From 7211155732244ac527c075d033164ec253ee0971 Mon Sep 17 00:00:00 2001 From: Eric Botcazou Date: Tue, 29 Oct 2024 21:40:34 +0100 Subject: [PATCH] Fix miscompilation of function containing __builtin_unreachable This is a wrong-code generation on the SPARC for a function containing a call to __builtin_unreachable caused by the delay slot scheduling pass, and more specifically the find_end_label function which has these lines: /* Otherwise, see if there is a label at the end of the function. If there is, it must be that RETURN insns aren't needed, so that is our return label and we don't have to do anything else. */ The comment was correct 20 years ago but no longer is nowadays in the presence of RTL epilogues and calls to __builtin_unreachable, so the patch just removes the associated two lines of code: else if (LABEL_P (insn)) *plabel = as_a (insn); and otherwise contains just adjustments to the commentary. gcc/ PR rtl-optimization/117327 * reorg.cc (find_end_label): Do not return a dangling label at the end of the function and adjust commentary. gcc/testsuite/ * gcc.c-torture/execute/20241029-1.c: New test. --- gcc/reorg.cc | 38 ++++++++----------- .../gcc.c-torture/execute/20241029-1.c | 23 +++++++++++ 2 files changed, 38 insertions(+), 23 deletions(-) create mode 100644 gcc/testsuite/gcc.c-torture/execute/20241029-1.c diff --git a/gcc/reorg.cc b/gcc/reorg.cc index 51321ce7b80..68bf30801cf 100644 --- a/gcc/reorg.cc +++ b/gcc/reorg.cc @@ -336,13 +336,14 @@ insn_sets_resource_p (rtx insn, struct resources *res, return resource_conflicts_p (&insn_sets, res); } -/* Find a label at the end of the function or before a RETURN. If there - is none, try to make one. If that fails, returns 0. +/* Find a label before a RETURN. If there is none, try to make one; if this + fails, return 0. KIND is either ret_rtx or simple_return_rtx, indicating + which type of RETURN we're looking for. - The property of such a label is that it is placed just before the - epilogue or a bare RETURN insn, so that another bare RETURN can be - turned into a jump to the label unconditionally. In particular, the - label cannot be placed before a RETURN insn with a filled delay slot. + The property of the label is that it is placed just before a bare RETURN + insn, so that another bare RETURN can be turned into a jump to the label + unconditionally. In particular, the label cannot be placed before a + RETURN insn with a filled delay slot. ??? There may be a problem with the current implementation. Suppose we start with a bare RETURN insn and call find_end_label. It may set @@ -353,9 +354,7 @@ insn_sets_resource_p (rtx insn, struct resources *res, Note that this is probably mitigated by the following observation: once function_return_label is made, it is very likely the target of a jump, so filling the delay slot of the RETURN will be much more - difficult. - KIND is either simple_return_rtx or ret_rtx, indicating which type of - return we're looking for. */ + difficult. */ static rtx_code_label * find_end_label (rtx kind) @@ -375,10 +374,7 @@ find_end_label (rtx kind) if (*plabel) return *plabel; - /* Otherwise, see if there is a label at the end of the function. If there - is, it must be that RETURN insns aren't needed, so that is our return - label and we don't have to do anything else. */ - + /* Otherwise, scan the insns backward from the end of the function. */ insn = get_last_insn (); while (NOTE_P (insn) || (NONJUMP_INSN_P (insn) @@ -386,9 +382,8 @@ find_end_label (rtx kind) || GET_CODE (PATTERN (insn)) == CLOBBER))) insn = PREV_INSN (insn); - /* When a target threads its epilogue we might already have a - suitable return insn. If so put a label before it for the - function_return_label. */ + /* First, see if there is a RETURN at the end of the function. If so, + put the label before it. */ if (BARRIER_P (insn) && JUMP_P (PREV_INSN (insn)) && PATTERN (PREV_INSN (insn)) == kind) @@ -397,8 +392,8 @@ find_end_label (rtx kind) rtx_code_label *label = gen_label_rtx (); LABEL_NUSES (label) = 0; - /* Put the label before any USE insns that may precede the RETURN - insn. */ + /* Put the label before any USE insns that may precede the + RETURN insn. */ while (GET_CODE (temp) == USE) temp = PREV_INSN (temp); @@ -406,15 +401,12 @@ find_end_label (rtx kind) *plabel = label; } - else if (LABEL_P (insn)) - *plabel = as_a (insn); + /* If the basic block reordering pass has moved the return insn to some + other place, try to locate it again and put the label there. */ else { rtx_code_label *label = gen_label_rtx (); LABEL_NUSES (label) = 0; - /* If the basic block reorder pass moves the return insn to - some other place try to locate it again and put our - function_return_label there. */ while (insn && ! (JUMP_P (insn) && (PATTERN (insn) == kind))) insn = PREV_INSN (insn); if (insn) diff --git a/gcc/testsuite/gcc.c-torture/execute/20241029-1.c b/gcc/testsuite/gcc.c-torture/execute/20241029-1.c new file mode 100644 index 00000000000..1090edd9c7c --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/20241029-1.c @@ -0,0 +1,23 @@ +/* PR rtl-optimization/117327 */ +/* Testcase by Brad Moody */ + +__attribute__((noinline)) +void foo(int *self, int *x) +{ + __builtin_puts ("foo\n"); + + if (x) { + while (1) { + ++*self; + if (*self == 6) break; + if (*self == 7) __builtin_unreachable(); + } + } +} + +int main (void) +{ + int y = 0; + foo (&y, 0); + return 0; +}