[compiler-rt] [TySan] Fixed false positive when accessing offset member variables (PR #120406)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (gbMattN)

<details>
<summary>Changes</summary>

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.

---
Full diff: https://github.com/llvm/llvm-project/pull/120406.diff


2 Files Affected:

- (modified) compiler-rt/lib/tysan/tysan.cpp (+24-1) 
- (added) compiler-rt/test/tysan/global-struct-members.c (+30) 


``````````diff
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index 39d78e7c95e0cd..8595d9014d14e8 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -227,8 +227,31 @@ __tysan_check(void *addr, int size, tysan_type_descriptor *td, int flags) {
     int i = -((sptr)OldTD);
     OldTDPtr -= i;
     OldTD = *OldTDPtr;
+  
+    tysan_type_descriptor *AccessedType = OldTD;
+    
+    // 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;
+          }
+        }
+      }
+    }
 
-    if (!isAliasingLegal(td, OldTD, 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
new file mode 100644
index 00000000000000..ed8f21bb84f3c5
--- /dev/null
+++ b/compiler-rt/test/tysan/global-struct-members.c
@@ -0,0 +1,30 @@
+// RUN: %clang_tysan -O0 %s -o %t && %run %t >%t.out 2>&1
+// RUN: FileCheck %s --implicit-check-not ERROR < %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);
+
+  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-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]]
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/120406