public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH] Make EXCEPTION_POINTERS available to signal handlers
@ 2015-03-26 15:25 Jon TURNEY
  2015-03-30 10:21 ` Corinna Vinschen
  0 siblings, 1 reply; 9+ messages in thread
From: Jon TURNEY @ 2015-03-26 15:25 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Jon TURNEY

Add ucontext.h header, defining ucontext_t and mcontext_t types.

Provide sigaction sighandlers with a ucontext_t parameter containing a
mcontext_t with exception context information, if available.

	* include/sys/ucontext.h (__ucontext): New header.
	* include/ucontext.h (_UCONTEXT_H): Ditto.
	* exception.h (cygwin_exception): Add exception_record accessor.
	* exceptions.cc (call_signal_handler): Provide ucontext_t
	parameter to signal handler function, if available.

Signed-off-by: Jon TURNEY <jon.turney@dronecode.org.uk>
---
 winsup/cygwin/ChangeLog              |  8 ++++++++
 winsup/cygwin/exception.h            |  1 +
 winsup/cygwin/exceptions.cc          | 16 ++++++++++++++--
 winsup/cygwin/include/sys/ucontext.h | 27 +++++++++++++++++++++++++++
 winsup/cygwin/include/ucontext.h     | 16 ++++++++++++++++
 5 files changed, 66 insertions(+), 2 deletions(-)
 create mode 100644 winsup/cygwin/include/sys/ucontext.h
 create mode 100644 winsup/cygwin/include/ucontext.h

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 869beee..ce09390 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,11 @@
+2015-03-26  Jon TURNEY  <jon.turney@dronecode.org.uk>
+
+	* include/sys/ucontext.h (__ucontext): New header.
+	* include/ucontext.h (_UCONTEXT_H): Ditto.
+	* exception.h (cygwin_exception): Add exception_record accessor.
+	* exceptions.cc (call_signal_handler): Provide ucontext_t
+	parameter to signal handler function, if available.
+
 2015-03-25  Corinna Vinschen  <corinna@vinschen.de>
 
 	* include/sys/termios.h: Add CMIN and CTIME.
diff --git a/winsup/cygwin/exception.h b/winsup/cygwin/exception.h
index 3686bb0..0478daf 100644
--- a/winsup/cygwin/exception.h
+++ b/winsup/cygwin/exception.h
@@ -175,4 +175,5 @@ public:
     framep (in_framep), ctx (in_ctx), e (in_e), h (NULL) {}
   void dumpstack ();
   PCONTEXT context () const {return ctx;}
+  EXCEPTION_RECORD *exception_record () const {return e;}
 };
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 3af9a54..33ff989 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -16,6 +16,7 @@ details. */
 #include <stdlib.h>
 #include <syslog.h>
 #include <wchar.h>
+#include <ucontext.h>
 
 #include "cygtls.h"
 #include "pinfo.h"
@@ -1487,8 +1488,20 @@ _cygtls::call_signal_handler ()
       /* Save information locally on stack to pass to handler. */
       int thissig = sig;
       siginfo_t thissi = infodata;
+      ucontext_t *thiscontext = NULL;
       void (*thisfunc) (int, siginfo_t *, void *) = func;
 
+      ucontext_t context;
+      memset(&context, 0, sizeof(ucontext_t)); /* no ucontext_t information provided yet */
+      EXCEPTION_POINTERS ep;
+      if (thissi.si_cyg)
+        {
+          ep.ExceptionRecord = ((cygwin_exception *)thissi.si_cyg)->exception_record();
+          ep.ContextRecord = ((cygwin_exception *)thissi.si_cyg)->context();
+          context.uc_mcontext.ep = &ep;
+          thiscontext = &context;
+        }
+
       sigset_t this_oldmask = set_process_mask_delta ();
       int this_errno = saved_errno;
       reset_signal_arrived ();
@@ -1496,8 +1509,7 @@ _cygtls::call_signal_handler ()
       sig = 0;		/* Flag that we can accept another signal */
       unlock ();	/* unlock signal stack */
 
-      /* no ucontext_t information provided yet, so third arg is NULL */
-      thisfunc (thissig, &thissi, NULL);
+      thisfunc (thissig, &thissi, thiscontext);
       incyg = true;
 
       set_signal_mask (_my_tls.sigmask, this_oldmask);
diff --git a/winsup/cygwin/include/sys/ucontext.h b/winsup/cygwin/include/sys/ucontext.h
new file mode 100644
index 0000000..a44f854
--- /dev/null
+++ b/winsup/cygwin/include/sys/ucontext.h
@@ -0,0 +1,27 @@
+/* ucontext.h
+
+   Copyright 2015 Red Hat, Inc.
+
+This file is part of Cygwin.
+
+This software is a copyrighted work licensed under the terms of the
+Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
+details. */
+
+#ifndef _SYS_UCONTEXT_H_
+#define _SYS_UCONTEXT_H_
+
+#include <signal.h>
+
+typedef struct __mcontext {
+	EXCEPTION_POINTERS *ep;
+} mcontext_t;
+
+typedef struct __ucontext {
+	struct __ucontext *uc_link;
+	sigset_t	uc_sigmask;
+	// We don't have a type stack_t, so we don't have a uc_stack member
+	mcontext_t	uc_mcontext;
+} ucontext_t;
+
+#endif /* !_SYS_UCONTEXT_H_ */
diff --git a/winsup/cygwin/include/ucontext.h b/winsup/cygwin/include/ucontext.h
new file mode 100644
index 0000000..4240597
--- /dev/null
+++ b/winsup/cygwin/include/ucontext.h
@@ -0,0 +1,16 @@
+/* ucontext.h
+
+   Copyright 2015 Red Hat, Inc.
+
+This file is part of Cygwin.
+
+This software is a copyrighted work licensed under the terms of the
+Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
+details. */
+
+#ifndef _UCONTEXT_H
+#define _UCONTEXT_H
+
+#include <sys/ucontext.h>
+
+#endif /* _UCONTEXT_H */
-- 
2.1.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Make EXCEPTION_POINTERS available to signal handlers
  2015-03-26 15:25 [PATCH] Make EXCEPTION_POINTERS available to signal handlers Jon TURNEY
@ 2015-03-30 10:21 ` Corinna Vinschen
  2015-03-30 17:33   ` Jon TURNEY
  0 siblings, 1 reply; 9+ messages in thread
From: Corinna Vinschen @ 2015-03-30 10:21 UTC (permalink / raw)
  To: cygwin-patches

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

Hi Jon,

On Mar 26 15:25, Jon TURNEY wrote:
> Add ucontext.h header, defining ucontext_t and mcontext_t types.
> 
> Provide sigaction sighandlers with a ucontext_t parameter containing a
> mcontext_t with exception context information, if available.
> 
> 	* include/sys/ucontext.h (__ucontext): New header.
> 	* include/ucontext.h (_UCONTEXT_H): Ditto.
> 	* exception.h (cygwin_exception): Add exception_record accessor.
> 	* exceptions.cc (call_signal_handler): Provide ucontext_t
> 	parameter to signal handler function, if available.

Thanks for this patch.  Looks like a good idea.

But there are a few problems we should discuss first.

> +typedef struct __mcontext {
> +	EXCEPTION_POINTERS *ep;
> +} mcontext_t;
> +
> +typedef struct __ucontext {
> +	struct __ucontext *uc_link;
> +	sigset_t	uc_sigmask;
> +	// We don't have a type stack_t, so we don't have a uc_stack member
> +	mcontext_t	uc_mcontext;
> +} ucontext_t;

* Per the Linux man page this structure should be called `struct
  ucontext' without the underscores.  However, we already have
  definitions of `struct ucontext' in include/cygwin/signal.h... which
  have nothing at all to do with the definition of ucontext on Linux.

  This needs fixing.

  Historically, the ucontext definition in cygwin/signal.h is equivalent
  to the Windows thread CONTEXT definition.  Why ever CONTEXT was used to
  define ucontext beats me in retrospect.
  
  It is used only when sending the thread context to a debugger when a
  signal occured.  The definition of struct ucontext from cygwin/signal.h
  is only used as part of struct _cygtls (struct ucontext thread_context)
  and then thread_context is used in _cygtls::signal_debugger, that's it.
  Cygwin doesn't use it anywhere else.
  
  GDB uses only the definition of __COPY_CONTEXT_SIZE from cygwin/signal.h,
  but not the definition of struct ucontext.  I sent a patch upstream to
  get rid of that dependency.

  We should remove or rename struct ucontext in cygwin/signal.h, so we
  can use that name for your above struct __ucontext.  That leads to the
  next point...

* Since struct ucontext from cygwin/signal.h is actually a redefinition
  of CONTEXT + an oldmask value. it's basically the Cygwin/Windows
  representation of gregset_t + fpregset_t + cr2 + oldmask, aka
  mcontext_t.

  As for oldmask, this can be fetched easily from _my_tls, so in theory
  we can use the current definition of ucontext from cygwin/signal.h as
  mcontext_t.

  But this drops the EXCEPTION_RECORD.  I'm not sure it belongs here.
  Keep in mind that a signal handler is not only called in case an
  exception occured.  I think the context is all a signal handler needs.

* As for stack_t, we have that.  It's defined in newlib's sys/signal.h.
  The stack base and stack size can be fetched from the TEB; with a
  test for a user-defined stack, see pthread_wrapper in miscfuncs.cc.

  While we're at it we should contemplate to define SIGSTKSZ and
  MINSIGSTKSZ along the lines of 64K, I guess.


Thanks,
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 --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/2] Make CONTEXT available to signal handlers
  2015-03-30 17:33   ` Jon TURNEY
  2015-03-30 17:33     ` [PATCH 1/2] Rename struct ucontext to struct mcontext Jon TURNEY
@ 2015-03-30 17:33     ` Jon TURNEY
  2015-03-30 19:12       ` Corinna Vinschen
  1 sibling, 1 reply; 9+ messages in thread
From: Jon TURNEY @ 2015-03-30 17:33 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Jon TURNEY

Add ucontext.h header, defining ucontext_t and mcontext_t types.

Provide sigaction sighandlers with a ucontext_t parameter containing a
mcontext_t with exception context information, if available.

	* include/sys/ucontext.h : New header.
	* include/ucontext.h : Ditto.
	* exceptions.cc (call_signal_handler): Provide ucontext_t
	parameter to signal handler function, if available.
---
 winsup/cygwin/ChangeLog              |  7 +++++++
 winsup/cygwin/exceptions.cc          | 13 +++++++++++--
 winsup/cygwin/include/sys/ucontext.h | 22 ++++++++++++++++++++++
 winsup/cygwin/include/ucontext.h     | 16 ++++++++++++++++
 4 files changed, 56 insertions(+), 2 deletions(-)
 create mode 100644 winsup/cygwin/include/sys/ucontext.h
 create mode 100644 winsup/cygwin/include/ucontext.h

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index cfc29a8..8da5fa2 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,5 +1,12 @@
 2015-03-30  Jon TURNEY  <jon.turney@dronecode.org.uk>
 
+	* include/sys/ucontext.h : New header.
+	* include/ucontext.h : Ditto.
+	* exceptions.cc (call_signal_handler): Provide ucontext_t
+	parameter to signal handler function, if available.
+
+2015-03-30  Jon TURNEY  <jon.turney@dronecode.org.uk>
+
 	* include/cygwin/signal.h : Rename struct ucontext to struct mcontext.
 	Remove unused member oldmask and simplify __COPY_CONTEXT_SIZE.
 
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index af53457..80899d1 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -16,6 +16,7 @@ details. */
 #include <stdlib.h>
 #include <syslog.h>
 #include <wchar.h>
+#include <ucontext.h>
 
 #include "cygtls.h"
 #include "pinfo.h"
@@ -1487,8 +1488,17 @@ _cygtls::call_signal_handler ()
       /* Save information locally on stack to pass to handler. */
       int thissig = sig;
       siginfo_t thissi = infodata;
+      ucontext_t *thiscontext = NULL;
       void (*thisfunc) (int, siginfo_t *, void *) = func;
 
+      ucontext_t context;
+      memset(&context, 0, sizeof(ucontext_t)); /* no ucontext_t information provided yet */
+      if (thissi.si_cyg)
+        {
+          memcpy(&context.uc_mcontext, ((cygwin_exception *)thissi.si_cyg)->context(), sizeof(CONTEXT));
+          thiscontext = &context;
+        }
+
       sigset_t this_oldmask = set_process_mask_delta ();
       int this_errno = saved_errno;
       reset_signal_arrived ();
@@ -1496,8 +1506,7 @@ _cygtls::call_signal_handler ()
       sig = 0;		/* Flag that we can accept another signal */
       unlock ();	/* unlock signal stack */
 
-      /* no ucontext_t information provided yet, so third arg is NULL */
-      thisfunc (thissig, &thissi, NULL);
+      thisfunc (thissig, &thissi, thiscontext);
       incyg = true;
 
       set_signal_mask (_my_tls.sigmask, this_oldmask);
diff --git a/winsup/cygwin/include/sys/ucontext.h b/winsup/cygwin/include/sys/ucontext.h
new file mode 100644
index 0000000..df9b32b
--- /dev/null
+++ b/winsup/cygwin/include/sys/ucontext.h
@@ -0,0 +1,22 @@
+/* ucontext.h
+
+   Copyright 2015 Red Hat, Inc.
+
+This file is part of Cygwin.
+
+This software is a copyrighted work licensed under the terms of the
+Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
+details. */
+
+#ifndef _SYS_UCONTEXT_H_
+#define _SYS_UCONTEXT_H_
+
+#include <signal.h>
+
+typedef struct mcontext mcontext_t;
+
+typedef struct ucontext {
+	mcontext_t	uc_mcontext;
+} ucontext_t;
+
+#endif /* !_SYS_UCONTEXT_H_ */
diff --git a/winsup/cygwin/include/ucontext.h b/winsup/cygwin/include/ucontext.h
new file mode 100644
index 0000000..4240597
--- /dev/null
+++ b/winsup/cygwin/include/ucontext.h
@@ -0,0 +1,16 @@
+/* ucontext.h
+
+   Copyright 2015 Red Hat, Inc.
+
+This file is part of Cygwin.
+
+This software is a copyrighted work licensed under the terms of the
+Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
+details. */
+
+#ifndef _UCONTEXT_H
+#define _UCONTEXT_H
+
+#include <sys/ucontext.h>
+
+#endif /* _UCONTEXT_H */
-- 
2.1.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] Rename struct ucontext to struct mcontext
  2015-03-30 17:33   ` Jon TURNEY
@ 2015-03-30 17:33     ` Jon TURNEY
  2015-03-30 18:47       ` Corinna Vinschen
  2015-03-30 17:33     ` [PATCH 2/2] Make CONTEXT available to signal handlers Jon TURNEY
  1 sibling, 1 reply; 9+ messages in thread
From: Jon TURNEY @ 2015-03-30 17:33 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Jon TURNEY

	* include/cygwin/signal.h : Rename struct ucontext to struct mcontext.
	Remove unused member oldmask and simplify __COPY_CONTEXT_SIZE.
---
 winsup/cygwin/ChangeLog               |  5 +++++
 winsup/cygwin/include/cygwin/signal.h | 16 +++++++++-------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 505f4ce..cfc29a8 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,8 @@
+2015-03-30  Jon TURNEY  <jon.turney@dronecode.org.uk>
+
+	* include/cygwin/signal.h : Rename struct ucontext to struct mcontext.
+	Remove unused member oldmask and simplify __COPY_CONTEXT_SIZE.
+
 2015-03-30  Corinna Vinschen  <corinna@vinschen.de>
 
 	* cygtls.h (struct _cygtls): Convert thread_context to type CONTEXT.
diff --git a/winsup/cygwin/include/cygwin/signal.h b/winsup/cygwin/include/cygwin/signal.h
index 58bbff0..47093b9 100644
--- a/winsup/cygwin/include/cygwin/signal.h
+++ b/winsup/cygwin/include/cygwin/signal.h
@@ -18,6 +18,10 @@
 extern "C" {
 #endif
 
+/*
+  Define a struct mcontext, which should be identical to the Win32 API type
+  CONTEXT
+*/
 #ifdef __x86_64__
 
 struct _uc_fpxreg {
@@ -45,7 +49,7 @@ struct _fpstate
   __uint32_t padding[24];
 };
 
-struct ucontext
+struct mcontext
 {
   __uint64_t p1home;
   __uint64_t p2home;
@@ -93,7 +97,6 @@ struct ucontext
   __uint64_t etr;
   __uint64_t efr;
   __uint8_t _internal;
-  __uint64_t oldmask;
 };
 
 #else /* !x86_64 */
@@ -117,7 +120,7 @@ struct _fpstate
   __uint32_t nxst;
 };
 
-struct ucontext
+struct mcontext
 {
   __uint32_t cr2;
   __uint32_t dr0;
@@ -144,14 +147,13 @@ struct ucontext
   __uint32_t esp;
   __uint32_t ss;
   __uint8_t _internal;
-  __uint32_t oldmask;
 };
 
 #endif /* !x86_64 */
 
-/* Needed for GDB.   It only compiles in the context copy code if this
-   macro s defined. */
-#define __COPY_CONTEXT_SIZE ((size_t) (uintptr_t) &((struct ucontext *) 0)->_internal)
+/* Needed for GDB 7.9 and earlier.  It only compiles in the context copy code if
+   this macro is defined. */
+#define __COPY_CONTEXT_SIZE (sizeof(struct mcontext))
 
 typedef union sigval
 {
-- 
2.1.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Make EXCEPTION_POINTERS available to signal handlers
  2015-03-30 10:21 ` Corinna Vinschen
@ 2015-03-30 17:33   ` Jon TURNEY
  2015-03-30 17:33     ` [PATCH 1/2] Rename struct ucontext to struct mcontext Jon TURNEY
  2015-03-30 17:33     ` [PATCH 2/2] Make CONTEXT available to signal handlers Jon TURNEY
  0 siblings, 2 replies; 9+ messages in thread
From: Jon TURNEY @ 2015-03-30 17:33 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Jon TURNEY

Thanks for your comments.

>    We should remove or rename struct ucontext in cygwin/signal.h, so we
>    can use that name for your above struct __ucontext.  That leads to the
>    next point...

Ok, building on your clean-up, this is [1/2]

> * Since struct ucontext from cygwin/signal.h is actually a redefinition
>    of CONTEXT + an oldmask value. it's basically the Cygwin/Windows
>    representation of gregset_t + fpregset_t + cr2 + oldmask, aka
>    mcontext_t.
> 
>    As for oldmask, this can be fetched easily from _my_tls, so in theory
>    we can use the current definition of ucontext from cygwin/signal.h as
>    mcontext_t.
> 
>    But this drops the EXCEPTION_RECORD.  I'm not sure it belongs here.
>    Keep in mind that a signal handler is not only called in case an
>    exception occured.  I think the context is all a signal handler needs.

Yes, I had my doubts that EXCEPTION_RECORD didn't belong ucontext.

I've writen [2/2] to just provide mcontext_t (= CONTEXT) as part of the 
ucontext_t.

But ultimately, I'd like to have access to EXCEPTION_RECORD to drive some 
Windows-specific crash reporting code, so I'll take another look at how that 
might be done...

> * As for stack_t, we have that.  It's defined in newlib's sys/signal.h.
>    The stack base and stack size can be fetched from the TEB; with a
>    test for a user-defined stack, see pthread_wrapper in miscfuncs.cc.
> 
>    While we're at it we should contemplate to define SIGSTKSZ and
>    MINSIGSTKSZ along the lines of 64K, I guess.

For the moment, I have omitted the POSIX-required uc_link, uc_sigmask and 
uc_stack members from ucontext_t. They can be added when I understand how to 
give them meaningful values.

Jon TURNEY (2):
  Rename struct ucontext to struct mcontext
  Make CONTEXT available to signal handlers

 winsup/cygwin/ChangeLog               | 12 ++++++++++++
 winsup/cygwin/exceptions.cc           | 13 +++++++++++--
 winsup/cygwin/include/cygwin/signal.h | 16 +++++++++-------
 winsup/cygwin/include/sys/ucontext.h  | 22 ++++++++++++++++++++++
 winsup/cygwin/include/ucontext.h      | 16 ++++++++++++++++
 5 files changed, 70 insertions(+), 9 deletions(-)
 create mode 100644 winsup/cygwin/include/sys/ucontext.h
 create mode 100644 winsup/cygwin/include/ucontext.h

-- 
2.1.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] Rename struct ucontext to struct mcontext
  2015-03-30 17:33     ` [PATCH 1/2] Rename struct ucontext to struct mcontext Jon TURNEY
@ 2015-03-30 18:47       ` Corinna Vinschen
  2015-03-31 17:48         ` Jon TURNEY
  0 siblings, 1 reply; 9+ messages in thread
From: Corinna Vinschen @ 2015-03-30 18:47 UTC (permalink / raw)
  To: cygwin-patches

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

Just for the records what we talked about on IRC:

On Mar 30 18:32, Jon TURNEY wrote:
> @@ -45,7 +49,7 @@ struct _fpstate
>    __uint32_t padding[24];
>  };
>  
> -struct ucontext
> +struct mcontext

__mcontext so as not to pollute the namespace.

>    __uint64_t etr;
>    __uint64_t efr;
>    __uint8_t _internal;
> -  __uint64_t oldmask;
>  };

Remove _internal, keep oldmask.  As a result, __mcontext is still
basically equivalent to Linux' mcontext_t.  __mcontext can be
taken from _my_tls.oldmask.


Thanks,
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 --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] Make CONTEXT available to signal handlers
  2015-03-30 17:33     ` [PATCH 2/2] Make CONTEXT available to signal handlers Jon TURNEY
@ 2015-03-30 19:12       ` Corinna Vinschen
  0 siblings, 0 replies; 9+ messages in thread
From: Corinna Vinschen @ 2015-03-30 19:12 UTC (permalink / raw)
  To: cygwin-patches

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

On Mar 30 18:32, Jon TURNEY wrote:
> +typedef struct ucontext {
> +	mcontext_t	uc_mcontext;
> +} ucontext_t;

I liked that better as before.  Keep in mind that changing, improving
user-space provided structures practically always requires to add
code to account for old and for newly built applications, the ones
which only know the old struct layout, and the new ones knowning the
new layout.

Therefore we should define ucontext_t in a way which does not require
to change it later.  Looking at the Linux definition:

  typedef struct ucontext
  {
    unsigned long int uc_flags;
    struct ucontext *uc_link;
    stack_t uc_stack;
    mcontext_t uc_mcontext;
    __sigset_t uc_sigmask;
    struct _libc_fpstate __fpregs_mem;
  } ucontext_t;

We won't have __fpregs_mem, ever, since it's part of uc_mcontext (see
the comments in sys/ucontext.h) so this can be dropped.  But everything
else we can and should provide.  uc_link might come in handy as well at
one point, who knows?  Just set it to NULL for now.


Thanks,
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 --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] Rename struct ucontext to struct mcontext
  2015-03-30 18:47       ` Corinna Vinschen
@ 2015-03-31 17:48         ` Jon TURNEY
  2015-03-31 19:03           ` Corinna Vinschen
  0 siblings, 1 reply; 9+ messages in thread
From: Jon TURNEY @ 2015-03-31 17:48 UTC (permalink / raw)
  To: cygwin-patches

On 30/03/2015 19:47, Corinna Vinschen wrote:
> Just for the records what we talked about on IRC:
>
> On Mar 30 18:32, Jon TURNEY wrote:
>> @@ -45,7 +49,7 @@ struct _fpstate
>>     __uint32_t padding[24];
>>   };
>>
>> -struct ucontext
>> +struct mcontext
>
> __mcontext so as not to pollute the namespace.
>
>>     __uint64_t etr;
>>     __uint64_t efr;
>>     __uint8_t _internal;
>> -  __uint64_t oldmask;
>>   };
>
> Remove _internal, keep oldmask.  As a result, __mcontext is still
> basically equivalent to Linux' mcontext_t.  __mcontext can be
> taken from _my_tls.oldmask.

Thanks for your help with this.

You'll have to help me understand what the difference in meaning between 
ucontext_t.uc_sigmask and ucontext_t.uc_mcontext.oldmask is.

In the context of _cygtls::call_signal_handler() is _my_tls.oldmask 
correct and not this_oldmask?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] Rename struct ucontext to struct mcontext
  2015-03-31 17:48         ` Jon TURNEY
@ 2015-03-31 19:03           ` Corinna Vinschen
  0 siblings, 0 replies; 9+ messages in thread
From: Corinna Vinschen @ 2015-03-31 19:03 UTC (permalink / raw)
  To: cygwin-patches

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

On Mar 31 18:48, Jon TURNEY wrote:
> On 30/03/2015 19:47, Corinna Vinschen wrote:
> >Just for the records what we talked about on IRC:
> >
> >On Mar 30 18:32, Jon TURNEY wrote:
> >>@@ -45,7 +49,7 @@ struct _fpstate
> >>    __uint32_t padding[24];
> >>  };
> >>
> >>-struct ucontext
> >>+struct mcontext
> >
> >__mcontext so as not to pollute the namespace.
> >
> >>    __uint64_t etr;
> >>    __uint64_t efr;
> >>    __uint8_t _internal;
> >>-  __uint64_t oldmask;
> >>  };
> >
> >Remove _internal, keep oldmask.  As a result, __mcontext is still
> >basically equivalent to Linux' mcontext_t.  __mcontext can be
> >taken from _my_tls.oldmask.
> 
> Thanks for your help with this.
> 
> You'll have to help me understand what the difference in meaning between
> ucontext_t.uc_sigmask and ucontext_t.uc_mcontext.oldmask is.
> 
> In the context of _cygtls::call_signal_handler() is _my_tls.oldmask correct
> and not this_oldmask?

Yes, this_oldmask should be the right one.


Thanks,
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 --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-03-31 19:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-26 15:25 [PATCH] Make EXCEPTION_POINTERS available to signal handlers Jon TURNEY
2015-03-30 10:21 ` Corinna Vinschen
2015-03-30 17:33   ` Jon TURNEY
2015-03-30 17:33     ` [PATCH 1/2] Rename struct ucontext to struct mcontext Jon TURNEY
2015-03-30 18:47       ` Corinna Vinschen
2015-03-31 17:48         ` Jon TURNEY
2015-03-31 19:03           ` Corinna Vinschen
2015-03-30 17:33     ` [PATCH 2/2] Make CONTEXT available to signal handlers Jon TURNEY
2015-03-30 19:12       ` Corinna Vinschen

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).