Subject: Re: bug#39892: 28.0.50; Crash when running async command

From: Eli Zaretskii <[email-masked]>
Date: Thu, 05 Mar 2020 11:46:59 +0200

> Date: Wed, 04 Mar 2020 18:13:38 +0200
> From: Eli Zaretskii <[email-masked]>
> Cc: [email-masked], [email-masked]
> 
> >   for (i = 0; i < size; i++)
> >     {
> >       Lisp_Object rfont_def = AREF (vec, i);
> >       Lisp_Object font_def = RFONT_DEF_FONT_DEF (rfont_def);
> > 
> > 'vec' at that point is
> > 
> > [
> > [nil [#<font-spec nil nil nil nil iso10646-1 nil nil nil nil nil nil nil nil> 144 nil] nil 0]
> > nil
> > [nil [#<font-spec nil nil Noto\ Sans\ Symbols2 nil nil nil nil nil nil nil nil nil ((:name . "Noto Sans Symbols2"))> 0 0] nil 2]
> > ]
> > 
> > and i == 1, so rfont_def ends up as nil.
> > 
> > How did that 'nil' get in there?
> 
> I don't know.  The code seems to expect one nil as the last element of
> the vector, but not in the middle.  Note that the commentary at the
> beginning of fontset.c explains when RFONT_DEF may be nil.

How about the patch below?  It works for me, and seems to be TRT in
general, since it makes the code more defensive in the face of
possible nil values in that vector, which are legitimate according to
my reading of the commentary at the beginning of fontset.c.

CC'ing Handa-san in the hope that he might have comments or
suggestions.

Btw, in general using such fontset customizations make little sense,
since you are in effect telling Emacs this font covers all of the
Unicode, which cannot be true.  Not specifying the registry is also
not recommended, definitely not for such large ranges of codepoints.

> Btw, U+E9F8 is a PUA character; how did it get into a context where we
> are trying to display it?

I'd still like to know the answer to this.  It seems like something
fishy is going on in the original use case that requires us to try to
display this PUA codepoint.

Here's the patch I propose to install on the emacs-27 branch:

diff --git a/src/fontset.c b/src/fontset.c
index bca9452..c2bb8b2 100644
--- a/src/fontset.c
+++ b/src/fontset.c
@@ -367,8 +367,14 @@ fontset_add (Lisp_Object fontset, Lisp_Object range, Lisp_Object elt, Lisp_Objec
 static int
 fontset_compare_rfontdef (const void *val1, const void *val2)
 {
-  return (RFONT_DEF_SCORE (*(Lisp_Object *) val1)
-	  - RFONT_DEF_SCORE (*(Lisp_Object *) val2));
+  Lisp_Object v1 = *(Lisp_Object *) val1, v2 = *(Lisp_Object *) val2;
+  if (NILP (v1) && NILP (v2))
+    return 0;
+  else if (NILP (v1))
+    return INT_MIN;
+  else if (NILP (v2))
+    return INT_MAX;
+  return (RFONT_DEF_SCORE (v1) - RFONT_DEF_SCORE (v2));
 }
 
 /* Update a cons cell which has this form:
@@ -400,6 +406,8 @@ reorder_font_vector (Lisp_Object font_group, struct font *font)
   for (i = 0; i < size; i++)
     {
       Lisp_Object rfont_def = AREF (vec, i);
+      if (NILP (rfont_def))
+	continue;
       Lisp_Object font_def = RFONT_DEF_FONT_DEF (rfont_def);
       Lisp_Object font_spec = FONT_DEF_SPEC (font_def);
       int score = RFONT_DEF_SCORE (rfont_def) & 0xFF;