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

[Jon TURNEY <jon.turney@dronecode.org.uk>]

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

On 01/04/2015 18:36, 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.
>>
>> What we really need is the context of the thread when calling
>> call_signal_handler I think.
>
> I had the same thought, but this is going to be quite tricky to achieve.

>> It would be better to call RtlCaptureContext
>> before calling call_signal_handler.  But this requires a change in how
>> call_signal_handler is called.
>>
>> We should discuss this at one point, I think.

I noticed that we already prepare a context for continuing after the 
signal for the debugger, so perhaps this is not quite as complex as I 
thought and something like the attached is needed.

It's very hard to reason about if this is doing the right thing when the 
signal is delivered across threads, though.


[-- Attachment #2: 0001-Use-the-same-continuation-context-in-a-signal-as-we-.patch --]
[-- Type: text/plain, Size: 1694 bytes --]

From d6385aa66e26e86832e1e95222e8026146bb63df Mon Sep 17 00:00:00 2001
From: Jon TURNEY <jon.turney@dronecode.org.uk>
Date: Thu, 23 Apr 2015 14:45:05 +0100
Subject: [PATCH] Use the same continuation context in a signal as we would
 send to the debugger

---
 winsup/cygwin/exceptions.cc | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 4a6c21e..c97cc19 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1503,12 +1503,7 @@ _cygtls::call_signal_handler ()
 	  if (thissi.si_cyg)
 	    memcpy (&context.uc_mcontext, ((cygwin_exception *)thissi.si_cyg)->context(), sizeof(CONTEXT));
 	  else
-	    {
-	      /* FIXME: Really this should be the context which the signal interrupted? */
-	      memset(&context.uc_mcontext, 0, sizeof(struct __mcontext));
-	      context.uc_mcontext.ctxflags = CONTEXT_FULL;
-	      RtlCaptureContext ((CONTEXT *)&context.uc_mcontext);
-	    }
+	    memcpy (&context.uc_mcontext, &thread_context, sizeof(CONTEXT));
 
 	  /* FIXME: If/when sigaltstack is implemented, this will need to do
 	     something more complicated */
@@ -1549,9 +1544,10 @@ void
 _cygtls::signal_debugger (siginfo_t& si)
 {
   HANDLE th;
-  /* If si.si_cyg is set then the signal was already sent to the debugger. */
+  /* If si.si_cyg is set then the signal was caused by an exception which has
+     already been sent to the debugger. */
   if (isinitialized () && !si.si_cyg && (th = (HANDLE) *this)
-      && being_debugged () && SuspendThread (th) >= 0)
+      && SuspendThread (th) >= 0)
     {
       CONTEXT c;
       c.ContextFlags = CONTEXT_FULL;
-- 
2.1.4