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

[Pip Cet via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

"Eli Zaretskii" <eliz@gnu.org> writes:

>> Date: Sun, 18 Aug 2024 13:44:41 +0000
>> From: Pip Cet <pipcet@protonmail.com>
>> Cc: execvy@gmail.com, 72692@debbugs.gnu.org
>>
>> "Eli Zaretskii" <eliz@gnu.org> writes:
>>
>> >> Cc: 72692@debbugs.gnu.org
>> >> 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" <bug-gnu-emacs@gnu.org>
>> >>
>> >> 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?
>
> We are talking about a fontset identified by face->fontset.  AFAIU,
> face->fontset is only non-negative for so-called "ASCII faces".

Not what I'm seeing. base_face->fontset is copied into the new face by
realize_non_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...
>
> But AFAIK a non-ASCII face is always released together with its ASCII
> face,

Also not what I'm seeing. When 'realize_basic_faces' is called, we call
'realize_face', which destroys the fontset, even though it's still in
use by the non-ASCII face.

> so how can this be a problem?

I don't understand yet what underlying assumption is violated, and what
precisely happened.

But I have just reproduced the crash, I think. It does need this patch,
which means we will actually crash when accessing a formerly-valid
fontset, rather than accessing random and inappropriate data, so I think
we need to first establish that this patch doesn't break things and
cause a different crash.

diff --git a/src/fontset.c b/src/fontset.c
index 16d14669c89..41d845c9bc5 100644
--- a/src/fontset.c
+++ b/src/fontset.c
@@ -921,8 +921,6 @@ free_face_fontset (struct frame *f, struct face *face)
   eassert (! BASE_FONTSET_P (fontset));
   eassert (f == XFRAME (FONTSET_FRAME (fontset)));
   ASET (Vfontset_table, face->fontset, Qnil);
-  if (face->fontset < next_fontset_id)
-    next_fontset_id = face->fontset;
   if (! NILP (FONTSET_DEFAULT (fontset)))
     {
       int id = XFIXNUM (FONTSET_ID (FONTSET_DEFAULT (fontset)));
@@ -931,8 +929,6 @@ free_face_fontset (struct frame *f, struct face *face)
       eassert (!NILP (fontset) && ! BASE_FONTSET_P (fontset));
       eassert (f == XFRAME (FONTSET_FRAME (fontset)));
       ASET (Vfontset_table, id, Qnil);
-      if (id < next_fontset_id)
-	next_fontset_id = face->fontset;
     }
   face->fontset = -1;
 }

and this Emacs Lisp script:

(display-time-mode t)
(let ((i 0))
  (while t
    (dolist (f (frame-list))
      (push (concat (make-string 1 (floor (random 132000)))
                    (make-string 1 (floor (random 132000)))
                    (make-string 1 (floor (random 132000)))
                    (make-string 1 (floor (random 132000))))
            mode-line-format)
      (set-frame-parameter f 'alpha-background 1.0)
      (sit-for 0)
      (cl-incf i)
      (message "%S" i)
      (set-frame-parameter f 'alpha-background 0.9)
      (sit-for 0)
      (garbage-collect))))

(Not minimized yet).

> A "non-ASCII face" is basically
> the same face as its "ASCII face" counterpart, it just uses a
> different font.  An example would be some well-known face, like 'bold'
> or 'variable-pitch' or 'region' -- when we need to display a non-ASCII
> character in this face, and the "ASCII face"s font doesn't support the
> character, we internally create a new face that uses the same fontset
> as the "ASCII face".  This new face basically shadows the "ASCII face"
> (and is never exposed to Lisp) and is for every practical purpose an
> integral part of that "ASCII face" -- they always go together.

Except they're not freed together?

>> > 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.
>
> That shouldn't happen, AFAIU, except for very brief periods of time,
> since we free the cached faces one by one, see free_realized_faces.

Again, not what I'm seeing, because 'free_realized_faces' isn't where the
font is actually removed from the cache; it's 'free_realized_face'.

I'd like to understand what is happening a bit better before submitting
a proposed fix.

Pip