On 2024-08-13 16:53, Richard Sandiford wrote:
Matthieu Longo <matthieu.lo...@arm.com> writes:
gcc/testsuite/ChangeLog:
* g++.target/aarch64/pr94515-1.C: Improve test documentation.
* g++.target/aarch64/pr94515-2.C: Same.
The patch is OK as-is, since it's clearly a strict improvement over
the status quo. But a suggestion below:
---
gcc/testsuite/g++.target/aarch64/pr94515-1.C | 8 ++++++
gcc/testsuite/g++.target/aarch64/pr94515-2.C | 28 +++++++++++++++-----
2 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/gcc/testsuite/g++.target/aarch64/pr94515-2.C
b/gcc/testsuite/g++.target/aarch64/pr94515-2.C
index f4a3333beed..af6b128b8fd 100644
--- a/gcc/testsuite/g++.target/aarch64/pr94515-2.C
+++ b/gcc/testsuite/g++.target/aarch64/pr94515-2.C
@@ -6,6 +6,7 @@
volatile int zero = 0;
int global = 0;
+/* This is a leaf function, so no .cfi_negate_ra_state directive is expected. */
__attribute__((noinline))
int bar(void)
{
@@ -13,29 +14,44 @@ int bar(void)
return 0;
}
+/* This function does not return normally, so the address is signed but no
+ * authentication code is emitted. It means that only one CFI directive is
+ * supposed to be emitted at signing time. */
__attribute__((noinline, noreturn))
void unwind (void)
{
throw 42;
}
+/* This function has several return instructions, and alternates different RA
+ * states. 4 .cfi_negate_ra_state and a .cfi_remember_state/.cfi_restore_state
+ * should be emitted.
+ */
__attribute__((noinline, noipa))
int test(int x)
{
- if (x==1) return 2; /* This return path may not use the stack. */
+ // This return path may not use the stack. This means that the return address
+ // won't be signed.
+ if (x==1) return 2;
+
+ // All the return paths of the code below must have RA mangle state set, and
+ // the return address must be signed.
int y = bar();
if (y > global) global=y;
- if (y==3) unwind(); /* This return path must have RA mangle state set. */
- return 0;
+ if (y==3) unwind(); // authentication of the return address is not required.
+ return 0; // authentication of the return address is required.
}
I wasn't sure from reading this why it would give 4 .cfi_negate_ra_states,
and had to run the test to find out. I think a key part is that the
current block layout is:
A: path to return 0 without assignment to global
B: global=y + branch back into A
C: return 2
D: unwind
so we have:
A: sign -> authenticate [2 negate_ra_states + remember_state for B]
B: signed [restore_state]
C: unsigned [negate_ra_state]
D: signed [negate_ra_state]
If the order had been ABDC then we would only need 3 negate_ra_states.
If the x==1 branch had been:
cmp w0, 1
bne .L10
mov w0, 2
ret
.L10:
followed by the rest of A, B and D, then we would only have needed
2 negate_ra_states.
So it might be helpful to give the block layout above too.
Like I say, it's just a suggestion though.
Thanks,
Richard
Done in the next revision.
Thanks,
Matthieu
+/* This function requires only 2 .cfi_negate_ra_state. */
int main ()
{
+ // paciasp -> cfi_negate_ra_state: RA_no_signing -> RA_signing_SP
try {
test (zero);
- __builtin_abort ();
+ __builtin_abort (); // authentication of the return address is not
required.
} catch (...) {
+ // autiasp -> cfi_negate_ra_state: RA_signing_SP -> RA_no_signing
return 0;
}
- __builtin_abort ();
-}
+ __builtin_abort (); // authentication of the return address is not required.
+}
\ No newline at end of file
