[compiler-rt] [TySan] Fix struct access with different bases (PR #120412)

[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/108385)
Fixes issue https://github.com/llvm/llvm-project/issues/105960

If a member in a struct is also a struct, accessing a member partway through this inner struct currently causes a false positive. This is because when checking aliasing, the access offset is seen as greater than the starting offset of the inner struct, so the loop continues one iteration, and believes we are accessing the member after the inner struct.

The next member's offset is greater than the offset we are looking for, so when we subtract the next member's offset from what we are looking for, the offset underflows.

To fix this, we check if the member we think we are accessing has a greater offset than the offset we are looking for. If so, we take a step back. We cannot do this in the loop, since the loop does not check the final member. This means the penultimate member would still cause false positives.

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


2 Files Affected:

- (modified) compiler-rt/lib/tysan/tysan.cpp (+8) 
- (added) compiler-rt/test/tysan/struct-offset-different-base.cpp (+31) 


``````````diff
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index 39d78e7c95e0cd..94498287f17d19 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -131,6 +131,14 @@ static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
           break;
       }
 
+      // This offset can't be negative. Therefore we must be accessing something
+      // partially inside the last type
+      // We shouldn't check this if we are on the first member, Idx will
+      // underflow The first member can be offset in rare cases such as
+      // llvm::cl::Option
+      if (TDA->Struct.Members[Idx].Offset > OffsetA && Idx > 0)
+        Idx -= 1;
+
       OffsetA -= TDA->Struct.Members[Idx].Offset;
       TDA = TDA->Struct.Members[Idx].Type;
     } else {
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..da0efd2cb6503c
--- /dev/null
+++ b/compiler-rt/test/tysan/struct-offset-different-base.cpp
@@ -0,0 +1,31 @@
+// RUN: %clangxx_tysan -O0 %s -o %t && %run %t >%t.out 2>&1
+// RUN: FileCheck %s --implicit-check-not ERROR < %t.out
+
+// Modified reproducer from https://github.com/llvm/llvm-project/issues/105960
+
+#include <stdio.h>
+
+struct inner {
+  char buffer;
+  int i;
+};
+
+void init_inner(inner *iPtr) { iPtr->i = 200; }
+
+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("Accessed value is %d\n", access_offsets_with_different_base);
+
+  return 0;
+}
+
+// CHECK: Accessed value is 200

``````````

</details>


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