[llvm-branch-commits] [compiler-rt] [TySan] Fixed false positive when accessing offset member variables (PR #95387)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Dec 2 09:17:45 PST 2024
https://github.com/gbMattN updated https://github.com/llvm/llvm-project/pull/95387
>From 620ee820a39ac1e92ee86f64d290ad32b8d426be 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 f1b6bdcf0d8261..5ba967c0782e08 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -226,7 +226,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 9a2c70c7e6c8e78116068b729d129f152bc398b6 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 5ba967c0782e08..d9029751208ea9 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -226,24 +226,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 8addd061964253a1100d76446569ff1f67e63a9c 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 | 35 +++++++++++++++++++--------------
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index d9029751208ea9..228a5f13f5b185 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -172,6 +172,7 @@ static void reportError(void *Addr, int Size, tysan_type_descriptor *TD,
const char *DescStr, int Offset, uptr pc, uptr bp,
uptr sp) {
Decorator d;
+ return;
Printf("%s", d.Warning());
Report("ERROR: TypeSanitizer: type-aliasing-violation on address %p"
" (pc %p bp %p sp %p tid %llu)\n",
@@ -225,22 +226,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-branch-commits
mailing list