bug#32265: 26.1; yank-excluded-properties set to t triggers "Error in syntax_table logic for intervals <-" signal

[Eli Zaretskii <eliz@gnu.org>]

> From: Noam Postavsky <npostavs@gmail.com>
> Cc: sand@blarg.net,  32265@debbugs.gnu.org
> Date: Tue, 31 Jul 2018 21:06:08 -0400
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I presume just calling set-text-properties with the same arguments
> > there doesn't trigger the problem?  If so, I think we need to
> > understand how come we got offset=-1 in frame #4.
> 
> Actually, it looks like it can be triggered with just
> set-text-properties, but there is some interaction with
> c-before-change.  I've reduced the testcase to this:
> 
>     (defconst dir (file-name-directory
>                    (or load-file-name buffer-file-name)))
> 
>     (pop-to-buffer-same-window (get-buffer-create "*test*"))
>     (erase-buffer)
>     (insert-file-contents (expand-file-name "U.java" dir))
>     (java-mode)
>     (set-text-properties 10467 10542 nil)

Thanks for an easy test case.  This is bug #13743 striking again in a
slightly different incarnation: removing the text properties calls
before-change-functions, which modify the buffer's interval tree, but
we continue using the interval calculated from the unmodified tree.
So the solution should be similar to what we used in that bug.

If the patch below fixes the original problem (Derek, can you
confirm?), I will install it on the emacs-26 branch.

> The backtrace no longer has a negative offset to blame:
> 
> ../../src/intervals.c:371: Emacs fatal error: assertion failed: LENGTH (i) > 0
> 
> (gdb) bt 8
> #0  terminate_due_to_signal (sig=6, backtrace_limit=2147483647) at ../../src/emacs.c:364
> #1  0x00000000006161d3 in die (msg=0x7795cd "LENGTH (i) > 0", file=0x7794f0 "../../src/intervals.c", 
>     line=371) at ../../src/alloc.c:7410
> #2  0x00000000006b51c4 in balance_an_interval (i=0x324b0b0) at ../../src/intervals.c:371
> #3  0x00000000006b5792 in split_interval_right (interval=0x181f380 <bss_sbrk_buffer+10524736>, 
>     offset=4048) at ../../src/intervals.c:504

But the offset is still bogus, because:

  #3  0x012d2f46 in split_interval_right (interval=0xfa9c10, offset=4048)
      at intervals.c:504
  504           balance_an_interval (new);
  (gdb) p new_length
  $8 = -107

IOW, OFFSET is greater than LENGTH(interval) in split_interval_right,
which must not happen.

Here's the proposed patch:

diff --git a/src/textprop.c b/src/textprop.c
index 984f2e6..904e226 100644
--- a/src/textprop.c
+++ b/src/textprop.c
@@ -1350,6 +1350,7 @@ set_text_properties (Lisp_Object start, Lisp_Object end, Lisp_Object properties,
 {
   register INTERVAL i;
   Lisp_Object ostart, oend;
+  bool first_time = true;
 
   ostart = start;
   oend = end;
@@ -1372,6 +1373,7 @@ set_text_properties (Lisp_Object start, Lisp_Object end, Lisp_Object properties,
       return Qt;
     }
 
+ retry:
   i = validate_interval_range (object, &start, &end, soft);
 
   if (!i)
@@ -1391,8 +1393,22 @@ set_text_properties (Lisp_Object start, Lisp_Object end, Lisp_Object properties,
 	return Qnil;
     }
 
-  if (BUFFERP (object) && !NILP (coherent_change_p))
-    modify_text_properties (object, start, end);
+  if (BUFFERP (object) && !NILP (coherent_change_p) && first_time)
+    {
+      ptrdiff_t prev_length = LENGTH (i);
+      ptrdiff_t prev_pos = i->position;
+
+      modify_text_properties (object, start, end);
+      /* If someone called us recursively as a side effect of
+	 modify_text_properties, and changed the intervals behind our
+	 back, we cannot continue with I, because its data changed.
+	 So we restart the interval analysis anew.  */
+      if (LENGTH (i) != prev_length || i->position != prev_pos)
+	{
+	  first_time = false;
+	  goto retry;
+	}
+    }
 
   set_text_properties_1 (start, end, properties, object, i);