[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
Thu Sep 12 03:52:29 PDT 2024


https://github.com/gbMattN updated https://github.com/llvm/llvm-project/pull/95387

>From 8099113d68bd7c47c29f635bb10a048ddb99833b 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/2] [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 f627851d049e6a..8235b0ec2b55e7 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -221,7 +221,24 @@ __tysan_check(void *addr, int size, tysan_type_descriptor *td, int flags) {
     OldTDPtr -= i;
     OldTD = *OldTDPtr;
 
-    if (!isAliasingLegal(td, 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 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))
       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 83a368867533e316b4272c19d0bf61da842c5b4b Mon Sep 17 00:00:00 2001
From: Matthew Nagy <gbmatt at tiger-linux2.domain.snsys.com>
Date: Thu, 12 Sep 2024 10:52:19 +0000
Subject: [PATCH 2/2] Fix more member offset bugs

---
 compiler-rt/lib/tysan/tysan.cpp               | 25 +++++++++------
 .../tysan/struct-offset-different-base.cpp    | 31 +++++++++++++++++++
 2 files changed, 47 insertions(+), 9 deletions(-)
 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 8235b0ec2b55e7..abad429de7ed9b 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -128,8 +128,13 @@ static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
           break;
       }
 
-      OffsetA -= TDA->Struct.Members[Idx].Offset;
-      TDA = TDA->Struct.Members[Idx].Type;
+      if (TDA->Struct.Members[Idx].Offset > OffsetA) {
+        OffsetA = TDA->Struct.Members[Idx].Offset - OffsetA;
+        TDA = TDA->Struct.Members[Idx - 1].Type;
+      } else {
+        OffsetA -= TDA->Struct.Members[Idx].Offset;
+        TDA = TDA->Struct.Members[Idx].Type;
+      }
     } else {
       DCHECK(0);
       break;
@@ -221,13 +226,15 @@ __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.
+    // 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++) {
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..c2986522198ab4
--- /dev/null
+++ b/compiler-rt/test/tysan/struct-offset-different-base.cpp
@@ -0,0 +1,31 @@
+// RUN: %clangxx_tysan -O0 %s -c -o %t.o
+// RUN: %run %t 2>&1 | FileCheck %s
+
+#include <stdio.h>
+
+struct inner {
+	char buffer;
+	int i;
+};
+
+void init_inner(inner *list) {
+	list->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



More information about the llvm-branch-commits mailing list