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.


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);
        }
     }


brgds, H-P

Reply via email to