Message ID | alpine.BSF.2.00.1106152239550.62222@dair.pair.com |
---|---|
State | New |
Headers | show |
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 06/15/11 21:46, Hans-Peter Nilsson wrote: > A generic bug found in one of the less common targets. The MMIX > port usually saves the return-address from the special-register rJ to > a general call-saved register (helped by get_hard_reg_initial_val), > restored to rJ immediately on return from the call. I know, the > "restore immediately on return" is odd and I don't remember exactly > why I did it that way and not in the "return" insn; I might change that, > but that's only incidental. IRA dutifully refuses to allocate a > register for the return-address seeing that it's live across the setjmp, > but reload comes to the "rescue" and when fixing up the accesses to the > resulting stack-slot, reuses the reload register across the setjmp, so > the generated code looks just like an ordinary register allocation, > save for the sometimes-unused stack-slot (in sjtest, completely > unused; in sjtest3 just not used across the setjmp). The test-case is > slightly changed from the submitted original to avoid the > miscompilation causing an endless loop and a test-timeout. > > There are two fixes, because when register contents is invalidated for > the main path (the patch to reload1.c), there's fallback code (patch > to reload.c) that also iterates over code, and which also lacks proper > handling of setjmp-type calls. > > You will see this bug on other targets if you're unlucky enough to get > a reuse of a register that held something live at the time of the > setjmp but which died and the register reused for something else some > time after the setjmp call but before the longjmp call, and the > original value is live again after the second setjmp return. This > just naturally happens more often for MMIX (read: always when there's > a temporary held in a register like the loop counter in the > test-case), given that (set rJ (mem stack-slot)) is emitted after every > call and always needs a reload through a general register, and is always > reloaded from the same stack-slot. A trivial attempt failed to come > up with a test-case that fails for x86_64. > > When working on the test-case, I noticed IPA discovering the > noreturn-ness of (calls to) functions calling longjmp but which were > declared noinline; naturally I didn't want gcc to look at the called > function or assume anything of its contents. Adding attribute weak > helped, but that's a kludge that LTO will (at some time) see through. > So, isn't there now a need for a "noipa" attribute? (Better call it > "extern" or "foreign" or some other color to avoid leaking internal > terms.) The attribute would create the effect of a function that gcc > doesn't inspect (or really, ignores knowledge at calls), so we can > keep having self-contained tests in a single-file now and for future > IPA-like improvements. > > Regtested mmix all open branches and native x86_64-linux trunk. > > Ok? > For all open branches (after native regtesting)? > > gcc/testsuite: > PR rtl-optimization/48542 > * gcc.dg/torture/pr48542.c: New test. > > gcc: > PR rtl-optimization/48542 > * reload.c (find_equiv_reg): Stop looking when finding a > setjmp-type call. > * reload1.c (reload_as_needed): Invalidate all reload > registers when crossing a setjmp-type call. OK. Jeff -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJN+n4AAAoJEBRtltQi2kC7QzkH/RjqeAN+7a4jJxyUsgMBJ/1l N3iLC0JlZ50WmKBeT0R358wuoO17+J/RfM3Im3qmLYlf/16g70DxfrkT+Bs/LtEm 3wuGyt67kJrSXagw/7+cF2JW5hNDPMJKq1fWZNnyJYudPmdLY3s3Iki/D21c95nI NZeMRKzbNUVMFrilrjVKtcAtQj7UM/BHwNgOFen0Q3WlSwvVa5eB1xQqiHigjXwh g03Vtax1iXaK/8ziQG7Ww7E0fgejCcfMFSTKIH1BK6mjmAjy1flGvYidlCSKmd9P OrDJ7SQvPGeKrLzoymrMUJWw+EmMaOor8f7rW3xnwbkDtyBRgPyI0OUqESzGxcg= =72ZM -----END PGP SIGNATURE-----
Index: gcc/testsuite/gcc.dg/torture/pr48542.c =================================================================== --- gcc/testsuite/gcc.dg/torture/pr48542.c (revision 0) +++ gcc/testsuite/gcc.dg/torture/pr48542.c (revision 0) @@ -0,0 +1,57 @@ +/* { dg-do run } */ +/* The return-address was clobbered. */ +#include <stdlib.h> +#include <setjmp.h> + +jmp_buf env; +extern void sub(void); +extern void sub3(void); +int called; +__attribute__ ((__noinline__)) +int sjtest() +{ + int i; + if (setjmp(env)) + return 99; + + for (i = 0; i < 10; i++) + sub(); + + longjmp(env, 1); +} + +__attribute__ ((__noinline__)) +void sub(void) +{ + called++; +} + +int called3; +__attribute__ ((__noinline__)) +int sjtest3() +{ + int i; + if (setjmp(env)) + return 42; + + for (i = 0; i < 10; i++) + sub3(); + return 0; +} + +__attribute__ ((__noinline__)) +void sub3(void) +{ + called3++; + if (called3 == 10) + longjmp (env, 1); +} + +int main(void) +{ + if (sjtest() != 99 || called != 10) + abort(); + if (sjtest3() != 42 || called3 != 10) + abort(); + exit (0); +} Index: gcc/reload.c =================================================================== --- gcc/reload.c (revision 174961) +++ gcc/reload.c (working copy) @@ -6791,6 +6791,15 @@ find_equiv_reg (rtx goal, rtx insn, enum || num > PARAM_VALUE (PARAM_MAX_RELOAD_SEARCH_INSNS)) return 0; + /* Don't reuse register contents from before a setjmp-type + function call; on the second return (from the longjmp) it + might have been clobbered by a later reuse. It doesn't + seem worthwhile to actually go and see if it is actually + reused even if that information would be readily available; + just don't reuse it across the setjmp call. */ + if (CALL_P (p) && find_reg_note (p, REG_SETJMP, NULL_RTX)) + return 0; + if (NONJUMP_INSN_P (p) /* If we don't want spill regs ... */ && (! (reload_reg_p != 0 Index: gcc/reload1.c =================================================================== --- gcc/reload1.c (revision 174961) +++ gcc/reload1.c (working copy) @@ -4844,6 +4844,13 @@ reload_as_needed (int live_known) { AND_COMPL_HARD_REG_SET (reg_reloaded_valid, call_used_reg_set); AND_COMPL_HARD_REG_SET (reg_reloaded_valid, reg_reloaded_call_part_clobbered); + + /* If this is a call to a setjmp-type function, we must not + reuse any reload reg contents across the call; that will + just be clobbered by other uses of the register in later + code, before the longjmp. */ + if (find_reg_note (insn, REG_SETJMP, NULL_RTX)) + CLEAR_HARD_REG_SET (reg_reloaded_valid); } }