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

From: Pip Cet <[email-masked]>
Date: Fri, 17 Jan 2025 21:30:41 +0000

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

>> Date: Fri, 17 Jan 2025 20:52:58 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], [email-masked]
>>
>> "Eli Zaretskii" <[email-masked]> writes:
>>
>> > This breaks the MS-Windows build: temacs crashes as below:
>>
>> Sorry.  Can you try the fix I just pushed?  While I arrived at that
>> independently (I was about to push when I got your email), I believe it
>> explains your crash as well.
>
> It fixes the crash, thanks.

Thanks!

I still think we should initialize the vector header of subrs and the
thread structure to contain the appropriate size fields.  Subrs
currently claim size 0 (and no auto-marked Lisp fields), which is
confusing.  Threads report totally incorrect values, because of what
seems like a bug to me: VECSIZE is used where VECSIZE - PSEUDOVECSIZE is
intended.

But this problem was caused by the GC header, not the vectorlike header.
Initializing the GC header is harder because of the hash value.  (No
great harm in leaving it set to 0 for those very few objects, though).

>> > The following patch seems to fix that, but I'm not sure it's the
>> > correct fix.  Can you explain why we need to add-variable-watcher for
>> > this in init_keyboard, in particular in temacs?
>>
>> I'm not sure what you are suggesting here.  Is there a good place that
>> is initialized only in the !pdumper || !temacs case?
>
> The "if (initialized)" test is exactly for that.  pdumper_load sets
> initialized = true before it returns.  So in temacs initialized will
> be false when init_keyboard is called.

But if we build without pdumper (after removing the relevant #error and
a few minor fixes, it works just fine), initialized is false, so SIGUSR2
wouldn't work at all in those builds.

>> I don't think so, but it might call Fadd_variable_watcher twice.  We
>> should avoid that.
>
> So maybe the patch I proposed should be installed regardless.

I had a look: Fadd_variable_watcher handles the case of several
identical watchers by ignoring the second call.

If we want to avoid calling Fadd_variable_watcher twice, we can use "if
(!initialized)", which will run the code just once (however, the
explicit call to watch_debug_on_event needs to be performed twice,
before and after the dump, because the xstrdup()ed string isn't dumped).

In the pdumper case, it'd run in temacs, put the right subr in the
symbol's plist, dump it, and then it'd be restored from the dump and
we'd only need the initial call.

Would that be okay?

Pip