[compiler-rt] [TySan] Fix struct access with different bases (PR #120412)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 18 04:08:05 PST 2024
https://github.com/gbMattN created https://github.com/llvm/llvm-project/pull/120412
Original pull request [here](https://github.com/llvm/llvm-project/pull/108385)
Fixes issue https://github.com/llvm/llvm-project/issues/105960
If a member in a struct is also a struct, accessing a member partway through this inner struct currently causes a false positive. This is because when checking aliasing, the access offset is seen as greater than the starting offset of the inner struct, so the loop continues one iteration, and believes we are accessing the member after the inner struct.
The next member's offset is greater than the offset we are looking for, so when we subtract the next member's offset from what we are looking for, the offset underflows.
To fix this, we check if the member we think we are accessing has a greater offset than the offset we are looking for. If so, we take a step back. We cannot do this in the loop, since the loop does not check the final member. This means the penultimate member would still cause false positives.
>From f645295ad59e99ef4efc62fca302b387904abbbc Mon Sep 17 00:00:00 2001
From: Matthew Nagy <gbmatt at tiger-linux2.domain.snsys.com>
Date: Thu, 12 Sep 2024 12:36:57 +0000
Subject: [PATCH 1/4] [TySan] Fix struct access with different bases
---
compiler-rt/lib/tysan/tysan.cpp | 5 +++
.../tysan/struct-offset-different-base.cpp | 33 +++++++++++++++++++
2 files changed, 38 insertions(+)
create mode 100644 compiler-rt/test/tysan/struct-offset-different-base.cpp
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index 39d78e7c95e0cd..6640f83c4553cf 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -131,6 +131,11 @@ static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
break;
}
+ // This offset can't be negative. Therefore we must be accessing something
+ // partially inside the last type
+ if (TDA->Struct.Members[Idx].Offset > OffsetA)
+ Idx -= 1;
+
OffsetA -= TDA->Struct.Members[Idx].Offset;
TDA = TDA->Struct.Members[Idx].Type;
} else {
diff --git a/compiler-rt/test/tysan/struct-offset-different-base.cpp b/compiler-rt/test/tysan/struct-offset-different-base.cpp
new file mode 100644
index 00000000000000..3e1d6f2a6a42f5
--- /dev/null
+++ b/compiler-rt/test/tysan/struct-offset-different-base.cpp
@@ -0,0 +1,33 @@
+// RUN: %clangxx_tysan -O0 %s -o %t && %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+
+// Modified reproducer from https://github.com/llvm/llvm-project/issues/105960
+
+#include <stdio.h>
+
+struct inner {
+ char buffer;
+ int i;
+};
+
+void init_inner(inner *iPtr) {
+ iPtr->i = 0;
+}
+
+struct outer {
+ inner foo;
+ char buffer;
+};
+
+int main(void) {
+ outer *l = new outer();
+
+ init_inner(&l->foo);
+
+ int access_offsets_with_different_base = l->foo.i;
+ printf("%d\n", access_offsets_with_different_base);
+
+ return 0;
+}
+
+// CHECK-NOT: ERROR: TypeSanitizer: type-aliasing-violation
>From 6e663807274a28f3b2f90d72c749133711fae1e0 Mon Sep 17 00:00:00 2001
From: gbMattN <matthew.nagy at sony.com>
Date: Tue, 12 Nov 2024 17:05:44 +0000
Subject: [PATCH 2/4] Changed test to check for output
---
compiler-rt/test/tysan/struct-offset-different-base.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/compiler-rt/test/tysan/struct-offset-different-base.cpp b/compiler-rt/test/tysan/struct-offset-different-base.cpp
index 3e1d6f2a6a42f5..4563f7025bea48 100644
--- a/compiler-rt/test/tysan/struct-offset-different-base.cpp
+++ b/compiler-rt/test/tysan/struct-offset-different-base.cpp
@@ -1,5 +1,5 @@
// RUN: %clangxx_tysan -O0 %s -o %t && %run %t >%t.out 2>&1
-// RUN: FileCheck %s < %t.out
+// RUN: FileCheck %s --implicit-check-not ERROR < %t.out
// Modified reproducer from https://github.com/llvm/llvm-project/issues/105960
@@ -11,7 +11,7 @@ struct inner {
};
void init_inner(inner *iPtr) {
- iPtr->i = 0;
+ iPtr->i = 200;
}
struct outer {
@@ -25,9 +25,9 @@ int main(void) {
init_inner(&l->foo);
int access_offsets_with_different_base = l->foo.i;
- printf("%d\n", access_offsets_with_different_base);
+ printf("Accessed value is %d\n", access_offsets_with_different_base);
return 0;
}
-// CHECK-NOT: ERROR: TypeSanitizer: type-aliasing-violation
+// CHECK: Accessed value is 200
>From 9c92143a330ddaa87bdc595ed37c0cd94bc600b1 Mon Sep 17 00:00:00 2001
From: gbMattN <matthew.nagy at sony.com>
Date: Tue, 26 Nov 2024 16:44:43 +0000
Subject: [PATCH 3/4] More format changes
---
compiler-rt/lib/tysan/tysan.cpp | 2 +-
.../tysan/struct-offset-different-base.cpp | 28 +++++++++----------
2 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index 6640f83c4553cf..f17a0f7007ac69 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -135,7 +135,7 @@ static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
// partially inside the last type
if (TDA->Struct.Members[Idx].Offset > OffsetA)
Idx -= 1;
-
+
OffsetA -= TDA->Struct.Members[Idx].Offset;
TDA = TDA->Struct.Members[Idx].Type;
} else {
diff --git a/compiler-rt/test/tysan/struct-offset-different-base.cpp b/compiler-rt/test/tysan/struct-offset-different-base.cpp
index 4563f7025bea48..da0efd2cb6503c 100644
--- a/compiler-rt/test/tysan/struct-offset-different-base.cpp
+++ b/compiler-rt/test/tysan/struct-offset-different-base.cpp
@@ -6,28 +6,26 @@
#include <stdio.h>
struct inner {
- char buffer;
- int i;
+ char buffer;
+ int i;
};
-void init_inner(inner *iPtr) {
- iPtr->i = 200;
-}
+void init_inner(inner *iPtr) { iPtr->i = 200; }
struct outer {
- inner foo;
- char buffer;
+ inner foo;
+ char buffer;
};
int main(void) {
- outer *l = new outer();
-
- init_inner(&l->foo);
-
- int access_offsets_with_different_base = l->foo.i;
- printf("Accessed value is %d\n", access_offsets_with_different_base);
-
- return 0;
+ outer *l = new outer();
+
+ init_inner(&l->foo);
+
+ int access_offsets_with_different_base = l->foo.i;
+ printf("Accessed value is %d\n", access_offsets_with_different_base);
+
+ return 0;
}
// CHECK: Accessed value is 200
>From 7ba829ef4e5aff36c1d433d9ba6dc8e935ab873b Mon Sep 17 00:00:00 2001
From: gbMattN <matthew.nagy at sony.com>
Date: Fri, 29 Nov 2024 11:41:03 +0000
Subject: [PATCH 4/4] Fixed occasional segfault
---
compiler-rt/lib/tysan/tysan.cpp | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index f17a0f7007ac69..94498287f17d19 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -133,7 +133,10 @@ static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
// This offset can't be negative. Therefore we must be accessing something
// partially inside the last type
- if (TDA->Struct.Members[Idx].Offset > OffsetA)
+ // We shouldn't check this if we are on the first member, Idx will
+ // underflow The first member can be offset in rare cases such as
+ // llvm::cl::Option
+ if (TDA->Struct.Members[Idx].Offset > OffsetA && Idx > 0)
Idx -= 1;
OffsetA -= TDA->Struct.Members[Idx].Offset;
More information about the llvm-commits
mailing list