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

From: Pip Cet <[email-masked]>
Date: Wed, 22 Jan 2025 15:55:05 +0000

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

>> Date: Wed, 22 Jan 2025 12:30:45 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], [email-masked]
>>
>> "Eli Zaretskii" <[email-masked]> writes:
>>
>> >> Date: Sun, 19 Jan 2025 13:37:18 +0000
>> >> From: Pip Cet <[email-masked]>
>> >> Cc: [email-masked], [email-masked]
>> >>
>> >> "Eli Zaretskii" <[email-masked]> writes:
>> >>
>> >> > and at temacs time?
>> >>
>> >> I don't think it's a realistic scenario that someone needs to debug
>> >> non-interactive temacs, cannot use the default signal, and wants to
>> >> change it.
>> >
>> > I meant the interactive temacs, which begins by loading all the
>> > preloaded packages.  So one of those could add the watcher.
>>
>> Do we really need this to be added early?
>
> Only if we care about sending SIGUSR2 before bindings.el is loaded.
>
> I guess we can leave it in bindings.el, and wait for someone to
> complain that it doesn't work earlier.

I'm not sure I understand: you're thinking someone might modify one of
the dozen or so Lisp files that are loaded before, in the Emacs tree, to
attempt to set an obsolete variable to allow entering the debugger,
which would also require changing things around so the debugger could
even be loaded at that point?  I'm confident that we'd have to wait for
some time.

>> > One of the first files we load in loadup, I think?  Like subr.el,
>> > perhaps?
>>
>> See above.  I think if you need to put an --eval '(debug--on-event
>> (quote sigusr1))' in your temacs invocation, you're likely knowledgeable
>> enough to do so.
>
> Does it really help?  I think temacs loads the preloaded files before
> it processes --eval on the command line, isn't that so?

You're right, but it turns out SIGUSR2 doesn't do anything noticeable
with or without the patch, so SIGUSR1 can't, either.

>> > It's not optimal, indeed.  See a somewhat better arrangement we have
>> > with the variables which should trigger redisplay of the current
>> > buffer, at the end of frame.el.
>>
>> I've tried this:
>>
>> diff --git a/lisp/help-fns.el b/lisp/help-fns.el
>> index 9324cf85454..f4f595aedbe 100644
>> --- a/lisp/help-fns.el
>> +++ b/lisp/help-fns.el
>> @@ -1712,7 +1712,9 @@ help-fns--var-watchpoints
>>      (when watchpoints
>>        (princ "  Calls these functions when changed: ")
>>        ;; FIXME: Turn function names into hyperlinks.
>> -      (princ watchpoints)
>> +      (dolist (watchpoint watchpoints)
>> +        (princ (help-fns-function-name watchpoint))
>> +        (princ "\n  "))
>>        (terpri))))
>>
>> it generates a three-digit hex hash of the function, which is a lot
>> nicer than bytecode, but still not ideal.
>>
>> Adding a docstring and a name to the function produces:
>>
>>   Calls these functions when changed: (#[1028 \300!\207 [debug--on-event] 6 (bindings.elc . 52251)])
>>
>> If there's a better way to fix this minor problem, we probably should.
>
> This might be a misunderstanding.  I meant to use a subr, as we do in
> frame.el.  "C-h v" then says:

You said:
> Can we add the watcher in Lisp instead?  Are there any reasons to have
> it in C and at temacs time?

I realize now that you can read the first sentence two ways, but the
second one seems clear to me: there are no reasons to have the watcher
in C.  But as that patch had the defun in C, we can switch back to it.

>     Calls these functions when changed: (#<subr set-buffer-redisplay>)
>
> Isn't that slightly nicer than
>
>   (#[1028 \300!\207 [debug--on-event] 6 (bindings.elc . 52251)])
>
> ?  That would mean make debug--on-event a subr, not a DEFUN.

Let's add the symbol, not the function cell, and everything will be
fine.

We still need to fix the output of C-h v when watchers are in effect,
but that's a separate issue.

However, I also don't understand how a subr, which is defined by DEFUN,
is different from "a DEFUN".  Did you mean to write "defun"?

>> @@ -8280,6 +8284,7 @@ add_user_signal (int sig, const char *name)
>>    p = xmalloc (sizeof *p);
>>    p->sig = sig;
>>    p->name = xstrdup (name);
>> +  p->debug_on_event = !strcmp (p->name, "sigusr2");
>
> Hmm... why only sigusr2? what about sigusr1?

The default behavior is for sigusr2 to enter the debugger and for
sigusr1 to execute its binding, which is 'ignore.  Changing SIGUSR1 to
also enter the debugger by default would be a breaking change and
require significant documentation adjustments.  (Also, why?)

Pip