Subject: Re: bug#79131: 31.0.50; igc: nested signal, SIGSEGV

From: Pip Cet <[email-masked]>
Date: Fri, 08 Aug 2025 13:08:12 +0000

"Eli Zaretskii" <[email-masked]> writes:

>> From: Gerd Möllmann <[email-masked]>
>> Cc: [email-masked],  [email-masked],  [email-masked],
>>   [email-masked]
>> Date: Fri, 08 Aug 2025 09:10:07 +0200
>>
>> Eli Zaretskii <[email-masked]> writes:
>>
>> >> From: Gerd Möllmann <[email-masked]>
>> >> Cc: [email-masked],  [email-masked],  [email-masked],
>> >>   [email-masked]
>> >> Date: Fri, 08 Aug 2025 08:25:04 +0200
>> >>
>> >> Eli Zaretskii <[email-masked]> writes:
>> >>
>> >> >> While an algorithm is working on tree, the tree may temporarily become
>> >> >> inconsisten. Quitting out of the algorithm where that is the case is a
>> >> >> no-go.
>> >> >
>> >> > Is that related to igc in some way?
>> >>
>> >> No.
>> >>
>> >> > The discussion was about igc making this happen, or making it happen
>> >> > more frequently. What you say seem to be unrelated.
>> >>
>> >> More frequently was a hypotheses of Pip, as a side note.
>> >
>> > I responded only to that side note.
>>
>> Then Ipropose to go with whatever Pip finds out to be the best: either
>> specbind or *_no_quit.
>
> Before we do, I'd like to see the actual places where we currently
> call functions which can quit, while intervals are in inconsistent
> state.

The explicit calls to Fmemq in intervals.c would be two such places:

static INTERVAL
adjust_intervals_for_insertion (INTERVAL tree,
				ptrdiff_t position, ptrdiff_t length)
{
    ....
      /* Does any actual property pose an actual problem?  We break
         the loop if we find a nonsticky property.  */
      for (; CONSP (tail); tail = Fcdr (XCDR (tail)))
	{
	  Lisp_Object prop, tmp;
	  prop = XCAR (tail);

	  /* Is this particular property front-sticky?  */
	  if (CONSP (front) && ! NILP (Fmemq (prop, front))) <--- quit happens here
	    continue;

	  /* Is this particular property rear-nonsticky?  */
	  if (CONSP (rear) && ! NILP (Fmemq (prop, rear)))
	    break;
    ....
  }

  /* If we are positioned between intervals, check the stickiness of
     both of them.  We have to do this too, if we are at BEG or Z.  */
  if (position == i->position || eobp)
    {
      ...
      for (temp = prev ? prev : i; temp; temp = INTERVAL_PARENT_OR_NULL (temp))
	{
	  temp->total_length += length; <--- adjustment happens here
          ...
    }
  else
    {
      for (temp = i; temp; temp = INTERVAL_PARENT_OR_NULL (temp))
	{
	  temp->total_length += length;
          ...
	}
    }

  return tree;

Here's the case for insertion:

Intervals are in an inconsistent state from the point where Z is
increased in insert_from_gap_1 until the total length of the root
interval is increased to match it in adjust_intervals_for_insertion
(both of these functions are called from insert_from_gap).

So adjust_intervals_for_insertion must not call any function that can
quit (if it does so before the total length is updated, intervals will
be in an inconsistent state. If it does so afterwards, we'll fail to
adjust point after the insertion).

But it calls Fmemq and Fassq, both directly and via TMEM and textget, in
certain circumstances. Those calls happen before total_length is
adjusted. My gdb session indicates that Vinhibit_quit is nil when this
function is reached for a normal character insertion.

Fmemq and Fassq sometimes quit, though they will also sometimes ignore
the quit flag and return normally, because of the rather complicated
logic in FOR_EACH_TAIL_INTERNAL; my guess from looking at the code is
that we need to walk to at least the third element of a list before we
first check the quit flag.

We might be able to produce an inconsistent state by setting a
breakpoint in gdb and setting Vquit_flag = Qt at the wrong moment; would
that help in any way?

Pip