[compiler-rt] [TySan] Fix struct access with different bases (PR #120412)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 13 05:42:10 PST 2025
https://github.com/gbMattN updated https://github.com/llvm/llvm-project/pull/120412
>From f645295ad59e99ef4efc62fca302b387904abbbc Mon Sep 17 00:00:00 2001
From: Matthew Nagy <gbmatt at tiger-linux2.domain.snsys.com>
Date: Thu, 12 Sep 2024 12:36:57 +0000
Subject: [PATCH 1/9] [TySan] Fix struct access with different bases
---
compiler-rt/lib/tysan/tysan.cpp | 5 +++
.../tysan/struct-offset-different-base.cpp | 33 +++++++++++++++++++
2 files changed, 38 insertions(+)
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 39d78e7c95e0cd..6640f83c4553cf 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -131,6 +131,11 @@ 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
+ if (TDA->Struct.Members[Idx].Offset > OffsetA)
+ 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..3e1d6f2a6a42f5
--- /dev/null
+++ b/compiler-rt/test/tysan/struct-offset-different-base.cpp
@@ -0,0 +1,33 @@
+// RUN: %clangxx_tysan -O0 %s -o %t && %run %t >%t.out 2>&1
+// RUN: FileCheck %s < %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 = 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
>From 6e663807274a28f3b2f90d72c749133711fae1e0 Mon Sep 17 00:00:00 2001
From: gbMattN <matthew.nagy at sony.com>
Date: Tue, 12 Nov 2024 17:05:44 +0000
Subject: [PATCH 2/9] Changed test to check for output
---
compiler-rt/test/tysan/struct-offset-different-base.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/compiler-rt/test/tysan/struct-offset-different-base.cpp b/compiler-rt/test/tysan/struct-offset-different-base.cpp
index 3e1d6f2a6a42f5..4563f7025bea48 100644
--- a/compiler-rt/test/tysan/struct-offset-different-base.cpp
+++ b/compiler-rt/test/tysan/struct-offset-different-base.cpp
@@ -1,5 +1,5 @@
// RUN: %clangxx_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
// Modified reproducer from https://github.com/llvm/llvm-project/issues/105960
@@ -11,7 +11,7 @@ struct inner {
};
void init_inner(inner *iPtr) {
- iPtr->i = 0;
+ iPtr->i = 200;
}
struct outer {
@@ -25,9 +25,9 @@ int main(void) {
init_inner(&l->foo);
int access_offsets_with_different_base = l->foo.i;
- printf("%d\n", access_offsets_with_different_base);
+ printf("Accessed value is %d\n", access_offsets_with_different_base);
return 0;
}
-// CHECK-NOT: ERROR: TypeSanitizer: type-aliasing-violation
+// CHECK: Accessed value is 200
>From 9c92143a330ddaa87bdc595ed37c0cd94bc600b1 Mon Sep 17 00:00:00 2001
From: gbMattN <matthew.nagy at sony.com>
Date: Tue, 26 Nov 2024 16:44:43 +0000
Subject: [PATCH 3/9] More format changes
---
compiler-rt/lib/tysan/tysan.cpp | 2 +-
.../tysan/struct-offset-different-base.cpp | 28 +++++++++----------
2 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index 6640f83c4553cf..f17a0f7007ac69 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -135,7 +135,7 @@ static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
// partially inside the last type
if (TDA->Struct.Members[Idx].Offset > OffsetA)
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
index 4563f7025bea48..da0efd2cb6503c 100644
--- a/compiler-rt/test/tysan/struct-offset-different-base.cpp
+++ b/compiler-rt/test/tysan/struct-offset-different-base.cpp
@@ -6,28 +6,26 @@
#include <stdio.h>
struct inner {
- char buffer;
- int i;
+ char buffer;
+ int i;
};
-void init_inner(inner *iPtr) {
- iPtr->i = 200;
-}
+void init_inner(inner *iPtr) { iPtr->i = 200; }
struct outer {
- inner foo;
- char buffer;
+ 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;
+ 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
>From 7ba829ef4e5aff36c1d433d9ba6dc8e935ab873b Mon Sep 17 00:00:00 2001
From: gbMattN <matthew.nagy at sony.com>
Date: Fri, 29 Nov 2024 11:41:03 +0000
Subject: [PATCH 4/9] Fixed occasional segfault
---
compiler-rt/lib/tysan/tysan.cpp | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index f17a0f7007ac69..94498287f17d19 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -133,7 +133,10 @@ static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
// This offset can't be negative. Therefore we must be accessing something
// partially inside the last type
- if (TDA->Struct.Members[Idx].Offset > OffsetA)
+ // 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;
>From e4e57a3ebaf18b1ebcec31081b5df9a84c6a0408 Mon Sep 17 00:00:00 2001
From: gbMattN <matthew.nagy at sony.com>
Date: Fri, 10 Jan 2025 16:05:20 +0000
Subject: [PATCH 5/9] Updated test
---
.../tysan/struct-offset-different-base.cpp | 32 +++++++++++++++----
1 file changed, 25 insertions(+), 7 deletions(-)
diff --git a/compiler-rt/test/tysan/struct-offset-different-base.cpp b/compiler-rt/test/tysan/struct-offset-different-base.cpp
index da0efd2cb6503c..4b14c51aaa37fd 100644
--- a/compiler-rt/test/tysan/struct-offset-different-base.cpp
+++ b/compiler-rt/test/tysan/struct-offset-different-base.cpp
@@ -5,27 +5,45 @@
#include <stdio.h>
-struct inner {
+struct inner1 {
char buffer;
int i;
};
-void init_inner(inner *iPtr) { iPtr->i = 200; }
+struct inner2 {
+ char buffer;
+ int i;
+ float endBuffer;
+};
+
+void init_inner1(inner1 *iPtr) { iPtr->i = 200; }
+void init_inner2(inner2 *iPtr) {
+ iPtr->i = 400;
+ iPtr->endBuffer = 413.0f;
+}
struct outer {
- inner foo;
+ inner1 foo;
+ inner2 bar;
char buffer;
};
int main(void) {
outer *l = new outer();
- init_inner(&l->foo);
+ init_inner1(&l->foo);
+ init_inner2(&l->bar);
- int access_offsets_with_different_base = l->foo.i;
- printf("Accessed value is %d\n", access_offsets_with_different_base);
+ int access = l->foo.i;
+ printf("Accessed value 1 is %d\n", access);
+ access = l->bar.i;
+ printf("Accessed value 2 is %d\n", access);
+ float fAccess = l->bar.endBuffer;
+ printf("Accessed value 3 is %f\n", fAccess);
return 0;
}
-// CHECK: Accessed value is 200
+// CHECK: Accessed value 1 is 200
+// CHECK: Accessed value 2 is 400
+// CHECK: Accessed value 3 is 413.0
>From a7f05f6f4c9859846b08b28e257883f4d42f3253 Mon Sep 17 00:00:00 2001
From: gbMattN <matthew.nagy at sony.com>
Date: Fri, 10 Jan 2025 16:06:41 +0000
Subject: [PATCH 6/9] Comments updated
---
compiler-rt/lib/tysan/tysan.cpp | 4 ++--
compiler-rt/test/tysan/struct-offset-different-base.cpp | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index f9012c66450106..74f0919a2b4444 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -134,8 +134,8 @@ static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
// 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
+ // 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;
diff --git a/compiler-rt/test/tysan/struct-offset-different-base.cpp b/compiler-rt/test/tysan/struct-offset-different-base.cpp
index 4b14c51aaa37fd..862595de8dc81f 100644
--- a/compiler-rt/test/tysan/struct-offset-different-base.cpp
+++ b/compiler-rt/test/tysan/struct-offset-different-base.cpp
@@ -17,7 +17,7 @@ struct inner2 {
};
void init_inner1(inner1 *iPtr) { iPtr->i = 200; }
-void init_inner2(inner2 *iPtr) {
+void init_inner2(inner2 *iPtr) {
iPtr->i = 400;
iPtr->endBuffer = 413.0f;
}
>From 10652dadee7a2b3987c702d94f38f6b23ab3bb4e Mon Sep 17 00:00:00 2001
From: gbMattN <matthew.nagy at sony.com>
Date: Mon, 13 Jan 2025 12:30:12 +0000
Subject: [PATCH 7/9] Change comment to reflect change
---
compiler-rt/lib/tysan/tysan.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index 74f0919a2b4444..cf9c10a470f931 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -134,8 +134,7 @@ static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
// 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.
+ // underflow.
if (TDA->Struct.Members[Idx].Offset > OffsetA && Idx > 0)
Idx -= 1;
>From a42ab69033f0401ecea682946359df31117d06d6 Mon Sep 17 00:00:00 2001
From: gbMattN <146744444+gbMattN at users.noreply.github.com>
Date: Mon, 13 Jan 2025 12:30:46 +0000
Subject: [PATCH 8/9] Update compiler-rt/lib/tysan/tysan.cpp
Co-authored-by: Florian Hahn <flo at fhahn.com>
---
compiler-rt/lib/tysan/tysan.cpp | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index cf9c10a470f931..48df1ce20376e2 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -135,8 +135,13 @@ static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
// partially inside the last type
// We shouldn't check this if we are on the first member, Idx will
// underflow.
- if (TDA->Struct.Members[Idx].Offset > OffsetA && Idx > 0)
+ if (TDA->Struct.Members[Idx].Offset > OffsetA) {
+ // Trying to access something before the current type.
+ if (!Idx)
+ return false;
+
Idx -= 1;
+ }
OffsetA -= TDA->Struct.Members[Idx].Offset;
TDA = TDA->Struct.Members[Idx].Type;
>From d0b20a81e4f3a02dbcb6bb123c3b592901ae91ac Mon Sep 17 00:00:00 2001
From: gbMattN <146744444+gbMattN at users.noreply.github.com>
Date: Mon, 13 Jan 2025 13:41:56 +0000
Subject: [PATCH 9/9] Update compiler-rt/lib/tysan/tysan.cpp
Co-authored-by: Florian Hahn <flo at fhahn.com>
---
compiler-rt/lib/tysan/tysan.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp
index 48df1ce20376e2..41dac438c76db9 100644
--- a/compiler-rt/lib/tysan/tysan.cpp
+++ b/compiler-rt/lib/tysan/tysan.cpp
@@ -132,7 +132,7 @@ static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
}
// This offset can't be negative. Therefore we must be accessing something
- // partially inside the last type
+ // before the current type (not legal) or partially inside the last type. In the latter case, we adjust Idx.
// We shouldn't check this if we are on the first member, Idx will
// underflow.
if (TDA->Struct.Members[Idx].Offset > OffsetA) {
More information about the llvm-commits
mailing list