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 18:11:47 +0000

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

>> Date: Sun, 18 Aug 2024 16:08:39 +0000
>> From: Pip Cet <[email-masked]>
>> Cc: [email-masked], [email-masked]
>>
>> "Eli Zaretskii" <[email-masked]> writes:
>>
>> > I don't understand: is this patch needed to trigger a crash, or are
>> > you saying we need it to fix crashes?
>>
>> It helps trigger the crash, which might take a long time without the
>> patch.
>>
>> > next_fontset_id is used to assign an ID to the next fontset we create,
>> > AFAIK.
>>
>> Indeed, and if we never reset it to a low value, we won't reuse slots in
>> Vfontset_table.  So the entries, rather than containing a different and
>> incorrect fontset for our font, will remain zeroed.
>
> How can incorrect fontset in Vfontset_table cause a segfault of the
> kind the OP reported?

I said "rather than containing a different and incorrect fontset"; in
the OP's case (and after the patch, on my system), it was Qnil.

>> >> > 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?
>> >
>> > How do you see that?
>>
>> printf debugging.
>
> Which shows what exactly?

See below.

>> >> >> 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'.
>> >
>> > Yes, but free_realized_faces calls free_realized_face, no?
>>
>> Yes, it does, but in my case, 'free_realized_face' is called by
>> 'realize_face', and 'free_realized_faces' isn't.
>
> Which face was freed, the ASCII face or not?

The ASCII face was freed, the non-ASCII face remained in the cache. In
the log below, the ASCII face is 0x117cca0, the non-ASCII face is
0x2a7a3c0.

>> I'll continue debugging this, but if there are any questions or further
>> helpful information, that would be much appreciated.
>
> I don't yet understand well what you are seeing because there isn't
> enough detailed information.

Okay, maybe this helps.

Here's the patch:

diff --git a/src/fontset.c b/src/fontset.c
index 16d14669c89..52149ebd6b2 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;
 }
@@ -1000,6 +996,7 @@ face_for_char (struct frame *f, struct face *face, int c,
      and display it as "glyphless".  That is certainly better than
      violating the assertion below or crashing when assertions are not
      compiled in.  */
+  fprintf (stderr, "fontset %d used for face %p\n", face->fontset, face);
   if (face->fontset < 0 && !face->font)
     return face->id;
 
diff --git a/src/xfaces.c b/src/xfaces.c
index 684b6ccfac7..86ab8e1328a 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -4598,7 +4598,25 @@ free_realized_face (struct frame *f, struct face *face)
 	{
 	  /* Free fontset of FACE if it is ASCII face.  */
 	  if (face->fontset >= 0 && face == face->ascii_face)
-	    free_face_fontset (f, face);
+	    {
+	      struct face_cache *cache = FRAME_FACE_CACHE (f);
+	      if (cache)
+		{
+		  for (int i = 0; i < cache->used; i++)
+		    {
+		      struct face *face2 = cache->faces_by_id[i];
+		      if (face2 != 0 && face2 != face && face2->fontset == face->fontset)
+			{
+			  fprintf (stderr, "Freeing fontset %d that's still in use by %p!\n", face->fontset,
+				   face2);
+			}
+		    }
+		}
+	      free_face_fontset (f, face);
+	    }
+	  else
+	    fprintf (stderr, "fontset %d not freed, used by %p\n", face->fontset, face);
+	dont:
 
 #ifdef HAVE_X_WINDOWS
 	  /* This function might be called with the frame's display

Here's hibiscus.el:

(display-time-mode t)
(setq display-time-interval .1)
(let ((i 0))
  (while t
    (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)
    (dolist (f (frame-list))
      (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))))

And here's the output of emacs -Q --load hibiscus.el:

fontset 103 used for face 0x117cca0
fontset 103 used for face 0x2a7a3c0
fontset 103 used for face 0x117cca0
fontset 103 used for face 0x2a62860
fontset 103 used for face 0x117cca0
fontset 103 used for face 0x2fb0dc0
fontset 103 used for face 0x117cca0
fontset 103 used for face 0x2a7a3c0
fontset 103 used for face 0x117cca0
fontset 103 used for face 0x2a62860
fontset 103 used for face 0x117cca0
fontset 103 used for face 0x2fb0dc0
fontset 103 used for face 0x117cca0
fontset 103 used for face 0x2a7a3c0
fontset 103 used for face 0x117cca0
fontset 103 used for face 0x2a62860
fontset 103 used for face 0x117cca0
fontset 103 used for face 0x2fb0dc0
Freeing fontset 103 that's still in use by 0x2a7a3c0!
Freeing fontset 103 that's still in use by 0x2a62860!
Freeing fontset 103 that's still in use by 0x2fb0dc0!
Freeing fontset 103 that's still in use by 0x2fdb7a0!
fontset 126 used for face 0x117cca0
fontset 103 used for face 0x2a7a3c0

lisp.h:2126: Emacs fatal error: assertion failed: CHAR_TABLE_P (a)
Fatal error 6: Aborted
Backtrace:
./emacs() [0x60f113]
./emacs() [0x5da076]
./emacs() [0x684c6d]
./emacs() [0x78d649]
./emacs() [0x790239]
./emacs() [0x44b7bd]
./emacs() [0x46933b]
./emacs() [0x4a9aef]
./emacs() [0x4a63af]
./emacs() [0x4a6dc3]
./emacs() [0x4a542f]
./emacs() [0x4a512e]
./emacs() [0x48dce3]
./emacs() [0x483053]
./emacs() [0x6b96a0]
./emacs() [0x482ec5]
./emacs() [0x4816c0]
./emacs() [0x482434]
./emacs() [0x433222]
./emacs() [0x6bd608]
./emacs() [0x718884]
./emacs() [0x6bdc5f]
./emacs() [0x6bdaf6]
./emacs() [0x6bc027]
./emacs() [0x6b6ae6]
./emacs() [0x6b8679]
./emacs() [0x6bbaae]
./emacs() [0x6b6ae6]
./emacs() [0x6b6b16]
./emacs() [0x6b86e1]
./emacs() [0x6bbaae]
./emacs() [0x6b6ae6]
./emacs() [0x6b8679]
./emacs() [0x6bbaae]
./emacs() [0x6b6ae6]
./emacs() [0x6b6b16]
./emacs() [0x6b86e1]
./emacs() [0x6bbaae]
./emacs() [0x6b6ae6]
./emacs() [0x6b8679]
./emacs() [0x6bbaae]
./emacs() [0x6fd2d6]
./emacs() [0x6fdb40]
./emacs() [0x6fde8f]
./emacs() [0x6bd6e6]
./emacs() [0x718884]
./emacs() [0x6bdc5f]
./emacs() [0x6bd059]
./emacs() [0x6bd308]
./emacs() [0x6fb7c7]
./emacs() [0x6bd6e6]
./emacs() [0x718884]
./emacs() [0x6bdc5f]
./emacs() [0x6bdaf6]
./emacs() [0x6bc027]
./emacs() [0x6bb55a]
./emacs() [0x5e1fdd]
./emacs() [0x6b95c9]
./emacs() [0x5e2038]
./emacs() [0x6b8a8d]
./emacs() [0x5e1ec0]
./emacs() [0x5e1327]
./emacs() [0x5e153a]
./emacs() [0x5dd18f]
/lib64/libc.so.6(+0x26200) [0x7ffff5b63200]
/lib64/libc.so.6(__libc_start_main+0x89) [0x7ffff5b632b9]
./emacs() [0x422155]
Aborted

Pip