bug#36431: Crash in marker.c:337

[Stefan Monnier]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> No, it doesn't fix the problem, unfortunately – mew doesn't start at
>> all with this patch; the error message shows a string that is
>> definitely cropped.
>
> Yes, I also saw other problems with the patch.
> I'll send an updated patch later.

Can you try the patch below?


        Stefan
        

diff --git a/src/fileio.c b/src/fileio.c
index ed1d2aedf3..1fea93fa8e 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -3741,6 +3741,7 @@ by calling `format-decode', which see.  */)
          CHECK_CODING_SYSTEM (Vcoding_system_for_read);
          Fset (Qbuffer_file_coding_system, Vcoding_system_for_read);
        }
+      eassert (inserted == 0);
       goto notfound;
     }
 
@@ -3767,7 +3768,10 @@ by calling `format-decode', which see.  */)
       not_regular = 1;
 
       if (! NILP (visit))
-       goto notfound;
+        {
+          eassert (inserted == 0);
+         goto notfound;
+        }
 
       if (! NILP (replace) || ! NILP (beg) || ! NILP (end))
        xsignal2 (Qfile_error,
@@ -4435,19 +4439,6 @@ by calling `format-decode', which see.  */)
   if (how_much < 0)
     report_file_error ("Read error", orig_filename);
 
-  /* Make the text read part of the buffer.  */
-  GAP_SIZE -= inserted;
-  GPT      += inserted;
-  GPT_BYTE += inserted;
-  ZV       += inserted;
-  ZV_BYTE  += inserted;
-  Z        += inserted;
-  Z_BYTE   += inserted;
-
-  if (GAP_SIZE > 0)
-    /* Put an anchor to ensure multi-byte form ends at gap.  */
-    *GPT_ADDR = 0;
-
  notfound:
 
   if (NILP (coding_system))
@@ -4457,6 +4448,7 @@ by calling `format-decode', which see.  */)
 
         Note that we can get here only if the buffer was empty
         before the insertion.  */
+      eassert (Z == BEG);
 
       if (!NILP (Vcoding_system_for_read))
        coding_system = Vcoding_system_for_read;
@@ -4477,6 +4469,10 @@ by calling `format-decode', which see.  */)
          bset_undo_list (current_buffer, Qt);
          record_unwind_protect (decide_coding_unwind, unwind_data);
 
+          /* Make the text read part of the buffer.  */
+          eassert (NILP (BVAR (current_buffer, enable_multibyte_characters)));
+          insert_from_gap_1 (inserted, inserted, false);
+
          if (inserted > 0 && ! NILP (Vset_auto_coding_function))
            {
              coding_system = call2 (Vset_auto_coding_function,
@@ -4493,8 +4489,22 @@ by calling `format-decode', which see.  */)
              if (CONSP (coding_system))
                coding_system = XCAR (coding_system);
            }
-         unbind_to (count1, Qnil);
+
+          /* Move the text back to the beginning of the gap.
+             Do it now, before we set the buffer back to multibyte, since the
+             bytes may very well not be valid for a multibyte buffer.  */
+          set_point_both (BEG, BEG_BYTE);
+          /* In general this may have to move all the bytes, but here
+             this can't move more bytes than were moved during the execution
+             of Vset_auto_coding_function, which is normally 0 (because it
+             normally doesn't modify the buffer).  */
+          move_gap_both (Z, Z_BYTE);
          inserted = Z_BYTE - BEG_BYTE;
+          GAP_SIZE += inserted;
+          ZV = Z = GPT = BEG;
+          ZV_BYTE = Z_BYTE = GPT_BYTE = BEG_BYTE;
+
+         unbind_to (count1, Qnil);
        }
 
       if (NILP (coding_system))
@@ -4528,22 +4538,29 @@ by calling `format-decode', which see.  */)
        }
     }
 
-  coding.dst_multibyte = ! NILP (BVAR (current_buffer, 
enable_multibyte_characters));
+  eassert (PT == GPT);
+
+  coding.dst_multibyte
+    = ! NILP (BVAR (current_buffer, enable_multibyte_characters));
   if (CODING_MAY_REQUIRE_DECODING (&coding)
       && (inserted > 0 || CODING_REQUIRE_FLUSHING (&coding)))
     {
-      move_gap_both (PT, PT_BYTE);
-      GAP_SIZE += inserted;
-      ZV_BYTE -= inserted;
-      Z_BYTE -= inserted;
-      ZV -= inserted;
-      Z -= inserted;
+      /* Now we have all the new bytes at the beginning of the gap,
+         but `decode_coding_gap` needs them at the end of the gap, so
+         we need to move them.
+         FIXME: We should arrange for the bytes to be already at the right
+         place so we don't need to memmove them in the common case!  */
+      memmove (GAP_END_ADDR - inserted, GPT_ADDR, inserted);
       decode_coding_gap (&coding, inserted, inserted);
       inserted = coding.produced_char;
       coding_system = CODING_ID_NAME (coding.id);
     }
   else if (inserted > 0)
     {
+      /* Make the text read part of the buffer.  */
+      eassert (NILP (BVAR (current_buffer, enable_multibyte_characters)));
+      insert_from_gap_1 (inserted, inserted, false);
+
       invalidate_buffer_caches (current_buffer, PT, PT + inserted);
       adjust_after_insert (PT, PT_BYTE, PT + inserted, PT_BYTE + inserted,
                           inserted);
diff --git a/src/insdel.c b/src/insdel.c
index 85fffd8fd1..51371ee13c 100644
--- a/src/insdel.c
+++ b/src/insdel.c
@@ -115,7 +115,7 @@ gap_left (ptrdiff_t charpos, ptrdiff_t bytepos, bool newgap)
   i = GPT_BYTE;
   to = GAP_END_ADDR;
   from = GPT_ADDR;
-  new_s1 = GPT_BYTE;
+  new_s1 = GPT_BYTE; /* May point in the middle of multibyte sequences.  */
 
   /* Now copy the characters.  To move the gap down,
      copy characters up.  */
@@ -133,6 +133,7 @@ gap_left (ptrdiff_t charpos, ptrdiff_t bytepos, bool newgap)
         make_gap_smaller set inhibit-quit.  */
       if (QUITP)
        {
+          /* FIXME: This can point in the middle of a multibyte character.  */
          bytepos = new_s1;
          charpos = BYTE_TO_CHAR (bytepos);
          break;
@@ -164,7 +165,7 @@ gap_right (ptrdiff_t charpos, ptrdiff_t bytepos)
 {
   register unsigned char *to, *from;
   register ptrdiff_t i;
-  ptrdiff_t new_s1;
+  ptrdiff_t new_s1; /* May point in the middle of multibyte sequences.  */
 
   BUF_COMPUTE_UNCHANGED (current_buffer, charpos, GPT);
 
@@ -189,6 +190,7 @@ gap_right (ptrdiff_t charpos, ptrdiff_t bytepos)
         make_gap_smaller set inhibit-quit.  */
       if (QUITP)
        {
+          /* FIXME: This can point in the middle of a multibyte character.  */
          bytepos = new_s1;
          charpos = BYTE_TO_CHAR (bytepos);
          break;
@@ -1072,6 +1074,31 @@ insert_from_string_1 (Lisp_Object string, ptrdiff_t pos, 
ptrdiff_t pos_byte,

 /* Insert a sequence of NCHARS chars which occupy NBYTES bytes
    starting at GAP_END_ADDR - NBYTES (if text_at_gap_tail) and at
+   GPT_ADDR (if not text_at_gap_tail).
+   Contrary to insert_from_gap, this does not invalidate any cache,
+   nor update any markers, nor record any buffer modification information
+   of any sort.  */
+void
+insert_from_gap_1 (ptrdiff_t nchars, ptrdiff_t nbytes, bool text_at_gap_tail)
+{
+  GAP_SIZE -= nbytes;
+  if (! text_at_gap_tail)
+    {
+      GPT += nchars;
+      GPT_BYTE += nbytes;
+    }
+  ZV += nchars;
+  Z += nchars;
+  ZV_BYTE += nbytes;
+  Z_BYTE += nbytes;
+
+  /* Put an anchor to ensure multi-byte form ends at gap.  */
+  if (GAP_SIZE > 0) *(GPT_ADDR) = 0;
+  eassert (GPT <= GPT_BYTE);
+}
+
+/* Insert a sequence of NCHARS chars which occupy NBYTES bytes
+   starting at GAP_END_ADDR - NBYTES (if text_at_gap_tail) and at
    GPT_ADDR (if not text_at_gap_tail).  */
 
 void
@@ -1090,19 +1117,7 @@ insert_from_gap (ptrdiff_t nchars, ptrdiff_t nbytes, 
bool text_at_gap_tail)
   record_insert (GPT, nchars);
   modiff_incr (&MODIFF);
 
-  GAP_SIZE -= nbytes;
-  if (! text_at_gap_tail)
-    {
-      GPT += nchars;
-      GPT_BYTE += nbytes;
-    }
-  ZV += nchars;
-  Z += nchars;
-  ZV_BYTE += nbytes;
-  Z_BYTE += nbytes;
-  if (GAP_SIZE > 0) *(GPT_ADDR) = 0; /* Put an anchor.  */
-
-  eassert (GPT <= GPT_BYTE);
+  insert_from_gap_1 (nchars, nbytes, text_at_gap_tail);
 
   adjust_overlays_for_insert (ins_charpos, nchars);
   adjust_markers_for_insert (ins_charpos, ins_bytepos,
diff --git a/src/lisp.h b/src/lisp.h
index a0619e64f2..1a1d8ee7e4 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3667,6 +3667,7 @@ extern void insert (const char *, ptrdiff_t);
 extern void insert_and_inherit (const char *, ptrdiff_t);
 extern void insert_1_both (const char *, ptrdiff_t, ptrdiff_t,
                           bool, bool, bool);
+extern void insert_from_gap_1 (ptrdiff_t, ptrdiff_t, bool text_at_gap_tail);
 extern void insert_from_gap (ptrdiff_t, ptrdiff_t, bool text_at_gap_tail);
 extern void insert_from_string (Lisp_Object, ptrdiff_t, ptrdiff_t,
                                ptrdiff_t, ptrdiff_t, bool);