bug#23386: Segfault when messing with font-backend

[npostavs@users.sourceforge.net]

[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]

found 23386 24.1
found 23386 24.5
found 23386 25.0.95
tags 23386 confirmed
quit

- Emacs versions 24.1, 24.2, 24.4, 24.5 all segfault this case.
- Emacs versions 23.4 and 24.3 don't segfault, but the first frame
  shows boxes for the characters in the modeline, and still seems to
  be using the Xft font in the initial frame (subsequently created
  frames use a font from the X backend).

In all cases this error is triggerred on startup:
"frame-notice-user-settings: Font `-PfEd-DejaVu Sans
Mono-normal-normal-normal-*-15-*-*-*-m-0-iso10646-1' is not defined",
although only in the latter case is Emacs able to display it, otherwise
it segfaults first.

AFAICT, this it's the same bug in all versions, some happen not to
segfault by accident.

The segfault happens when using font with

  (font->driver == &xftfont_driver) &&
  ((struct xftfont_info *)font)->xftfont == NULL

Passing NULL xftfont to Xft library triggers a segfault.

The way we end up with this kind of bad font object, is that
x_set_font_backend calls font_update_drivers which eventually calls
xftfont_close which sets the xftfont field of the frame's font to NULL,
but the frame still refers to this closed object.  The chosen font is
not updated, because it's set in the frame-parameters, so when
x_set_font_backend tries to honour this choice, it gets the error "Font
... is not defined" mentioned above (the font was defined only for the
xft backend, not the remaining x backend), and leaves the invalid font
object as the frame's default font.

Here is a patch that attempts to fix the issue by resetting the font
after the backend is changed.  It does let Emacs successfully open the
frame with the new font (no funny box characters in the modeline), but
I'm not sure if it's the best way of marking the font object invalid.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 3169 bytes --]

From 190e70acf940ad7678812e069e74fce93668a8a8 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 9 Jul 2016 14:20:53 -0400
Subject: [PATCH v1] Don't segfault on font backend change (Bug #23386)

* src/font.c (font_finish_cache): Kill frame's font if it used the
driver we just turned off.
* src/frame.c (x_set_font_backend): Reset the frame's font if it's been
killed.
---
 src/font.c  |  7 +++++++
 src/frame.c | 16 ++++++++++++++--
 src/frame.h |  1 +
 src/xfns.c  |  2 +-
 4 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/font.c b/src/font.c
index 2519599..e48b566 100644
--- a/src/font.c
+++ b/src/font.c
@@ -2587,6 +2587,13 @@ font_finish_cache (struct frame *f, struct font_driver *driver)
       font_clear_cache (f, XCAR (val), driver);
       XSETCDR (cache, XCDR (val));
     }
+
+  if (FRAME_FONT (f)->driver == driver)
+    {
+      /* Don't leave the frame's font pointing to a closed driver. */
+      store_frame_param(f, Qfont, Qnil);
+      FRAME_FONT (f) = NULL;
+    }
 }
 
 
diff --git a/src/frame.c b/src/frame.c
index 00f25f7..d7454d9 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -3677,6 +3677,8 @@ x_set_font (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
 void
 x_set_font_backend (struct frame *f, Lisp_Object new_value, Lisp_Object old_value)
 {
+  Lisp_Object frame;
+
   if (! NILP (new_value)
       && !CONSP (new_value))
     {
@@ -3718,11 +3720,21 @@ x_set_font_backend (struct frame *f, Lisp_Object new_value, Lisp_Object old_valu
     }
   store_frame_param (f, Qfont_backend, new_value);
 
+  XSETFRAME (frame, f);
+
+  /* If closing the font driver killed the frame's font, we need to
+     get a new one.  */
+  if (!FRAME_FONT (f))
+    x_default_font_parameter (f, Fframe_parameters (frame));
+  if (!FRAME_FONT (f))
+    {
+      delete_frame (frame, Qnoelisp);
+      error ("Invalid frame font");
+    }
+
   if (FRAME_FONT (f))
     {
-      Lisp_Object frame;
 
-      XSETFRAME (frame, f);
       x_set_font (f, Fframe_parameter (frame, Qfont), Qnil);
       face_change = true;
       windows_or_buffers_changed = 18;
diff --git a/src/frame.h b/src/frame.h
index f0cdcd4..5b5349e 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -1356,6 +1356,7 @@ extern void x_set_scroll_bar_default_height (struct frame *);
 extern void x_set_offset (struct frame *, int, int, int);
 extern void x_wm_set_size_hint (struct frame *f, long flags, bool user_position);
 extern Lisp_Object x_new_font (struct frame *, Lisp_Object, int);
+extern void x_default_font_parameter (struct frame *f, Lisp_Object parms);
 extern void x_set_frame_parameters (struct frame *, Lisp_Object);
 extern void x_set_fullscreen (struct frame *, Lisp_Object, Lisp_Object);
 extern void x_set_line_spacing (struct frame *, Lisp_Object, Lisp_Object);
diff --git a/src/xfns.c b/src/xfns.c
index 7c1bb1c..1b9dd48 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -3071,7 +3071,7 @@ do_unwind_create_frame (Lisp_Object frame)
   unwind_create_frame (frame);
 }
 
-static void
+void
 x_default_font_parameter (struct frame *f, Lisp_Object parms)
 {
   struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (f);
-- 
2.8.0