[Bug target/94515] New: aarch64: broken unwind information for pac-ret

[Bug target/94515] New: aarch64: broken unwind information for pac-ret

[nsz at gcc dot gnu.org]

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

            Bug ID: 94515
           Summary: aarch64: broken unwind information for pac-ret
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nsz at gcc dot gnu.org
  Target Milestone: ---

pac-ret uses the .cfi_window_save directive to toggle between signed/unsigned
return address, alternatively .cfi_remember_state and .cfi_restore_state pair
can be used to keep track of the "return address signedness" state.

in some cases, when there are several return paths, gcc fails to generate
the correct cfi directives for all return paths which can cause the unwinder
not to authenticate a signed return address leading to a runtime crash on
pauth enabled systems.

example c++ test that segfaults (after fixing bug 94514 ):

volatile int zero = 0;

__attribute__((noinline))
void unwind (void)
{
  if (zero == 0)
    throw 42;
}

__attribute__((noinline,noipa))
static int test (int z)
{
  if (z) {
    asm volatile("":::"x20","x21");
    unwind();
    return 1;
  } else {
    unwind();
    return 2;
  }
}

int main ()
{
  try {
    test (zero);
    __builtin_abort ();
  } catch (...) {
    return 0;
  }
  __builtin_abort ();
}

the test() function with -mbranch-protection=standard -O2 compiles to

_ZL4testi:
.LFB1:
        .cfi_startproc
        hint    25 // paciasp
        .cfi_window_save  // pauth on
        stp     x29, x30, [sp, -32]!
        .cfi_def_cfa_offset 32
        .cfi_offset 29, -32
        .cfi_offset 30, -24
        mov     x29, sp
        cbz     w0, .L9
        stp     x20, x21, [sp, 16]
        .cfi_offset 21, -8
        .cfi_offset 20, -16
        bl      _Z6unwindv
        mov     w0, 1
        ldp     x20, x21, [sp, 16]
        .cfi_restore 21
        .cfi_restore 20
        ldp     x29, x30, [sp], 32
        .cfi_restore 30
        .cfi_restore 29
        .cfi_def_cfa_offset 0
        hint    29 // autiasp
        .cfi_window_save  // pauth off
        ret
        .p2align 2,,3
.L9:
        //// ret addr pauth state is wrong here !
        .cfi_def_cfa_offset 32
        .cfi_offset 29, -32
        .cfi_offset 30, -24
        bl      _Z6unwindv
        ldp     x29, x30, [sp], 32
        .cfi_restore 30
        .cfi_restore 29
        .cfi_def_cfa_offset 0
        hint    29 // autiasp
        .cfi_window_save
        mov     w0, 2
        ret
        .cfi_endproc
.LFE1:
        .size   _ZL4testi, .-_ZL4testi

[Bug target/94515] aarch64: broken unwind information for pac-ret

[nsz at gcc dot gnu.org]

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

--- Comment #1 from nsz at gcc dot gnu.org ---
i had a fix but it's not enough, so here is another test case:

__attribute__((noreturn)) void unwind(void);
int bar(void);
int global;

int foo(int x)
{
  if (x==1) return 2;
  int y = bar();
  if (y > global) global=y;
  if (y==3) unwind();
  return 0;
}

-O2 -S -mbranch-protection=pac-ret the asm:

foo:
        .cfi_startproc
        cmp     w0, 1
        beq     .L4
        hint    25 // paciasp
        .cfi_window_save  //// pauth on
        stp     x29, x30, [sp, -16]!
        .cfi_def_cfa_offset 16
        .cfi_offset 29, -16
        .cfi_offset 30, -8
        mov     x29, sp
        bl      bar
        mov     w1, w0
        adrp    x2, .LANCHOR0
        ldr     w0, [x2, #:lo12:.LANCHOR0]
        cmp     w0, w1
        blt     .L11
.L3:
        mov     w0, 0
        cmp     w1, 3
        beq     .L12
        ldp     x29, x30, [sp], 16
        .cfi_remember_state
        .cfi_restore 30
        .cfi_restore 29
        .cfi_def_cfa_offset 0
        hint    29 // autiasp
        .cfi_window_save  //// pauth off
        ret
        .p2align 2,,3
.L11:
        .cfi_restore_state  //// pauth on
        str     w1, [x2, #:lo12:.LANCHOR0]
        b       .L3
        .p2align 2,,3
.L4:
        .cfi_def_cfa_offset 0
        .cfi_restore 29
        .cfi_restore 30
        mov     w0, 2  //// pauth should be off but it's on 
        ret
.L12:
        .cfi_def_cfa_offset 16
        .cfi_offset 29, -16
        .cfi_offset 30, -8
        bl      unwind
        .cfi_endproc

[Bug target/94515] aarch64: broken unwind information for pac-ret

[cvs-commit at gcc dot gnu.org]

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

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Szabolcs Nagy <nsz@gcc.gnu.org>:

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

commit r10-7986-gacdf733634745548c0167c40bad80e6140ac2eeb
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Mon Apr 27 09:07:15 2020 +0100

    aarch64: Fix .cfi_window_save with pac-ret [PR94515]

    On aarch64 -mbranch-protection=pac-ret reuses the dwarf
    opcode for window_save to mean "toggle the return address
    mangle state", but in the dwarf2cfi internal logic the
    state was not updated when an opcode was emitted, the
    currently present update logic is only valid for the
    original sparc use of window_save so a separate bool is
    used on aarch64 to track the state.

    This bug can cause the unwinder not to authenticate return
    addresses that were signed (or vice versa) which means a
    runtime crash on a pauth enabled system.

    Currently only aarch64 pac-ret uses REG_CFA_TOGGLE_RA_MANGLE.

    This should be backported to gcc-9 and gcc-8 branches.

    gcc/ChangeLog:

            PR target/94515
            * dwarf2cfi.c (struct GTY): Add ra_mangled.
            (cfi_row_equal_p): Check ra_mangled.
            (dwarf2out_frame_debug_cfa_window_save): Remove the argument,
            this only handles the sparc logic now.
            (dwarf2out_frame_debug_cfa_toggle_ra_mangle): New function for
            the aarch64 specific logic.
            (dwarf2out_frame_debug): Update to use the new subroutines.
            (change_cfi_row): Check ra_mangled.

    gcc/testsuite/ChangeLog:

            PR target/94515
            * g++.target/aarch64/pr94515-1.C: New test.
            * g++.target/aarch64/pr94515-2.C: New test.

[Bug target/94515] aarch64: broken unwind information for pac-ret

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Szabolcs Nagy <nsz@gcc.gnu.org>:

https://gcc.gnu.org/g:95833c34424f340a7e465ee38b6a41369bc7c90b

commit r9-8593-g95833c34424f340a7e465ee38b6a41369bc7c90b
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Mon Apr 27 09:07:15 2020 +0100

    aarch64: Fix .cfi_window_save with pac-ret [PR94515]

    On aarch64 -mbranch-protection=pac-ret reuses the dwarf
    opcode for window_save to mean "toggle the return address
    mangle state", but in the dwarf2cfi internal logic the
    state was not updated when an opcode was emitted, the
    currently present update logic is only valid for the
    original sparc use of window_save so a separate bool is
    used on aarch64 to track the state.

    This bug can cause the unwinder not to authenticate return
    addresses that were signed (or vice versa) which means a
    runtime crash on a pauth enabled system.

    Currently only aarch64 pac-ret uses REG_CFA_TOGGLE_RA_MANGLE.

    This should be backported to gcc-9 and gcc-8 branches.

    gcc/ChangeLog:

            Backport from mainline.
            2020-04-27  Szabolcs Nagy  <szabolcs.nagy@arm.com>

            PR target/94515
            * dwarf2cfi.c (struct GTY): Add ra_mangled.
            (cfi_row_equal_p): Check ra_mangled.
            (dwarf2out_frame_debug_cfa_window_save): Remove the argument,
            this only handles the sparc logic now.
            (dwarf2out_frame_debug_cfa_toggle_ra_mangle): New function for
            the aarch64 specific logic.
            (dwarf2out_frame_debug): Update to use the new subroutines.
            (change_cfi_row): Check ra_mangled.

    gcc/testsuite/ChangeLog:

            Backport from mainline.
            2020-04-27  Szabolcs Nagy  <szabolcs.nagy@arm.com>

            PR target/94515
            * g++.target/aarch64/pr94515-1.C: New test.
            * g++.target/aarch64/pr94515-2.C: New test.

[Bug target/94515] aarch64: broken unwind information for pac-ret

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Szabolcs Nagy <nsz@gcc.gnu.org>:

https://gcc.gnu.org/g:62ab8b9114b0bdae508ed76fa9028e0040d35e6b

commit r8-10253-g62ab8b9114b0bdae508ed76fa9028e0040d35e6b
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Mon Apr 27 09:07:15 2020 +0100

    aarch64: Fix .cfi_window_save with pac-ret [PR94515]

    On aarch64 -mbranch-protection=pac-ret reuses the dwarf
    opcode for window_save to mean "toggle the return address
    mangle state", but in the dwarf2cfi internal logic the
    state was not updated when an opcode was emitted, the
    currently present update logic is only valid for the
    original sparc use of window_save so a separate bool is
    used on aarch64 to track the state.

    This bug can cause the unwinder not to authenticate return
    addresses that were signed (or vice versa) which means a
    runtime crash on a pauth enabled system.

    Currently only aarch64 pac-ret uses REG_CFA_TOGGLE_RA_MANGLE.

    This should be backported to gcc-9 and gcc-8 branches.

    Changed branch-protection=pac-ret to sign-return-address=all etc
    in the tests for the backport and adjusted the dwarf2cfi.c
    changes because the sparc change was missing.

    gcc/ChangeLog:

            Backport from mainline.
            2020-04-27  Szabolcs Nagy  <szabolcs.nagy@arm.com>

            PR target/94515
            * dwarf2cfi.c (struct GTY): Add ra_mangled.
            (cfi_row_equal_p): Check ra_mangled.
            (dwarf2out_frame_debug_cfa_window_save): Remove the argument,
            this only handles the sparc logic now.
            (dwarf2out_frame_debug_cfa_toggle_ra_mangle): New function for
            the aarch64 specific logic.
            (dwarf2out_frame_debug): Update to use the new subroutines.
            (change_cfi_row): Check ra_mangled.

    gcc/testsuite/ChangeLog:

            Backport from mainline.
            2020-04-27  Szabolcs Nagy  <szabolcs.nagy@arm.com>

            PR target/94515
            * g++.target/aarch64/pr94515-1.C: New test.
            * g++.target/aarch64/pr94515-2.C: New test.

[Bug target/94515] aarch64: broken unwind information for pac-ret

[nsz at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94515
Bug 94515 depends on bug 94514, which changed state.

Bug 94514 Summary: aarch64: unwinding across mixed pac-ret and non-pac-ret frames is broken
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94514

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

[Bug target/94515] aarch64: broken unwind information for pac-ret

[nsz at gcc dot gnu.org]

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

nsz at gcc dot gnu.org changed:

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

--- Comment #5 from nsz at gcc dot gnu.org ---
fixed for gcc-10.1 and on gcc-9 and gcc-8 branches.