Hi all,

(sorry for top posting here, I just mean to keep the patch at th end).

I think this patch raise the question of the intended use of FatalError() in 
the Xserver.

 - as an exit() on error:
   port out of range, host not on supported network type, unknown host, etc.
   screen init failed, invalid configuration, 
   
 - as a abort() / assert()
   memory allocation failures, fork failure, GLSL compilation failures, 

 - and in Xwayland for socket issues

For /some/ it would make sense to (optionally, with "-core") generate a core 
dump (things liek memory allocation, GLSL, shaders isses, etc.), for other not 
so much (Wayland socket errors, wrong configuration or command line options).

So, which is the intended use of FatalError() and do we have an equivalent for 
those cases where it's not the intended use?

Cheers,
Olivier

   
----- Original Message -----
> Xwayland is a pretty standard Wayland client, we want to be able to
> capture core dumps on crashes.
> 
> Yet using "-core" in the command line is not what we want, because that
> causes any FatalError() to generate a core dump, meaning that we would
> get a core file for all Wayland server crashes, which would generate a
> lot of false positives.
> 
> Besides, for most case, the xorg_backtrace() generated is not sufficient
> and rather ineffective compared to a regular debugger session.
> 
> Restore the default signal handlers for Xwayland so that the usual
> signals (SIGSEGV, SIGABRT, SIGILL, etc.) will possibly generate a core
> file (depending on the OS configuration of course).
> 
> See also: https://bugzilla.gnome.org/show_bug.cgi?id=790502
>      and: https://bugzilla.gnome.org/show_bug.cgi?id=789086
> 
> Signed-off-by: Olivier Fourdan <ofour...@redhat.com>
> ---
>  hw/xwayland/xwayland.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
> index 81e669cae..f327db748 100644
> --- a/hw/xwayland/xwayland.c
> +++ b/hw/xwayland/xwayland.c
> @@ -845,6 +845,22 @@ wm_selection_callback(CallbackListPtr *p, void *data,
> void *arg)
>      DeleteCallback(&SelectionCallback, wm_selection_callback, xwl_screen);
>  }
>  
> +static void
> +reset_default_signals (void)
> +{
> +    OsSignal(SIGSEGV, SIG_DFL);
> +    OsSignal(SIGABRT, SIG_DFL);
> +    OsSignal(SIGILL, SIG_DFL);
> +#ifdef SIGEMT
> +    OsSignal(SIGEMT, SIG_DFL);
> +#endif
> +    OsSignal(SIGFPE, SIG_DFL);
> +    OsSignal(SIGBUS, SIG_DFL);
> +    OsSignal(SIGSYS, SIG_DFL);
> +    OsSignal(SIGXCPU, SIG_DFL);
> +    OsSignal(SIGXFSZ, SIG_DFL);
> +}
> +
>  static Bool
>  xwl_screen_init(ScreenPtr pScreen, int argc, char **argv)
>  {
> @@ -853,6 +869,9 @@ xwl_screen_init(ScreenPtr pScreen, int argc, char **argv)
>      Pixel red_mask, blue_mask, green_mask;
>      int ret, bpc, green_bpc, i;
>  
> +    /* For Xwayland, we want to be able to capture core files on crashes */
> +    reset_default_signals();
> +
>      xwl_screen = calloc(1, sizeof *xwl_screen);
>      if (xwl_screen == NULL)
>          return FALSE;
> --
> 2.14.3
> 
> _______________________________________________
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to