unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: --with-cairo Emacs server crash and fix
@ 2019-08-09 15:40 Liam Quinlan
  2019-08-10  9:39 ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Liam Quinlan @ 2019-08-09 15:40 UTC (permalink / raw)
  To: emacs-devel

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

It is definitely specific to cairo builds.  The 'fringe_bmp' array (not to
be confused with the 'fringe_bitmaps' array) has a cairo api type and is
declared behind #ifdef USE_CAIRO.

I *think* the minimal init file to reproduce is actually '-Q'... provided
you can get flymake to display its error fringe indicator, which
embarrassingly I cannot (no backends will run... definitely unrelated,
probably self inflicted).   Assuming you can accomplish this
(flymake-error-bitmap uses flymake-double-exclaimation-mark,
flymake-fringe-indicator-position isn't nil, fringe-mode is on, etc), the
steps to reproduce are very simple:

1. build --with-cairo, using any x toolkit
2. start an emacs server with --daemon option  (ie don't 'M-x server-mode')
3: connect normally with emacsclient
4: open a file with syntax errors and more than one screen of text
5: get flymake to display its error indicators
6: scroll up and down.  It won't take very long.

Should it prove easier somehow flycheck and sesman also define custom
bitmaps, either of which will do.  It can be any fringe-bitmap defined in
package that gets loaded during the daemon process's own startup.  Packages
that don't load until emacsclient connects are fine, as by that point an x
frame exists and SELECTED_FRAME will work.

I'm experiencing this building from head, but last I checked building the
26 branch didn't change anything.



Possibly also useful: the following gdb script is more or less what my
actual investigation lead to.  It will start the server process, and break
each time a fringe bitmap initialization needs to call
rif->define_fringe_bitmap but isn't able to. The values of 'which' when
this occurs will be the fringe_bmp indexes that go uninitialized and cause
the crash (culprit index for a given crash can be recovered from the
coredump).  It is possible to monkey-patch a server instance during startup
by manually calling x_redisplay_interface.define_fringe_bitmap yourself
each time the breakpoint fires.


set print pretty on
set print array on
set print array-indexes on

file path/to/built/emacs
set args --fg-daemon=cairo

break src/fringe.c:1481 if ! rif
commands
  printf "\nwhich: %d\n\n",which
  print "init_fringe_bitmap(): !(once_p || rif)"
  print "  after [...]"
  print "  if (!once_p) {"
  print "     struct redisplay_interface *rif = FRAME_RIF (SELECTED_FRAME
());'\n"
  list
end





also, improved diff below: exposes the x_redisplay_interface structure via
const pointer instead of directly

diff --git a/src/frame.h b/src/frame.h
index fa45a32d6b..fa4659eed0 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -1587,6 +1587,9 @@ #define EMACS_CLASS "Emacs"

 #if defined HAVE_X_WINDOWS
 extern void x_wm_set_icon_position (struct frame *, int, int);
+  #if defined USE_CAIRO
+  extern struct redisplay_interface const * const
x_redisplay_interface_ptr;
+  #endif
 #if !defined USE_X_TOOLKIT
 extern const char *x_get_resource_string (const char *, const char *);
 #endif
diff --git a/src/fringe.c b/src/fringe.c
index d0d599223d..a97c5e98c1 100644
--- a/src/fringe.c
+++ b/src/fringe.c
@@ -1482,6 +1482,10 @@ init_fringe_bitmap (int which, struct fringe_bitmap
*fb, int once_p)

       if (rif && rif->define_fringe_bitmap)
        rif->define_fringe_bitmap (which, fb->bits, fb->height, fb->width);
+#if (defined HAVE_X_WINDOWS) && (defined USE_CAIRO)
+      else
+        x_redisplay_interface_ptr->define_fringe_bitmap(which, fb->bits,
fb->height, fb->width);
+#endif

       fringe_bitmaps[which] = fb;
       if (which >= max_used_fringe_bitmap)
diff --git a/src/xterm.c b/src/xterm.c
index bbe68ef622..3cb779b39c 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -13334,6 +13334,7 @@ x_activate_timeout_atimer (void)
     x_hide_hourglass
   };

+struct redisplay_interface const * const x_redisplay_interface_ptr =
&x_redisplay_interface;

 /* This function is called when the last frame on a display is deleted. */
 void

[-- Attachment #2: Type: text/html, Size: 6093 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread
* --with-cairo Emacs server crash and fix
@ 2019-08-08  7:01 Liam Quinlan
  2019-08-09  7:11 ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Liam Quinlan @ 2019-08-08  7:01 UTC (permalink / raw)
  To: emacs-devel

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

If emacs is compiled for xwindows --with-cairo, then started with --daemon,
and any packages that define a fringe bitmap are used (including builtin
flymake), then 'define-fringe-bitmap' will be called from lisp during
startup and attempt to define dynamic bitmaps when emacs has no active
frame.

This is a problem because 'init_fringe_bitmap' (in src/fringe.c) relies on
the SELECTED_FRAME macro in order to access a redisplay_interface structure
and call 'rif->define_fringe_bitmap'.  Until a frame is created for a
client session, this process will fail, causing emacs to skip calling
'rif->define_fringe_bitmap' (which should initialize 'fringe_bmp[which]'
for cairo builds), but otherwise proceed as if the bitmap in question
exists.  (note the code in question already contains the following comment:
'/* XXX Is SELECTED_FRAME OK here? */' ... it is not.)

The upshot from there is, if[/when] emacs tries to *draw* these bitmaps, it
pulls a null pointer out of 'fringe_bmp[which]' and hands it straight to
libcairo.  Predictably, libcairo retaliates with the SEGV, and it's all
over but the coredumping.


As best as I can tell, the following diff resolves the issue (at least as
it manifests when using X; I haven't experimented with other
configurations).  Style-wise it's a touch crude and hamfisted, but tbh
trying to figure out the right approach for this codebase seemed to mean a
whole lot of stumbling through giant briar patches of macro indirection,
and I bailed.  Hopefully someone familiar with all that stuff can translate
it readily enough.
[git diff against commit d5622eb6fff94714c5d5a64c98c5e02bc1be478c]

diff --git a/src/frame.h b/src/frame.h
index fa45a32d6b..94a880f4eb 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -1587,6 +1587,9 @@ #define EMACS_CLASS "Emacs"

 #if defined HAVE_X_WINDOWS
 extern void x_wm_set_icon_position (struct frame *, int, int);
+  #if defined USE_CAIRO
+  extern struct redisplay_interface x_redisplay_interface;
+  #endif
 #if !defined USE_X_TOOLKIT
 extern const char *x_get_resource_string (const char *, const char *);
 #endif
diff --git a/src/fringe.c b/src/fringe.c
index d0d599223d..7a93afd418 100644
--- a/src/fringe.c
+++ b/src/fringe.c
@@ -1482,6 +1482,10 @@ init_fringe_bitmap (int which, struct fringe_bitmap
*fb, int once_p)

       if (rif && rif->define_fringe_bitmap)
        rif->define_fringe_bitmap (which, fb->bits, fb->height, fb->width);
+#if (defined HAVE_X_WINDOWS) && (defined USE_CAIRO)
+      else
+        x_redisplay_interface.define_fringe_bitmap(which, fb->bits,
fb->height, fb->width);
+#endif

       fringe_bitmaps[which] = fb;
       if (which >= max_used_fringe_bitmap)
diff --git a/src/xterm.c b/src/xterm.c
index bbe68ef622..76fc5c21ac 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -13298,7 +13298,7 @@ x_activate_timeout_atimer (void)

 extern frame_parm_handler x_frame_parm_handlers[];

-static struct redisplay_interface x_redisplay_interface =
+struct redisplay_interface x_redisplay_interface =
   {
     x_frame_parm_handlers,
     gui_produce_glyphs,

[-- Attachment #2: Type: text/html, Size: 4397 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-08-13 17:36 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-09 15:40 --with-cairo Emacs server crash and fix Liam Quinlan
2019-08-10  9:39 ` Eli Zaretskii
2019-08-12 10:42   ` Liam Quinlan
2019-08-12 10:48   ` Liam Quinlan
2019-08-12 16:59     ` Eli Zaretskii
2019-08-12 20:00       ` Liam Quinlan
2019-08-13  2:52       ` Liam Quinlan
2019-08-13 15:00         ` Eli Zaretskii
2019-08-13 17:36           ` Liam Quinlan
  -- strict thread matches above, loose matches on Subject: below --
2019-08-08  7:01 Liam Quinlan
2019-08-09  7:11 ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).