User account creation filtered due to spam.

Bug 50678 - [4.7/4.8 Regression] FAIL: c52104y on x86_64-apple-darwin10
Summary: [4.7/4.8 Regression] FAIL: c52104y on x86_64-apple-darwin10
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.7.0
: P4 normal
Target Milestone: 4.7.3
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-09 12:56 UTC by Dominique d'Humieres
Modified: 2013-02-09 18:10 UTC (History)
9 users (show)

See Also:
Host: x86_64-apple-darwin10
Target: x86_64-apple-darwin10
Build: x86_64-apple-darwin10
Known to work:
Known to fail:
Last reconfirmed: 2011-10-11 00:00:00


Attachments
asm for c52104y (4.69 KB, text/plain)
2011-10-11 15:57 UTC, Iain Sandoe
Details
test of -fstack-check (1.50 KB, text/plain)
2011-10-11 18:45 UTC, Iain Sandoe
Details
Assembly file for the code in attachment 25466 (1.28 KB, text/plain)
2011-10-12 11:55 UTC, Dominique d'Humieres
Details
updated stack check test (1.63 KB, text/plain)
2011-10-12 12:27 UTC, Iain Sandoe
Details
optimized tree dump for c52104y (3.55 KB, text/plain)
2011-10-12 14:07 UTC, Iain Sandoe
Details
test of a foreign except thrown from a sig handler in c++ (3.10 KB, text/plain)
2011-10-17 09:58 UTC, Iain Sandoe
Details
revised (3.23 KB, application/octet-stream)
2011-10-17 12:13 UTC, Iain Sandoe
Details
demonstration of the fault using c++/vendor's tools (2.75 KB, text/plain)
2011-10-18 11:05 UTC, Iain Sandoe
Details
Patch to suppress register swap on Darwin >= 12 (902 bytes, patch)
2013-02-05 15:33 UTC, simon
Details | Diff
Patch to suppress register swap on Darwin >= 12, v2 (935 bytes, patch)
2013-02-05 22:37 UTC, simon
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dominique d'Humieres 2011-10-09 12:56:15 UTC
Between revisions 179635 (OK) and 179703 (fails) the acats test c52104y has started to fail on x86_64-apple-darwin10 (it is one of the three failing tests on powerpc-apple-darwin9). The failure is:

splitting /opt/gcc/build_w/gcc/testsuite/ada/acats/tests/c5/c52104y.ada into:
   c52104y.adb
BUILD c52104y.adb
gnatmake --GCC="/opt/gcc/build_w/gcc/xgcc -B/opt/gcc/build_w/gcc/" -gnatws -O2 -fstack-check -I/opt/gcc/build_w/gcc/testsuite/ada/acats/support c52104y.adb -largs --GCC="/opt/gcc/build_w/gcc/xgcc -B/opt/gcc/build_w/gcc/"
/opt/gcc/build_w/gcc/xgcc -c -B/opt/gcc/build_w/gcc/ -gnatws -O2 -fstack-check -I/opt/gcc/build_w/gcc/testsuite/ada/acats/support c52104y.adb
gnatbind -I/opt/gcc/build_w/gcc/testsuite/ada/acats/support -x c52104y.ali
gnatlink c52104y.ali -O2 -fstack-check --GCC=/opt/gcc/build_w/gcc/xgcc -B/opt/gcc/build_w/gcc/
RUN c52104y

,.,. C52104Y ACATS 2.5 11-10-08 14:57:10
---- C52104Y CHECK THAT IN ARRAY ASSIGNMENTS AND IN SLICE ASSIGNMENTS,
                THE LENGTHS MUST MATCH.
   - C52104Y NO CONSTRAINT_ERROR FOR NON-NULL ARRAY SUBTYPE WHEN ONE
                DIMENSION HAS INTEGER'LAST + 3 COMPONENTS.

raised CONSTRAINT_ERROR : erroneous memory access
FAIL:   c52104y
Comment 1 Eric Botcazou 2011-10-09 20:59:46 UTC
It is one of the 6 stack checking tests so this isn't totally unexpected.  Fixing this probably requires low-level fiddling with the OS (e.g. see ada/init.c) so I cannot help for Darwin.
Comment 2 Dominique d'Humieres 2011-10-10 15:41:05 UTC
This is due to/exposed by revision 179655:

Author:	vries
Date:	Fri Oct 7 12:49:49 2011 UTC (3 days, 2 hours ago)
Changed paths:	15
Log Message:	
2011-10-07  Tom de Vries  <tom@codesourcery.com>

	PR middle-end/50527
	* tree.c (build_common_builtin_nodes): Add local_define_builtin for
	* builtins.c (expand_builtin_alloca): Handle BUILT_IN_ALLOCA_WITH_ALIGN
	* tree-ssa-ccp.c (evaluate_stmt): Set align for
	* builtins.def (BUILT_IN_ALLOCA_WITH_ALIGN): Declare using
	* ipa-pure-const.c (special_builtin_state): Handle
	* tree-ssa-alias.c (ref_maybe_used_by_call_p_1)
	* function.c (gimplify_parameters): Lower vla to
	* gimplify.c (gimplify_vla_decl): Same.
	* cfgexpand.c (expand_call_stmt): Handle BUILT_IN_ALLOCA_WITH_ALIGN.
	* tree-mudflap.c (mf_xform_statements): Same.
	* tree-ssa-dce.c (mark_stmt_if_obviously_necessary)
	* varasm.c (incorporeal_function_p): Same.
	* tree-object-size.c (alloc_object_size): Same.
	* gimple.c (gimple_build_call_from_tree): Same.
Comment 3 Dominique d'Humieres 2011-10-10 19:34:06 UTC
On powerpc-apple-darwin9.8.0, c52103x, c52104x, and c52104y fail with a different error:

raised STORAGE_ERROR : stack overflow detected
Comment 4 Iain Sandoe 2011-10-10 20:04:41 UTC
(In reply to comment #3)

No acats regressions on i686-darwin9 (at 179745).

> On powerpc-apple-darwin9.8.0, c52103x, c52104x, and c52104y fail with a
> different error:
> 
> raised STORAGE_ERROR : stack overflow detected

I suspect that  that these tests have never passed on powerpc-darwin9 - since they were introduced during the time that the platform would not bootstrap Ada.  in any event, this is a distinct problem from the one reported here.
Comment 5 Dominique d'Humieres 2011-10-10 20:11:10 UTC
(In reply to comment #4)

> > On powerpc-apple-darwin9.8.0, c52103x, c52104x, and c52104y fail with a
> > different error:
> > 
> > raised STORAGE_ERROR : stack overflow detected
>
> I suspect that  that these tests have never passed on powerpc-darwin9 - since
> they were introduced during the time that the platform would not bootstrap Ada.
> in any event, this is a distinct problem from the one reported here.

Note pr20548 (now closed as fixed) have been opened for failures of these tests.
Comment 6 Eric Botcazou 2011-10-11 06:19:39 UTC
> 2011-10-07  Tom de Vries  <tom@codesourcery.com>
> 
>     PR middle-end/50527
>     * tree.c (build_common_builtin_nodes): Add local_define_builtin for
>     * builtins.c (expand_builtin_alloca): Handle BUILT_IN_ALLOCA_WITH_ALIGN
>     * tree-ssa-ccp.c (evaluate_stmt): Set align for
>     * builtins.def (BUILT_IN_ALLOCA_WITH_ALIGN): Declare using
>     * ipa-pure-const.c (special_builtin_state): Handle
>     * tree-ssa-alias.c (ref_maybe_used_by_call_p_1)
>     * function.c (gimplify_parameters): Lower vla to
>     * gimplify.c (gimplify_vla_decl): Same.
>     * cfgexpand.c (expand_call_stmt): Handle BUILT_IN_ALLOCA_WITH_ALIGN.
>     * tree-mudflap.c (mf_xform_statements): Same.
>     * tree-ssa-dce.c (mark_stmt_if_obviously_necessary)
>     * varasm.c (incorporeal_function_p): Same.
>     * tree-object-size.c (alloc_object_size): Same.
>     * gimple.c (gimple_build_call_from_tree): Same.

OK, I didn't realize that things were still ongoing on this front.  Frankly, this alloca folding seems to introduce far too much complication for what it's worth.

Tom, did you consider reverting the whole thing and doing things differently?

If your canonical example is something like:

const int n = 5;
int array[n];

it's very easy to fold this in the front-end into:

const int n = 5;
int array[5];

As a matter of fact, that's what we do in Ada, which is by far the biggest user of alloca in the compiler.
Comment 7 Tom de Vries 2011-10-11 10:45:04 UTC
> OK, I didn't realize that things were still ongoing on this front.

There was still a bug lingering: PR50527, an inconsistency between actual alignment after folding and alignment propagated by ccp.

That got us to rethink the alignment for allocas. Using the alloca_with_align,
we can transport the DECL_ALIGN of the vla decl to:
- the alloca folding, where we can use it on the replacement decl, and
- expand_builtin_alloca where we can lower the required alignment passed to
  allocate_dynamic_stack_space

The latter means that we change something also when we're not folding allocas.

> Frankly, this alloca folding seems to introduce far too much complication for
> what it's worth.
> Tom, did you consider reverting the whole thing and doing things differently?
> If your canonical example is something like:
>
> const int n = 5;
> int array[n];
>
> it's very easy to fold this in the front-end into:
>
> const int n = 5;
> int array[5];

I think I tried that in the beginning, but didn't manage to get that working. 
But apart from that, doing it in the front-end doesn't take care of more complicated examples, f.i. where the array size only becomes constant after inlining.

I'm now trying to reproduce this failure on x86_64.
Comment 8 Richard Biener 2011-10-11 11:02:08 UTC
(In reply to comment #6)
> > 2011-10-07  Tom de Vries  <tom@codesourcery.com>
> > 
> >     PR middle-end/50527
> >     * tree.c (build_common_builtin_nodes): Add local_define_builtin for
> >     * builtins.c (expand_builtin_alloca): Handle BUILT_IN_ALLOCA_WITH_ALIGN
> >     * tree-ssa-ccp.c (evaluate_stmt): Set align for
> >     * builtins.def (BUILT_IN_ALLOCA_WITH_ALIGN): Declare using
> >     * ipa-pure-const.c (special_builtin_state): Handle
> >     * tree-ssa-alias.c (ref_maybe_used_by_call_p_1)
> >     * function.c (gimplify_parameters): Lower vla to
> >     * gimplify.c (gimplify_vla_decl): Same.
> >     * cfgexpand.c (expand_call_stmt): Handle BUILT_IN_ALLOCA_WITH_ALIGN.
> >     * tree-mudflap.c (mf_xform_statements): Same.
> >     * tree-ssa-dce.c (mark_stmt_if_obviously_necessary)
> >     * varasm.c (incorporeal_function_p): Same.
> >     * tree-object-size.c (alloc_object_size): Same.
> >     * gimple.c (gimple_build_call_from_tree): Same.
> 
> OK, I didn't realize that things were still ongoing on this front.  Frankly,
> this alloca folding seems to introduce far too much complication for what it's
> worth.

I think more properly tracking alignment requirements on VLAs makes sense
to avoid excessive stack (re-)alignment.  So the above patch would be
independent on whether we fold the allocas to automatic vars or not.

> Tom, did you consider reverting the whole thing and doing things differently?
> 
> If your canonical example is something like:
> 
> const int n = 5;
> int array[n];
> 
> it's very easy to fold this in the front-end into:
> 
> const int n = 5;
> int array[5];
> 
> As a matter of fact, that's what we do in Ada, which is by far the biggest user
> of alloca in the compiler.

And now Fortran with -fstack-arrays.  Though those are very likely never
folded to automatic vars.

It would be nice to know whether this particular FAIL is the failure
of some checking mechanism or a genuine wrong-code bug.  I suppose
it's the former, and for -fstack-check we could simply disable alloca
folding.
Comment 9 Eric Botcazou 2011-10-11 11:20:41 UTC
> It would be nice to know whether this particular FAIL is the failure
> of some checking mechanism or a genuine wrong-code bug.  I suppose
> it's the former, and for -fstack-check we could simply disable alloca
> folding.

I'd rather not, -fstack-check should affect code generation as little as possible beyond the checking code itself.  A conforming Ada compiler is supposed to do stack checking by default.  I'll try to get my hands on a Darwin machine.
Comment 10 Tom de Vries 2011-10-11 14:47:34 UTC
I don't know whether it's related, but while reviewing the code once more I found this problem in function.c. I forgot to update the number of arguments in build_call_expr to 2:
...
		  t = built_in_decls[BUILT_IN_ALLOCA_WITH_ALIGN];
		  t = build_call_expr (t, 1, DECL_SIZE_UNIT (parm),
				       size_int (DECL_ALIGN (parm)));
...

I'll test and submit to gcc-patches.
Comment 11 Iain Sandoe 2011-10-11 15:57:05 UTC
Created attachment 25465 [details]
asm for c52104y

changing the number of args doesn't seem to fix the problem (off a stage3 bubble + rebuild of libada).

Attached asm - if there's a tree dump that would help then I can upload as wanted..
Comment 12 Iain Sandoe 2011-10-11 18:45:39 UTC
Created attachment 25466 [details]
test of -fstack-check

a simple test program for Darwin .. 
.. AFAICT this DTRT under 'c' on {powerpc,i386}-darwin9 and also on x86_64-darwin10.

If one bumps up the stack usage in stack_meltdown() then checks the tree dumps, __builtin_alloca is being invoked - but still things seem to be as expected.
Comment 13 Maxim Kuvyrkov 2011-10-11 20:41:35 UTC
Dominique,
Eric,

Would you please try and reproduce the failure with a Linux target?  [Setting up Darwin GCC development environment (especially with GNAT in it) is a very time-consuming task.]

Thank you.
Comment 14 Dominique d'Humieres 2011-10-12 11:31:57 UTC
(In reply to comment #11)
> Created attachment 25466 [details]
> test of -fstack-check
>
> a simple test program for Darwin .. 
> .. AFAICT this DTRT under 'c' on {powerpc,i386}-darwin9 and also on
> x86_64-darwin10.

The attached test aborts on x86_64-darwin10 with/without -fstack-check, pre(r179600)/post revision 179655. What is my mistake?
Comment 15 Dominique d'Humieres 2011-10-12 11:40:17 UTC
(In reply to comment #13)
>Would you please try and reproduce the failure with a Linux target?  

What for? AFAICT the failure occurs only on x86_64-apple-darwin10. One possibility would be to convince our friends at Intel to also test Ada with the hope that the bug will trigger with one of their several configs (that happened in the past).

> [Setting up Darwin GCC development environment (especially with GNAT in it) 
> is a very time-consuming task.]

The converse is also true: I have once installed gcc (4.6) on one of our linux box, it took me a full week-end to meet all the prerequisites and I have only be able to do it for x86_64 (no access to the 32 bit mode)
and I did not enabled libjava nor Ada.
Comment 16 Iain Sandoe 2011-10-12 11:44:28 UTC
(In reply to comment #14)
> (In reply to comment #11)
> > Created attachment 25466 [details]
> > test of -fstack-check
> >
> > a simple test program for Darwin .. 
> > .. AFAICT this DTRT under 'c' on {powerpc,i386}-darwin9 and also on
> > x86_64-darwin10.
> 
> The attached test aborts on x86_64-darwin10 with/without -fstack-check,
> pre(r179600)/post revision 179655. What is my mistake?

if it's aborting, that is wrong .. send me the .s file please.

it should do (something like) this (with or without stack check):

$ ./sc
[address=bf7ff848 pc=00001ced insn=00040c83] Segmentation Fault

on ppc the message should also identify that the stack has overflowed.

The program is meant to allow one to check that the correct code has been
generated - by scanning the RTL and/or asm.  The result of execution should be a caught segfault.
Comment 17 Dominique d'Humieres 2011-10-12 11:55:33 UTC
Created attachment 25471 [details]
Assembly file for the code in attachment 25466 [details]

> if it's aborting, that is wrong .. send me the .s file please.

It aborts also on powerpc-apple-darwin9. I am attaching the assembly file compiled with -fstack-check.
Comment 18 Iain Sandoe 2011-10-12 12:27:19 UTC
Created attachment 25472 [details]
updated stack check test

mea culpa...

... I should have said to put ulimit -s 8192.

I've attached an updated version which iterates more times and does what's expected with default ulimits on ppc/x86 d9 and x86 d10.

it's slightly refined on ppc - and should give a different termination message when -fstack-check is in force.

(the old version is still perfectly OK for checking code-gen, BTW).
Comment 19 Iain Sandoe 2011-10-12 13:35:05 UTC
built with ; 
$ gnatmake -f -O2 -gnatws -fstack-check -I../../../support c52104y --GCC="/GCC/gcc-4-7-trunk-build/gcc/xgcc -B/GCC/gcc-4-7-trunk-build/gcc" -cargs -save-temps -fverbose-asm -fdump-tree-all -largs --GCC="/GCC/gcc-4-7-trunk-build/gcc/xgcc -B/GCC/gcc-4-7-trunk-build/gcc" 

(gdb) run
Starting program: /Volumes/GCC/gcc-4-7-trunk-build/gcc/testsuite/ada/acats/tests/c5/c52104y/c52104y 
Reading symbols for shared libraries ++. done

,.,. C52104Y ACATS 2.5 11-10-12 14:25:13
---- C52104Y CHECK THAT IN ARRAY ASSIGNMENTS AND IN SLICE ASSIGNMENTS,
                THE LENGTHS MUST MATCH.
   - C52104Y NO CONSTRAINT_ERROR FOR NON-NULL ARRAY SUBTYPE WHEN ONE
                DIMENSION HAS INTEGER'LAST + 3 COMPONENTS.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00007fff5bc00380
0x0000000100003658 in _ada_c52104y () at c52104y.adb:115
115                         ARRX52  :  TABOX52 ;     -- BIG ARRAY HERE.

===

0x0000000100003649 <_ada_c52104y+521>:  sub    %rdx,%rsi
0x000000010000364c <_ada_c52104y+524>:  cmp    %rsi,%rcx
0x000000010000364f <_ada_c52104y+527>:  je     0x100003661 <_ada_c52104y+545>
0x0000000100003651 <_ada_c52104y+529>:  sub    $0x1000,%rcx
0x0000000100003658 <_ada_c52104y+536>:  orq    $0x0,(%rcx)
0x000000010000365c <_ada_c52104y+540>:  cmp    %rsi,%rcx
0x000000010000365f <_ada_c52104y+543>:  jne    0x100003651 <_ada_c52104y+529>
0x0000000100003661 <_ada_c52104y+545>:  mov    %rax,%rsi
0x0000000100003664 <_ada_c52104y+548>:  sub    %rdx,%rsi

which looks like a stack probe.
Comment 20 Iain Sandoe 2011-10-12 13:47:32 UTC
rax            0x10000010       268435472
rbx            0x7fff5fbff380   140734799803264
rcx            0x7fff5bc00380   140734732698496
rdx            0x10000000       268435456
rsi            0x7fff4fbfc380   140734531355520
rdi            0x0      0
rbp            0x7fff5fbff4a0   0x7fff5fbff4a0
rsp            0x7fff5fbff380   0x7fff5fbff380

.. so it looks like the requested stack frame size is circa 256Mb .. 
(c.f. Darwin's 64-sih Mb).
Comment 21 Iain Sandoe 2011-10-12 14:07:45 UTC
Created attachment 25474 [details]
optimized tree dump for c52104y

shows the builtin_alloca_with_align
Comment 22 Iain Sandoe 2011-10-12 14:54:01 UTC
(In reply to comment #13)

> Would you please try and reproduce the failure with a Linux target?  [Setting
> up Darwin GCC development environment (especially with GNAT in it) is a very
> time-consuming task.]

the test passes on x86_64-unknown-linux-gnu, even with ulimit -s set to a very
low value.
Comment 23 Eric Botcazou 2011-10-12 18:04:00 UTC
It turns out that Tom's patch is innocent, you can reproduce the problem at the preceding revision if you compiled at -O1 instead of -O2.

This appears to be a problem in the signal unwinder on Darwin.  Here's the status of the registers when the probe hits the guard page:

Program received signal SIGSEGV, Segmentation fault.
0x0000000100002a44 in _ada_c52104y () at c52104y.adb:31
31                          ARRX52  :  TABOX52 ;     -- BIG ARRAY HERE.
(gdb) info reg
rax            0x10000010       268435472
rbx            0x7fff5fbffa40   140734799804992
rcx            0x7fff5f3ffa30   140734791416368
rdx            0xf      15
rsi            0x7fff5fbffa30   140734799804976
rdi            0x7fff4fbfca30   140734531357232
rbp            0x7fff5fbffa80   0x7fff5fbffa80
rsp            0x7fff5fbffa30   0x7fff5fbffa30
r8             0x80000002       2147483650
r9             0x10000000       268435456
r10            0x80000002       2147483650
r11            0x10000001       268435457
r12            0xfffffffffffffffa       -6
r13            0xd      13
r14            0x0      0
r15            0x1      1
rip            0x100002a44      0x100002a44 <_ada_c52104y+416>

And here's the status of the registers when execution resumes:

 Breakpoint 1, 0x0000000100002aa2 in _ada_c52104y () at c52104y.adb:49
49      END C52104Y;
(gdb) info reg
rax            0x100100080      4296016000
rbx            0xf      15
rcx            0x7fff5f3ffa30   140734791416368
rdx            0x1      1
rsi            0x7fff5fbffa30   140734799804976
rdi            0x7fff4fbfca30   140734531357232
rbp            0x7fff5fbffa80   0x7fff5fbffa80
rsp            0x7fff5fbffa30   0x7fff5fbffa30
r8             0x80000002       2147483650
r9             0x10000000       268435456
r10            0x80000002       2147483650
r11            0x10000001       268435457
r12            0xfffffffffffffffa       -6
r13            0xd      13
r14            0x0      0
r15            0x1      1
rip            0x100002aa2      0x100002aa2 <_ada_c52104y+510>


Note how the value of rdx has apparently been moved to rbx; this is the bug, rbx is a call-saved register so its value is supposed to be preserved here.
Comment 24 Iain Sandoe 2011-10-12 19:57:15 UTC
reproducible (using gdb 7.1) on darwin9 @ m64 (m32 is OK on D9 and D10) - 
- so where are we looking for a problem- in the m64 libgcc_s unwinder - or in the ada handers? .. or in libSystem ?
Comment 25 Iain Sandoe 2011-10-12 20:07:28 UTC
passes at -O0 on darwin9 and 10
Comment 26 Eric Botcazou 2011-10-12 20:55:31 UTC
> reproducible (using gdb 7.1) on darwin9 @ m64 (m32 is OK on D9 and D10) - 
> - so where are we looking for a problem- in the m64 libgcc_s unwinder - or in
> the ada handers? .. or in libSystem ?

First the signal unwinder, i.e. how it restores registers that have been saved by the kernel when the signal was raised.  So libgcc or the system equivalent if there is one.  Unfortunately my Darwin knowledge is pretty limited...
Comment 27 Iain Sandoe 2011-10-12 22:48:18 UTC
(In reply to comment #26)
> > reproducible (using gdb 7.1) on darwin9 @ m64 (m32 is OK on D9 and D10) - 
> > - so where are we looking for a problem- in the m64 libgcc_s unwinder - or in
> > the ada handers? .. or in libSystem ?
> 
> First the signal unwinder, i.e. how it restores registers that have been saved
> by the kernel when the signal was raised.  So libgcc or the system equivalent
> if there is one.  Unfortunately my Darwin knowledge is pretty limited...

OK. well  libgcc_s or libSystem contains the unwinder, depending on whether it's darwin9 or darwin10 (and assuming that there's no insertion caused by a DYLD_LIBRARY_PATH).  I'll have to trawl through it in more detail.

I suppose that the fact that the tests pass at -O0 might simply reflect different register usage...  I was hoping that it might indicate a code-gen problem ;-)
Comment 28 Eric Botcazou 2011-10-12 22:54:57 UTC
> OK. well  libgcc_s or libSystem contains the unwinder, depending on whether
> it's darwin9 or darwin10 (and assuming that there's no insertion caused by a
> DYLD_LIBRARY_PATH).  I'll have to trawl through it in more detail.

I'll try as well.

> I suppose that the fact that the tests pass at -O0 might simply reflect
> different register usage...  I was hoping that it might indicate a code-gen
> problem ;-)

Yes, it passes at -O0 because the stack pointer is restored from a stack slot instead of from %rbx.  Tom's patch changed %r12 into %rbx here, which is OK.
Comment 29 Iain Sandoe 2011-10-15 19:33:25 UTC
(In reply to comment #28)
> > OK. well  libgcc_s or libSystem contains the unwinder, depending on whether
> > it's darwin9 or darwin10 (and assuming that there's no insertion caused by a
> > DYLD_LIBRARY_PATH).  I'll have to trawl through it in more detail.
> 
> I'll try as well.

so far, I added a little debugging code to init.c and looked at the params into __gnat_error_handler.  AFAICT it looks perfectly sensible - and the "Right Thing" seems to happen with the stack area being recognized (for the probe) and the exception correctly chosen.  The saved exception state seems OK (if one arranges to examine ucontext).

however I've not got far through Raise_From_Signal_Handler  () - if one continues from there it ends with a loop on x86-64/darwin9 and another segv on x86-64/darwin10.

The thing that's itching slightly is that there is a syscall (in __gnat_error_handler ) to switch the signal stack - before the raise, and I wonder if the use of the alt sigstack is what's causing the problem.  One would imagine that anything as radical as a missing reg. save/rest in eh would be spotted outside Ada ;-)

I might try to cook up a simplified 'c' mimicry and see if that fails too.

The generated eh_frame data is unchanged from O0-O2 .. 

... the exception tables differ - but I guess that reflects code motion in the different optimizations.
Comment 30 Eric Botcazou 2011-10-15 20:49:03 UTC
> however I've not got far through Raise_From_Signal_Handler  () - if one
> continues from there it ends with a loop on x86-64/darwin9 and another segv on
> x86-64/darwin10.

You need to compile with -fstack-check.  Then you'll be able to debug the unwinding phase.  Here are some possible breakpoints:

0x00007fff85b7a260 in unw_init_local () from /usr/lib/libSystem.B.dylib
(gdb) disass
Dump of assembler code for function unw_init_local:

Breakpoint 3, 0x00007fff85b9fe32 in unwind_phase2 ()
   from /usr/lib/libSystem.B.dylib
(gdb) disass
Dump of assembler code for function unwind_phase2:

0x00007fff85ba0150 in libunwind::Registers_x86_64::jumpto() ()
   from /usr/lib/libSystem.B.dylib
(gdb) disass
Dump of assembler code for function _ZN9libunwind16Registers_x86_646jumptoEv:
=> 0x00007fff85ba0150 <+0>:     mov    0x38(%rdi),%rax

The context from which jumpto restores the registers has the wrong %rbx line.

> The thing that's itching slightly is that there is a syscall (in
> __gnat_error_handler ) to switch the signal stack - before the raise, and I
> wonder if the use of the alt sigstack is what's causing the problem.

This call doesn't actually do anything.  The stack is switched by the system before __gnat_error_handler is entered and all the processing is done using the alternate stack, until execution is resumed in the exception handler.

> One would imagine that anything as radical as a missing reg. save/rest in eh
> would be spotted outside Ada ;-)

Java is the only other language that supports this kind of things though.
Comment 31 Eric Botcazou 2011-10-15 21:34:32 UTC
There is some suspicious code in

#0  0x00007fff85c75d48 in libunwind::DwarfInstructions<libunwind::LocalAddressSpace, libunwind::Registers_x86_64>::stepWithDwarf(libunwind::LocalAddressSpace&, unsigned long long, unsigned long long, libunwind::Registers_x86_64&) ()
   from /usr/lib/libSystem.B.dylib

(gdb) disass
Dump of assembler code for function _ZN9libunwind17DwarfInstructionsINS_17LocalAddressSpaceENS_16Registers_x86_64EE13stepWithDwarfERS1_yyRS2_:
   0x00007fff85c75990 <+0>:     push   %rbp

   0x00007fff85c75c81 <+753>:   mov    -0x860(%rbp),%rax
   0x00007fff85c75c88 <+760>:   mov    %rax,0xa0(%r13)
   0x00007fff85c75c8f <+767>:   mov    -0x868(%rbp),%rdx
   0x00007fff85c75c96 <+774>:   mov    %rdx,0x98(%r13)
   0x00007fff85c75c9d <+781>:   mov    -0x870(%rbp),%rcx
   0x00007fff85c75ca4 <+788>:   mov    %rcx,0x90(%r13)
   0x00007fff85c75cab <+795>:   mov    -0x878(%rbp),%rax
   0x00007fff85c75cb2 <+802>:   mov    %rax,0x88(%r13)
   0x00007fff85c75cb9 <+809>:   mov    -0x8e8(%rbp),%rdx
   0x00007fff85c75cc0 <+816>:   mov    %rdx,0x78(%r13)
   0x00007fff85c75cc4 <+820>:   mov    -0x8e0(%rbp),%rcx
   0x00007fff85c75ccb <+827>:   mov    %rcx,0x70(%r13)
   0x00007fff85c75ccf <+831>:   mov    -0x8d8(%rbp),%rax
   0x00007fff85c75cd6 <+838>:   mov    %rax,0x68(%r13)
   0x00007fff85c75cda <+842>:   mov    -0x8d0(%rbp),%rdx
   0x00007fff85c75ce1 <+849>:   mov    %rdx,0x60(%r13)
   0x00007fff85c75ce5 <+853>:   mov    -0x8c8(%rbp),%rcx
   0x00007fff85c75cec <+860>:   mov    %rcx,0x58(%r13)
   0x00007fff85c75cf0 <+864>:   mov    -0x8c0(%rbp),%rax
   0x00007fff85c75cf7 <+871>:   mov    %rax,0x50(%r13)
   0x00007fff85c75cfb <+875>:   mov    -0x8b8(%rbp),%rdx
   0x00007fff85c75d02 <+882>:   mov    %rdx,0x48(%r13)
   0x00007fff85c75d06 <+886>:   mov    -0x8b0(%rbp),%rcx
   0x00007fff85c75d0d <+893>:   mov    %rcx,0x40(%r13)
   0x00007fff85c75d11 <+897>:   mov    -0x8a8(%rbp),%rax
   0x00007fff85c75d18 <+904>:   mov    %rax,0x30(%r13)
   0x00007fff85c75d1c <+908>:   mov    -0x8a0(%rbp),%rdx
   0x00007fff85c75d23 <+915>:   mov    %rdx,0x20(%r13)
   0x00007fff85c75d27 <+919>:   mov    -0x898(%rbp),%rcx
   0x00007fff85c75d2e <+926>:   mov    %rcx,0x28(%r13)
   0x00007fff85c75d32 <+930>:   mov    -0x890(%rbp),%rax
   0x00007fff85c75d39 <+937>:   mov    %rax,0x8(%r13)
   0x00007fff85c75d3d <+941>:   mov    -0x888(%rbp),%rdx
   0x00007fff85c75d44 <+948>:   mov    %rdx,0x10(%r13)
   0x00007fff85c75d48 <+952>:   mov    -0x880(%rbp),%rcx
   0x00007fff85c75d4f <+959>:   mov    %rcx,0x18(%r13)
   0x00007fff85c75d53 <+963>:   mov    %r15,0x0(%r13)
   0x00007fff85c75d57 <+967>:   mov    %r12,0x38(%r13)
   0x00007fff85c75d5b <+971>:   mov    %rbx,0x80(%r13)

-0x860(%rbp) seems to be a saved context:

(gdb) x/gx (-0x860 + $rbp - 8 * 4)
0x100036900:    0x00007fff5fbff9f0
(gdb) x/gx (-0x860 + $rbp - 8 * 5)
0x1000368f8:    0x00007fff5f3ff9f0
(gdb) x/gx (-0x860 + $rbp - 8 * 6)
0x1000368f0:    0x0000000000000000
(gdb) x/gx (-0x860 + $rbp - 8 * 7)
0x1000368e8:    0x00007fff5fbff9f0
(gdb) x/gx (-0x860 + $rbp - 8 * 8)
0x1000368e0:    0x00007fff4fbfc9f0
(gdb) x/gx (-0x860 + $rbp - 8 * 9)
0x1000368d8:    0x00007fff5fbffa30
(gdb) x/gx (-0x860 + $rbp - 8 * 10)
0x1000368d0:    0x0000000080000002
(gdb) x/gx (-0x860 + $rbp - 8 * 11)
0x1000368c8:    0x0000000010000000
(gdb) x/gx (-0x860 + $rbp - 8 * 12)
0x1000368c0:    0x0000000080000002
(gdb) x/gx (-0x860 + $rbp - 8 * 13)
0x1000368b8:    0x0000000010000001
(gdb) x/gx (-0x860 + $rbp - 8 * 14)
0x1000368b0:    0xfffffffffffffffa
(gdb) x/gx (-0x860 + $rbp - 8 * 15)
0x1000368a8:    0x000000000000000d
(gdb) x/gx (-0x860 + $rbp - 8 * 16)
0x1000368a0:    0x0000000000000000
(gdb) x/gx (-0x860 + $rbp - 8 * 17)
0x100036898:    0x0000000000000001

This is the same context as the one displayed by GDB when the probe hits:

(gdb) info reg
rax            0x10000010       268435472
rbx            0x7fff5fbff9f0   140734799804912
rcx            0x7fff5f3ff9f0   140734791416304
rdx            0x0      0
rsi            0x7fff5fbff9f0   140734799804912
rdi            0x7fff4fbfc9f0   140734531357168
rbp            0x7fff5fbffa30   0x7fff5fbffa30
rsp            0x7fff5fbff9f0   0x7fff5fbff9f0
r8             0x80000002       2147483650
r9             0x10000000       268435456
r10            0x80000002       2147483650
r11            0x10000001       268435457
r12            0xfffffffffffffffa       -6
r13            0xd      13
r14            0x0      0
r15            0x1      1

Now, at the end of the code sequence, the context pointed to by %r13 is:

(gdb) x/gx ($r13 + 8 * 0)
0x1000372b0:    0x0000000010000010
(gdb) x/gx ($r13 + 8 * 1)
0x1000372b8:    0x0000000000000000
(gdb) x/gx ($r13 + 8 * 2)
0x1000372c0:    0x00007fff5f3ff9f0
(gdb) x/gx ($r13 + 8 * 3)
0x1000372c8:    0x00007fff5fbff9f0
(gdb) x/gx ($r13 + 8 * 4)
0x1000372d0:    0x00007fff4fbfc9f0
(gdb) x/gx ($r13 + 8 * 5)
0x1000372d8:    0x00007fff5fbff9f0
(gdb) x/gx ($r13 + 8 * 6)
0x1000372e0:    0x00007fff5fbffa30
(gdb) x/gx ($r13 + 8 * 7)
0x1000372e8:    0x00007fff5fbff9f0
(gdb) x/gx ($r13 + 8 * 8)
0x1000372f0:    0x0000000080000002
(gdb) x/gx ($r13 + 8 * 9)
0x1000372f8:    0x0000000010000000
(gdb) x/gx ($r13 + 8 * 10)
0x100037300:    0x0000000080000002
(gdb) x/gx ($r13 + 8 * 11)
0x100037308:    0x0000000010000001
(gdb) x/gx ($r13 + 8 * 12)
0x100037310:    0xfffffffffffffffa
(gdb) x/gx ($r13 + 8 * 13)
0x100037318:    0x000000000000000d
(gdb) x/gx ($r13 + 8 * 14)
0x100037320:    0x0000000000000000
(gdb) x/gx ($r13 + 8 * 15)
0x100037328:    0x0000000000000001

which will be the context restored when the execution resumes.  Note how the lines of %rbx and %rdx have been swapped.  Looking at the end of the code:

   0x00007fff85c75d27 <+919>:   mov    -0x898(%rbp),%rcx
   0x00007fff85c75d2e <+926>:   mov    %rcx,0x28(%r13)
   0x00007fff85c75d32 <+930>:   mov    -0x890(%rbp),%rax
   0x00007fff85c75d39 <+937>:   mov    %rax,0x8(%r13)
   0x00007fff85c75d3d <+941>:   mov    -0x888(%rbp),%rdx
   0x00007fff85c75d44 <+948>:   mov    %rdx,0x10(%r13)
   0x00007fff85c75d48 <+952>:   mov    -0x880(%rbp),%rcx
   0x00007fff85c75d4f <+959>:   mov    %rcx,0x18(%r13)

it seems that the 0x8(%r13) and the 0x18(%r13) have been swapped.
Comment 32 Iain Sandoe 2011-10-15 22:37:41 UTC
thanks Eric, you're going much quicker then me ;) ... had some other things to do.
I think libUnwind is released as OS these days, so it should be possible to look ... will try to find it tomorrow.
Comment 33 Iain Sandoe 2011-10-17 09:58:53 UTC
Created attachment 25518 [details]
test of a foreign except thrown from a sig handler in c++

1. the code for the D10 libSystem unwind library is available from here:
http://www.opensource.apple.com/tarballs/libunwind/

(the D9 unwinder is just that of gcc-4.0).

2. Looking at a build of this, the order of the assignments (R newRegisters = register) seems generally scrambled when the getCFA function is inlined.  This is reproducible with the vendor's tools and the source in 1 at optimization levels > 0 and not Os.

3. The scrambling is consistent (in and out) - and I'm not 100% sure about whether this is the fault... ISTM that so long as the re-ordering is local (and consistent) to optimized code, it could be harmless.

4. the code attached tries to simplify things by emulating the effect from c++.  I hope that it tries to test the the Right Thing - i.e. that register that should be preserved across the call to do_fail () are, indeed, preserved to the catch.

5. I find that the test code succeeds on i386 (D9 and D10).

6. I find that the test code fails on x86-64 (D9 - using the _current_ libgcc_s unwinder - DYLD_LIBRARY_PATH inserted).  Fails on x86-64 D10 using the libSystem unwinder (with the scrambling as noted).

... i.e. it fails on two different unwinders.  On both rbx seems to end up as 0.

any comments on the validity of the test would be appreciated.
Comment 34 Iain Sandoe 2011-10-17 12:13:53 UTC
Created attachment 25520 [details]
revised

some bad constraints in the last version (there are some odd cases, like this where it would be nice to be able to force r8..r15).

- now this passes everywhere (m32/m64 on D9 and D10).
... not sure how to interpret that presently (likely more asm mistakes on my part).  

It would be easy if one could write the asm separately - but we're trying to trick the compiler into making the unwind tables etc. ...
Comment 35 Eric Botcazou 2011-10-17 15:36:11 UTC
> 1. the code for the D10 libSystem unwind library is available from here:
> http://www.opensource.apple.com/tarballs/libunwind/

Thanks for the pointer.

> 2. Looking at a build of this, the order of the assignments (R newRegisters =
> register) seems generally scrambled when the getCFA function is inlined.  This
> is reproducible with the vendor's tools and the source in 1 at optimization
> levels > 0 and not Os.
> 
> 3. The scrambling is consistent (in and out) - and I'm not 100% sure about
> whether this is the fault... ISTM that so long as the re-ordering is local
> (and consistent) to optimized code, it could be harmless.

It wasn't so much the order of assignments as the difference in the offsets between the contexts.  But, you're right, this isn't the problem as the local variable newRegisters is very likely scalarized, so the final offsets are entirely meaningless.  In any case, the problem is elsewhere, namely in the unwind info for the _sigtramp function of the libc:

(gdb) b *0x00007fff85b9b1b8
Breakpoint 1 at 0x7fff85b9b1b8
(gdb) run
Starting program: /nfs/nas/homes/botcazou/c52104y_0
     C52104Y CHECK THAT IN ARRAY ASSIGNMENTS AND IN SLICE ASSIGNMENTS,
                THE LENGTHS MUST MATCH.
   - C52104Y NO CONSTRAINT_ERROR FOR NON-NULL ARRAY SUBTYPE WHEN ONE
                DIMENSION HAS INTEGER'LAST + 3 COMPONENTS.

Program received signal SIGSEGV, Segmentation fault.
0x0000000100002428 in c52104y_0 ()
(gdb) continue
Continuing.

Breakpoint 1, <signal handler called>
(gdb) disass
Dump of assembler code for function _sigtramp:
   0x00007fff85b9b1a0 <+0>:     push   %rbp
   0x00007fff85b9b1a1 <+1>:     mov    %rsp,%rbp
   0x00007fff85b9b1a4 <+4>:     mov    %rdi,%rax
   0x00007fff85b9b1a7 <+7>:     incl   -0x15261b75(%rip)        # 0x7fff70939638
   0x00007fff85b9b1ad <+13>:    mov    %r8,%rbx
   0x00007fff85b9b1b0 <+16>:    mov    %edx,%edi
   0x00007fff85b9b1b2 <+18>:    mov    %rcx,%rsi
   0x00007fff85b9b1b5 <+21>:    mov    %r8,%rdx
=> 0x00007fff85b9b1b8 <+24>:    callq  *%rax
   0x00007fff85b9b1ba <+26>:    decl   -0x15261b88(%rip)        # 0x7fff70939638
   0x00007fff85b9b1c0 <+32>:    mov    %rbx,%rdi
   0x00007fff85b9b1c3 <+35>:    mov    $0x1e,%esi
   0x00007fff85b9b1c8 <+40>:    jmpq   0x7fff85b9b1d0 <__sigreturn>
   0x00007fff85b9b1cd <+45>:    nop
   0x00007fff85b9b1ce <+46>:    nop
   0x00007fff85b9b1cf <+47>:    nop
End of assembler dump.

The CFI of the unwind info for _sigtramp is at this address:

(gdb) x/167bx 0x00007fff85ccff59
0x7fff85ccff59: 0x10    0x00    0x05    0x73    0x30    0x06    0x23    0x10
0x7fff85ccff61: 0x10    0x01    0x05    0x73    0x30    0x06    0x23    0x18
0x7fff85ccff69: 0x10    0x02    0x05    0x73    0x30    0x06    0x23    0x20
0x7fff85ccff71: 0x10    0x03    0x05    0x73    0x30    0x06    0x23    0x28
0x7fff85ccff79: 0x10    0x04    0x05    0x73    0x30    0x06    0x23    0x38
0x7fff85ccff81: 0x10    0x05    0x05    0x73    0x30    0x06    0x23    0x30
0x7fff85ccff89: 0x10    0x06    0x05    0x73    0x30    0x06    0x23    0x40
0x7fff85ccff91: 0x10    0x07    0x05    0x73    0x30    0x06    0x23    0x48
0x7fff85ccff99: 0x10    0x08    0x05    0x73    0x30    0x06    0x23    0x50
0x7fff85ccffa1: 0x10    0x09    0x05    0x73    0x30    0x06    0x23    0x58
0x7fff85ccffa9: 0x10    0x0a    0x05    0x73    0x30    0x06    0x23    0x60
0x7fff85ccffb1: 0x10    0x0b    0x05    0x73    0x30    0x06    0x23    0x68
0x7fff85ccffb9: 0x10    0x0c    0x05    0x73    0x30    0x06    0x23    0x70
0x7fff85ccffc1: 0x10    0x0d    0x05    0x73    0x30    0x06    0x23    0x78
0x7fff85ccffc9: 0x10    0x0e    0x06    0x73    0x30    0x06    0x23    0x80

DW_CFA_expression reg_num len DW_OP_breg3 off deref DW_OP_plus_uconst offset

So, for example, register 1 is at offset 0x18 of the deref of (%rdx + 0x30):

(gdb) x/gx ($rdx + 0x30)
0x100038958:    0x00000001000384bc

(gdb) x/gx 0x00000001000384bc + 0x18
0x1000384d4:    0x00007fff5fbff910

And register 3 is at offset 0x18 of the deref of (%rdx + 0x30):

(gdb) x/gx 0x00000001000384bc + 0x28
0x1000384e4:    0x0000000010000000

The former is the saved %rbx and the latter is the saved %rdx.  The problem is that they are numbered differently by libunwind.h:

// 64-bit x86_64 registers
enum {
    UNW_X86_64_RAX =  0,
    UNW_X86_64_RDX =  1,
    UNW_X86_64_RCX =  2,
    UNW_X86_64_RBX =  3,
    UNW_X86_64_RSI =  4,
    UNW_X86_64_RDI =  5,
    UNW_X86_64_RBP =  6,
    UNW_X86_64_RSP =  7,
    UNW_X86_64_R8  =  8,
    UNW_X86_64_R9  =  9,
    UNW_X86_64_R10 = 10,
    UNW_X86_64_R11 = 11,
    UNW_X86_64_R12 = 12,
    UNW_X86_64_R13 = 13,
    UNW_X86_64_R14 = 14,
    UNW_X86_64_R15 = 15
};

so %rbx is register 3 and %rdx is register 1 for libunwind.  Therefore, if you swap the saved values, the program works fine:

(gdb) set { unsigned long } 0x1000384d4 = 0x0000000010000000
(gdb) set { unsigned long } 0x1000384e4 = 0x00007fff5fbff910
(gdb) continue
Continuing.
   - C52104Y STORAGE_ERROR RAISED WHEN DECLARING ONE PACKED BOOLEAN
                ARRAY WITH INTEGER'LAST + 3 COMPONENTS.
==== C52104Y PASSED ============================.



The unwind info for _sigtramp is in i386/sys/_sigtramp.s for i386:

	/* Now for the expressions, which all compute
	   uctx->uc_mcontext->register
	   for each register.
	   
	   Describe even the registers that are not call-saved because they
	   might be being used in the prologue to save other registers.
	   Only integer registers are described at present.    */

	loc_expr_for_reg (0, MCONTEXT_SS_EAX)
	loc_expr_for_reg (1, MCONTEXT_SS_ECX)
	loc_expr_for_reg (2, MCONTEXT_SS_EDX)
	loc_expr_for_reg (3, MCONTEXT_SS_EBX)
	loc_expr_for_reg (4, MCONTEXT_SS_EBP) # note that GCC switches
	loc_expr_for_reg (5, MCONTEXT_SS_ESP) # DWARF registers 4 & 5
	loc_expr_for_reg (6, MCONTEXT_SS_ESI)
	loc_expr_for_reg (7, MCONTEXT_SS_EDI)
	loc_expr_for_reg (9, MCONTEXT_SS_EFLAGS)

It is in keeping with libunwind.h:

// 32-bit x86 registers
enum {
    UNW_X86_EAX = 0,
    UNW_X86_ECX = 1,
    UNW_X86_EDX = 2,
    UNW_X86_EBX = 3,
    UNW_X86_EBP = 4,
    UNW_X86_ESP = 5,
    UNW_X86_ESI = 6,
    UNW_X86_EDI = 7
};


The unwind info for _sigtramp is in x86_64/sys/_sigtramp.s for x86-64:

	/* Now for the expressions, which all compute
	   uctx->uc_mcontext->register
	   for each register.
	   
	   Describe even the registers that are not call-saved because they
	   might be being used in the prologue to save other registers.
	   Only integer registers are described at present.    */

	loc_expr_for_reg (0, MCONTEXT_SS_RAX)
	loc_expr_for_reg (1, MCONTEXT_SS_RBX)
	loc_expr_for_reg (2, MCONTEXT_SS_RCX)
	loc_expr_for_reg (3, MCONTEXT_SS_RDX)
	loc_expr_for_reg (4, MCONTEXT_SS_RSI)
	loc_expr_for_reg (5, MCONTEXT_SS_RDI)
	loc_expr_for_reg (6, MCONTEXT_SS_RBP)
	loc_expr_for_reg (7, MCONTEXT_SS_RSP)
	loc_expr_rN (8)
	loc_expr_rN (9)
	loc_expr_rN (10)
	loc_expr_rN (11)
	loc_expr_rN (12)
	loc_expr_rN (13)
	loc_expr_rN_long (14)
	loc_expr_rN_long (15)

and it is _not_ in keeping with libunwind.h:

// 64-bit x86_64 registers
enum {
    UNW_X86_64_RAX =  0,
    UNW_X86_64_RDX =  1,
    UNW_X86_64_RCX =  2,
    UNW_X86_64_RBX =  3,
    UNW_X86_64_RSI =  4,
    UNW_X86_64_RDI =  5,
    UNW_X86_64_RBP =  6,
    UNW_X86_64_RSP =  7,
    UNW_X86_64_R8  =  8,
    UNW_X86_64_R9  =  9,
    UNW_X86_64_R10 = 10,
    UNW_X86_64_R11 = 11,
    UNW_X86_64_R12 = 12,
    UNW_X86_64_R13 = 13,
    UNW_X86_64_R14 = 14,
    UNW_X86_64_R15 = 15
};

The discrepancy in the x86-64 case is the bug.


This should be reported to Apple and probably fixed in the libc.  In the meantime, we can work around it in ada/init.c/__gnat_error_handler:

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

by patching up the context reachable through the ucontext argument.  According to the comment in the libc file, it is at ucontext->uc_mcontext.
Comment 36 Iain Sandoe 2011-10-17 18:07:09 UTC
(In reply to comment #35)
>   In any case, the problem is elsewhere, namely in the
> unwind info for the _sigtramp function of the libc:

apropos the i386 variant:
>     loc_expr_for_reg (0, MCONTEXT_SS_EAX)
>     loc_expr_for_reg (1, MCONTEXT_SS_ECX)
>     loc_expr_for_reg (2, MCONTEXT_SS_EDX)

note that (despite the consistency within the code here) this differs from the order in gcc/config/i386/i386.h... (although these are not call-saved, still ... )

apropos the x86_64 variant:
> This should be reported to Apple and probably fixed in the libc.  

is there a definitive "right" or "wrong'  - should I be patching gcc/config/i386/darwin.h to match the sigtramp and bug-file against libunwind?  or just file a bug agains the sigtramp (the order there matches the context order).

It should be possible to produce a test-case w/out Ada (filing a bug that requires GCC+Ada is not likely to get far).   I wonder why my c++ case doesn't appear to fail.

> In the
> meantime, we can work around it in ada/init.c/__gnat_error_handler:
> 
> static void
> __gnat_error_handler (int sig, siginfo_t *si, void *ucontext ATTRIBUTE_UNUSED)
> {
> 
> by patching up the context reachable through the ucontext argument.  According
> to the comment in the libc file, it is at ucontext->uc_mcontext.

poof-of-principle (needs some careful consideration of how to wrap it so we can control its removal when the bug is fixed).. [ using __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ is on my mind]
 
#if defined (__x86_64__)
  /* Work around an unwinder bug in libSystem where the unwinder
     and the context/sigtramp have different ideas about the reg.
     numbers.  */
  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
Comment 37 Iain Sandoe 2011-10-17 18:28:47 UTC
.. this doesn't fix the problem on Darwin9/m64 (with either the system libgcc_s or the one from current gcc/4.7).

I suppose it's failing to unwind through the sigtramp - that might require some fiddling to solve.
Comment 38 Iain Sandoe 2011-10-17 18:38:49 UTC
(In reply to comment #37)
> .. this doesn't fix the problem on Darwin9/m64 (with either the system libgcc_s
erm....
*oops* shoulda reinstalled the RTS.
Comment 39 Eric Botcazou 2011-10-17 20:37:01 UTC
> note that (despite the consistency within the code here) this differs from the
> order in gcc/config/i386/i386.h... (although these are not call-saved, still
> ... )

Yes, but this is OK as long as libc and libunwind agree.

> apropos the x86_64 variant:
> > This should be reported to Apple and probably fixed in the libc.  
> 
> is there a definitive "right" or "wrong'  - should I be patching
> gcc/config/i386/darwin.h to match the sigtramp and bug-file against
> libunwind? or just file a bug agains the sigtramp (the order there matches 
> the context order).

GCC isn't concerned here, this is a pure Darwin inconsistency, namely:

	loc_expr_for_reg (1, MCONTEXT_SS_RBX)

	loc_expr_for_reg (3, MCONTEXT_SS_RDX)

vs

    UNW_X86_64_RDX =  1,

    UNW_X86_64_RBX =  3,

The index number must be consistent: either 1 or 3 in both cases.

I think that the bug is in the unwind info of the libc, so the fix would be:

--- x86_64/sys/_sigtramp.s.0    2011-10-17 22:08:31.000000000 +0200
+++ x86_64/sys/_sigtramp.s      2011-10-17 22:08:42.000000000 +0200
@@ -195,9 +195,9 @@ LASFDE1:
           Only integer registers are described at present.    */

        loc_expr_for_reg (0, MCONTEXT_SS_RAX)
-       loc_expr_for_reg (1, MCONTEXT_SS_RBX)
+       loc_expr_for_reg (1, MCONTEXT_SS_RDX)
        loc_expr_for_reg (2, MCONTEXT_SS_RCX)
-       loc_expr_for_reg (3, MCONTEXT_SS_RDX)
+       loc_expr_for_reg (3, MCONTEXT_SS_RBX)
        loc_expr_for_reg (4, MCONTEXT_SS_RSI)
        loc_expr_for_reg (5, MCONTEXT_SS_RDI)
        loc_expr_for_reg (6, MCONTEXT_SS_RBP)

> It should be possible to produce a test-case w/out Ada (filing a bug that
> requires GCC+Ada is not likely to get far).   I wonder why my c++ case doesn't
> appear to fail.

Because %rbx is saved in the prologue of do_fail.

> poof-of-principle (needs some careful consideration of how to wrap it so we
> can control its removal when the bug is fixed).. [ using
> __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ is on my mind]
> 
> #if defined (__x86_64__)
>   /* Work around an unwinder bug in libSystem where the unwinder
>      and the context/sigtramp have different ideas about the reg.
>      numbers.  */
>   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

This looks good to me.
Comment 40 Eric Botcazou 2011-10-17 22:44:06 UTC
> is there a definitive "right" or "wrong'  - should I be patching
> gcc/config/i386/darwin.h to match the sigtramp and bug-file against libunwind? 
> or just file a bug agains the sigtramp (the order there matches the context
> order).

config/i386/darwin.h is indeed compatible with libunwind so, from a pure GCC's viewpoint, the problem is definitely in the libc.
Comment 41 Iain Sandoe 2011-10-17 22:52:28 UTC
(In reply to comment #39)

I'll try and cook up a radar...  (against sigtramp unwind data in Libc taking into account your following comment).

I guess we should not expect a fix for Darwin 9 - so the fix will likely be needed in any case.

> > It should be possible to produce a test-case w/out Ada (filing a bug that
> > requires GCC+Ada is not likely to get far).   I wonder why my c++ case doesn't
> > appear to fail.
> 
> Because %rbx is saved in the prologue of do_fail.

hm.  Isn't that the correct action?
also - _ada_c52104y saves %rbx in its prologue.  

I thought we were looking for a situation where the saves were OK, but the restores were wrong - and I was expecting the c++ emulation to do roughly the same thing.  Perhaps there's a difference in the way it unwinds.
Comment 42 Eric Botcazou 2011-10-17 22:58:24 UTC
> > Because %rbx is saved in the prologue of do_fail.
> 
> hm.  Isn't that the correct action?

Yes, this is correct, but this will also restore the correct %rbx in the caller.

> also - _ada_c52104y saves %rbx in its prologue.  

Yes, but the thrower and the handler are within _ada_c52104y, so %rbx isn't restored in-between.

> I thought we were looking for a situation where the saves were OK, but the
> restores were wrong - and I was expecting the c++ emulation to do roughly the
> same thing.  Perhaps there's a difference in the way it unwinds.

You need to make sure that %rbx isn't saved/restored between thrower and handler.
Comment 43 Iain Sandoe 2011-10-18 11:05:38 UTC
Created attachment 25540 [details]
demonstration of the fault using c++/vendor's tools

after Eric solved my inverted-logic thinko ... 
.. I reproduced using g++-4.2
bug filed as radar #10302855.

I think we'll need to apply the patch in the short/medium term and then figure out how to control it - which will depend on which system(s) a fix is released for.
Comment 44 Eric Botcazou 2011-10-18 15:22:14 UTC
> Created attachment 25540 [details]
> demonstration of the fault using c++/vendor's tools
> 
> after Eric solved my inverted-logic thinko ... 
> .. I reproduced using g++-4.2
> bug filed as radar #10302855.

Thanks!

> I think we'll need to apply the patch in the short/medium term and then figure
> out how to control it - which will depend on which system(s) a fix is released
> for.

One approach could be to scan the unwind info of _sigtramp live and check for the problematic pattern.  You call __builtin_return_address from the handler to get the PC of _sigtramp, then _Unwind_Find_FDE on this PC and you scan starting from the address you get (the length of the FDE of _sigtramp is 0xc0 currently).

The problematic pattern are the lines:

0x7fff85ccff61: 0x10    0x01    0x05    0x73    0x30    0x06    0x23    0x18

and

0x7fff85ccff71: 0x10    0x03    0x05    0x73    0x30    0x06    0x23    0x28

The register number is the second field (1 or 3) and the offset in the context is the 8th and last field (0x18 or 0x28).  The problem is here if they are in the same relative order (the likely fix will be to swap 0x18 and 0x28 in the unwind info).
Comment 45 Iain Sandoe 2011-10-18 15:32:33 UTC
(In reply to comment #44)
> > I think we'll need to apply the patch in the short/medium term and then figure
> > out how to control it - which will depend on which system(s) a fix is released
> > for.
> 
> One approach could be to scan the unwind info of _sigtramp live and check for
> the problematic pattern.  You call __builtin_return_address from the handler to
> get the PC of _sigtramp, then _Unwind_Find_FDE on this PC and you scan starting
> from the address you get (the length of the FDE of _sigtramp is 0xc0
> currently).
> 
> The problematic pattern are the lines:
> 
> 0x7fff85ccff61: 0x10    0x01    0x05    0x73    0x30    0x06    0x23    0x18
> 
> and
> 
> 0x7fff85ccff71: 0x10    0x03    0x05    0x73    0x30    0x06    0x23    0x28
> 
> The register number is the second field (1 or 3) and the offset in the context
> is the 8th and last field (0x18 or 0x28).  The problem is here if they are in
> the same relative order (the likely fix will be to swap 0x18 and 0x28 in the
> unwind info).

that seems reasonable if the result can be cached - otherwise it's potentially a big hit.
... or it could be done from a crt (I have in mind a new crt to try and solve some other unwind issues).
Comment 46 Eric Botcazou 2011-10-18 16:03:20 UTC
> that seems reasonable if the result can be cached - otherwise it's potentially
> a big hit.

We don't really care about performances here: a signal has been raised and we're about to throw a DWARF exception, so everything is already quite costly.  But saving the result in a static variable should indeed be possible.
Comment 47 Iain Sandoe 2011-10-18 16:22:10 UTC
(In reply to comment #46)
> > that seems reasonable if the result can be cached - otherwise it's potentially
> > a big hit.
> 
> We don't really care about performances here: a signal has been raised and
> we're about to throw a DWARF exception, so everything is already quite costly. 
> But saving the result in a static variable should indeed be possible.

OK, that's fair... mine for now (will try and cook sth up).
Comment 48 Dominique d'Humieres 2011-10-18 17:06:44 UTC
I have bootstrapped gcc on x86_64-apple-darwin10 at revision 180138 with the patch at http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01617.html. All the Ada tests pass with it.
Thanks for the hard work.

I have one question: how this unwinder problem relates to the other ones reported in bugzilla?
Comment 49 Iain Sandoe 2011-10-18 17:26:28 UTC
(In reply to comment #48)
> I have one question: how this unwinder problem relates to the other ones
> reported in bugzilla?

To the bugs I am aware of, totally unrelated ... 

This problem is caused by a discrepancy in the unwind data attached to  a specific routine.

...  the other (wider) problems are caused by the compiler emitting unwind data that the (older <=D9) system unwinders doesn't understand (and that the D10 unwind compacter doesn't either).
Comment 50 Iain Sandoe 2011-10-18 19:18:38 UTC
well I've hit a few issues.
1. _Unwind_Find_FDE is not part of the public interface (nor are the types it needs).

2. if the vendor decides to 'fix' libunwind .. we won't detect this ... 
(although I still think this idea is worth pursuing, on the grounds that 'no fix' or a fix to Libc are more likely).

3. _Unwind_Find_FDE is returning NULL in the following proof-of-concept hack..

.. not having any joy debugging this from the c++ case (maybe have to try a newer debugger)..

(have to do some other things for a while - but if you can spot my obvious mistake .. then let me know).

#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).  */

#define DX86FIX_CHECKED 0x01
#define DX86FIX_DOFIXUP 0x02

struct dwarf_eh_bases
{
  void *tbase;
  void *dbase;
  void *func;
};

typedef          int  sword __attribute__ ((mode (SI)));
typedef unsigned int  uword __attribute__ ((mode (SI)));

struct dwarf_fde
{
  uword length;
  sword CIE_delta;
  unsigned char pc_begin[];
} __attribute__ ((packed, aligned (__alignof__ (void *))));

extern struct dwarf_fde * _Unwind_Find_FDE (void *, struct dwarf_eh_bases *);

static void
__gnat_darwin_maybe_fixup_unwind_context (ucontext_t *uc, void *ad)
{
  static unsigned need_fixup = 0;

  /* Look for the FDE for the sig tramp.  */
  if ((need_fixup & DX86FIX_CHECKED) == 0)
    {
      struct dwarf_eh_bases bases;
      const struct dwarf_fde *myfde = _Unwind_Find_FDE (ad, &bases);
      
      if (myfde == NULL)
        need_fixup |= DX86FIX_DOFIXUP;  /* We're likely broken.  */
      else
	{
	  unsigned MCONTEXT_SS_RBX, MCONTEXT_SS_RDX, ts;
	  /* These are the offsets for the registers.  */
	  ts = __builtin_offsetof (_STRUCT_MCONTEXT,__ss);
	  MCONTEXT_SS_RBX = __builtin_offsetof (_STRUCT_X86_THREAD_STATE64,__rbx);
	  MCONTEXT_SS_RBX += ts;
	  MCONTEXT_SS_RDX = __builtin_offsetof (_STRUCT_X86_THREAD_STATE64,__rdx);
	  MCONTEXT_SS_RDX += ts;

// UNFINISHED
	}
      need_fixup |= DX86FIX_CHECKED;
    }

  if (need_fixup & DX86FIX_DOFIXUP)
    {
      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)
{
  struct Exception_Data *exception;
  const char *msg;

#if defined (__x86_64__)
  /* We pass the address from here so that we don't need to worry about
     inlining.  */
  __gnat_darwin_maybe_fixup_unwind_context ((ucontext_t *)ucontext,
					    __builtin_return_address (0));
#endif
Comment 51 Eric Botcazou 2011-10-18 20:04:38 UTC
> 2. if the vendor decides to 'fix' libunwind .. we won't detect this ... 
> (although I still think this idea is worth pursuing, on the grounds that 'no
> fix' or a fix to Libc are more likely).

They cannot change libunwind as this will break binary compatibility for all the programs using it.  In contrast, 99.99% of the programs don't care about the unwind info of _sigtramp (otherwise the bug wouldn't have survived long).

> 3. _Unwind_Find_FDE is returning NULL in the following proof-of-concept hack..

IIRC this can happen if the frame uses "compact unwind info", but I don't know what this means.

We could probably get away by using the core routines of libunwind, but I wonder if this is really worth the hassle.  Let's wait a little and see what kind of reply we get from the Apple side.
Comment 52 simon 2011-10-26 19:48:42 UTC
(In reply to comment #48)
> I have bootstrapped gcc on x86_64-apple-darwin10 at revision 180138 with the
> patch at http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01617.html. All the Ada
> tests pass with it.
> Thanks for the hard work.

Same here, on x86_64-apple-darwin11 at r180524.
Comment 53 Iain Sandoe 2011-10-26 19:57:25 UTC
well, no reply to my radar yet -
-  so, shall I apply the proposed patch as "sticking plaster" until we decide the Right Way forward ?
(I believe it's generally been approved, just a question of could we do sth better)
Comment 54 Eric Botcazou 2011-10-26 20:15:43 UTC
> well, no reply to my radar yet -
> -  so, shall I apply the proposed patch as "sticking plaster" until we decide
> the Right Way forward ?

OK, let's go ahead.
Comment 55 Iain Sandoe 2011-10-28 11:59:10 UTC
Author: iains
Date: Fri Oct 28 11:59:07 2011
New Revision: 180613

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=180613
Log:

ada:

	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.


Modified:
    trunk/gcc/ada/ChangeLog
    trunk/gcc/ada/init.c
Comment 56 Iain Sandoe 2011-10-28 12:01:10 UTC
please leave this bug open for now - it is not really fixed, the patch applied is a workaround.
once we get a response to the radar, we'll know better how to proceed.
Comment 57 Iain Sandoe 2011-11-18 13:19:29 UTC
Author: iains
Date: Fri Nov 18 13:19:25 2011
New Revision: 181474

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181474
Log:

gcc/ada:

	PR target/50678
	* init.c (__gnat_error_handler) [Darwin]: Move work-around to the
	bug filed as radar #10302855 from __gnat_error_handler ...
	... to (__gnat_adjust_context_for_raise) [Darwin]: New.
	(HAVE_GNAT_ADJUST_CONTEXT_FOR_RAISE) [Darwin]: Define.
	(__gnat_error_handler) [Darwin]: Use __gnat_adjust_context_for_raise.


Modified:
    trunk/gcc/ada/ChangeLog
    trunk/gcc/ada/init.c
Comment 58 Iain Sandoe 2011-11-21 09:04:14 UTC
Author: iains
Date: Mon Nov 21 09:04:08 2011
New Revision: 181553

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181553
Log:

gcc/ada:

	Backport from mainline r181474
	PR target/50678
	* init.c (__gnat_error_handler) [Darwin]: Move work-around to the
	bug filed as radar #10302855 from __gnat_error_handler ...
	... to (__gnat_adjust_context_for_raise) [Darwin]: New.
	(HAVE_GNAT_ADJUST_CONTEXT_FOR_RAISE) [Darwin]: Define.
	(__gnat_error_handler) [Darwin]: Use __gnat_adjust_context_for_raise.


Modified:
    branches/gcc-4_6-branch/gcc/ada/ChangeLog
    branches/gcc-4_6-branch/gcc/ada/init.c
Comment 59 Iain Sandoe 2011-11-21 09:05:55 UTC
I am marking this as 'waiting' - since it's probably better to get some feedback on the radar before cooking up a more complex fix.
Comment 60 Eric Botcazou 2011-11-21 09:14:56 UTC
> I am marking this as 'waiting' - since it's probably better to get some
> feedback on the radar before cooking up a more complex fix.

Let's use SUSPENDED for that.
Comment 61 Richard Biener 2012-03-22 08:26:40 UTC
GCC 4.7.0 is being released, adjusting target milestone.
Comment 62 Richard Biener 2012-06-14 08:27:29 UTC
GCC 4.7.1 is being released, adjusting target milestone.
Comment 63 Jakub Jelinek 2012-09-20 10:18:15 UTC
GCC 4.7.2 has been released.
Comment 64 simon 2013-01-31 20:48:10 UTC
I just built

Target: x86_64-apple-darwin12
gcc version 4.8.0 20130131 (experimental) [trunk revision 195611] (GCC) 

and c52104y fails again. Will try removing the call to __gnat_adjust_context_for_raise and report back.

Xcode 4.5.2
Darwin 12.2.1
Comment 65 Georg 2013-02-01 18:33:11 UTC
FWIW, with XCode 4.6 and
$ gcc/xgcc -v
Using built-in specs.
COLLECT_GCC=gcc/xgcc
Target: x86_64-apple-darwin11.4.2
Configured with: /Users/bauhaus/src/gcc/configure --prefix=/home/bauhaus/mine --disable-nls --disable-libstdcxx-pch --enable-languages=c,ada,c++,fortran CC=gcc
Thread model: posix
gcc version 4.8.0 20130130 (experimental) [trunk revision 195588] (GCC) 

                === acats Summary ===
# of expected passes            2320
# of unexpected failures        0

...
,.,. C52104Y ACATS 2.5 13-02-01 17:47:23
---- C52104Y CHECK THAT IN ARRAY ASSIGNMENTS AND IN SLICE ASSIGNMENTS,
                THE LENGTHS MUST MATCH.
   - C52104Y NO CONSTRAINT_ERROR FOR NON-NULL ARRAY SUBTYPE WHEN ONE
                DIMENSION HAS INTEGER'LAST + 3 COMPONENTS.
   - C52104Y STORAGE_ERROR RAISED WHEN DECLARING ONE PACKED BOOLEAN
                ARRAY WITH INTEGER'LAST + 3 COMPONENTS.
==== C52104Y PASSED ============================.
PASS:   c52104y
Comment 66 simon 2013-02-01 21:03:24 UTC
(In reply to comment #65)

Something amazing has happened with Xcode 4.6.

I'm running Darwin 12.2.1, Georg is running 11.4.2.

When I built r195611 with Xcode 4.5.2, the test case failed.

When I built with Xcode 4.6, the test case succeeded.

I have also built (regrettably, with Xcode 4.6) a version with the register-swap disabled; *and that succeeded too*. So it no longer matters whether we swap rbx, rdx.
Comment 67 Eric Botcazou 2013-02-01 22:01:57 UTC
> Something amazing has happened with Xcode 4.6.
> 
> I'm running Darwin 12.2.1, Georg is running 11.4.2.
> 
> When I built r195611 with Xcode 4.5.2, the test case failed.
> 
> When I built with Xcode 4.6, the test case succeeded.
> 
> I have also built (regrettably, with Xcode 4.6) a version with the
> register-swap disabled; *and that succeeded too*. So it no longer matters
> whether we swap rbx, rdx.

Can someone check whether radar #10302855 is fixed in Darwin 12?
Comment 68 simon 2013-02-02 17:27:12 UTC
(In reply to comment #66)
> (In reply to comment #65)
> 
> Something amazing has happened with Xcode 4.6.
> 
> I'm running Darwin 12.2.1, Georg is running 11.4.2.
> 
> When I built r195611 with Xcode 4.5.2, the test case failed.
> 
> When I built with Xcode 4.6, the test case succeeded.
> 
> I have also built (regrettably, with Xcode 4.6) a version with the
> register-swap disabled; *and that succeeded too*. So it no longer matters
> whether we swap rbx, rdx.

Apologies to all; I don't know what I did wrong, but now I've rebuilt the unpatched tree and a version with register-swap disabled in separate build trees, and the register-swap is no longer needed in Darwin 12.2.1 and, if applied, results in the same error that it was designed to cure.

I'm still using Xcode 4.6, but I no longer believe that affects this issue.

The compiler was:

Configured with: ../gcc-trunk-svn/configure --prefix=/opt/gcc-4.8-195682 --disable-multilib --enable-languages=c,ada --target=x86_64-apple-darwin12 --build=x86_64-apple-darwin12
Thread model: posix
gcc version 4.8.0 20130202 (experimental) [trunk revision 195682] (GCC) 

Unpatched:

		=== acats Summary ===
# of expected passes		2319
# of unexpected failures	1
*** FAILURES: c52104y 

		=== gnat Summary ===

# of expected passes		1158
# of expected failures		17
# of unsupported tests		5

The patch applied was to remove the register swap implemented at r181474:

Index: gcc/ada/init.c
===================================================================
--- gcc/ada/init.c	(revision 195682)
+++ gcc/ada/init.c	(working copy)
@@ -2093,25 +2093,6 @@
   return 0;
 }
 
-#define HAVE_GNAT_ADJUST_CONTEXT_FOR_RAISE
-
-void
-__gnat_adjust_context_for_raise (int signo ATTRIBUTE_UNUSED,
-				 void *ucontext ATTRIBUTE_UNUSED)
-{
-#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
-}
-
 static void
 __gnat_error_handler (int sig, siginfo_t *si, void *ucontext)
 {

Patched:

                === acats Summary ===
# of expected passes            2320
# of unexpected failures        0

		=== gnat Summary ===

# of expected passes		1158
# of expected failures		17
# of unsupported tests		5


I also ran Iain Sandoe's demo using c++/vendor tools (attachment 25540 [details]), which reported:

$ ./fail
caught and OK.. 
dummy cleanup, OK

The vendor G++ is
gcc version 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2336.11.00)

so I also ran with the G++ in a darwin11 build of GCC 4.7.0; again,

$ ./fail
caught and OK.. 
dummy cleanup, OK
Comment 69 Eric Botcazou 2013-02-02 18:38:02 UTC
> Apologies to all; I don't know what I did wrong, but now I've rebuilt the
> unpatched tree and a version with register-swap disabled in separate build
> trees, and the register-swap is no longer needed in Darwin 12.2.1 and, if
> applied, results in the same error that it was designed to cure.
> 
> I'm still using Xcode 4.6, but I no longer believe that affects this issue.

OK, thanks, so the bug has apparently been fixed in Darwin 12.  The last thing to do is to devise a _run time_ test to be added to __gnat_adjust_context_for_raise that will disable the code if the Darwin version is 12 or above.  Essentially anything that works is acceptable.  Any Darwin expert out there?
Comment 70 Georg 2013-02-02 23:53:35 UTC
Don't know whether this matters in any way, but I should perhaps mention that the system of comment #65 does not have autogen installed.
Comment 71 simon 2013-02-05 15:33:52 UTC
Created attachment 29360 [details]
Patch to suppress register swap on Darwin >= 12

(In reply to comment #69)

> OK, thanks, so the bug has apparently been fixed in Darwin 12.  The last thing
> to do is to devise a _run time_ test to be added to
> __gnat_adjust_context_for_raise that will disable the code if the Darwin
> version is 12 or above.  Essentially anything that works is acceptable.  Any
> Darwin expert out there?

This patch works here in Darwin 12.2.1:

                === acats Summary ===
# of expected passes            2320
# of unexpected failures        0

                === gnat Summary ===

# of expected passes            1158
# of expected failures          17
# of unsupported tests          5
/Users/simon/tmp/gcc-build-195682-patched/gcc/gnatmake version 4.8.0 20130202 (experimental) [trunk revision 195682]

It uses sysctl(3) and strtol(3), which might seem heavy but as Eric has pointed out it'll only be called when we're already in the middle of handling a signal.

Especially considering that, I may have gone OTT in caching the determined version and/or separating the version determination into a new function.

I haven't checked the returned values of the sysctl(3) calls or the length of the buffer required; what's policy? and what could be done if there was an error?

As an aside, what's the recommended technique for rebuilding and reinstalling just the RTS? ('cd gcc; make gnatlib' leaves the build in a state where 'make install' doesn't work).
Comment 72 Eric Botcazou 2013-02-05 17:48:16 UTC
> This patch works here in Darwin 12.2.1:
> 
>                 === acats Summary ===
> # of expected passes            2320
> # of unexpected failures        0
> 
>                 === gnat Summary ===
> 
> # of expected passes            1158
> # of expected failures          17
> # of unsupported tests          5

Great!

> Especially considering that, I may have gone OTT in caching the determined
> version and/or separating the version determination into a new function.

Not at all, it's not forbidden to be clever.

> I haven't checked the returned values of the sysctl(3) calls or the length of
> the buffer required; what's policy? and what could be done if there was an
> error?

We should probably check the return value of sysctl and immediately return 0 if there is a problem (it would be up to the caller to decide what to do with 0).
Then I don't think that we need to check the length.

Minor points:

 1. This should be

static int
__darwin_major_version (void)

 2. I think that you can pass NULL to strtol instead of dot_p.
 
 3. Double space before star-slash at the end of comments.

 4. No () for functions in comments.

> As an aside, what's the recommended technique for rebuilding and reinstalling
> just the RTS? ('cd gcc; make gnatlib' leaves the build in a state where 'make
> install' doesn't work).

rm $(target)/libada/stamp-libada gcc/stamp-gnatlib*
make all-target-libada


CCing Tristan who knows more about Darwin than me...
Comment 73 simon 2013-02-05 22:37:36 UTC
Created attachment 29361 [details]
Patch to suppress register swap on Darwin >= 12, v2

(In reply to comment #72)

> We should probably check the return value of sysctl and immediately return 0 if
> there is a problem (it would be up to the caller to decide what to do with 0).
> Then I don't think that we need to check the length.

Done.

I do wonder when Apple introduced this problem. It was first reported with Darwin 10, but GCC 4.6 and GNAT GPL 2011, 2012 (both based on GCC 4.5) don't show the problem, so would we have known? Should the test be 10 <= version <12

> Minor points:
> 
>  1. This should be
> 
> static int
> __darwin_major_version (void)
> 
>  2. I think that you can pass NULL to strtol instead of dot_p.
> 
>  3. Double space before star-slash at the end of comments.
> 
>  4. No () for functions in comments.

Done.

> > As an aside, what's the recommended technique for rebuilding and reinstalling
> > just the RTS? ('cd gcc; make gnatlib' leaves the build in a state where 'make
> > install' doesn't work).
> 
> rm $(target)/libada/stamp-libada gcc/stamp-gnatlib*
> make all-target-libada

and 'make install-target-libada'.
Comment 74 Eric Botcazou 2013-02-06 08:27:36 UTC
> I do wonder when Apple introduced this problem. It was first reported with
> Darwin 10, but GCC 4.6 and GNAT GPL 2011, 2012 (both based on GCC 4.5) don't
> show the problem, so would we have known? Should the test be 10 <= version <12

Ideally we should try Iain's testcase on each version but, given that we made GCC releases with the unconditional workaround, it doesn't seem sensible to change the policy for earlier versions without a solid evidence.

Tristan, any comments here?
Comment 75 gingold@gcc.gnu.org 2013-02-06 08:44:51 UTC
(In reply to comment #74)
> 
> Tristan, any comments here?

This is fine with me.

As a possible minor optimization (for the future), we could avoid the version test if __MAC_OS_X_VERSION_MIN_REQUIRED is >= 1080
according to Availability.h

Tristan.
Comment 76 Eric Botcazou 2013-02-06 09:31:15 UTC
> This is fine with me.

OK, let's go for this v2 patch, thanks.  Now we need to test it on a version of Darwin with the bug.  Although I can do it, this will take a couple of days, so if someone already has a working setup... (Georg maybe?)
Comment 77 Dominique d'Humieres 2013-02-06 19:02:27 UTC
> Created attachment 29361 [details]
> Patch to suppress register swap on Darwin >= 12, v2

With this patch applied on top of r195808 for

Target: x86_64-apple-darwin10.8.0

Configured with: ../p_work/configure --prefix=/opt/gcc/gcc4.8p-195808p1 --enable-languages=c,c++,lto,fortran,ada,objc,obj-c++ --with-gmp=/opt/mp --with-system-zlib --enable-checking=release --with-isl=/opt/mp --enable-lto --enable-plugin --enable-build-with-cxx

make -k check-ada RUNTESTFLAGS="--target_board=unix'{-m32,-m64}'"
gives

		=== acats Summary ===
# of expected passes		2320
# of unexpected failures	0

		=== gnat Summary for unix/-m64 ===

# of expected passes		1159
# of expected failures		17
# of unsupported tests		4

		=== gnat Summary ===

# of expected passes		2318
# of expected failures		34
# of unsupported tests		8
Comment 78 Eric Botcazou 2013-02-07 08:34:04 UTC
> With this patch applied on top of r195808 for
> 
> Target: x86_64-apple-darwin10.8.0
> 
> Configured with: ../p_work/configure --prefix=/opt/gcc/gcc4.8p-195808p1
> --enable-languages=c,c++,lto,fortran,ada,objc,obj-c++ --with-gmp=/opt/mp
> --with-system-zlib --enable-checking=release --with-isl=/opt/mp --enable-lto
> --enable-plugin --enable-build-with-cxx
> 
> make -k check-ada RUNTESTFLAGS="--target_board=unix'{-m32,-m64}'"
> gives
> 
>         === acats Summary ===
> # of expected passes        2320
> # of unexpected failures    0
> 
>         === gnat Summary for unix/-m64 ===
> 
> # of expected passes        1159
> # of expected failures        17
> # of unsupported tests        4
> 
>         === gnat Summary ===
> 
> # of expected passes        2318
> # of expected failures        34
> # of unsupported tests        8

Thanks!  I'm going to apply the patch on all branches.
Comment 79 Eric Botcazou 2013-02-07 18:07:40 UTC
Author: ebotcazou
Date: Thu Feb  7 18:07:18 2013
New Revision: 195862

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195862
Log:
	PR target/50678
	* init.c (__darwin_major_version): New function for x86-64/Darwin.
	(__gnat_adjust_context_for_raise) [Darwin]: Disable the workaround
	on Darwin 12 and above.

Modified:
    trunk/gcc/ada/ChangeLog
    trunk/gcc/ada/init.c
Comment 80 Eric Botcazou 2013-02-07 18:08:21 UTC
Author: ebotcazou
Date: Thu Feb  7 18:07:58 2013
New Revision: 195863

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195863
Log:
	PR target/50678
	* init.c (__darwin_major_version): New function for x86-64/Darwin.
	(__gnat_adjust_context_for_raise) [Darwin]: Disable the workaround
	on Darwin 12 and above.

Modified:
    branches/gcc-4_7-branch/gcc/ada/ChangeLog
    branches/gcc-4_7-branch/gcc/ada/init.c
Comment 81 Eric Botcazou 2013-02-07 18:09:02 UTC
Author: ebotcazou
Date: Thu Feb  7 18:08:41 2013
New Revision: 195864

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195864
Log:
	PR target/50678
	* init.c (__darwin_major_version): New function for x86-64/Darwin.
	(__gnat_adjust_context_for_raise) [Darwin]: Disable the workaround
	on Darwin 12 and above.

Modified:
    branches/gcc-4_6-branch/gcc/ada/ChangeLog
    branches/gcc-4_6-branch/gcc/ada/init.c
Comment 82 Eric Botcazou 2013-02-07 18:13:13 UTC
Hopefully fixed once for all.
Comment 83 Georg 2013-02-09 18:10:26 UTC
(In reply to comment #82)
> Hopefully fixed once for all.

Just to confirm:

                === acats Summary ===
# of expected passes            2320
# of unexpected failures        0

                === gnat Summary ===

# of expected passes            1158
# of expected failures          17
# of unsupported tests          5

$ gcc/xgcc -v
Using built-in specs.
COLLECT_GCC=gcc/xgcc
Target: x86_64-apple-darwin11.4.2
Configured with: /Users/bauhaus/src/gcc/configure --prefix=/Users/bauhaus/bauhaus/mine --disable-nls --disable-libstdcxx-pch --enable-languages=c,ada,c++,fortran CC=gcc
Thread model: posix
gcc version 4.8.0 20130208 (experimental) [trunk revision 195897] (GCC)