[compiler-rt] [TySan] Fixed false positive when accessing offset member variables (PR #120406)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 18 03:36:26 PST 2024
https://github.com/gbMattN created https://github.com/llvm/llvm-project/pull/120406
Original pull request [here](https://github.com/llvm/llvm-project/pull/95387)
This patch fixes a false positive bug in TySan. If you try access a member variable other than the first, TySan currently believes you are trying to access a type part-way through and reports an error.
In this patch we check to see if the type accessed has members, and if so searches through to find the internal type we are accessing.
>From 579f4bd64bae7cbd4886707af034584a20a5585a Mon Sep 17 00:00:00 2001
From: Matthew Nagy <gbmatt at tiger-linux2.domain.snsys.com>
Date: Fri, 28 Jun 2024 16:12:31 +0000
Subject: [PATCH 1/3] [TySan] Fixed false positive when accessing global
object's member variables
---
compiler-rt/lib/tysan/tysan.cpp | 19 +++++++++++-
.../test/tysan/global-struct-members.c | 31 +++++++++++++++++++
2 files changed, 49 insertions(+), 1 deletion(-)
create mode 100644 compiler-rt/test/tysan/global-struct-members.c
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index 39d78e7c95e0cd..093d3b22d1039f 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -228,7 +228,24 @@ __tysan_check(void *addr, int size, tysan_type_descriptor *td, int flags) {
OldTDPtr -= i;
OldTD = *OldTDPtr;
- if (!isAliasingLegal(td, OldTD, i))
+ // When shadow memory is set for global objects, the entire object is tagged with the struct type
+ // This means that when you access a member variable, tysan reads that as you accessing a struct midway
+ // through, with 'i' being the offset
+ // Therefore, if you are accessing a struct, we need to find the member type. We can go through the
+ // members of the struct type and see if there is a member at the offset you are accessing the struct by.
+ // If there is indeed a member starting at offset 'i' in the struct, we should check aliasing legality
+ // with that type. If there isn't, we run alias checking on the struct with will give us the correct error.
+ tysan_type_descriptor *InternalMember = OldTD;
+ if (OldTD->Tag == TYSAN_STRUCT_TD) {
+ for (int j = 0; j < OldTD->Struct.MemberCount; j++) {
+ if (OldTD->Struct.Members[j].Offset == i) {
+ InternalMember = OldTD->Struct.Members[j].Type;
+ break;
+ }
+ }
+ }
+
+ if (!isAliasingLegal(td, InternalMember, i))
reportError(addr, size, td, OldTD, AccessStr,
"accesses part of an existing object", -i, pc, bp, sp);
diff --git a/compiler-rt/test/tysan/global-struct-members.c b/compiler-rt/test/tysan/global-struct-members.c
new file mode 100644
index 00000000000000..76ea3c431dd7bc
--- /dev/null
+++ b/compiler-rt/test/tysan/global-struct-members.c
@@ -0,0 +1,31 @@
+// RUN: %clang_tysan -O0 %s -o %t && %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %t.out
+
+#include <stdio.h>
+
+struct X {
+ int a, b, c;
+} x;
+
+static struct X xArray[2];
+
+int main() {
+ x.a = 1;
+ x.b = 2;
+ x.c = 3;
+
+ printf("%d %d %d\n", x.a, x.b, x.c);
+ // CHECK-NOT: ERROR: TypeSanitizer: type-aliasing-violation
+
+ for (size_t i = 0; i < 2; i++) {
+ xArray[i].a = 1;
+ xArray[i].b = 1;
+ xArray[i].c = 1;
+ }
+
+ struct X *xPtr = (struct X *)&(xArray[0].c);
+ xPtr->a = 1;
+ // CHECK: ERROR: TypeSanitizer: type-aliasing-violation
+ // CHECK: WRITE of size 4 at {{.*}} with type int (in X at offset 0) accesses an existing object of type int (in X at offset 8)
+ // CHECK: {{#0 0x.* in main .*struct-members.c:}}[[@LINE-3]]
+}
>From a08f79a65ae07afe929ce5604a57e7de2dab845c Mon Sep 17 00:00:00 2001
From: gbMattN <matthew.nagy at sony.com>
Date: Tue, 12 Nov 2024 16:52:27 +0000
Subject: [PATCH 2/3] [style] Tidied up comments, formatting, and polished test
---
compiler-rt/lib/tysan/tysan.cpp | 24 ++++++++++---------
.../test/tysan/global-struct-members.c | 7 +++---
2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index 093d3b22d1039f..34dfecee8d8826 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -228,24 +228,26 @@ __tysan_check(void *addr, int size, tysan_type_descriptor *td, int flags) {
OldTDPtr -= i;
OldTD = *OldTDPtr;
- // When shadow memory is set for global objects, the entire object is tagged with the struct type
- // This means that when you access a member variable, tysan reads that as you accessing a struct midway
- // through, with 'i' being the offset
- // Therefore, if you are accessing a struct, we need to find the member type. We can go through the
- // members of the struct type and see if there is a member at the offset you are accessing the struct by.
- // If there is indeed a member starting at offset 'i' in the struct, we should check aliasing legality
- // with that type. If there isn't, we run alias checking on the struct with will give us the correct error.
- tysan_type_descriptor *InternalMember = OldTD;
+ // When shadow memory is set for global objects, the entire object is tagged
+ // with the struct type This means that when you access a member variable,
+ // tysan reads that as you accessing a struct midway through, with 'i' being
+ // the offset Therefore, if you are accessing a struct, we need to find the
+ // member type. We can go through the members of the struct type and see if
+ // there is a member at the offset you are accessing the struct by. If there
+ // is indeed a member starting at offset 'i' in the struct, we should check
+ // aliasing legality with that type. If there isn't, we run alias checking
+ // on the struct which will give us the correct error.
+ tysan_type_descriptor *AccessedType = OldTD;
if (OldTD->Tag == TYSAN_STRUCT_TD) {
- for (int j = 0; j < OldTD->Struct.MemberCount; j++) {
+ for (int j = 0; j < OldTD->Struct.MemberCount; ++j) {
if (OldTD->Struct.Members[j].Offset == i) {
- InternalMember = OldTD->Struct.Members[j].Type;
+ AccessedType = OldTD->Struct.Members[j].Type;
break;
}
}
}
- if (!isAliasingLegal(td, InternalMember, i))
+ if (!isAliasingLegal(td, AccessedType, i))
reportError(addr, size, td, OldTD, AccessStr,
"accesses part of an existing object", -i, pc, bp, sp);
diff --git a/compiler-rt/test/tysan/global-struct-members.c b/compiler-rt/test/tysan/global-struct-members.c
index 76ea3c431dd7bc..ed8f21bb84f3c5 100644
--- a/compiler-rt/test/tysan/global-struct-members.c
+++ b/compiler-rt/test/tysan/global-struct-members.c
@@ -1,5 +1,5 @@
// RUN: %clang_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
#include <stdio.h>
@@ -15,7 +15,6 @@ int main() {
x.c = 3;
printf("%d %d %d\n", x.a, x.b, x.c);
- // CHECK-NOT: ERROR: TypeSanitizer: type-aliasing-violation
for (size_t i = 0; i < 2; i++) {
xArray[i].a = 1;
@@ -26,6 +25,6 @@ int main() {
struct X *xPtr = (struct X *)&(xArray[0].c);
xPtr->a = 1;
// CHECK: ERROR: TypeSanitizer: type-aliasing-violation
- // CHECK: WRITE of size 4 at {{.*}} with type int (in X at offset 0) accesses an existing object of type int (in X at offset 8)
- // CHECK: {{#0 0x.* in main .*struct-members.c:}}[[@LINE-3]]
+ // CHECK-NEXT: WRITE of size 4 at {{.*}} with type int (in X at offset 0) accesses an existing object of type int (in X at offset 8)
+ // CHECK-NEXT: #0 0x{{.*}} in main {{.*}}struct-members.c:[[@LINE-3]]
}
>From 4d70fda2f8047cb7145776be78b9caf260ea67cf Mon Sep 17 00:00:00 2001
From: gbMattN <matthew.nagy at sony.com>
Date: Mon, 2 Dec 2024 17:17:31 +0000
Subject: [PATCH 3/3] Removed false positive/segfault when accessing member of
global for the first time
---
compiler-rt/lib/tysan/tysan.cpp | 34 ++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index 34dfecee8d8826..8595d9014d14e8 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -227,22 +227,26 @@ __tysan_check(void *addr, int size, tysan_type_descriptor *td, int flags) {
int i = -((sptr)OldTD);
OldTDPtr -= i;
OldTD = *OldTDPtr;
-
- // When shadow memory is set for global objects, the entire object is tagged
- // with the struct type This means that when you access a member variable,
- // tysan reads that as you accessing a struct midway through, with 'i' being
- // the offset Therefore, if you are accessing a struct, we need to find the
- // member type. We can go through the members of the struct type and see if
- // there is a member at the offset you are accessing the struct by. If there
- // is indeed a member starting at offset 'i' in the struct, we should check
- // aliasing legality with that type. If there isn't, we run alias checking
- // on the struct which will give us the correct error.
+
tysan_type_descriptor *AccessedType = OldTD;
- if (OldTD->Tag == TYSAN_STRUCT_TD) {
- for (int j = 0; j < OldTD->Struct.MemberCount; ++j) {
- if (OldTD->Struct.Members[j].Offset == i) {
- AccessedType = OldTD->Struct.Members[j].Type;
- break;
+
+ // Only check if we are accessing members if the type exists
+ if(OldTD != nullptr){
+ // When shadow memory is set for global objects, the entire object is tagged
+ // with the struct type This means that when you access a member variable,
+ // tysan reads that as you accessing a struct midway through, with 'i' being
+ // the offset Therefore, if you are accessing a struct, we need to find the
+ // member type. We can go through the members of the struct type and see if
+ // there is a member at the offset you are accessing the struct by. If there
+ // is indeed a member starting at offset 'i' in the struct, we should check
+ // aliasing legality with that type. If there isn't, we run alias checking
+ // on the struct which will give us the correct error.
+ if (OldTD->Tag == TYSAN_STRUCT_TD) {
+ for (int j = 0; j < OldTD->Struct.MemberCount; ++j) {
+ if (OldTD->Struct.Members[j].Offset == i) {
+ AccessedType = OldTD->Struct.Members[j].Type;
+ break;
+ }
}
}
}
More information about the llvm-commits
mailing list