https://github.com/gbMattN updated https://github.com/llvm/llvm-project/pull/95387
>From 620ee820a39ac1e92ee86f64d290ad32b8d426be Mon Sep 17 00:00:00 2001 From: Matthew Nagy <gbm...@tiger-linux2.domain.snsys.com> Date: Fri, 28 Jun 2024 16:12:31 +0000 Subject: [PATCH 1/3] [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 f1b6bdcf0d8261..5ba967c0782e08 100644 --- a/compiler-rt/lib/tysan/tysan.cpp +++ b/compiler-rt/lib/tysan/tysan.cpp @@ -226,7 +226,24 @@ __tysan_check(void *addr, int size, tysan_type_descriptor *td, int flags) { OldTDPtr -= i; OldTD = *OldTDPtr; - if (!isAliasingLegal(td, OldTD, i)) + // 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, 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..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 9a2c70c7e6c8e78116068b729d129f152bc398b6 Mon Sep 17 00:00:00 2001 From: gbMattN <matthew.n...@sony.com> Date: Tue, 12 Nov 2024 16:52:27 +0000 Subject: [PATCH 2/3] [style] Tidied up comments, formatting, and polished test --- compiler-rt/lib/tysan/tysan.cpp | 24 ++++++++++--------- .../test/tysan/global-struct-members.c | 7 +++--- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp index 5ba967c0782e08..d9029751208ea9 100644 --- a/compiler-rt/lib/tysan/tysan.cpp +++ b/compiler-rt/lib/tysan/tysan.cpp @@ -226,24 +226,26 @@ __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. - tysan_type_descriptor *InternalMember = 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 which will give us the correct error. + tysan_type_descriptor *AccessedType = OldTD; if (OldTD->Tag == TYSAN_STRUCT_TD) { - for (int j = 0; j < OldTD->Struct.MemberCount; j++) { + for (int j = 0; j < OldTD->Struct.MemberCount; ++j) { if (OldTD->Struct.Members[j].Offset == i) { - InternalMember = OldTD->Struct.Members[j].Type; + AccessedType = OldTD->Struct.Members[j].Type; break; } } } - if (!isAliasingLegal(td, InternalMember, 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 index 76ea3c431dd7bc..ed8f21bb84f3c5 100644 --- a/compiler-rt/test/tysan/global-struct-members.c +++ b/compiler-rt/test/tysan/global-struct-members.c @@ -1,5 +1,5 @@ // RUN: %clang_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 #include <stdio.h> @@ -15,7 +15,6 @@ int main() { 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; @@ -26,6 +25,6 @@ int main() { 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]] + // 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]] } >From 8addd061964253a1100d76446569ff1f67e63a9c Mon Sep 17 00:00:00 2001 From: gbMattN <matthew.n...@sony.com> Date: Mon, 2 Dec 2024 17:17:31 +0000 Subject: [PATCH 3/3] Removed false positive/segfault when accessing member of global for the first time --- compiler-rt/lib/tysan/tysan.cpp | 35 +++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/compiler-rt/lib/tysan/tysan.cpp b/compiler-rt/lib/tysan/tysan.cpp index d9029751208ea9..228a5f13f5b185 100644 --- a/compiler-rt/lib/tysan/tysan.cpp +++ b/compiler-rt/lib/tysan/tysan.cpp @@ -172,6 +172,7 @@ static void reportError(void *Addr, int Size, tysan_type_descriptor *TD, const char *DescStr, int Offset, uptr pc, uptr bp, uptr sp) { Decorator d; + return; Printf("%s", d.Warning()); Report("ERROR: TypeSanitizer: type-aliasing-violation on address %p" " (pc %p bp %p sp %p tid %llu)\n", @@ -225,22 +226,26 @@ __tysan_check(void *addr, int size, tysan_type_descriptor *td, int flags) { int i = -((sptr)OldTD); 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 which will give us the correct error. + tysan_type_descriptor *AccessedType = OldTD; - 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; + + // 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; + } } } } _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits