public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/94515] New: aarch64: broken unwind information for pac-ret
@ 2020-04-07 11:48 nsz at gcc dot gnu.org
2020-04-21 14:26 ` [Bug target/94515] " nsz at gcc dot gnu.org
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: nsz at gcc dot gnu.org @ 2020-04-07 11:48 UTC (permalink / raw)
To: gcc-bugs
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug target/94515] aarch64: broken unwind information for pac-ret
2020-04-07 11:48 [Bug target/94515] New: aarch64: broken unwind information for pac-ret nsz at gcc dot gnu.org
@ 2020-04-21 14:26 ` nsz at gcc dot gnu.org
2020-04-27 8:11 ` cvs-commit at gcc dot gnu.org
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: nsz at gcc dot gnu.org @ 2020-04-21 14:26 UTC (permalink / raw)
To: gcc-bugs
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug target/94515] aarch64: broken unwind information for pac-ret
2020-04-07 11:48 [Bug target/94515] New: aarch64: broken unwind information for pac-ret nsz at gcc dot gnu.org
2020-04-21 14:26 ` [Bug target/94515] " nsz at gcc dot gnu.org
@ 2020-04-27 8:11 ` cvs-commit at gcc dot gnu.org
2020-05-14 15:17 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-04-27 8:11 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug target/94515] aarch64: broken unwind information for pac-ret
2020-04-07 11:48 [Bug target/94515] New: aarch64: broken unwind information for pac-ret nsz at gcc dot gnu.org
2020-04-21 14:26 ` [Bug target/94515] " nsz at gcc dot gnu.org
2020-04-27 8:11 ` cvs-commit at gcc dot gnu.org
@ 2020-05-14 15:17 ` cvs-commit at gcc dot gnu.org
2020-05-14 15:48 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-05-14 15:17 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug target/94515] aarch64: broken unwind information for pac-ret
2020-04-07 11:48 [Bug target/94515] New: aarch64: broken unwind information for pac-ret nsz at gcc dot gnu.org
` (2 preceding siblings ...)
2020-05-14 15:17 ` cvs-commit at gcc dot gnu.org
@ 2020-05-14 15:48 ` cvs-commit at gcc dot gnu.org
2020-05-14 15:51 ` nsz at gcc dot gnu.org
2020-05-14 15:52 ` nsz at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-05-14 15:48 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug target/94515] aarch64: broken unwind information for pac-ret
2020-04-07 11:48 [Bug target/94515] New: aarch64: broken unwind information for pac-ret nsz at gcc dot gnu.org
` (3 preceding siblings ...)
2020-05-14 15:48 ` cvs-commit at gcc dot gnu.org
@ 2020-05-14 15:51 ` nsz at gcc dot gnu.org
2020-05-14 15:52 ` nsz at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: nsz at gcc dot gnu.org @ 2020-05-14 15:51 UTC (permalink / raw)
To: gcc-bugs
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Bug target/94515] aarch64: broken unwind information for pac-ret
2020-04-07 11:48 [Bug target/94515] New: aarch64: broken unwind information for pac-ret nsz at gcc dot gnu.org
` (4 preceding siblings ...)
2020-05-14 15:51 ` nsz at gcc dot gnu.org
@ 2020-05-14 15:52 ` nsz at gcc dot gnu.org
5 siblings, 0 replies; 7+ messages in thread
From: nsz at gcc dot gnu.org @ 2020-05-14 15:52 UTC (permalink / raw)
To: gcc-bugs
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-14 15:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-07 11:48 [Bug target/94515] New: aarch64: broken unwind information for pac-ret nsz at gcc dot gnu.org
2020-04-21 14:26 ` [Bug target/94515] " nsz at gcc dot gnu.org
2020-04-27 8:11 ` cvs-commit at gcc dot gnu.org
2020-05-14 15:17 ` cvs-commit at gcc dot gnu.org
2020-05-14 15:48 ` cvs-commit at gcc dot gnu.org
2020-05-14 15:51 ` nsz at gcc dot gnu.org
2020-05-14 15:52 ` nsz at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).