[Bug target/114846] New: powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[Bug target/114846] New: powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

            Bug ID: 114846
           Summary: powerpc: epilogue in _Unwind_RaiseException corrupts
                    return value due to __builtin_eh_return
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: pinskia at gcc dot gnu.org
  Target Milestone: ---
            Target: powerpc64-linux-gnu powerpc-linux-gnu

I reduced an miscompile for aarch64 inside _Unwind_RaiseException and I noticed
the same issue can be reproduced on powerpc64-linux-gnu and powerpc-linux-gnu.

Reduced testcase:
```
__attribute__((noipa,noinline))
int f(int *a, long offset, void *handler)
{
  if (*a == 5)
    return 5;
  __builtin_eh_return (offset, handler);
}

int main()
{
  int t = 5;
  if (f(&t, 0, 0) != 5)
    __builtin_abort();
}
```

This produces a load in the epilogue part for the `return 5` path (which is
reduced from the end of stack path inside _Unwind_RaiseException).
```
        li 3,5
        li 10,0
.L4:
        ld 2,40(1)
        ld 4,-24(1)
        ld 5,-16(1)
        ld 6,-8(1)
        ld 3,-32(1)
        add 1,1,10
        blr
```

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

Kewen Lin <linkw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2024-04-25
             Status|UNCONFIRMED                 |NEW
                 CC|                            |bergner at gcc dot gnu.org,
                   |                            |linkw at gcc dot gnu.org,
                   |                            |segher at gcc dot gnu.org
             Target|powerpc64-linux-gnu         |powerpc64*-linux-gnu
                   |powerpc-linux-gnu           |powerpc-linux-gnu

--- Comment #1 from Kewen Lin <linkw at gcc dot gnu.org> ---
Thanks for reporting, confirmed, it also fails on LE (ppc64le-linux).

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #2 from Kewen Lin <linkw at gcc dot gnu.org> ---
As https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114843#c8, we may need some
similar handling like r14-6440-g4b421728289e6f.

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Kewen Lin from comment #2)
> As https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114843#c8, we may need some
> similar handling like r14-6440-g4b421728289e6f.

Note rs6000_emit_epilogue mostly handles eh_returns so it might not be as hard
as other targets.

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

Kewen Lin <linkw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |linkw at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #4 from Kewen Lin <linkw at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> (In reply to Kewen Lin from comment #2)
> > As https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114843#c8, we may need some
> > similar handling like r14-6440-g4b421728289e6f.
> 
> Note rs6000_emit_epilogue mostly handles eh_returns so it might not be as
> hard as other targets.

Yes, making a patch.

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #5 from Kewen Lin <linkw at gcc dot gnu.org> ---
Created attachment 58067
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58067&action=edit
untested patch

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

Kewen Lin <linkw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #58067|0                           |1
        is obsolete|                            |

--- Comment #6 from Kewen Lin <linkw at gcc dot gnu.org> ---
Created attachment 58201
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58201&action=edit
tested patch

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Kewen Lin <linkw@gcc.gnu.org>:

https://gcc.gnu.org/g:e5fc5d42d25c86ae48178db04ce64d340a834614

commit r15-884-ge5fc5d42d25c86ae48178db04ce64d340a834614
Author: Kewen Lin <linkw@linux.ibm.com>
Date:   Tue May 28 21:13:40 2024 -0500

    rs6000: Don't clobber return value when eh_return called [PR114846]

    As the associated test case in PR114846 shows, currently
    with eh_return involved some register restoring for EH
    RETURN DATA in epilogue can clobber the one which holding
    the return value.  Referring to the existing handlings in
    some other targets, this patch makes eh_return expander
    call one new define_insn_and_split eh_return_internal which
    directly calls rs6000_emit_epilogue with epilogue_type
    EPILOGUE_TYPE_EH_RETURN instead of the previous treating
    normal return with crtl->calls_eh_return specially.

            PR target/114846

    gcc/ChangeLog:

            * config/rs6000/rs6000-logue.cc (rs6000_emit_epilogue): As
            EPILOGUE_TYPE_EH_RETURN would be passed as epilogue_type directly
            now, adjust the relevant handlings on it.
            * config/rs6000/rs6000.md (eh_return expander): Append by calling
            gen_eh_return_internal and emit_barrier.
            (eh_return_internal): New define_insn_and_split, call function
            rs6000_emit_epilogue with epilogue type EPILOGUE_TYPE_EH_RETURN.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/pr114846.c: New test.

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #8 from Kewen Lin <linkw at gcc dot gnu.org> ---
Should be fixed on trunk, it's not a regression, but we probably want
backporting this?

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #9 from Peter Bergner <bergner at gcc dot gnu.org> ---
(In reply to Kewen Lin from comment #8)
> Should be fixed on trunk, it's not a regression, but we probably want
> backporting this?

For code correctness bugs, yes, we want them backported.

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #10 from Kewen Lin <linkw at gcc dot gnu.org> ---
(In reply to Peter Bergner from comment #9)
> (In reply to Kewen Lin from comment #8)
> > Should be fixed on trunk, it's not a regression, but we probably want
> > backporting this?
> 
> For code correctness bugs, yes, we want them backported.

Thanks for confirming!  Will do backporting after burn-in time.

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #11 from Peter Bergner <bergner at gcc dot gnu.org> ---
(In reply to Kewen Lin from comment #10)
> (In reply to Peter Bergner from comment #9)
> > (In reply to Kewen Lin from comment #8)
> > > Should be fixed on trunk, it's not a regression, but we probably want
> > > backporting this?
> > 
> > For code correctness bugs, yes, we want them backported.
> 
> Thanks for confirming!  Will do backporting after burn-in time.

Have we done the backports so we can just mark this bug a FIXED?  ...or do we
still need to push the backports?

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[segher at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #12 from Segher Boessenkool <segher at gcc dot gnu.org> ---
The backports have not been done yet.

It would be good if the blockage / barrier would get some comment btw, saying
what exactly it is intended to do!  It is very much cargo-cult the way it is.

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #13 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-14 branch has been updated by Kewen Lin <linkw@gcc.gnu.org>:

https://gcc.gnu.org/g:2b5e8f918ef0027d2af8e53c4e114e1d133fc609

commit r14-10342-g2b5e8f918ef0027d2af8e53c4e114e1d133fc609
Author: Kewen Lin <linkw@linux.ibm.com>
Date:   Tue May 28 21:13:40 2024 -0500

    rs6000: Don't clobber return value when eh_return called [PR114846]

    As the associated test case in PR114846 shows, currently
    with eh_return involved some register restoring for EH
    RETURN DATA in epilogue can clobber the one which holding
    the return value.  Referring to the existing handlings in
    some other targets, this patch makes eh_return expander
    call one new define_insn_and_split eh_return_internal which
    directly calls rs6000_emit_epilogue with epilogue_type
    EPILOGUE_TYPE_EH_RETURN instead of the previous treating
    normal return with crtl->calls_eh_return specially.

            PR target/114846

    gcc/ChangeLog:

            * config/rs6000/rs6000-logue.cc (rs6000_emit_epilogue): As
            EPILOGUE_TYPE_EH_RETURN would be passed as epilogue_type directly
            now, adjust the relevant handlings on it.
            * config/rs6000/rs6000.md (eh_return expander): Append by calling
            gen_eh_return_internal and emit_barrier.
            (eh_return_internal): New define_insn_and_split, call function
            rs6000_emit_epilogue with epilogue type EPILOGUE_TYPE_EH_RETURN.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/pr114846.c: New test.

    (cherry picked from commit e5fc5d42d25c86ae48178db04ce64d340a834614)

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #14 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Kewen Lin <linkw@gcc.gnu.org>:

https://gcc.gnu.org/g:dd54ed4ae417935300a3c4bb356d37c2ae7f731e

commit r13-8866-gdd54ed4ae417935300a3c4bb356d37c2ae7f731e
Author: Kewen Lin <linkw@linux.ibm.com>
Date:   Tue May 28 21:13:40 2024 -0500

    rs6000: Don't clobber return value when eh_return called [PR114846]

    As the associated test case in PR114846 shows, currently
    with eh_return involved some register restoring for EH
    RETURN DATA in epilogue can clobber the one which holding
    the return value.  Referring to the existing handlings in
    some other targets, this patch makes eh_return expander
    call one new define_insn_and_split eh_return_internal which
    directly calls rs6000_emit_epilogue with epilogue_type
    EPILOGUE_TYPE_EH_RETURN instead of the previous treating
    normal return with crtl->calls_eh_return specially.

            PR target/114846

    gcc/ChangeLog:

            * config/rs6000/rs6000-logue.cc (rs6000_emit_epilogue): As
            EPILOGUE_TYPE_EH_RETURN would be passed as epilogue_type directly
            now, adjust the relevant handlings on it.
            * config/rs6000/rs6000.md (eh_return expander): Append by calling
            gen_eh_return_internal and emit_barrier.
            (eh_return_internal): New define_insn_and_split, call function
            rs6000_emit_epilogue with epilogue type EPILOGUE_TYPE_EH_RETURN.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/pr114846.c: New test.

    (cherry picked from commit e5fc5d42d25c86ae48178db04ce64d340a834614)

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #15 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Kewen Lin <linkw@gcc.gnu.org>:

https://gcc.gnu.org/g:0fd6ae9b20913ab84d596448e14411eedbd324f9

commit r12-10579-g0fd6ae9b20913ab84d596448e14411eedbd324f9
Author: Kewen Lin <linkw@linux.ibm.com>
Date:   Tue May 28 21:13:40 2024 -0500

    rs6000: Don't clobber return value when eh_return called [PR114846]

    As the associated test case in PR114846 shows, currently
    with eh_return involved some register restoring for EH
    RETURN DATA in epilogue can clobber the one which holding
    the return value.  Referring to the existing handlings in
    some other targets, this patch makes eh_return expander
    call one new define_insn_and_split eh_return_internal which
    directly calls rs6000_emit_epilogue with epilogue_type
    EPILOGUE_TYPE_EH_RETURN instead of the previous treating
    normal return with crtl->calls_eh_return specially.

            PR target/114846

    gcc/ChangeLog:

            * config/rs6000/rs6000-logue.cc (rs6000_emit_epilogue): As
            EPILOGUE_TYPE_EH_RETURN would be passed as epilogue_type directly
            now, adjust the relevant handlings on it.
            * config/rs6000/rs6000.md (eh_return expander): Append by calling
            gen_eh_return_internal and emit_barrier.
            (eh_return_internal): New define_insn_and_split, call function
            rs6000_emit_epilogue with epilogue type EPILOGUE_TYPE_EH_RETURN.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/pr114846.c: New test.

    (cherry picked from commit e5fc5d42d25c86ae48178db04ce64d340a834614)

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #16 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Kewen Lin <linkw@gcc.gnu.org>:

https://gcc.gnu.org/g:549701628b64a7c4ac9bb5f9623e83a8dc1d828c

commit r11-11535-g549701628b64a7c4ac9bb5f9623e83a8dc1d828c
Author: Kewen Lin <linkw@linux.ibm.com>
Date:   Tue May 28 21:13:40 2024 -0500

    rs6000: Don't clobber return value when eh_return called [PR114846]

    As the associated test case in PR114846 shows, currently
    with eh_return involved some register restoring for EH
    RETURN DATA in epilogue can clobber the one which holding
    the return value.  Referring to the existing handlings in
    some other targets, this patch makes eh_return expander
    call one new define_insn_and_split eh_return_internal which
    directly calls rs6000_emit_epilogue with epilogue_type
    EPILOGUE_TYPE_EH_RETURN instead of the previous treating
    normal return with crtl->calls_eh_return specially.

            PR target/114846

    gcc/ChangeLog:

            * config/rs6000/rs6000-logue.c (rs6000_emit_epilogue): As
            EPILOGUE_TYPE_EH_RETURN would be passed as epilogue_type directly
            now, adjust the relevant handlings on it.
            * config/rs6000/rs6000.md (eh_return expander): Append by calling
            gen_eh_return_internal and emit_barrier.
            (eh_return_internal): New define_insn_and_split, call function
            rs6000_emit_epilogue with epilogue type EPILOGUE_TYPE_EH_RETURN.

    gcc/testsuite/ChangeLog:

            * gcc.target/powerpc/pr114846.c: New test.

    (cherry picked from commit e5fc5d42d25c86ae48178db04ce64d340a834614)

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

--- Comment #17 from Kewen Lin <linkw at gcc dot gnu.org> ---

(In reply to Peter Bergner from comment #11)
> Have we done the backports so we can just mark this bug a FIXED?  ...or do
> we still need to push the backports?

(In reply to Segher Boessenkool from comment #12)
> The backports have not been done yet.
> 

Yeah, I just backported it to all active release branches, previously when I
wanted to backport them, I noticed release 12.4 is ongoing, so I held it for a
while.

> It would be good if the blockage / barrier would get some comment btw, saying
> what exactly it is intended to do!  It is very much cargo-cult the way it is.

Good idea, sorry that didn't specify it originally, the reason why adding the
barrier is that: bb verifier requires returnjump_p should be followed by a
barrier:

      if (JUMP_P (x)
          && returnjump_p (x) && ! condjump_p (x)
          && ! ((y = next_nonnote_nondebug_insn (x))
                && BARRIER_P (y)))
            fatal_insn ("return not followed by barrier", x); 

, otherwise we will encounter an ICE. Adding blockage follows the existing
practice when calling rs6000_emit_epilogue to respect option -mno-sched-epilog
(put one scheduling boundary on the emitted insns from rs6000_emit_epilogue).

[Bug target/114846] powerpc: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[linkw at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114846

Kewen Lin <linkw at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #18 from Kewen Lin <linkw at gcc dot gnu.org> ---
Should be fixed on trunk and all active release branches.