Re: [PATCH 2/3] Provide ucontext to signal handlers

[Corinna Vinschen <corinna-cygwin@cygwin.com>]

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

On Apr  3 23:09, Jon TURNEY wrote:
> On 01/04/2015 15:22, Corinna Vinschen wrote:
> >On Apr  1 14:19, Jon TURNEY wrote:
> >>Add ucontext.h header, defining ucontext_t and mcontext_t types.
> >>
> >>Provide sigaction sighandlers with a ucontext_t parameter, containing stack and
> >>context information.
> >>
> >>	* include/sys/ucontext.h : New header.
> >>	* include/ucontext.h : Ditto.
> >>	* exceptions.cc (call_signal_handler): Provide ucontext_t
> >>	parameter to signal handler function.
> >
> >Patch is ok with a single change:  Please add a "FIXME?" comment to:
> >
> >   else
> >     RtlCaptureContext();
> >
> >On second thought, calling RtlCaptureContext here is probably wrong.
> 
> Wrong and also dangerous.
> 
> This causes random crashes on x86.
> 
> It seems that RtlCaptureContext requires the framepointer of the calling
> function in ebp, which it uses to report the rip and rsp of it's caller.
> 
> It also seems that gcc can decide to optimize the setting of the
> framepointer away, irrespective of the fact that -fomit-frame-pointer is not
> used when building exceptions.cc
> 
> If _cygtls::call_signal_handler() happens to get called with ebp pointing to
> an invalid memory address, as seems to happen occasionally, we will fault in
> RtlCaptureContext.  (in all cases, the eip and ebp in the returned context
> are incorrect)
> 
> I wrote the attached patch, which fakes a callframe for RtlCaptureContext to
> avoid these possible crashes, but this needs more work to correctly report
> eip and ebp

Maybe it's simpler than that?  Looking into the GCC info pages, I found
this:

     Starting with GCC version 4.6, the default setting (when not
     optimizing for size) for 32-bit GNU/Linux x86 and 32-bit Darwin x86
     targets has been changed to '-fomit-frame-pointer'.  The default
     can be reverted to '-fno-omit-frame-pointer' by configuring GCC
     with the '--enable-frame-pointer' configure option.

     Enabled at levels '-O', '-O2', '-O3', '-Os'.
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

So it seems adding -fomit-frame-pointer file by file in Makefile.in
(when building with -O2) is moot and only has an effect when building
unoptimized, otherwise all files are built with -fomit-frame-pointer
anyway.

So, what if we drop all the -fomit-frame-pointer from Makefile.in and
add an

  exceptions_CFLAGS:=-fno-omit-frame-pointer

Does that help?


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]