Subject: bug#75632: 31.0.50; igc: Crash report

From: Pip Cet <[email-masked]>
Date: Sat, 18 Jan 2025 19:53:50 +0000

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

>> Date: Sat, 18 Jan 2025 17:31:57 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], [email-masked]
>>
>> "Eli Zaretskii" <[email-masked]> writes:
>>
>> >> I think it's important to keep temacs/emacs without dumping runnable.
>> >> It's certainly still interesting when temacs is called emacs.
>> >
>> > Yes, of course, no argument here.
>>
>> So (!initialized) it is?  That works for temacs and emacs (I originally
>> thought it would be wasteful for non-interactive temacs, but I'm sure
>> someone's using that :-) ).
>
> As long as we understand well what it means to free in emacs a string
> that was xstrdup'ed in temacs, yes.  Suppose debug-on-event is changed
> in emacs to something else, how would xfree work in that case?

pdumper doesn't work that way (unexec did, I think): a static variable
that isn't "remembered" by pdumper isn't saved, and it starts out as
NULL in the dumped Emacs, and is replaced by the new xstrdup.  The old
xstrdup (and the memory it allocated) are gone after the dump.

>> > I'd be much happier if we instead treated this as we treat SIGIO that
>> > is delivered when the user presses a keyboard key: set a flag saying
>> > that input is available, and then process it as we do with keyboard.
>>
>> IIRC, there are some loops that test Vinhibit_quit or Vquit_flag
>> directly (or use QUITP); we wouldn't be able to break out of one of
>> those if we only set Vinhibit_quit from what's currently
>> store_user_signal_events, would we?
>
> Sorry, I don't follow.  Why does it matter whether we set those
> variables from a signal handler or in gobble_input?

We might never reach gobble_input if the variable isn't set in the
signal handler: some loops check QUITP (etc.), and if there's a bug in
them and Vinhibit_quit is set, the loop might never call maybe_quit.

For example, SIGUSR2 should be able to interrupt this loop in gap_right:

  while (1)
    {
      /* I gets number of characters left to copy.  */
      i = bytepos - new_s1;
      if (i == 0)
	break;
      /* If a quit is requested, stop copying now.
	 Change BYTEPOS to be where we have actually moved the gap to.
	 Note that this cannot happen when we are called to make the
	 gap larger or smaller, since make_gap_larger and
	 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;
	}
      /* Move at most 32000 chars before checking again for a quit.  */
      if (i > 32000)
	i = 32000;
      new_s1 += i;
      memmove (to, from, i);
      from += i, to += i;
    }

Since QUITP is defined as

#define QUITP (!NILP (Vquit_flag) && NILP (Vinhibit_quit))

that means the signal handler (not gobble_input) needs to touch these
flags.

It's possible to add something like

if (pending_signals)
  {
    ptrdiff_t old_bytepos = bytepos;
    ptrdiff_t old_charpos = charpos;
    bytepos = new_s1;
    charpos = BYTE_TO_CHAR (bytepos);
    maybe_quit ();
  }

to this loop, and then it would be interruptible by SIGUSR2, but it
would also be interruptible by other input events, so I'm not sure that
wouldn't cause crashes.

>> > The SIGUSR event is eventually entered into the keyboard buffer
>> > anyway, so why should we treat it so differently from SIGIO?
>>
>> Vdebug_on_event isn't, AFAICS.  Not a major change to make it so,
>> though.  I don't know why we eat this event, but we do.
>
> I guess because we want the debugger to pop up before any queued
> events are processed.

"instead of", I think.  That's a detail I don't care much for: I think
it'd be helpful for sigusr2 to show up in M-x view-lossage even if it
triggered the debugger.

> But I don't see why doing the same in
> gobble_input should produce different behavior.

while (true)
  {
    while (!QUITP);
    maybe_quit();
  )

should be interruptible by SIGUSR2.

Pip