From 999d8fa570a4ca6aa5f8e2ba0233edfc4ddec357 Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Mon, 16 Oct 2023 23:05:53 +0000 Subject: [PATCH 1/2] [hwasan] Fix and re-enable deep-recursion.c deep-recursion.c was disabled (https://github.com/llvm/llvm-project/commit/c007e0f66ee3f96467fd12f6200218fb4c38c2c9) because the test may get unlucky and end up with a zero-tagged variable, leading to a false negative (https://github.com/llvm/llvm-project/issues/69221). This patch re-enables the test and adds a workaround: it checks if the variable is zero-tagged, and if so, it will instead use the neighboring variable, which must have a different (hence non-zero) tag. Fixing the stack allocation tagging is left as an exercise for the reader. It is non-trivial because, even if the stackTagBase is non-zero, tags for subsequent allocations in the stack frame may wrap around to zero; working around this would require adding multiple instructions to each alloca. --- .../test/hwasan/TestCases/deep-recursion.c | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/compiler-rt/test/hwasan/TestCases/deep-recursion.c b/compiler-rt/test/hwasan/TestCases/deep-recursion.c index bf390d051d472..c992f205917ba 100644 --- a/compiler-rt/test/hwasan/TestCases/deep-recursion.c +++ b/compiler-rt/test/hwasan/TestCases/deep-recursion.c @@ -17,9 +17,6 @@ // Stack histories are currently not recorded on x86. // XFAIL: target=x86_64{{.*}} -// Flaky on AArch64 Linux, see https://github.com/llvm/llvm-project/issues/69221. -// UNSUPPORTED: target=aarch64{{.*}} - #include // At least -O1 is needed for this function to not have a stack frame on // AArch64. @@ -29,7 +26,23 @@ void USE(void *x) { // pretend_to_do_something(void *x) volatile int four = 4; -__attribute__((noinline)) void OOB() { int x[4]; x[four] = 0; USE(&x[0]); } +__attribute__((noinline)) void OOB() { + int x[4]; + int y[4]; + + // Tags for stack-allocated variables can occasionally be zero, resulting in + // a false negative for this test. This is not easy to fix, hence we work + // around it: if the tag is zero, we use the neighboring variable instead, + // which must have a different (hence non-zero) tag. + // This tag check assumes aarch64. + if(((unsigned long)&x) >> 56 == 0) { + y[four] = 0; + } else { + x[four] = 0; + } + USE(&x[0]); + USE(&y[0]); +} __attribute__((noinline)) void FUNC1() { int x; USE(&x); OOB(); } __attribute__((noinline)) void FUNC2() { int x; USE(&x); FUNC1(); } __attribute__((noinline)) void FUNC3() { int x; USE(&x); FUNC2(); } From a209d083fb9cdf62f21db950771ac784316bec2e Mon Sep 17 00:00:00 2001 From: Thurston Dang Date: Tue, 17 Oct 2023 00:22:14 +0000 Subject: [PATCH 2/2] Fix whitespace and use uintptr_t --- compiler-rt/test/hwasan/TestCases/deep-recursion.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler-rt/test/hwasan/TestCases/deep-recursion.c b/compiler-rt/test/hwasan/TestCases/deep-recursion.c index c992f205917ba..792f758958270 100644 --- a/compiler-rt/test/hwasan/TestCases/deep-recursion.c +++ b/compiler-rt/test/hwasan/TestCases/deep-recursion.c @@ -35,7 +35,7 @@ __attribute__((noinline)) void OOB() { // around it: if the tag is zero, we use the neighboring variable instead, // which must have a different (hence non-zero) tag. // This tag check assumes aarch64. - if(((unsigned long)&x) >> 56 == 0) { + if (((uintptr_t)&x) >> 56 == 0) { y[four] = 0; } else { x[four] = 0;