[compiler-rt] 0670675 - [TySan] Fix false positives with derived classes (#126260)

[via llvm-commits]

Author: gbMattN
Date: 2025-04-25T10:55:20+01:00
New Revision: 067067580f35d897e5aa4dcf94e9f35173984254

URL: https://github.com/llvm/llvm-project/commit/067067580f35d897e5aa4dcf94e9f35173984254
DIFF: https://github.com/llvm/llvm-project/commit/067067580f35d897e5aa4dcf94e9f35173984254.diff

LOG: [TySan] Fix false positives with derived classes (#126260)

Fixes issue #125079 as well as another case I discovered while trying to
build LLVM with TySan.

Added: 
    compiler-rt/test/tysan/derrived_default_constructor.cpp
    compiler-rt/test/tysan/inherited_member.cpp

Modified: 
    compiler-rt/lib/tysan/tysan.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index f0230df9260e3..4fa8166986d76 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -102,20 +102,10 @@ static tysan_type_descriptor *getRootTD(tysan_type_descriptor *TD) {
   return RootTD;
 }
 
-static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
-                              tysan_type_descriptor *TDB, int TDAOffset) {
-  // Walk up the tree starting with TDA to see if we reach TDB.
-  uptr OffsetA = 0, OffsetB = 0;
-  if (TDB->Tag == TYSAN_MEMBER_TD) {
-    OffsetB = TDB->Member.Offset;
-    TDB = TDB->Member.Base;
-  }
-
-  if (TDA->Tag == TYSAN_MEMBER_TD) {
-    OffsetA = TDA->Member.Offset - TDAOffset;
-    TDA = TDA->Member.Base;
-  }
-
+// Walk up TDA to see if it reaches TDB.
+static bool walkAliasTree(tysan_type_descriptor *TDA,
+                          tysan_type_descriptor *TDB, uptr OffsetA,
+                          uptr OffsetB) {
   do {
     if (TDA == TDB)
       return OffsetA == OffsetB;
@@ -153,8 +143,50 @@ static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
   return false;
 }
 
+// Walk up the tree starting with TDA to see if we reach TDB.
+static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
+                              tysan_type_descriptor *TDB) {
+  uptr OffsetA = 0, OffsetB = 0;
+  if (TDB->Tag == TYSAN_MEMBER_TD) {
+    OffsetB = TDB->Member.Offset;
+    TDB = TDB->Member.Base;
+  }
+
+  if (TDA->Tag == TYSAN_MEMBER_TD) {
+    OffsetA = TDA->Member.Offset;
+    TDA = TDA->Member.Base;
+  }
+
+  return walkAliasTree(TDA, TDB, OffsetA, OffsetB);
+}
+
+static bool isAliasingLegalWithOffset(tysan_type_descriptor *TDA,
+                                      tysan_type_descriptor *TDB,
+                                      uptr OffsetB) {
+  // This is handled by calls to isAliasingLegalUp.
+  if (OffsetB == 0)
+    return false;
+
+  // You can't have an offset into a member.
+  if (TDB->Tag == TYSAN_MEMBER_TD)
+    return false;
+
+  uptr OffsetA = 0;
+  if (TDA->Tag == TYSAN_MEMBER_TD) {
+    OffsetA = TDA->Member.Offset;
+    TDA = TDA->Member.Base;
+  }
+
+  // Since the access was partially inside TDB (the shadow), it can be assumed
+  // that we are accessing a member in an object. This means that rather than
+  // walk up the scalar access TDA to reach an object, we should walk up the
+  // object TBD to reach the scalar we are accessing it with. The offsets will
+  // still be checked at the end to make sure this alias is legal.
+  return walkAliasTree(TDB, TDA, OffsetB, OffsetA);
+}
+
 static bool isAliasingLegal(tysan_type_descriptor *TDA,
-                            tysan_type_descriptor *TDB, int TDAOffset = 0) {
+                            tysan_type_descriptor *TDB, uptr OffsetB = 0) {
   if (TDA == TDB || !TDB || !TDA)
     return true;
 
@@ -165,8 +197,8 @@ static bool isAliasingLegal(tysan_type_descriptor *TDA,
   // TDB may have been adjusted by offset TDAOffset in the caller to point to
   // the outer type. Check for aliasing with and without adjusting for this
   // offset.
-  return isAliasingLegalUp(TDA, TDB, 0) || isAliasingLegalUp(TDB, TDA, 0) ||
-         isAliasingLegalUp(TDA, TDB, TDAOffset);
+  return isAliasingLegalUp(TDA, TDB) || isAliasingLegalUp(TDB, TDA) ||
+         isAliasingLegalWithOffset(TDA, TDB, OffsetB);
 }
 
 namespace __tysan {

diff  --git a/compiler-rt/test/tysan/derrived_default_constructor.cpp b/compiler-rt/test/tysan/derrived_default_constructor.cpp
new file mode 100644
index 0000000000000..39705bfbb3eae
--- /dev/null
+++ b/compiler-rt/test/tysan/derrived_default_constructor.cpp
@@ -0,0 +1,25 @@
+// RUN: %clangxx_tysan %s -o %t && %run %t 2>&1 | FileCheck --implicit-check-not ERROR %s
+
+#include <stdio.h>
+
+class Inner {
+public:
+  void *ptr = nullptr;
+};
+
+class Base {
+public:
+  void *buffer1;
+  Inner inside;
+  void *buffer2;
+};
+
+class Derrived : public Base {};
+
+Derrived derr;
+
+int main() {
+  printf("%p", derr.inside.ptr);
+
+  return 0;
+}

diff  --git a/compiler-rt/test/tysan/inherited_member.cpp b/compiler-rt/test/tysan/inherited_member.cpp
new file mode 100644
index 0000000000000..f23d69e6c8065
--- /dev/null
+++ b/compiler-rt/test/tysan/inherited_member.cpp
@@ -0,0 +1,21 @@
+// RUN: %clangxx_tysan %s -o %t && %run %t 2>&1 | FileCheck --implicit-check-not ERROR %s
+
+#include <stdio.h>
+
+class Base {
+public:
+  void *first;
+  void *second;
+  void *third;
+};
+
+class Derrived : public Base {};
+
+Derrived derr;
+
+int main() {
+  derr.second = nullptr;
+  printf("%p", derr.second);
+
+  return 0;
+}