[Patch Darwin/Ada] work around PR target/50678

[Patch Darwin/Ada] work around PR target/50678

[Iain Sandoe]

The audit trail in the PR contains the detective work (mostly by Eric)  
that concludes we have a long-standing bug in the Darwin x86-64  
sigtramp unwind data.
This has been filed as radar #10302855, but we need a work-around  
until that is resolved (possibly forever on older systems).

OK for trunk?
(what opinion about 4.6?)

ada:

	PR target/50678
	* init.c (Darwin/__gnat_error_handler): Transpose rbx and rdx in the
	handler.

Index: gcc/ada/init.c
===================================================================
--- gcc/ada/init.c	(revision 180097)
+++ gcc/ada/init.c	(working copy)
@@ -2287,6 +2287,16 @@ __gnat_error_handler (int sig, siginfo_t *si,  
void
  {
    struct Exception_Data *exception;
    const char *msg;
+#if defined (__x86_64__)
+  /* Work around radar #10302855/pr50678, where the unwinders  
(libunwind or
+     libgcc_s depending on the system revision) and the DWARF unwind  
data for
+     the sigtramp have different ideas about register numbering  
(causing rbx
+     and rdx to be transposed).  */
+  ucontext_t *uc = (ucontext_t *)ucontext ;
+  unsigned long t = uc->uc_mcontext->__ss.__rbx;
+  uc->uc_mcontext->__ss.__rbx = uc->uc_mcontext->__ss.__rdx;
+  uc->uc_mcontext->__ss.__rdx =t;
+#endif

    switch (sig)
      {

Re: [Patch Darwin/Ada] work around PR target/50678

[Arnaud Charlet]

> The audit trail in the PR contains the detective work (mostly by Eric) that
> concludes we have a long-standing bug in the Darwin x86-64 sigtramp unwind
> data.
> This has been filed as radar #10302855, but we need a work-around until
> that is resolved (possibly forever on older systems).
> 
> OK for trunk?
> (what opinion about 4.6?)
> 
> ada:
> 
> 	PR target/50678
> 	* init.c (Darwin/__gnat_error_handler): Transpose rbx and rdx in the
> 	handler.

Here is Tristan's review on the above patch:

--
According to the PR, this is ok.
(Maybe we should restrict the swap to the known broken libc ?)

Tristan.

Re: [Patch Darwin/Ada] work around PR target/50678

[Iain Sandoe]

On 18 Oct 2011, at 13:22, Arnaud Charlet wrote:

>> The audit trail in the PR contains the detective work (mostly by  
>> Eric) that
>> concludes we have a long-standing bug in the Darwin x86-64 sigtramp  
>> unwind
>> data.
>> This has been filed as radar #10302855, but we need a work-around  
>> until
>> that is resolved (possibly forever on older systems).
>>
>> OK for trunk?
>> (what opinion about 4.6?)
>>
>> ada:
>>
>> 	PR target/50678
>> 	* init.c (Darwin/__gnat_error_handler): Transpose rbx and rdx in the
>> 	handler.
>
> Here is Tristan's review on the above patch:
>
> --
> According to the PR, this is ok.
> (Maybe we should restrict the swap to the known broken libc ?)

It's broken in all Libc versions that are in the wild (AFAICT from  
looking at the released sources).

We will need to deal with configury/ 
__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ stuff once there is a  
fixed Libc.

Iain

Re: [Patch Darwin/Ada] work around PR target/50678

[Arnaud Charlet]

> It's broken in all Libc versions that are in the wild (AFAICT from looking
> at the released sources).
> 
> We will need to deal with 
> configury/__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ stuff once
> there is a fixed Libc.

OK, would be good to follow up with such patch when/if this is fixed.

Arno

Re: [Patch Darwin/Ada] work around PR target/50678

[Iain Sandoe]

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


On 18 Oct 2011, at 13:31, Arnaud Charlet wrote:

>> It's broken in all Libc versions that are in the wild (AFAICT from  
>> looking
>> at the released sources).
>>
>> We will need to deal with
>> configury/__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ stuff once
>> there is a fixed Libc.
>
> OK, would be good to follow up with such patch when/if this is fixed.

We are waiting for input re. the actual bug (in system code).

In the meantime,  I've applied the patch as a (hopefully temporary)  
workaround.

cheers
Iain


[-- Attachment #2: 180612-pr50678-diff.txt --]
[-- Type: text/plain, Size: 1341 bytes --]

Index: gcc/ada/ChangeLog
===================================================================
--- gcc/ada/ChangeLog	(revision 180612)
+++ gcc/ada/ChangeLog	(working copy)
@@ -1,3 +1,11 @@
+2011-10-28  Iain Sandoe  <iains@gcc.gnu.org>
+	    Eric Botcazou  <ebotcazou@adacore.com>
+
+	PR target/50678
+	* init.c (Darwin/__gnat_error_handler): Apply a work-around to the
+	bug [filed as radar #10302855], which is inconsistent unwind data
+	for sigtramp.
+
 2011-10-28  Eric Botcazou  <ebotcazou@adacore.com>
 
 	PR ada/50842
Index: gcc/ada/init.c
===================================================================
--- gcc/ada/init.c	(revision 180612)
+++ gcc/ada/init.c	(working copy)
@@ -2287,6 +2287,16 @@ __gnat_error_handler (int sig, siginfo_t *si, void
 {
   struct Exception_Data *exception;
   const char *msg;
+#if defined (__x86_64__)
+  /* Work around radar #10302855/pr50678, where the unwinders (libunwind or
+     libgcc_s depending on the system revision) and the DWARF unwind data for
+     the sigtramp have different ideas about register numbering (causing rbx
+     and rdx to be transposed)..  */
+  ucontext_t *uc = (ucontext_t *)ucontext ;
+  unsigned long t = uc->uc_mcontext->__ss.__rbx;
+  uc->uc_mcontext->__ss.__rbx = uc->uc_mcontext->__ss.__rdx;
+  uc->uc_mcontext->__ss.__rdx = t;
+#endif
 
   switch (sig)
     {

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

Re: [Patch Darwin/Ada] work around PR target/50678

[Mike Stump]

On Oct 18, 2011, at 4:58 AM, Iain Sandoe wrote:
> The audit trail in the PR contains the detective work (mostly by Eric) that concludes we have a long-standing bug in the Darwin x86-64 sigtramp unwind data.

Yeah, well, it is a bug, if they ever fix it.  If they burn it in and define it as correct, then, it is an unfortunatism.  If they fix it, it should be software updated to older system and all tools fixed to use the new scheme.  I suspect they'll never fix it.

> This has been filed as radar #10302855, but we need a work-around until that is resolved (possibly forever on older systems).

> +#if defined (__x86_64__)

If this is for a target machine compile, this is fine.  Looking through the code, it seems like target machine code, though, I wasn't sure.  If it is for host code, then this isn't cross compile safe.  For host code DARWIN_X86 and TARGET_64BIT (from tm.h) both need to be true, for one to be compiling for a 64-bit x86 machine.

I think this is fine for relevant release branches.

Re: [Patch Darwin/Ada] work around PR target/50678

[Eric Botcazou]

> This has been filed as radar #10302855, but we need a work-around
> until that is resolved (possibly forever on older systems).
>
> OK for trunk?
> (what opinion about 4.6?)

Did you apply it to the 4.6 branch?  I think that this would be appropriate.

> ada:
>
> 	PR target/50678
> 	* init.c (Darwin/__gnat_error_handler): Transpose rbx and rdx in the
> 	handler.

Sorry, I overlooked something here: there is a specific procedure to make this 
kind of adjustments.  The reason is that the adjustment needs to be made in 
the tasking case as well and __gnat_error_handler isn't used for this case.

So HAVE_GNAT_ADJUST_CONTEXT_FOR_RAISE must be defined in the Darwin-specific 
section and __gnat_adjust_context_for_raise implemented (with the standard 
prototype) and __gnat_error_handler changed to call it instead.

Would you mind adjusting the fix that way?  Thanks in advance.

-- 
Eric Botcazou

Re: [Patch Darwin/Ada] work around PR target/50678

[Iain Sandoe]

On 12 Nov 2011, at 07:44, Eric Botcazou wrote:

>> This has been filed as radar #10302855, but we need a work-around
>> until that is resolved (possibly forever on older systems).
>>
>> OK for trunk?
>> (what opinion about 4.6?)
>
> Did you apply it to the 4.6 branch?  I think that this would be  
> appropriate.
>
>> ada:
>>
>> 	PR target/50678
>> 	* init.c (Darwin/__gnat_error_handler): Transpose rbx and rdx in the
>> 	handler.
>
> Sorry, I overlooked something here: there is a specific procedure to  
> make this
> kind of adjustments.  The reason is that the adjustment needs to be  
> made in
> the tasking case as well and __gnat_error_handler isn't used for  
> this case.
>
> So HAVE_GNAT_ADJUST_CONTEXT_FOR_RAISE must be defined in the Darwin- 
> specific
> section and __gnat_adjust_context_for_raise implemented (with the  
> standard
> prototype) and __gnat_error_handler changed to call it instead.
>
> Would you mind adjusting the fix that way?  Thanks in advance.

the following is reg-strapping on x86-64-darwin10,
OK for trunk (and 4.6) if it succeeds (with a ChangeLog, of course) ?
Iain

Index: gcc/ada/init.c
===================================================================
--- gcc/ada/init.c	(revision 181448)
+++ gcc/ada/init.c	(working copy)
@@ -2282,11 +2282,12 @@ __gnat_is_stack_guard (mach_vm_address_t addr)
    return 0;
  }

-static void
-__gnat_error_handler (int sig, siginfo_t *si, void *ucontext  
ATTRIBUTE_UNUSED)
+#define HAVE_GNAT_ADJUST_CONTEXT_FOR_RAISE
+
+void
+__gnat_adjust_context_for_raise (int signo ATTRIBUTE_UNUSED,
+				 void *ucontext ATTRIBUTE_UNUSED)
  {
-  struct Exception_Data *exception;
-  const char *msg;
  #if defined (__x86_64__)
    /* Work around radar #10302855/pr50678, where the unwinders  
(libunwind or
       libgcc_s depending on the system revision) and the DWARF unwind  
data for
@@ -2294,10 +2295,23 @@ __gnat_is_stack_guard (mach_vm_address_t addr)
       and rdx to be transposed)..  */
    ucontext_t *uc = (ucontext_t *)ucontext ;
    unsigned long t = uc->uc_mcontext->__ss.__rbx;
+
    uc->uc_mcontext->__ss.__rbx = uc->uc_mcontext->__ss.__rdx;
    uc->uc_mcontext->__ss.__rdx = t;
  #endif
+}

+static void
+__gnat_error_handler (int sig, siginfo_t *si, void *ucontext  
ATTRIBUTE_UNUSED)
+{
+  struct Exception_Data *exception;
+  const char *msg;
+
+#if defined (__x86_64__)
+  /* Work around #10302855/pr50678 on x86-64 Darwin.  */
+  __gnat_adjust_context_for_raise (sig, ucontext);
+#endif
+
    switch (sig)
      {
      case SIGSEGV:

Re: [Patch Darwin/Ada] work around PR target/50678

[Eric Botcazou]

> the following is reg-strapping on x86-64-darwin10,

Thanks a lot!

> OK for trunk (and 4.6) if it succeeds (with a ChangeLog, of course) ?

Yes, modulo:

> +static void
> +__gnat_error_handler (int sig, siginfo_t *si, void *ucontext
> ATTRIBUTE_UNUSED)

Remove ATTRIBUTE_UNUSED and...

> +#if defined (__x86_64__)
> +  /* Work around #10302855/pr50678 on x86-64 Darwin.  */
> +  __gnat_adjust_context_for_raise (sig, ucontext);
> +#endif

...remove everything except for the call to _gnat_adjust_context_for_raise.

-- 
Eric Botcazou