bug#23386: Segfault when messing with font-backend

bug#23386: Segfault when messing with font-backend

[Stefan Monnier]

Package: Emacs
Version: 25.0.50


If I do

    % emacs -Q --eval "(push '(font-backend x) default-frame-alist)"

I get a segfault.  This is with the latest emacs-25.
See the backtrace below.


        Stefan


Program received signal SIGSEGV, Segmentation fault.
0xf70b3b36 in XftCharIndex () from /usr/lib/i386-linux-gnu/libXft.so.2
(gdb) xbacktrace
(gdb) bt
#0  0xf70b3b36 in XftCharIndex () from /usr/lib/i386-linux-gnu/libXft.so.2
#1  0x082d4d69 in xftfont_encode_char (font=0x8779b00, c=92) at xftfont.c:537
#2  0x08074145 in get_char_glyph_code (c=<optimized out>, 
    font=font@entry=0x8779b00, char2b=char2b@entry=0xffffbbbe)
    at xdisp.c:24681
#3  0x080bdbfd in x_produce_glyphs (it=0xffffbc08) at xdisp.c:27011
#4  0x0807d80c in produce_special_glyphs (it=it@entry=0xffffc718, 
    what=what@entry=IT_CONTINUATION) at xdisp.c:26643
#5  0x0809bcd6 in init_iterator (it=<optimized out>, w=<optimized out>, 
    charpos=<optimized out>, bytepos=<optimized out>, row=<optimized out>, 
    base_face_id=<optimized out>) at xdisp.c:2856
#6  0x080b0bb4 in resize_mini_window (w=0x8852930, exact_p=true)
    at xdisp.c:10970
#7  0x080b0fa9 in resize_mini_window_1 (a1=142944560, exactly=...)
    at xdisp.c:10912
#8  0x080828d4 in with_echo_area_buffer (w=0x8852930, which=which@entry=0, 
    fn=fn@entry=0x80b0f91 <resize_mini_window_1>, a1=142944560, a2=...)
    at xdisp.c:10642
#9  0x080b7082 in resize_echo_area_exactly () at xdisp.c:10890
#10 0x081ad0d6 in command_loop_1 () at keyboard.c:1274
#11 0x0823977d in internal_condition_case (bfun=0x81ad053 <command_loop_1>, 
    handlers=..., hfun=0x819da9e <cmd_error>) at eval.c:1309
#12 0x081963cd in command_loop_2 (ignore=...) at keyboard.c:1099
#13 0x082396f4 in internal_catch (tag=..., func=0x81963ac <command_loop_2>, 
    arg=...) at eval.c:1074
#14 0x0819635c in command_loop () at keyboard.c:1078
#15 0x0819d535 in recursive_edit_1 () at keyboard.c:684
#16 0x0819d9b9 in Frecursive_edit () at keyboard.c:755
#17 0x08195adc in main (argc=<optimized out>, argv=0xffffd514) at emacs.c:1605
(gdb) 

bug#23386: Segfault when messing with font-backend

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Date: Wed, 27 Apr 2016 09:13:30 -0400
> 
> If I do
> 
>     % emacs -Q --eval "(push '(font-backend x) default-frame-alist)"
> 
> I get a segfault.  This is with the latest emacs-25.

Not reproducible here, but then I cannot run an X version.  (I did try
an equivalent for Windows; it didn't crash.)

> See the backtrace below.
> 
> 
>         Stefan
> 
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0xf70b3b36 in XftCharIndex () from /usr/lib/i386-linux-gnu/libXft.so.2
> (gdb) xbacktrace
> (gdb) bt
> #0  0xf70b3b36 in XftCharIndex () from /usr/lib/i386-linux-gnu/libXft.so.2
> #1  0x082d4d69 in xftfont_encode_char (font=0x8779b00, c=92) at xftfont.c:537

Can you tell which arguments to XftCharIndex were invalid in this
case?

Also, is 'x' a valid font-backend symbol?  It's strange that Emacs
uses xftfont when it should have been using xfont instead (AFAIU).
But I'm far from being an expert in this area.

Thanks.

bug#23386: Segfault when messing with font-backend

[Stefan Monnier]

>> % emacs -Q --eval "(push '(font-backend x) default-frame-alist)"
>> I get a segfault.  This is with the latest emacs-25.
> Not reproducible here, but then I cannot run an X version.  (I did try
> an equivalent for Windows; it didn't crash.)

Not surprised: it seems specific to the X11 code.

>> Program received signal SIGSEGV, Segmentation fault.
>> 0xf70b3b36 in XftCharIndex () from /usr/lib/i386-linux-gnu/libXft.so.2
>> (gdb) xbacktrace
>> (gdb) bt
>> #0  0xf70b3b36 in XftCharIndex () from /usr/lib/i386-linux-gnu/libXft.so.2
>> #1  0x082d4d69 in xftfont_encode_char (font=0x8779b00, c=92) at xftfont.c:537
> Can you tell which arguments to XftCharIndex were invalid in this
> case?

I guess it's the second arg (xftfont_info->xftfont), which is NULL.

    Program received signal SIGSEGV, Segmentation fault.
    0xf70b3b36 in XftCharIndex () from /usr/lib/i386-linux-gnu/libXft.so.2
    (gdb) up
    #1  0x082d4d69 in xftfont_encode_char (font=0x8779af0, c=92) at xftfont.c:537
    (gdb) p xftfont_info->display
    $1 = (Display *) 0x86f2368
    (gdb) p xftfont_info->xftfont
    $2 = (XftFont *) 0x0
    (gdb) p c
    $3 = 92
    (gdb)

> Also, is 'x' a valid font-backend symbol?

AFAIK yes.

> It's strange that Emacs uses xftfont when it should have been using
> xfont instead (AFAIU).

Indeed, that seems to be a big part of the problem.


        Stefan

bug#23386: Segfault when messing with font-backend

[npostavs]

[-- 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

bug#23386: Segfault when messing with font-backend

[npostavs]

npostavs@users.sourceforge.net writes:

> 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.

Definitely not the best way: it causes segfault on delete-frame.

bug#23386: Segfault when messing with font-backend

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Cc: 23386@debbugs.gnu.org,  Eli Zaretskii <eliz@gnu.org>
> Date: Sat, 09 Jul 2016 16:02:30 -0400
> 
> > 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.
> 
> Definitely not the best way: it causes segfault on delete-frame.

Backtrace from that segfault?

I think one idea that could be useful is to trace the creation of
relevant objects when Emacs starts up, starting with the call to
font_update_drivers, and then compare that with what happens in this
case.  That could delineate the missing parts and the differences
which could point the way to solving this cleanly.

Thanks.

bug#23386: Segfault when messing with font-backend

[npostavs]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: npostavs@users.sourceforge.net
>> Cc: 23386@debbugs.gnu.org,  Eli Zaretskii <eliz@gnu.org>
>> Date: Sat, 09 Jul 2016 16:02:30 -0400
>> 
>> > 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.
>> 
>> Definitely not the best way: it causes segfault on delete-frame.
>
> Backtrace from that segfault?

Hmm, looks like I just tried to kill the font twice, adding a NULL check
fixes it.


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

From f405410bf424b0a24a0eac54d12eeed758af95e9 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 9 Jul 2016 14:20:53 -0400
Subject: [PATCH v2] 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..21eb95a 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) && 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


[-- Attachment #3: backtrace --]
[-- Type: text/plain, Size: 2020 bytes --]

#0  0x000000000063f535 in font_finish_cache (f=0x1404710, driver=0xd0f800 <xfont_driver>) at font.c:2591
#1  0x0000000000642ccd in font_update_drivers (f=0x1404710, new_drivers=0) at font.c:3539
#2  0x0000000000429e82 in delete_frame (frame=20989717, force=0) at frame.c:1662
#3  0x000000000042a647 in Fdelete_frame (frame=0, force=0) at frame.c:1839
#4  0x0000000000624959 in Ffuncall (nargs=1, args=0x7fffffffdda8) at eval.c:2696
#5  0x000000000061afa2 in Ffuncall_interactively (nargs=1, args=0x7fffffffdda8) at callint.c:252
#6  0x00000000006247eb in Ffuncall (nargs=2, args=0x7fffffffdda0) at eval.c:2673
#7  0x000000000061d4ae in Fcall_interactively (function=17232, record_flag=0, keys=14537909) at callint.c:840
#8  0x0000000000624994 in Ffuncall (nargs=4, args=0x7fffffffe098) at eval.c:2700
#9  0x000000000066f3df in exec_byte_code (bytestr=10946140, vector=10946173, maxdepth=54, args_template=4102, nargs=1, 
    args=0x7fffffffe5f0) at bytecode.c:880
#10 0x0000000000625271 in funcall_lambda (fun=10946093, nargs=1, arg_vector=0x7fffffffe5e8) at eval.c:2855
#11 0x0000000000624ba8 in Ffuncall (nargs=2, args=0x7fffffffe5e0) at eval.c:2742
#12 0x00000000006242e6 in call1 (fn=14784, arg1=17232) at eval.c:2552
#13 0x00000000005789a1 in command_loop_1 () at keyboard.c:1479
#14 0x000000000062104a in internal_condition_case (bfun=0x578178 <command_loop_1>, handlers=19056, hfun=0x5777e6 <cmd_error>)
    at eval.c:1309
#15 0x0000000000577da5 in command_loop_2 (ignore=0) at keyboard.c:1107
#16 0x00000000006205dc in internal_catch (tag=45840, func=0x577d7c <command_loop_2>, arg=0) at eval.c:1074
#17 0x0000000000577d47 in command_loop () at keyboard.c:1086
#18 0x00000000005772be in recursive_edit_1 () at keyboard.c:692
#19 0x00000000005774ca in Frecursive_edit () at keyboard.c:763
#20 0x0000000000575262 in main (argc=2, argv=0x7fffffffea98) at emacs.c:1606

Lisp Backtrace:
"delete-frame" (0xffffddb0)
"funcall-interactively" (0xffffdda8)
"call-interactively" (0xffffe0a0)
"command-execute" (0xffffe5e8)

[-- Attachment #4: Type: text/plain, Size: 395 bytes --]


>
> I think one idea that could be useful is to trace the creation of
> relevant objects when Emacs starts up, starting with the call to
> font_update_drivers, and then compare that with what happens in this
> case.  That could delineate the missing parts and the differences
> which could point the way to solving this cleanly.

Yeah, I'm still having trouble seeing the forest for the trees.

bug#23386: Segfault when messing with font-backend

[Dmitry Antipov]

On 07/09/2016 10:11 PM, npostavs@users.sourceforge.net wrote:

> 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.

IMHO the original trick (request to drop font backend when there is a font
opened by using this backend) is practically meaningless, so why just not
prohibit it explicitly? For example, with:

diff --git a/src/frame.c b/src/frame.c
index 22143ab..d8f89ed 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -3708,7 +3708,19 @@ x_set_font_backend (struct frame *f, Lisp_Object new_value, Lisp_Object old_valu
      return;

    if (FRAME_FONT (f))
-    free_all_realized_faces (Qnil);
+    {
+      if (!NILP (new_value))
+	{
+	  Lisp_Object backend = FRAME_FONT (f)->props[FONT_TYPE_INDEX];
+
+	  /* Do not release the backend used by F's default font.  */
+	  if (NILP (Fmemq (backend, new_value)))
+	    error ("Font backend '%s' is in use by font '%s'",
+		   SDATA (SYMBOL_NAME (backend)),
+		   SDATA (FRAME_FONT (f)->props[FONT_NAME_INDEX]));
+	}
+      free_all_realized_faces (Qnil);
+    }

    new_value = font_update_drivers (f, NILP (new_value) ? Qt : new_value);
    if (NILP (new_value))

Dmitry

bug#23386: Segfault when messing with font-backend

[npostavs]

Dmitry Antipov <dmantipov@yandex.ru> writes:

> On 07/09/2016 10:11 PM, npostavs@users.sourceforge.net wrote:
>
>> 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.
>
> IMHO the original trick (request to drop font backend when there is a font
> opened by using this backend) is practically meaningless, so why just not
> prohibit it explicitly? For example, with:

Maybe that is a better idea; it does prevent the segfault, and it's
certainly simpler.

bug#23386: Segfault when messing with font-backend

[Eli Zaretskii]

> From: npostavs@users.sourceforge.net
> Date: Sun, 10 Jul 2016 16:17:00 -0400
> Cc: Stefan Monnier <monnier@IRO.UMontreal.CA>, 23386@debbugs.gnu.org
> 
> Dmitry Antipov <dmantipov@yandex.ru> writes:
> 
> > IMHO the original trick (request to drop font backend when there is a font
> > opened by using this backend) is practically meaningless, so why just not
> > prohibit it explicitly? For example, with:
> 
> Maybe that is a better idea; it does prevent the segfault, and it's
> certainly simpler.

But it doesn't do what the user asked for.

I don't see why it would be meaningless to evict a backend and start
using another one, if all it takes is re-open a bunch of fonts.

So I'm in favor of Noam's proposal, assuming that it works.

Noam, I encourage you to dig some more into this "forest", until you
convince yourself that the patch is sound.  While at that, please
consider adding comments where you learn useful things that are not
trivial to understand from the code alone, as this area of Emacs
sources is notoriously under-documented.

One issue that bothers me is this: what if additional fonts were
already opened for non-default faces?  Should they also get some
treatment?  (This is relevant to Dmitry's suggestion as well.)

I have one comment about your patch:

> 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);

This cannot be done only in xfns.c, as it will then break the other
platforms, because x_set_font_backend is not specific to X.

Thanks.

bug#23386: Segfault when messing with font-backend

[Dmitry Antipov]

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

On 07/11/2016 05:33 PM, Eli Zaretskii wrote:

> But it doesn't do what the user asked for.
>
> I don't see why it would be meaningless to evict a backend and start
> using another one, if all it takes is re-open a bunch of fonts.

OK, the following patch basically works for me (not tested too much, BTW).

Dmitry



[-- Attachment #2: bug23386.patch --]
[-- Type: text/x-diff, Size: 2222 bytes --]

diff --git a/src/dispextern.h b/src/dispextern.h
index c2fcca5..88b4077 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -3605,6 +3605,7 @@ extern Lisp_Object x_default_parameter (struct frame *, Lisp_Object,
                                         Lisp_Object, Lisp_Object,
                                         const char *, const char *,
                                         enum resource_types);
+extern void x_default_font_parameter (struct frame *, Lisp_Object);
 extern char *x_get_string_resource (XrmDatabase, const char *,
 				    const char *);
 
diff --git a/src/frame.c b/src/frame.c
index 80e181f..40d2d32 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -3712,7 +3712,11 @@ x_set_font_backend (struct frame *f, Lisp_Object new_value, Lisp_Object old_valu
     return;
 
   if (FRAME_FONT (f))
-    free_all_realized_faces (Qnil);
+    {
+      Lisp_Object frame;
+      XSETFRAME (frame, f);
+      free_all_realized_faces (frame);
+    }
 
   new_value = font_update_drivers (f, NILP (new_value) ? Qt : new_value);
   if (NILP (new_value))
@@ -3726,10 +3730,8 @@ x_set_font_backend (struct frame *f, Lisp_Object new_value, Lisp_Object old_valu
 
   if (FRAME_FONT (f))
     {
-      Lisp_Object frame;
-
-      XSETFRAME (frame, f);
-      x_set_font (f, Fframe_parameter (frame, Qfont), Qnil);
+      /* Reconsider default font after backend(s) change (Bug#23386).  */
+      x_default_font_parameter (f, Qnil);
       face_change = true;
       windows_or_buffers_changed = 18;
     }
diff --git a/src/w32fns.c b/src/w32fns.c
index 0eb720e..cffa8c0 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -5249,7 +5249,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 w32_display_info *dpyinfo = FRAME_DISPLAY_INFO (f);
diff --git a/src/xfns.c b/src/xfns.c
index 16dbcfd..fcd3886 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -3065,7 +3065,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);

bug#23386: Segfault when messing with font-backend

[Eli Zaretskii]

> Cc: 23386@debbugs.gnu.org
> From: Dmitry Antipov <dmantipov@yandex.ru>
> Date: Tue, 12 Jul 2016 18:20:41 +0300
> 
> > But it doesn't do what the user asked for.
> >
> > I don't see why it would be meaningless to evict a backend and start
> > using another one, if all it takes is re-open a bunch of fonts.
> 
> OK, the following patch basically works for me (not tested too much, BTW).

Thanks.

> --- a/src/frame.c
> +++ b/src/frame.c
> @@ -3712,7 +3712,11 @@ x_set_font_backend (struct frame *f, Lisp_Object new_value, Lisp_Object old_valu
>      return;
>  
>    if (FRAME_FONT (f))
> -    free_all_realized_faces (Qnil);
> +    {
> +      Lisp_Object frame;
> +      XSETFRAME (frame, f);
> +      free_all_realized_faces (frame);
> +    }

Since free_all_realized_faces with a nil argument will free faces on
all frames, can you tell why this hunk was needed?

bug#23386: Segfault when messing with font-backend

[Dmitry Antipov]

On 07/12/2016 08:45 PM, Eli Zaretskii wrote:

>> --- a/src/frame.c
>> +++ b/src/frame.c
>> @@ -3712,7 +3712,11 @@ x_set_font_backend (struct frame *f, Lisp_Object new_value, Lisp_Object old_valu
>>      return;
>>
>>    if (FRAME_FONT (f))
>> -    free_all_realized_faces (Qnil);
>> +    {
>> +      Lisp_Object frame;
>> +      XSETFRAME (frame, f);
>> +      free_all_realized_faces (frame);
>> +    }
>
> Since free_all_realized_faces with a nil argument will free faces on
> all frames, can you tell why this hunk was needed?

Hmm...hopefully there are no reasons to disturb other frames when we change
font backend(s) on the only one.

Dmitry