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

[Jon TURNEY <jon dot turney at dronecode dot org dot uk>]

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

However, I'm not sure that is worthwhile effort as it's heading in the 
wrong direction, because ....

> What we really need is the context of the thread when calling
> call_signal_handler I think.  It would be better to call RtlCaptureContext
> before calling call_signal_handler.  But this requires a change in how
> call_signal_handler is called.

Attachment: 0001-Avoid-random-crashes-in-RtlCaptureContext-on-x86.patch
Description: Text document