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
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.
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.
On powerpc-apple-darwin9.8.0, c52103x, c52104x, and c52104y fail with a different error: raised STORAGE_ERROR : stack overflow detected
(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.
(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.
> 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.
> 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.
(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.
> 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.
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.
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..
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.
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.
(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?
(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.
(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.
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.
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).
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.
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).
Created attachment 25474 [details] optimized tree dump for c52104y shows the builtin_alloca_with_align
(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.
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.
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 ?
passes at -O0 on darwin9 and 10
> 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...
(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 ;-)
> 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.
(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.
> 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.
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.
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.
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.
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. ...
> 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.
(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
.. 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.
(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.
> 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.
> 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.
(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.
> > 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.
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.
> 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).
(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).
> 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.
(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).
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?
(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).
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
> 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.
(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.
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)
> 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.
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
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.
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
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
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.
> 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.
GCC 4.7.0 is being released, adjusting target milestone.
GCC 4.7.1 is being released, adjusting target milestone.
GCC 4.7.2 has been released.
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
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
(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.
> 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?
(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
> 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?
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.
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).
> 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...
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'.
> 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?
(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.
> 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?)
> 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
> 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.
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
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
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
Hopefully fixed once for all.
(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)