[Bug libgcc/114843] New: AArch64: Wrong Register Reload in _Unwind_RaiseException causes corrupt return value on _URC_END_OF_STACK

[Bug libgcc/114843] New: AArch64: Wrong Register Reload in _Unwind_RaiseException causes corrupt return value on _URC_END_OF_STACK

[hugo at algoriddim dot com]

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

            Bug ID: 114843
           Summary: AArch64: Wrong Register Reload in
                    _Unwind_RaiseException causes corrupt return value on
                    _URC_END_OF_STACK
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libgcc
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hugo at algoriddim dot com
  Target Milestone: ---

Created attachment 58034
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58034&action=edit
Small test for reproduction

When raising an Exception with `_Unwind_RaiseException` on Ubuntu 23.10
aarch64, instead of returning _URC_END_OF_STACK, the function returns the first
argument previously passed to `_Unwind_RaisedException` as part of an
(incorrect) reload.

This happens with the Ubuntu GCC Version (13.2.0), Clang 18.1.0 Using libgcc_s,
and the GCC version + libgcc_s build from master.

## Build GCC Version

GCC Version: commit ca00bf02dcc37f9ff1028ca1d90e8b8d95d69683 (HEAD -> master,
origin/trunk, origin/master, origin/HEAD)

```sh
hmelder@ubuntu:~/gcc/objdir$ ./gcc/xgcc --version
xgcc (GCC) 14.0.1 20240423 (experimental)
Copyright (C) 2024 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
```

```sh
mkdir objdir
cd objdir
../configure --enable-languages=c
make all-gcc -j 12
make all-target-libgcc -j 12
```

# Test

See `ReproduceUnwindTest-main.i`. Test was compiled with the following flags:

```sh
gcc -v -Wall -Wextra main.c -o ReproduceUnwindTest -save-temps
```

Expected: `Output RaiseException returned 0x5` (_URC_END_OF_STACK). This is the
behaviour on FreeBSD with CompilerRT and libunwind and Darwin.

Actual: Returns the first parameter passed to _Unwind_RaiseException because of
a reload of spilled registers before returning _URC_END_OF_STACK in
unwind.inc:108.

# Diagnosis

Here is the assembly of the `_Unwind_RaiseException` function.
We are currently right after `if (code == _URC_END_OF_STACK)` in
unwind.inc:108.
The return value of `uw_frame_state_for` is 5 (_URC_END_OF_STACK).

```asm
libgcc_s.so.1`_Unwind_RaiseException:
    0xfffff7f27700 <+0>:   sub    sp, sp, #0xc10
    0xfffff7f27704 <+4>:   stp    x29, x30, [sp]
    0xfffff7f27708 <+8>:   mov    x29, sp
    0xfffff7f2770c <+12>:  xpaclri 
    0xfffff7f27710 <+16>:  stp    x21, x22, [sp, #0x40]
    0xfffff7f27714 <+20>:  add    x22, sp, #0xc0
    0xfffff7f27718 <+24>:  add    x21, sp, #0x840
    0xfffff7f2771c <+28>:  stp    x0, x1, [sp, #0x10]
    0xfffff7f27720 <+32>:  add    x1, sp, #0xc10
    0xfffff7f27724 <+36>:  stp    x2, x3, [sp, #0x20]
    0xfffff7f27728 <+40>:  mov    x2, x30
    0xfffff7f2772c <+44>:  stp    x19, x20, [sp, #0x30]
    0xfffff7f27730 <+48>:  mov    x20, x0
    0xfffff7f27734 <+52>:  add    x19, sp, #0x480
    0xfffff7f27738 <+56>:  mov    x0, x22
    0xfffff7f2773c <+60>:  stp    x23, x24, [sp, #0x50]
    0xfffff7f27740 <+64>:  stp    x25, x26, [sp, #0x60]
    0xfffff7f27744 <+68>:  stp    x27, x28, [sp, #0x70]
    0xfffff7f27748 <+72>:  stp    d8, d9, [sp, #0x80]
    0xfffff7f2774c <+76>:  stp    d10, d11, [sp, #0x90]
    0xfffff7f27750 <+80>:  stp    d12, d13, [sp, #0xa0]
    0xfffff7f27754 <+84>:  stp    d14, d15, [sp, #0xb0]
    0xfffff7f27758 <+88>:  bl     0xfffff7f27020 ; uw_init_context_1 at
unwind-dw2.c:1324:1
    0xfffff7f2775c <+92>:  mov    x1, x22
    0xfffff7f27760 <+96>:  mov    x0, x19
    0xfffff7f27764 <+100>: mov    x2, #0x3c0 ; =960 
    0xfffff7f27768 <+104>: bl     0xfffff7f13740
    0xfffff7f2776c <+108>: b      0xfffff7f277a0 ; <+160> at unwind.inc:104:14
    0xfffff7f27770 <+112>: cbnz   w2, 0xfffff7f27814 ; <+276> at
unwind.inc:113:9
    0xfffff7f27774 <+116>: ldr    x5, [sp, #0xbe0]
    0xfffff7f27778 <+120>: cbz    x5, 0xfffff7f27794 ; <+148> at
unwind.inc:127:7
    0xfffff7f2777c <+124>: ldr    x2, [x20]
    0xfffff7f27780 <+128>: blr    x5
    0xfffff7f27784 <+132>: cmp    w0, #0x6
    0xfffff7f27788 <+136>: b.eq   0xfffff7f2781c ; <+284> at unwind.inc:132:18
    0xfffff7f2778c <+140>: cmp    w0, #0x8
    0xfffff7f27790 <+144>: b.ne   0xfffff7f27814 ; <+276> at unwind.inc:113:9
    0xfffff7f27794 <+148>: mov    x1, x21
    0xfffff7f27798 <+152>: mov    x0, x19
    0xfffff7f2779c <+156>: bl     0xfffff7f27264 ; uw_update_context at
unwind-dw2.c:1266:1
    0xfffff7f277a0 <+160>: mov    x1, x21
    0xfffff7f277a4 <+164>: mov    x0, x19
    0xfffff7f277a8 <+168>: bl     0xfffff7f25f00 ; uw_frame_state_for at
unwind-dw2.c:997:3
    0xfffff7f277ac <+172>: mov    w2, w0
    0xfffff7f277b0 <+176>: mov    w1, #0x1 ; =1 
    0xfffff7f277b4 <+180>: mov    x4, x19
    0xfffff7f277b8 <+184>: mov    x3, x20
    0xfffff7f277bc <+188>: mov    w0, w1
    0xfffff7f277c0 <+192>: cmp    w2, #0x5
    0xfffff7f277c4 <+196>: b.ne   0xfffff7f27770 ; <+112> at unwind.inc:110:10
->  0xfffff7f277c8 <+200>: mov    x4, #0x0 ; =0 
    0xfffff7f277cc <+204>: mov    w0, w2
    0xfffff7f277d0 <+208>: ldp    x29, x30, [sp]
    0xfffff7f277d4 <+212>: ldp    x0, x1, [sp, #0x10]
    0xfffff7f277d8 <+216>: ldp    x2, x3, [sp, #0x20]
    0xfffff7f277dc <+220>: ldp    x19, x20, [sp, #0x30]
    0xfffff7f277e0 <+224>: ldp    x21, x22, [sp, #0x40]
    0xfffff7f277e4 <+228>: ldp    x23, x24, [sp, #0x50]
    0xfffff7f277e8 <+232>: ldp    x25, x26, [sp, #0x60]
    0xfffff7f277ec <+236>: ldp    x27, x28, [sp, #0x70]
    0xfffff7f277f0 <+240>: ldp    d8, d9, [sp, #0x80]
    0xfffff7f277f4 <+244>: ldp    d10, d11, [sp, #0x90]
    0xfffff7f277f8 <+248>: ldp    d12, d13, [sp, #0xa0]
    0xfffff7f277fc <+252>: ldp    d14, d15, [sp, #0xb0]
    0xfffff7f27800 <+256>: add    sp, sp, #0xc10
    0xfffff7f27804 <+260>: cbz    x4, 0xfffff7f27810 ; <+272> at
unwind.inc:141:1
    0xfffff7f27808 <+264>: add    sp, sp, x5
    0xfffff7f2780c <+268>: br     x6
    0xfffff7f27810 <+272>: ret    
    0xfffff7f27814 <+276>: mov    w2, #0x3 ; =3 
    0xfffff7f27818 <+280>: b      0xfffff7f277c8 ; <+200> at unwind.inc:141:1
    0xfffff7f2781c <+284>: str    xzr, [x20, #0x10]
    0xfffff7f27820 <+288>: mov    x0, x19
    0xfffff7f27824 <+292>: bl     0xfffff7f13860 ; symbol stub for:
pthread_key_create
    0xfffff7f27828 <+296>: mov    x4, x0
    0xfffff7f2782c <+300>: ldr    x3, [sp, #0x7c0]
    0xfffff7f27830 <+304>: mov    x1, x22
    0xfffff7f27834 <+308>: mov    x2, #0x3c0 ; =960 
    0xfffff7f27838 <+312>: mov    x0, x19
    0xfffff7f2783c <+316>: sub    x3, x4, x3, lsr #63
    0xfffff7f27840 <+320>: str    x3, [x20, #0x18]
    0xfffff7f27844 <+324>: bl     0xfffff7f13740
    0xfffff7f27848 <+328>: mov    x2, x21
    0xfffff7f2784c <+332>: mov    x1, x19
    0xfffff7f27850 <+336>: mov    x0, x20
    0xfffff7f27854 <+340>: bl     0xfffff7f273ac ;
_Unwind_RaiseException_Phase2 at unwind.inc:41:1
    0xfffff7f27858 <+344>: mov    w2, #0x2 ; =2 
    0xfffff7f2785c <+348>: cmp    w0, #0x7
    0xfffff7f27860 <+352>: b.ne   0xfffff7f277c8 ; <+200> at unwind.inc:141:1
    0xfffff7f27864 <+356>: mov    x1, x19
    0xfffff7f27868 <+360>: mov    x0, x22
    0xfffff7f2786c <+364>: bl     0xfffff7f24a60 ; uw_install_context_1 at
unwind-dw2.c:1405:1
    0xfffff7f27870 <+368>: mov    x19, x0
    0xfffff7f27874 <+372>: ldr    x20, [sp, #0x798]
    0xfffff7f27878 <+376>: ldr    x0, [sp, #0x790]
    0xfffff7f2787c <+380>: mov    x1, x20
    0xfffff7f27880 <+384>: bl     0xfffff7f276f0 ; _Unwind_DebugHook at
unwind-dw2.c:1382:1
    0xfffff7f27884 <+388>: bl     0xfffff7f2af40 ; __arm_za_disable
    0xfffff7f27888 <+392>: mov    x5, x19
    0xfffff7f2788c <+396>: mov    x6, x20
    0xfffff7f27890 <+400>: mov    x4, #0x1 ; =1 
    0xfffff7f27894 <+404>: b      0xfffff7f277cc ; <+204> at unwind.inc:141:1
```

We write the return value from `uw_frame_state_for` which was moved into w2 at
0xfffff7f277ac into w0.

# After mov w0, w2 (->  0xfffff7f277d0 <+208>: ldp    x29, x30, [sp])
    0xfffff7f27894 <+404>: b      0xfffff7f277cc ; <+204> at unwind.inc:141:1
(lldb) register read
General Purpose Registers:
        x0 = 0x0000000000000005
        x1 = 0x0000000000000001
        x2 = 0x0000000000000005
        x3 = 0x0000aaaaaaafa790
        x4 = 0x0000000000000000
        x5 = 0x0000ffffffffe160
        x6 = 0xfffffffffffffff8
        x7 = 0x0000000000000004
        x8 = 0x0000000000000001
        x9 = 0x0000fffff7f402a8
       x10 = 0x0000000000000000
       x11 = 0x0000fffff7f40308
       x12 = 0x0000fffff7ff77c0
       x13 = 0x0000000000000010
       x14 = 0x0000000000000000
       x15 = 0x0000fffff7bc63c0  
       x16 = 0x0000fffff7f40000
       x17 = 0x0000000000000000
       x18 = 0x0000000000000007
       x19 = 0x0000ffffffffe610
       x20 = 0x0000aaaaaaafa790
       x21 = 0x0000ffffffffe9d0
       x22 = 0x0000ffffffffe250
       x23 = 0x0000ffffffffefc8
       x24 = 0x0000fffff7ffdb90  ld-linux-aarch64.so.1`_rtld_global_ro
       x25 = 0x0000000000000000
       x26 = 0x0000fffff7ffe008  _rtld_global
       x27 = 0x0000aaaaaaabfda0 
UnexpectedExceptionDebug`__do_global_dtors_aux_fini_array_entry
       x28 = 0x0000000000000000
        fp = 0x0000ffffffffe190
        lr = 0x0000fffff7f277ac  libgcc_s.so.1`_Unwind_RaiseException + 172 at
unwind.inc:104:14
        sp = 0x0000ffffffffe190
        pc = 0x0000fffff7f277d0  libgcc_s.so.1`_Unwind_RaiseException + 208 at
unwind.inc:141:1
      cpsr = 0x60201000

However, we then continue to reload the register state from the function entry.

# After ldp x0, x1

(lldb) register read
General Purpose Registers:
        x0 = 0x0000aaaaaaafa790
        x1 = 0x0000000000000000
        x2 = 0x0000000000000058
        x3 = 0x0000000000000000
        x4 = 0x0000000000000000
        x5 = 0x0000ffffffffe160
        x6 = 0xfffffffffffffff8
        x7 = 0x0000000000000004
        x8 = 0x0000000000000001
        x9 = 0x0000fffff7f402a8
       x10 = 0x0000000000000000
       x11 = 0x0000fffff7f40308
       x12 = 0x0000fffff7ff77c0
       x13 = 0x0000000000000010
       x14 = 0x0000000000000000
       x15 = 0x0000fffff7bc63c0  
       x16 = 0x0000fffff7f40000
       x17 = 0x0000000000000000
       x18 = 0x0000000000000007
       x19 = 0x0000ffffffffefb8
       x20 = 0x0000000000000001
       x21 = 0x0000aaaaaaabfda0 
UnexpectedExceptionDebug`__do_global_dtors_aux_fini_array_entry
       x22 = 0x0000aaaaaaaa0ae8  UnexpectedExceptionDebug`main at
UnexpectedException.m:33
       x23 = 0x0000ffffffffefc8
       x24 = 0x0000fffff7ffdb90  ld-linux-aarch64.so.1`_rtld_global_ro
       x25 = 0x0000000000000000
       x26 = 0x0000fffff7ffe008  _rtld_global
       x27 = 0x0000aaaaaaabfda0 
UnexpectedExceptionDebug`__do_global_dtors_aux_fini_array_entry
       x28 = 0x0000000000000000
        fp = 0x0000ffffffffee00
        lr = 0x0000fffff7f76d38  libobjc.so.4.6`objc_exception_throw + 520 at
eh_personality.c:256:22
        sp = 0x0000ffffffffeda0
        pc = 0x0000fffff7f27810  libgcc_s.so.1`_Unwind_RaiseException + 272 at
unwind.inc:141:1
      cpsr = 0x60201000

(lldb)

[Bug libgcc/114843] AArch64: Wrong Register Reload in _Unwind_RaiseException causes corrupt return value on _URC_END_OF_STACK

[hugo at algoriddim dot com]

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

--- Comment #1 from Hugo Melder <hugo at algoriddim dot com> ---
Output:

hmelder@ubuntu:/Users/hmelder/Downloads/gcc_bug_report$
temps/ReproduceUnwindTest
RaiseException returned 0xeb9e72a0

Compiler Output (With Ubuntu GCC 13.2.0) as example:

Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/aarch64-linux-gnu/13/lto-wrapper
Target: aarch64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu 13.2.0-4ubuntu3'
--with-bugurl=file:///usr/share/doc/gcc-13/README.Bugs
--enable-languages=c,ada,c++,go,d,fortran,objc,obj-c++,m2 --prefix=/usr
--with-gcc-major-version-only --program-suffix=-13
--program-prefix=aarch64-linux-gnu- --enable-shared --enable-linker-build-id
--libexecdir=/usr/libexec --without-included-gettext --enable-threads=posix
--libdir=/usr/lib --enable-nls --enable-bootstrap --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes
--with-default-libstdcxx-abi=new --enable-gnu-unique-object
--disable-libquadmath --disable-libquadmath-support --enable-plugin
--enable-default-pie --with-system-zlib --enable-libphobos-checking=release
--with-target-system-zlib=auto --enable-objc-gc=auto --enable-multiarch
--enable-fix-cortex-a53-843419 --disable-werror --enable-checking=release
--build=aarch64-linux-gnu --host=aarch64-linux-gnu --target=aarch64-linux-gnu
--with-build-config=bootstrap-lto-lean --enable-link-serialization=2
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 13.2.0 (Ubuntu 13.2.0-4ubuntu3)
COLLECT_GCC_OPTIONS='-v' '-Wall' '-Wextra' '-o' 'ReproduceUnwindTest'
'-save-temps' '-mlittle-endian' '-mabi=lp64' '-dumpdir' 'ReproduceUnwindTest-'
 /usr/libexec/gcc/aarch64-linux-gnu/13/cc1 -E -quiet -v -imultiarch
aarch64-linux-gnu ../main.c -mlittle-endian -mabi=lp64 -Wall -Wextra
-fpch-preprocess -fasynchronous-unwind-tables -fstack-protector-strong
-Wformat-security -fstack-clash-protection -o ReproduceUnwindTest-main.i
ignoring nonexistent directory "/usr/local/include/aarch64-linux-gnu"
ignoring nonexistent directory
"/usr/lib/gcc/aarch64-linux-gnu/13/include-fixed/aarch64-linux-gnu"
ignoring nonexistent directory
"/usr/lib/gcc/aarch64-linux-gnu/13/include-fixed"
ignoring nonexistent directory
"/usr/lib/gcc/aarch64-linux-gnu/13/../../../../aarch64-linux-gnu/include"
#include "..." search starts here:
#include <...> search starts here:
 /usr/lib/gcc/aarch64-linux-gnu/13/include
 /usr/local/include
 /usr/include/aarch64-linux-gnu
 /usr/include
End of search list.
COLLECT_GCC_OPTIONS='-v' '-Wall' '-Wextra' '-o' 'ReproduceUnwindTest'
'-save-temps' '-mlittle-endian' '-mabi=lp64' '-dumpdir' 'ReproduceUnwindTest-'
 /usr/libexec/gcc/aarch64-linux-gnu/13/cc1 -fpreprocessed
ReproduceUnwindTest-main.i -quiet -dumpdir ReproduceUnwindTest- -dumpbase
main.c -dumpbase-ext .c -mlittle-endian -mabi=lp64 -Wall -Wextra -version
-fasynchronous-unwind-tables -fstack-protector-strong -Wformat-security
-fstack-clash-protection -o ReproduceUnwindTest-main.s
GNU C17 (Ubuntu 13.2.0-4ubuntu3) version 13.2.0 (aarch64-linux-gnu)
        compiled by GNU C version 13.2.0, GMP version 6.3.0, MPFR version
4.2.1, MPC version 1.3.1, isl version isl-0.26-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: 4b1ebb5fb7f44afd7bdce2c9d09f1e65
COLLECT_GCC_OPTIONS='-v' '-Wall' '-Wextra' '-o' 'ReproduceUnwindTest'
'-save-temps' '-mlittle-endian' '-mabi=lp64' '-dumpdir' 'ReproduceUnwindTest-'
 as -v -EL -mabi=lp64 -o ReproduceUnwindTest-main.o ReproduceUnwindTest-main.s
GNU assembler version 2.41 (aarch64-linux-gnu) using BFD version (GNU Binutils
for Ubuntu) 2.41
COMPILER_PATH=/usr/libexec/gcc/aarch64-linux-gnu/13/:/usr/libexec/gcc/aarch64-linux-gnu/13/:/usr/libexec/gcc/aarch64-linux-gnu/:/usr/lib/gcc/aarch64-linux-gnu/13/:/usr/lib/gcc/aarch64-linux-gnu/
LIBRARY_PATH=/usr/lib/gcc/aarch64-linux-gnu/13/:/usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu/:/usr/lib/gcc/aarch64-linux-gnu/13/../../../../lib/:/lib/aarch64-linux-gnu/:/lib/../lib/:/usr/lib/aarch64-linux-gnu/:/usr/lib/../lib/:/usr/lib/gcc/aarch64-linux-gnu/13/../../../:/lib/:/usr/lib/
COLLECT_GCC_OPTIONS='-v' '-Wall' '-Wextra' '-o' 'ReproduceUnwindTest'
'-save-temps' '-mlittle-endian' '-mabi=lp64' '-dumpdir' 'ReproduceUnwindTest.'
 /usr/libexec/gcc/aarch64-linux-gnu/13/collect2 -plugin
/usr/libexec/gcc/aarch64-linux-gnu/13/liblto_plugin.so
-plugin-opt=/usr/libexec/gcc/aarch64-linux-gnu/13/lto-wrapper
-plugin-opt=-fresolution=ReproduceUnwindTest.res
-plugin-opt=-pass-through=-lgcc -plugin-opt=-pass-through=-lgcc_s
-plugin-opt=-pass-through=-lc -plugin-opt=-pass-through=-lgcc
-plugin-opt=-pass-through=-lgcc_s --build-id --eh-frame-hdr --hash-style=gnu
--as-needed -dynamic-linker /lib/ld-linux-aarch64.so.1 -X -EL -maarch64linux
--fix-cortex-a53-843419 -pie -z now -z relro -o ReproduceUnwindTest
/usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu/Scrt1.o
/usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu/crti.o
/usr/lib/gcc/aarch64-linux-gnu/13/crtbeginS.o
-L/usr/lib/gcc/aarch64-linux-gnu/13
-L/usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu
-L/usr/lib/gcc/aarch64-linux-gnu/13/../../../../lib -L/lib/aarch64-linux-gnu
-L/lib/../lib -L/usr/lib/aarch64-linux-gnu -L/usr/lib/../lib
-L/usr/lib/gcc/aarch64-linux-gnu/13/../../.. ReproduceUnwindTest-main.o -lgcc
--push-state --as-needed -lgcc_s --pop-state -lc -lgcc --push-state --as-needed
-lgcc_s --pop-state /usr/lib/gcc/aarch64-linux-gnu/13/crtendS.o
/usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu/crtn.o
COLLECT_GCC_OPTIONS='-v' '-Wall' '-Wextra' '-o' 'ReproduceUnwindTest'
'-save-temps' '-mlittle-endian' '-mabi=lp64' '-dumpdir' 'ReproduceUnwindTest.'

[Bug target/114843] AArch64: Wrong Register Reload in _Unwind_RaiseException causes corrupt return value on _URC_END_OF_STACK

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 58035
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58035&action=edit
Preprocessed source of unwind-dw2.i which shows the issue

cc1 -fpreprocessed unwind-dw2.i -quiet -dumpbase unwind-dw2.c -dumpbase-ext .c
-mlittle-endian -mabi=lp64 -g -g -g -O2 -O2 -O2 -Wextra -Wall -Wno-narrowing
-Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes
-Wold-style-definition -versi

[Bug target/114843] AArch64: Wrong Register Reload in _Unwind_RaiseException causes corrupt return value on _URC_END_OF_STACK

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
((insn 262 261 263 14 (parallel [
            (set (reg:DI 0 x0)
                (unspec:DI [
                        (mem/c:V2x8QI (plus:DI (reg/f:DI 31 sp)
                                (const_int 16 [0x10])) [0  S16 A8])
                    ] UNSPEC_LDP_FST))
            (set (reg:DI 1 x1)
                (unspec:DI [
                        (mem/c:V2x8QI (plus:DI (reg/f:DI 31 sp)
                                (const_int 16 [0x10])) [0  S16 A8])
                    ] UNSPEC_LDP_SND))
        ]) "../../../libgcc/unwind.inc":141:1 -1
     (nil))

It is added by pro_and_epilogue for some reason ...

[Bug target/114843] AArch64: Wrong Register Reload in _Unwind_RaiseException causes corrupt return value on _URC_END_OF_STACK

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2024-04-25

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed reduced runtime self contained testcase:
```
__attribute__((noipa))
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();
}
```

[Bug target/114843] aarch64: epoligue 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=114843

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|AArch64: Wrong Register     |aarch64: epoligue in
                   |Reload in                   |_Unwind_RaiseException
                   |_Unwind_RaiseException      |corrupts return value due
                   |causes corrupt return value |to __builtin_eh_return
                   |on _URC_END_OF_STACK        |

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I suspect this has been always a bug since the aarch64 back-end was added.

[Bug target/114843] aarch64: epoligue 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=114843

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |critical

[Bug target/114843] aarch64: epoligue 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=114843

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note I just happened to finish a build of aarch64 so I was able to create the
preprocessed source of unwind-dw2.i really quick. And then I just read the
aarch64.cc to see the saving of x0 happened due to __builtin_eh_return . That
is how I got a testcase so quickly.

If I read the assembly correctly a few other targets are broken too; let me
file a bug report for each of them ..

[Bug target/114843] aarch64: epoligue 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=114843

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Just an FYI on other targets on my reduced testcase (I just quickly looked at
the generated assembly to see if it worked or not):

backends that seems to work:
mips
riscv
x86
s390
m68k
sh
sparc

backends that don't work:
powerpc: PR 114846 
arm: PR 114847 
longarch: PR 114848

[Bug target/114843] aarch64: 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=114843

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #7)
> Just an FYI on other targets on my reduced testcase (I just quickly looked
> at the generated assembly to see if it worked or not):
> 
> backends that seems to work:
> mips
> riscv
> x86
> s390
> m68k
> sh
> sparc
> 
> backends that don't work:
> powerpc: PR 114846 
> arm: PR 114847 
> longarch: PR 114848

Looks like longarch was fixed in GCC 14: r14-6440 .
Most likely other targets should do something similar ...

[Bug target/114843] aarch64: 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=114843

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=77455

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Just a quick note here. Even though eh_return pattern was removed with
r7-6051-g8144a493ddc008, it was broken before that patch.

[Bug target/114843] aarch64: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[wilco at gcc dot gnu.org]

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

Wilco <wilco at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |wilco at gcc dot gnu.org

--- Comment #10 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #9)
> Just a quick note here. Even though eh_return pattern was removed with
> r7-6051-g8144a493ddc008, it was broken before that patch.

Yeah I only fixed the broken behaviours that I encountered at the time - no
tests tried to return a value on the non-exception path. There is no clear
specification (eg. making it clear that EH_RETURN_DATA_REGNO must not overlap
with registers used to return or if they do, you need to conditionally restore
them), so no wonder that many targets get this wrong. Who knew that introducing
lots of complex builtins that affect prolog and epilog generation in a major
way to avoid a few lines of assembly code was such a bad idea...

Since the whole eh_return is an internal ABI in libgcc, a fix would be to
change EH_RETURN_DATA_REGNO(N) to avoid x0 and x1. Since eh_return already
reserves 7 registers(!) and now need to avoid using x0/x1 too, using x2-x5 and
x6,x7 and x9 for the other special registers should work.

[Bug target/114843] aarch64: 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=114843

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

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

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I have a fix for aarch64, able to produce now:
```
f:
.LFB0:
        .cfi_startproc
        stp     x0, x1, [sp, -32]!
        .cfi_def_cfa_offset 32
        .cfi_offset 0, -32
        .cfi_offset 1, -24
        stp     x2, x3, [sp, 16]
        .cfi_offset 2, -16
        .cfi_offset 3, -8
        ldr     w0, [x0]
        cmp     w0, 5
        bne     .L8
        add     sp, sp, 32
        .cfi_remember_state
        .cfi_def_cfa_offset 0
        ret
.L8:
        .cfi_restore_state
        mov     x5, x1
        ldp     x2, x3, [sp, 16]
        ldp     x0, x1, [sp], 32
        .cfi_restore 1
        .cfi_restore 0
        .cfi_restore 2
        .cfi_restore 3
        .cfi_def_cfa_offset 0
        add     sp, sp, x5
        ret
        .cfi_endproc
```

Which is exactly what we should produce I think.
The patch is a bit more complex than I expected but that is due to how aarch64
has some of the most complex epilogues.

[Bug target/114843] aarch64: 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=114843

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Wilco from comment #10)
> Since the whole eh_return is an internal ABI in libgcc, a fix would be to
> change EH_RETURN_DATA_REGNO(N) to avoid x0 and x1. Since eh_return already
> reserves 7 registers(!) and now need to avoid using x0/x1 too, using x2-x5
> and x6,x7 and x9 for the other special registers should work.

You can't change EH_RETURN_DATA_REGNO now because it is not just internal ABI
to libgcc, it is part of libstdc++v3 too (libsupc++/eh_personality.cc):
  /* For targets with pointers smaller than the word size, we must extend the
     pointer, and this extension is target dependent.  */
  _Unwind_SetGR (context, __builtin_eh_return_data_regno (0),
                 __builtin_extend_pointer (ue_header));
  _Unwind_SetGR (context, __builtin_eh_return_data_regno (1),
                 handler_switch_value);


And libobjc:
exception.c:#define __builtin_eh_return_data_regno(x) x
exception.c:  _Unwind_SetGR (context, __builtin_eh_return_data_regno (0),
exception.c:  _Unwind_SetGR (context, __builtin_eh_return_data_regno (1),


And really any exception personality.

[Bug target/114843] aarch64: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[wilco at gcc dot gnu.org]

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

--- Comment #13 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #11)
> I have a fix for aarch64, able to produce now:
> ```
> f:
> .LFB0:
>         .cfi_startproc
>         stp     x0, x1, [sp, -32]!
>         .cfi_def_cfa_offset 32
>         .cfi_offset 0, -32
>         .cfi_offset 1, -24
>         stp     x2, x3, [sp, 16]
>         .cfi_offset 2, -16
>         .cfi_offset 3, -8
>         ldr     w0, [x0]
>         cmp     w0, 5
>         bne     .L8
>         add     sp, sp, 32
>         .cfi_remember_state
>         .cfi_def_cfa_offset 0
>         ret
> .L8:
>         .cfi_restore_state
>         mov     x5, x1
>         ldp     x2, x3, [sp, 16]
>         ldp     x0, x1, [sp], 32
>         .cfi_restore 1
>         .cfi_restore 0
>         .cfi_restore 2
>         .cfi_restore 3
>         .cfi_def_cfa_offset 0
>         add     sp, sp, x5
>         ret
>         .cfi_endproc
> ```
> 
> Which is exactly what we should produce I think.
> The patch is a bit more complex than I expected but that is due to how
> aarch64 has some of the most complex epilogues.

I'm not convinced that is an easy solution. Try various cases with large stack
sizes, alloca and other scalar and FP callee-saves. Getting all cases right and
writing good tests for them is a lot of work.

[Bug target/114843] aarch64: 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=114843

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://github.com/gnustep/
                   |                            |libobjc2/issues/247

--- Comment #14 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I tested alloca and large stack sizes, they both look good to me. I will add
runtime testcase for both. not omitting FP also looks correct.
The patch itself is most contained in
aarch64_pop_regs/aarch64_restore_callee_saves which just skips the restore if
the regno was eh_return_data_regno and returning normally.


> no tests tried to return a value on the non-exception path

well libobjc2 does and that is how this was originally root caused even:
https://github.com/gnustep/libobjc2/issues/247

[Bug target/114843] aarch64: 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=114843

--- Comment #15 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 58043
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58043&action=edit
Patch but no testcases yet

Will add testcases in a little bit. Also I have not tested this fully yet. Will
do tonight.

[Bug target/114843] aarch64: 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=114843

--- Comment #16 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Patch posted with all of the testcases included:
https://gcc.gnu.org/pipermail/gcc-patches/2024-April/650080.html

[Bug target/114843] aarch64: epilogue in _Unwind_RaiseException corrupts return value due to __builtin_eh_return

[wilco at gcc dot gnu.org]

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

--- Comment #17 from Wilco <wilco at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #16)
> Patch posted with all of the testcases included:
> https://gcc.gnu.org/pipermail/gcc-patches/2024-April/650080.html

Not nearly enough testcases... What about:

void g(void);
int f(long offset, void *handler)
{
  g();
  if (offset > 5)
    return arr[offset];
  __builtin_eh_return (offset, handler);
}

With -O2 -fomit-frame-pointer:

f:
.LFB0:
        .cfi_startproc
        stp     x30, x0, [sp, -64]!
        .cfi_def_cfa_offset 64
        .cfi_offset 30, -64
        .cfi_offset 0, -56
        stp     x1, x2, [sp, 16]
        stp     x3, x19, [sp, 32]
        .cfi_offset 1, -48
        .cfi_offset 2, -40
        .cfi_offset 3, -32
        .cfi_offset 19, -24
        mov     x19, x0
        str     x20, [sp, 48]
        .cfi_offset 20, -16
        mov     x20, x1
        bl      g
        cmp     x19, 5
        ble     .L8
        mov     w0, w19
        ldp     x19, x20, [sp, 40]
        ldp     x30, x0, [sp], 64    ****** oops
        .cfi_remember_state
        .cfi_restore 0
        .cfi_restore 30
        .cfi_restore 19
        .cfi_restore 20
        .cfi_def_cfa_offset 0
        ret
.L8:
        .cfi_restore_state
        mov     x5, x19
        ldp     x1, x2, [sp, 16]
        mov     x6, x20
        ldp     x3, x19, [sp, 32]
        ldr     x20, [sp, 48]
        ldp     x30, x0, [sp], 64
        .cfi_restore 0
        .cfi_restore 30
        .cfi_restore 20
        .cfi_restore 3
        .cfi_restore 19
        .cfi_restore 1
        .cfi_restore 2
        .cfi_def_cfa_offset 0
        add     sp, sp, x5
        br      x6
        .cfi_endproc

So I don't believe you should change aarch64_pop_regs at all - it's too late to
change things and just adds unnecessary complexity and more bugs. The best
option would be to handle eh_return explicitly and insert the extra push/pops
rather than treating them like a generic callee-save (because clearly they are
not anymore).

[Bug target/114843] aarch64: 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=114843

--- Comment #18 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Wilco from comment #17)
> So I don't believe you should change aarch64_pop_regs at all - it's too late
> to change things and just adds unnecessary complexity and more bugs. The
> best option would be to handle eh_return explicitly and insert the extra
> push/pops rather than treating them like a generic callee-save (because
> clearly they are not anymore).

Thinking about this I think you are right, I will work on an incremental patch
to change them to be in seperate array. Note I do have a fix for the issue in
comment #17, I had a premature optimization thinking only if regno1 was an
eh_handler data register, then regno2 could be one; rather they should be
checked independently. I will attach that patch to this bug once my testing is
finished. And then will work on splitting out the eh_handler data register
load/stores.

[Bug target/114843] aarch64: 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=114843

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

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

--- Comment #19 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 58052
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58052&action=edit
v2 of the patch

I am attaching this here just so I don't lose it; it includes the fix for the
case mentioned. I am not going to post this version to the list. I will be
doing a v3 which I will post which contains the rewrite part.

[Bug target/114843] aarch64: 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=114843

--- Comment #20 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
New patch posted:
https://gcc.gnu.org/pipermail/gcc-patches/2024-May/650757.html

It was simplier than I had expected too.