GNU bug report logs - #31373
26.1; frame-list-z-order segfaults on initial daemon frame

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 31373 in the body.
You can then email your comments to 31373 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.1; frame-list-z-order segfaults on initial daemon frame
Date: Sun, 06 May 2018 14:34:08 +0100

[gdb.txt (text/plain, attachment)]

[Message part 2 (text/plain, inline)]

On the latest emacs-26 and master branches, calling the function
frame-list-z-order crashes a daemon Emacs via segfault:

1. ./src/emacs -Q --daemon=<name>
2. ./lib-src/emacsclient --socket-name=<name> --create-frame
3. cd src
4. gdb -p <PID of step (1)>
5. Issue 'continue' in gdb
6. Evaluating (frame-list-z-order) in the emacsclient frame gives the
   following output in gdb:

Thread 1 "emacs" received signal SIGSEGV, Segmentation fault.
0x0000000000565112 in x_frame_list_z_order (dpy=0x33018f0, window=227) at xfns.c:5306
5306		    if (XFRAME (frame)->output_data.x->parent_desc == children[i]

Issuing 'set logging on' followed by 'bt full' in gdb at this point
gives the attached backtrace.

A quick step-through of x_frame_list_z_order indicates that the members
of the struct XFRAME (frame)->output_data are initialised to NULL for
the initial daemon frame F1.

Some information gathered by report-emacs-bug follows my signature.
Please let me know if I can provide any further information.

Thanks,

-- 
Basil

In GNU Emacs 26.1 (build 2, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2018-05-06 built on thunk
Repository revision: 91a68b5f61db50344c6a5df497f55370d54a7b15
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description:	Debian GNU/Linux testing (buster)

Configured using:
 'configure 'CC=ccache gcc' 'CFLAGS=-O0 -g3 -ggdb -gdwarf-4 -pipe'
 --config-cache --prefix=/home/blc/.local --program-suffix=26
 --enable-checking=yes,glyphs --enable-check-lisp-object-type
 --with-mailutils --with-x-toolkit=lucid --with-modules
 --with-file-notification=yes --with-x'

Configured features:
XAW3D XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS LUCID X11 MODULES THREADS LIBSYSTEMD LCMS2

Important settings:
  value of $LANG: en_IE.UTF-8
  locale-coding-system: utf-8-unix

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#31373: 26.1;
 frame-list-z-order segfaults on initial daemon frame
Date: Sun, 06 May 2018 10:14:13 -0400

[Message part 1 (text/plain, inline)]

tags 31373 + patch
quit

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> On the latest emacs-26 and master branches, calling the function
> frame-list-z-order crashes a daemon Emacs via segfault:
>
> 1. ./src/emacs -Q --daemon=<name>
> 2. ./lib-src/emacsclient --socket-name=<name> --create-frame
> 3. cd src
> 4. gdb -p <PID of step (1)>
> 5. Issue 'continue' in gdb

You can also use

1. gdb ./emacs -Q --fg-daemon=<name>
2. Issue 'run' in gdb
3. ../lib-src/emacsclient --socket-name=<name> --create-frame

> 6. Evaluating (frame-list-z-order) in the emacsclient frame gives the
>    following output in gdb:
>
> Thread 1 "emacs" received signal SIGSEGV, Segmentation fault.
> 0x0000000000565112 in x_frame_list_z_order (dpy=0x33018f0, window=227) at xfns.c:5306
> 5306		    if (XFRAME (frame)->output_data.x->parent_desc == children[i]

> A quick step-through of x_frame_list_z_order indicates that the members
> of the struct XFRAME (frame)->output_data are initialised to NULL for
> the initial daemon frame F1.

Right, I think we need to check the frame's output_method field first.
I wonder if there are other cases like this?

[0001-Don-t-check-non-X-frames-for-z-order-Bug-31373.patch (text/x-diff, inline)]

From b2c13a02a88fc8cc00c61ef8f56719fbb9b02897 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sun, 6 May 2018 10:07:25 -0400
Subject: [PATCH] Don't check non-X frames for z order (Bug#31373)

* src/xfns.c (x_frame_list_z_order): Only use frames with
`output_method' set to `output_x_window'.
---
 src/xfns.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/xfns.c b/src/xfns.c
index 20fe61bffd..29dd529e27 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -5303,8 +5303,9 @@ x_frame_list_z_order (Display* dpy, Window window)
 	    /* With a reparenting window manager the parent_desc field
 	       usually specifies the topmost windows of our frames.
 	       Otherwise FRAME_OUTER_WINDOW should do.  */
-	    if (XFRAME (frame)->output_data.x->parent_desc == children[i]
-		|| FRAME_OUTER_WINDOW (XFRAME (frame)) == children[i])
+	    if (XFRAME (frame)->output_method == output_x_window
+                && (XFRAME (frame)->output_data.x->parent_desc == children[i]
+                    || FRAME_OUTER_WINDOW (XFRAME (frame)) == children[i]))
 	      frames = Fcons (frame, frames);
 	}
 
-- 
2.11.0

Message #13 received at 31373 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 31373 <at> debbugs.gnu.org
Subject: Re: bug#31373: 26.1;
 frame-list-z-order segfaults on initial daemon frame
Date: Sun, 06 May 2018 15:58:37 +0100

Noam Postavsky <npostavs <at> gmail.com> writes:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> On the latest emacs-26 and master branches, calling the function
>> frame-list-z-order crashes a daemon Emacs via segfault:
>>
>> 1. ./src/emacs -Q --daemon=<name>
>> 2. ./lib-src/emacsclient --socket-name=<name> --create-frame
>> 3. cd src
>> 4. gdb -p <PID of step (1)>
>> 5. Issue 'continue' in gdb
>
> You can also use
>
> 1. gdb ./emacs -Q --fg-daemon=<name>
> 2. Issue 'run' in gdb
> 3. ../lib-src/emacsclient --socket-name=<name> --create-frame

Ah, yes, thank you.  I forgot about --fg-daemon (which I now remembered
is documented in '(emacs) Initial Options') because it is not mentioned
in the emacs(1) man page; do you think this option should be added there?

-- 
Basil

Message #16 received at 31373 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: 31373 <at> debbugs.gnu.org
Subject: Re: bug#31373: 26.1;
 frame-list-z-order segfaults on initial daemon frame
Date: Sun, 06 May 2018 12:28:16 -0400

[Message part 1 (text/plain, inline)]

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Ah, yes, thank you.  I forgot about --fg-daemon (which I now remembered
> is documented in '(emacs) Initial Options') because it is not mentioned
> in the emacs(1) man page; do you think this option should be added there?

Oh, yes it should be.

>> +	    if (XFRAME (frame)->output_method == output_x_window

Also, I noticed that there is FRAME_X_P for this.  I can't find any more
obvious instances of missing this check, although it's hard to tell in
many cases if the frame is really guaranteed to have output_x_window has
the output_method.

[v2-0001-Don-t-check-non-X-frames-for-z-order-Bug-31373.patch (text/x-diff, inline)]

From 5ad7571a59eb744c65a23198729ea2b56155a2c2 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sun, 6 May 2018 10:07:25 -0400
Subject: [PATCH v2] Don't check non-X frames for z order (Bug#31373)

* src/xfns.c (x_frame_list_z_order): Only use frames with
`output_method' set to `output_x_window'.
---
 src/xfns.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/xfns.c b/src/xfns.c
index 20fe61bffd..863fdefa5f 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -5303,8 +5303,9 @@ x_frame_list_z_order (Display* dpy, Window window)
 	    /* With a reparenting window manager the parent_desc field
 	       usually specifies the topmost windows of our frames.
 	       Otherwise FRAME_OUTER_WINDOW should do.  */
-	    if (XFRAME (frame)->output_data.x->parent_desc == children[i]
-		|| FRAME_OUTER_WINDOW (XFRAME (frame)) == children[i])
+            if (FRAME_X_P (frame)
+                && (XFRAME (frame)->output_data.x->parent_desc == children[i]
+                    || FRAME_OUTER_WINDOW (XFRAME (frame)) == children[i]))
 	      frames = Fcons (frame, frames);
 	}
 
-- 
2.11.0

Message #19 received at 31373 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 31373 <at> debbugs.gnu.org
Subject: Re: bug#31373: 26.1;
 frame-list-z-order segfaults on initial daemon frame
Date: Sun, 06 May 2018 19:49:50 +0300

> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Sun, 06 May 2018 12:28:16 -0400
> 
> >> +	    if (XFRAME (frame)->output_method == output_x_window
> 
> Also, I noticed that there is FRAME_X_P for this.  I can't find any more
> obvious instances of missing this check, although it's hard to tell in
> many cases if the frame is really guaranteed to have output_x_window has
> the output_method.

Don't we usually signal an error in such cases?  Through
check_x_display_info or somesuch?

Message #22 received at 31373 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 31373 <at> debbugs.gnu.org
Subject: Re: bug#31373: 26.1;
 frame-list-z-order segfaults on initial daemon frame
Date: Sun, 06 May 2018 18:03:03 +0100

Noam Postavsky <npostavs <at> gmail.com> writes:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> Ah, yes, thank you.  I forgot about --fg-daemon (which I now remembered
>> is documented in '(emacs) Initial Options') because it is not mentioned
>> in the emacs(1) man page; do you think this option should be added there?
>
> Oh, yes it should be.

Should I open a new bug for this?

-- 
Basil

Message #25 received at 31373 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 31373 <at> debbugs.gnu.org
Subject: Re: bug#31373: 26.1;
 frame-list-z-order segfaults on initial daemon frame
Date: Sun, 06 May 2018 13:09:33 -0400

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Noam Postavsky <npostavs <at> gmail.com>
>> Date: Sun, 06 May 2018 12:28:16 -0400
>> 
>> >> +	    if (XFRAME (frame)->output_method == output_x_window
>> 
>> Also, I noticed that there is FRAME_X_P for this.  I can't find any more
>> obvious instances of missing this check, although it's hard to tell in
>> many cases if the frame is really guaranteed to have output_x_window has
>> the output_method.
>
> Don't we usually signal an error in such cases?  Through
> check_x_display_info or somesuch?

Fx_frame_list_z_order already has a check_x_display_info call, but the
FRAME_X_P check is needed when looping over all the frames, like in
xic_create_xfontset and x_make_monitor_attribute_list.

Perhaps a FOR_EACH_FRAME_IN_TERMINAL macro would be less error-prone?

Message #28 received at 31373 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Noam Postavsky <npostavs <at> gmail.com>, 31373 <at> debbugs.gnu.org
Subject: Re: bug#31373: 26.1; frame-list-z-order segfaults on initial daemon
 frame
Date: Mon, 07 May 2018 08:26:01 +0200

-	    if (XFRAME (frame)->output_data.x->parent_desc == children[i]
-		|| FRAME_OUTER_WINDOW (XFRAME (frame)) == children[i])
+            if (FRAME_X_P (frame)
+                && (XFRAME (frame)->output_data.x->parent_desc == children[i]
+                    || FRAME_OUTER_WINDOW (XFRAME (frame)) == children[i]))

Thank you.  Eli, I think Noam should push this to 26.1, if still
possible.

martin

Message #31 received at 31373 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: npostavs <at> gmail.com, 31373 <at> debbugs.gnu.org
Subject: Re: bug#31373: 26.1;
 frame-list-z-order segfaults on initial daemon frame
Date: Mon, 07 May 2018 20:59:10 +0300

> Date: Mon, 07 May 2018 08:26:01 +0200
> From: martin rudalics <rudalics <at> gmx.at>
> 
> -	    if (XFRAME (frame)->output_data.x->parent_desc == children[i]
> -		|| FRAME_OUTER_WINDOW (XFRAME (frame)) == children[i])
> +            if (FRAME_X_P (frame)
> +                && (XFRAME (frame)->output_data.x->parent_desc == children[i]
> +                    || FRAME_OUTER_WINDOW (XFRAME (frame)) == children[i]))
> 
> Thank you.  Eli, I think Noam should push this to 26.1, if still
> possible.

Why?  The workaround is very simple: don't call that function in a
daemon session that has no GUI frames.

Message #34 received at 31373 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: martin rudalics <rudalics <at> gmx.at>, 31373 <at> debbugs.gnu.org
Subject: Re: bug#31373: 26.1;
 frame-list-z-order segfaults on initial daemon frame
Date: Mon, 7 May 2018 14:08:19 -0400

On 7 May 2018 at 13:59, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> Date: Mon, 07 May 2018 08:26:01 +0200
>> From: martin rudalics <rudalics <at> gmx.at>
>>
>> -         if (XFRAME (frame)->output_data.x->parent_desc == children[i]
>> -             || FRAME_OUTER_WINDOW (XFRAME (frame)) == children[i])
>> +            if (FRAME_X_P (frame)
>> +                && (XFRAME (frame)->output_data.x->parent_desc == children[i]
>> +                    || FRAME_OUTER_WINDOW (XFRAME (frame)) == children[i]))
>>
>> Thank you.  Eli, I think Noam should push this to 26.1, if still
>> possible.
>
> Why?  The workaround is very simple: don't call that function in a
> daemon session that has no GUI frames.

Ah, I think you missed that the segfault happens in a daemon session
that *does* have GUI frames.

I would also note that w32_frame_list_z_order makes the equivalent
FRAME_W32_P check via x_window_to_frame.

Message #37 received at 31373 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: rudalics <at> gmx.at, 31373 <at> debbugs.gnu.org
Subject: Re: bug#31373: 26.1;
 frame-list-z-order segfaults on initial daemon frame
Date: Mon, 07 May 2018 22:30:22 +0300

> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Mon, 7 May 2018 14:08:19 -0400
> Cc: martin rudalics <rudalics <at> gmx.at>, 31373 <at> debbugs.gnu.org
> 
> >> Thank you.  Eli, I think Noam should push this to 26.1, if still
> >> possible.
> >
> > Why?  The workaround is very simple: don't call that function in a
> > daemon session that has no GUI frames.
> 
> Ah, I think you missed that the segfault happens in a daemon session
> that *does* have GUI frames.

Ugh! why did no one bump into this until now??  We had this
function for a year!

OK, let's push it to emacs-26, and thanks...

Btw, is it reasonable for this function to return nil on TTY frames?
One frame is definitely displayed "on top".

Message #40 received at 31373 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rudalics <at> gmx.at, 31373 <at> debbugs.gnu.org
Subject: Re: bug#31373: 26.1;
 frame-list-z-order segfaults on initial daemon frame
Date: Mon, 07 May 2018 21:40:31 -0400

tags 31373 fixed
close 31373 26.1
quit

Eli Zaretskii <eliz <at> gnu.org> writes:

> OK, let's push it to emacs-26, and thanks...

Done [1: 1d9e66aea1].

> Btw, is it reasonable for this function to return nil on TTY frames?
> One frame is definitely displayed "on top".

I think it's not completely unreasonable to return nil, but it would
probably make more sense to return a list of frames starting with the
"on top" (displayed) frame.

[1: 1d9e66aea1]: 2018-05-07 21:35:25 -0400
  Don't check non-X frames for z order (Bug#31373)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=1d9e66aea17787e03954f32c6cd7561c881bb444

Message #47 received at 31373 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: 31373 <at> debbugs.gnu.org
Subject: Re: bug#31373: 26.1;
 frame-list-z-order segfaults on initial daemon frame
Date: Thu, 10 May 2018 22:51:12 -0400

"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Noam Postavsky <npostavs <at> gmail.com> writes:
>
>> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>>
>>> Ah, yes, thank you.  I forgot about --fg-daemon (which I now remembered
>>> is documented in '(emacs) Initial Options') because it is not mentioned
>>> in the emacs(1) man page; do you think this option should be added there?
>>
>> Oh, yes it should be.

> Should I open a new bug for this?

No need, I've pushed a fix to emacs-26.

[1: be2e8cb8be]: 2018-05-10 22:47:04 -0400
  * doc/man/emacs.1.in: Document --fg-daemon and --bg-daemon.
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=be2e8cb8be28affa830c21e2425573c3a1855a85>

Message #50 received at 31373 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 31373 <at> debbugs.gnu.org
Subject: Re: bug#31373: 26.1;
 frame-list-z-order segfaults on initial daemon frame
Date: Fri, 11 May 2018 09:29:21 +0100

Noam Postavsky <npostavs <at> gmail.com> writes:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> Should I open a new bug for this?
>
> No need, I've pushed a fix to emacs-26.
>
> [1: be2e8cb8be]: 2018-05-10 22:47:04 -0400
>   * doc/man/emacs.1.in: Document --fg-daemon and --bg-daemon.
>   https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=be2e8cb8be28affa830c21e2425573c3a1855a85

Thanks!

-- 
Basil

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.