Subject: bug#72692: Emacs 31.05 (40eecd594ac) get SIGSEGV on Linux (Linux 6.6.45 Kde Wayland)

From: Pip Cet <[email-masked]>
Date: Sun, 18 Aug 2024 13:44:41 +0000

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

>> Cc: [email-masked]
>> Date: Sun, 18 Aug 2024 12:43:06 +0000
>> From:  Pip Cet via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <[email-masked]>
>>
>> Thanks. That has a different fontset, so it looks like a fontset was
>> prematurely freed while still being referred to by a face.  I think the
>> assumption made in xfaces.c, that it's always safe to free a fontset if
>> we're freeing the realized ASCII face, is incorrect.
>
> Why do you think that?  free_realized_face frees a face, so what other
> face can still use the same fontset, if it's a so-called "ASCII face"?

I was under the impression two faces could share the same fontset.  That
certainly is what my debugging sessions so far indicate.  Maybe that's
the bug?

>> I can confirm that we're sometimes leaving a frame's fontset field
>> invalid by running this code:
>
> If face2->ascii_face is equal to face->ascii_face,

Yes, face2->ascii_face == face == face->ascii_face.

> then I see no
> reason not to free the fontset because of that other face.  The
> comment in dispextern.h says:
>
>   /* Fontset ID if for this face's fontset.  Non-ASCII faces derived
>      from the same ASCII face have the same fontset.  */
>   int fontset;

So, indeed, the fontset id is shared between the ASCII face and the
non-ASCII face.  If we free the fontset because the ASCII face is
unrealized, but the non-ASCII face is not, we hit the bug...

> And how did you see that a frame's fontset was left invalid here?  A
> frame doesn't have a fontset, AFAIK.

I meant "face", sorry!  The non-ASCII face remains in the font cache,
and its fontset is set to the newly freed fontset's ID, which is likely
soon to be reused; only if it isn't, we see a crash.

> I don't object to using this code as a means of investigation, of
> course, but I think we should have a clear idea of what the diagnostic
> it emits means.
>
> I also think it might be unsafe to call 'message' from this place,
> because 'message' might trigger redisplay.  It is better to call
> message_dolog or add_to_log (and ask the user to look at *Messages*).

It is, thank you! Yes, fprintf or message_dolog would have been better.

Pip