-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[hwasan] Fix rare false negative (zero tag) in stack-uar.c #69374
Conversation
stack-uar.c is flaky (1 in 256 executions) because the random tag may be zero (llvm#69221). This patch works around the issue in the same way as deep-recursion.c (llvm@aa4dfd3), by falling back to a neighboring object, which must have a different (non-zero) tag. This patch also does a minor cleanup of the aforementioned deep-recursion.c, for consistency with stack-uar.c.
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Thurston Dang (thurstond) Changesstack-uar.c is flaky (1 in 256 executions) because the random tag This patch works around the issue in the same way as deep-recursion.c This patch also does a minor cleanup of the aforementioned Full diff: https://github.com/llvm/llvm-project/pull/69374.diff 2 Files Affected:
diff --git a/compiler-rt/test/hwasan/TestCases/deep-recursion.c b/compiler-rt/test/hwasan/TestCases/deep-recursion.c
index 19d2b50726bee51..764ec5692372b4a 100644
--- a/compiler-rt/test/hwasan/TestCases/deep-recursion.c
+++ b/compiler-rt/test/hwasan/TestCases/deep-recursion.c
@@ -17,7 +17,8 @@
// Stack histories are currently not recorded on x86.
// XFAIL: target=x86_64{{.*}}
-#include <stdint.h>
+#include <assert.h>
+#include <sanitizer/hwasan_interface.h>
#include <stdlib.h>
// At least -O1 is needed for this function to not have a stack frame on
@@ -33,11 +34,12 @@ __attribute__((noinline)) void OOB() {
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 (((uintptr_t)&x) >> 56 == 0) {
+ // a false negative for this test. The tag allocation algorithm 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.
+ if (__hwasan_tag_pointer(x, 0) == x) {
+ assert(__hwasan_tag_pointer(y, 0) != y);
y[four] = 0;
} else {
x[four] = 0;
diff --git a/compiler-rt/test/hwasan/TestCases/stack-uar.c b/compiler-rt/test/hwasan/TestCases/stack-uar.c
index b4a817351029d9b..48440a47d5f5f42 100644
--- a/compiler-rt/test/hwasan/TestCases/stack-uar.c
+++ b/compiler-rt/test/hwasan/TestCases/stack-uar.c
@@ -9,14 +9,29 @@
// Stack histories currently are not recorded on x86.
// XFAIL: target=x86_64{{.*}}
+#include <assert.h>
+#include <sanitizer/hwasan_interface.h>
+
void USE(void *x) { // pretend_to_do_something(void *x)
__asm__ __volatile__("" : : "r" (x) : "memory");
}
__attribute__((noinline))
char *buggy() {
- char zzz[0x1000];
- char *volatile p = zzz;
+ char zzz[0x800];
+ char yyy[0x800];
+ // Tags for stack-allocated variables can occasionally be zero, resulting in
+ // a false negative for this test. The tag allocation algorithm 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.
+ char *volatile p;
+ if (__hwasan_tag_pointer(zzz, 0) == zzz) {
+ assert(__hwasan_tag_pointer(yyy, 0) != yyy);
+ p = yyy;
+ } else {
+ p = zzz;
+ }
return p;
}
@@ -35,7 +50,7 @@ int main() {
// CHECK: Cause: stack tag-mismatch
// CHECK: is located in stack of thread
// CHECK: Potentially referenced stack objects:
- // CHECK-NEXT: zzz in buggy {{.*}}stack-uar.c:[[@LINE-20]]
+ // CHECK-NEXT: {{zzz|yyy}} in buggy {{.*}}stack-uar.c:
// CHECK-NEXT: Memory tags around the buggy address
// NOSYM: Previously allocated frames:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the fixes.
Local branch amd-gfx 756ea0f Merged main:b0b8e83e668a into amd-gfx:b9f9dea74514 Remote branch main ddf1de2 [hwasan] Fix rare false negative (zero tag) in stack-uar.c (llvm#69374)
stack-uar.c is flaky (1 in 256 executions) because the random tag
may be zero (#69221).
This patch works around the issue in the same way as deep-recursion.c
(aa4dfd3),
by falling back to a neighboring object, which must have a different
(non-zero) tag.
This patch also does a minor cleanup of the aforementioned
deep-recursion.c, for consistency with stack-uar.c.