With revision 142890 this test passed. From revision 142900 and on, this test has failed as follows: Running /tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.c-torture/execute/execute.exp ... FAIL: gcc.c-torture/execute/built-in-setjmp.c execution, -O2 FAIL: gcc.c-torture/execute/built-in-setjmp.c execution, -O3 -fomit-frame-pointer FAIL: gcc.c-torture/execute/built-in-setjmp.c execution, -O3 -fomit-frame-pointer -funroll-loops FAIL: gcc.c-torture/execute/built-in-setjmp.c execution, -O3 -fomit-frame-pointer -funroll-all-loops -finline-function\ s FAIL: gcc.c-torture/execute/built-in-setjmp.c execution, -O3 -g With the message in the logfile being: PASS: gcc.c-torture/execute/built-in-setjmp.c compilation, -O2 program stopped with signal 6.^M FAIL: gcc.c-torture/execute/built-in-setjmp.c execution, -O2 (i.e. calls to abort) Author of patches in suspect revision range CC:ed. I understand that getting useful information out of an execution failure would seem daunting when you don't have experience with the run-time environmnent, so I'll try and assist with that, if needed.
The only change my patch does are two successful replace_read calls in DSE, nothing else changed, and those 2 replacements look correct to me. In *.optimized we have: <bb 2>: D.1233 = __builtin_alloca (20); <bb 3>: p = (char *) D.1233; __builtin_memcpy (p, &"test"[0], 5); <bb 4>: __builtin_setjmp_setup (&buf, &<L0>); goto <bb 6>; <L0>: __builtin_setjmp_receiver (&<L0>); goto <bb 14>; <bb 6>: D.1240 = p + 2; D.1244 = __builtin_alloca ((long unsigned int) *D.1240 * 4); goto <bb 9>; <bb 7>: abort (); <bb 8>: exit (0); <bb 9>: if (*D.1240 > 0) goto <bb 15>; else goto <bb 11>; and what DSE now can do and could not do, is replace the two *D.1240 reads with 0x73, as p[2] is known to contain 's' and nothing else could have changed it. In *.cse2 we have: (insn 35 34 36 2 B2.c:19 (set (mem:SI (reg/f:SI 14 sp) [0 S4 A8]) (reg:SI 57)) 38 {*movsi_internal} (expr_list:REG_EQUAL (const_int 1953719668 [0x74736574]) (nil))) (insn 36 35 38 2 B2.c:19 (set (mem:QI (plus:SI (reg/f:SI 14 sp) (const_int 4 [0x4])) [0 S1 A8]) (const_int 0 [0x0])) 47 {movqi} (nil)) ... # no sp related stores (insn 56 42 57 2 B2.c:30 (set (reg/f:SI 52 [ D.1240 ]) (plus:SI (reg/f:SI 14 sp) (const_int 2 [0x2]))) 75 {*addsi3_non_v32} (nil)) (insn 57 56 58 2 B2.c:30 (set (reg:SI 60) (sign_extend:SI (mem:QI (reg/f:SI 52 [ D.1240 ]) [0 S1 A8]))) 56 {extendqisi2} (nil)) ... # no sp related stores (insn 62 61 65 2 B2.c:30 (set (reg/f:SI 14 sp) (minus:SI (reg/f:SI 14 sp) (reg:SI 64))) 83 {*subsi3_non_v32} (nil)) (insn 65 62 79 2 B2.c:30 (set (reg/f:SI 51 [ D.1244 ]) (reg/f:SI 14 sp)) 38 {*movsi_internal} (nil)) (insn 79 65 80 2 B2.c:33 (set (cc0) (mem:QI (reg/f:SI 52 [ D.1240 ]) [0 S1 A8])) 6 {*tstqi_non_cmp} (nil)) (jump_insn 80 79 45 2 B2.c:33 (set (pc) (if_then_else (le (cc0) (const_int 0 [0x0])) (label_ref 91) (pc))) 198 {ble} (expr_list:REG_BR_PROB (const_int 900 [0x384]) (nil))) During DSE1 is: (insn 145 34 146 2 B2.c:19 (set (reg:QI 76) (const_int 115 [0x73])) -1 (nil)) (insn 146 145 35 2 B2.c:19 (set (reg:QI 77) (const_int 115 [0x73])) -1 (nil)) inserted after insn 34, insn 57 becomes: (insn 57 56 58 2 B2.c:30 (set (reg:SI 60) (sign_extend:SI (reg:QI 76))) 56 {extendqisi2} (nil)) and insn 79 becomes: (insn 79 65 80 2 B2.c:33 (set (cc0) (reg:QI 77)) 6 {*tstqi_non_cmp} (nil)) During combine, the jump_insn 80 is optimized out, as 0x73 is known to be >= 0. I haven't read the whole RTL till the last phase, so I don't know where things went wrong, but I'm pretty sure the bug isn't in DSE and the recent changes, so cris had just some latent bug.
(In reply to comment #1) > I haven't read the whole RTL till the last phase, so I don't know where things > went wrong, but I'm pretty sure the bug isn't in DSE and the recent changes, It might very well be that your change exposed a bug elsewhere... > so cris had just some latent bug. ...but FWIW in either case, this is a faulty conclusion; there is more code than the back-end. I'll try to point to it, but I hope I can get assistance if the trace points at code you understand.
I couldn't spot any bug eyeballing the assembly (or final RTL dump), so can you please debug how this now fails at runtime (abort, corruption (where), etc.)?
(In reply to comment #3) > I couldn't spot any bug eyeballing the assembly (or final RTL dump), so can you > please debug how this now fails at runtime (abort, corruption (where), etc.)? "(i.e. calls to abort)" Thanks for looking and for the DSE analysis. I'll have a look before the next year.
Glancing at the assembly and simulator trace (no looking at rtl or tree dumps yet), the value of "p" (sp after the first alloca) is somehow lost and after the __builtin_longjmp we effectively strcmp (NULL, "test") which FWIW, doesn't SEGV in the simulator, but nevertheless obviously doesn't match. At the longjmp-receiver, "p" is retrieved for the strcmp call as "move.d [$sp+8],$r10", i.e. $r10 = *((char *) $sp + 8), but that's bogus as nothing was stored there; "move.d $sp,$r10" (just copying the stack-pointer) would have been correct. I'll look further. HNY.
Hi HP, (In reply to comment #5) > Glancing at the assembly and simulator trace (no looking at rtl or tree dumps > yet), the value of "p" (sp after the first alloca) is somehow lost and after > the __builtin_longjmp we effectively strcmp (NULL, "test") which FWIW, doesn't > SEGV in the simulator, but nevertheless obviously doesn't match. > At the longjmp-receiver, "p" is retrieved for the strcmp call as "move.d > [$sp+8],$r10", i.e. $r10 = *((char *) $sp + 8), but that's bogus as nothing was > stored there; "move.d $sp,$r10" (just copying the stack-pointer) would have > been correct. Sounds like this could maybe be a dup of bug 38952, where the frame pointer is incorrectly calculated when setjmp saves it in the jmp_buf, and therefore restored to an incorrect value by longjmp. For a quick test without rebuilding your compiler, see if "-mpreferred-stack-boundary=2" or "-mno-stackrealign" fixes the failure in built-in-setjmp.c; if so there's a patch in the PR that should help. cheers, DaveK
(In reply to comment #6) > Sounds like this could maybe be a dup of bug 38952, where the frame pointer > is incorrectly calculated when setjmp saves it in the jmp_buf, and therefore > restored to an incorrect value by longjmp. For a quick test without rebuilding > your compiler, see if "-mpreferred-stack-boundary=2" or "-mno-stackrealign" > fixes the failure in built-in-setjmp.c; if so there's a patch in the PR that > should help. I don't have those options :) and the stack-alignment options I have make no difference - but I think you might on the right track. I'll investigate whether this is just a sign of CRIS suffering from lack of TARGET_BUILTIN_SETJMP_FRAME_VALUE. Can't be a dup though, same target bug but different targets. Fixing one won't fix the other. :) Right, changing component to target. Thanks!
Oh, bah, I misread the Host field for Target! Guess it probably won't be TARGET_BUILTIN_SETJMP_FRAME_VALUE then. You only need it if your stack frames have unpredicatable gaps in them so that you can't eliminate one reg against another that points the other side of the gap because the offset isn't known. Unless you've got those on CRIS, I've probably just been dangling a red herring under your nose, sorry!
(In reply to comment #8) > Guess it probably won't be TARGET_BUILTIN_SETJMP_FRAME_VALUE then. At any rate, changing it to hard_frame_pointer_rtx doesn't help by itself. (Resulting diffs in RTL dumps are gone after 132r.unshare, for r144898.) Either, GCC should punt and force p to the stack, or calculate p / keep track of the stack-pointer correctly: the value is off by 20 when used after the longjump. (It should be "move.d [$sp+28],$r10", not $sp+8.) Right, that's the "sp -= 20" due to the __builtin_alloca (20) before the __builtin_setjmp call.
Looks like I should punt and force "#define FRAME_POINTER_REQUIRED cfun->calls_alloca" or "#define FRAME_POINTER_REQUIRED cfun->has_nonlocal_label". Apparently this is *not* one of the cases where "The compiler recognizes those cases and automatically gives the function a frame pointer regardless of what @code{FRAME_POINTER_REQUIRED} says. You don't need to worry about them."
(In reply to comment #10) > Looks like I should punt and force "#define FRAME_POINTER_REQUIRED > cfun->calls_alloca" or "#define FRAME_POINTER_REQUIRED > cfun->has_nonlocal_label". > Apparently this is *not* one of the cases where "The compiler recognizes those > cases and automatically gives the function a frame pointer regardless of what > @code{FRAME_POINTER_REQUIRED} says. You don't need to worry about > them." > I think you may need a frame pointer for if (cfun->calls_alloca || cfun->has_nonlocal_label || crtl->has_nonlocal_goto) See expand_stack_alignment in cfgexpand.c. Joey and Xuepeng may know more about this.
(In reply to comment #11) > I think you may need a frame pointer for > > if (cfun->calls_alloca > || cfun->has_nonlocal_label > || crtl->has_nonlocal_goto) That should be covered by the generic code, not the target's FRAME_POINTER_REQUIRED, and besides, for this target (if I decode defaults.h correctly, and what seems intuitively correct): > See expand_stack_alignment in cfgexpand.c. For this target (if I decode defaults.h correctly, and what seems intuitively correct): if (! SUPPORTS_STACK_ALIGNMENT) return; No, this seems more like a case of reload not being able to keep track of stack-pointer adjustments. Actually, I see that for all labels that are reachable by indirect jumps, it assumes they are reached only by locations that have the initial elimination offsets. Instead, it should bail out on those (signalling that the offset is not known), always or optionally, if there is any call or any indirect jump in the function that does not have the initial elimination offset or optionally (set it to the offsets for the first), and if any call or indirect jump has different elimination offsets, bail out.
Noticed bug in middle-end (reload1.c:set_label_offsets) changing to middle-end.
Subject: Bug 38609 Author: hp Date: Thu Mar 19 03:52:58 2009 New Revision: 144951 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=144951 Log: PR middle-end/38609 * config/cris/cris.h (FRAME_POINTER_REQUIRED): Force for all functions with dynamic stack-pointer adjustments. Modified: trunk/gcc/ChangeLog trunk/gcc/config/cris/cris.h
Follow-up to 39499.