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

[Derek Upham <sand@blarg.net>]

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

Will do.  Is the patch against the stock 26.1 sources, or against some other commit?

Eli Zaretskii <eliz@gnu.org> writes:

>> 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);
>