Skip to content

Commit 0670675

Browse files
authored
[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.
1 parent d9bab33 commit 0670675

File tree

3 files changed

+95
-17
lines changed

3 files changed

+95
-17
lines changed

compiler-rt/lib/tysan/tysan.cpp

+49-17
Original file line numberDiff line numberDiff line change
@@ -102,20 +102,10 @@ static tysan_type_descriptor *getRootTD(tysan_type_descriptor *TD) {
102102
return RootTD;
103103
}
104104

105-
static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
106-
tysan_type_descriptor *TDB, int TDAOffset) {
107-
// Walk up the tree starting with TDA to see if we reach TDB.
108-
uptr OffsetA = 0, OffsetB = 0;
109-
if (TDB->Tag == TYSAN_MEMBER_TD) {
110-
OffsetB = TDB->Member.Offset;
111-
TDB = TDB->Member.Base;
112-
}
113-
114-
if (TDA->Tag == TYSAN_MEMBER_TD) {
115-
OffsetA = TDA->Member.Offset - TDAOffset;
116-
TDA = TDA->Member.Base;
117-
}
118-
105+
// Walk up TDA to see if it reaches TDB.
106+
static bool walkAliasTree(tysan_type_descriptor *TDA,
107+
tysan_type_descriptor *TDB, uptr OffsetA,
108+
uptr OffsetB) {
119109
do {
120110
if (TDA == TDB)
121111
return OffsetA == OffsetB;
@@ -153,8 +143,50 @@ static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
153143
return false;
154144
}
155145

146+
// Walk up the tree starting with TDA to see if we reach TDB.
147+
static bool isAliasingLegalUp(tysan_type_descriptor *TDA,
148+
tysan_type_descriptor *TDB) {
149+
uptr OffsetA = 0, OffsetB = 0;
150+
if (TDB->Tag == TYSAN_MEMBER_TD) {
151+
OffsetB = TDB->Member.Offset;
152+
TDB = TDB->Member.Base;
153+
}
154+
155+
if (TDA->Tag == TYSAN_MEMBER_TD) {
156+
OffsetA = TDA->Member.Offset;
157+
TDA = TDA->Member.Base;
158+
}
159+
160+
return walkAliasTree(TDA, TDB, OffsetA, OffsetB);
161+
}
162+
163+
static bool isAliasingLegalWithOffset(tysan_type_descriptor *TDA,
164+
tysan_type_descriptor *TDB,
165+
uptr OffsetB) {
166+
// This is handled by calls to isAliasingLegalUp.
167+
if (OffsetB == 0)
168+
return false;
169+
170+
// You can't have an offset into a member.
171+
if (TDB->Tag == TYSAN_MEMBER_TD)
172+
return false;
173+
174+
uptr OffsetA = 0;
175+
if (TDA->Tag == TYSAN_MEMBER_TD) {
176+
OffsetA = TDA->Member.Offset;
177+
TDA = TDA->Member.Base;
178+
}
179+
180+
// Since the access was partially inside TDB (the shadow), it can be assumed
181+
// that we are accessing a member in an object. This means that rather than
182+
// walk up the scalar access TDA to reach an object, we should walk up the
183+
// object TBD to reach the scalar we are accessing it with. The offsets will
184+
// still be checked at the end to make sure this alias is legal.
185+
return walkAliasTree(TDB, TDA, OffsetB, OffsetA);
186+
}
187+
156188
static bool isAliasingLegal(tysan_type_descriptor *TDA,
157-
tysan_type_descriptor *TDB, int TDAOffset = 0) {
189+
tysan_type_descriptor *TDB, uptr OffsetB = 0) {
158190
if (TDA == TDB || !TDB || !TDA)
159191
return true;
160192

@@ -165,8 +197,8 @@ static bool isAliasingLegal(tysan_type_descriptor *TDA,
165197
// TDB may have been adjusted by offset TDAOffset in the caller to point to
166198
// the outer type. Check for aliasing with and without adjusting for this
167199
// offset.
168-
return isAliasingLegalUp(TDA, TDB, 0) || isAliasingLegalUp(TDB, TDA, 0) ||
169-
isAliasingLegalUp(TDA, TDB, TDAOffset);
200+
return isAliasingLegalUp(TDA, TDB) || isAliasingLegalUp(TDB, TDA) ||
201+
isAliasingLegalWithOffset(TDA, TDB, OffsetB);
170202
}
171203

172204
namespace __tysan {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// RUN: %clangxx_tysan %s -o %t && %run %t 2>&1 | FileCheck --implicit-check-not ERROR %s
2+
3+
#include <stdio.h>
4+
5+
class Inner {
6+
public:
7+
void *ptr = nullptr;
8+
};
9+
10+
class Base {
11+
public:
12+
void *buffer1;
13+
Inner inside;
14+
void *buffer2;
15+
};
16+
17+
class Derrived : public Base {};
18+
19+
Derrived derr;
20+
21+
int main() {
22+
printf("%p", derr.inside.ptr);
23+
24+
return 0;
25+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// RUN: %clangxx_tysan %s -o %t && %run %t 2>&1 | FileCheck --implicit-check-not ERROR %s
2+
3+
#include <stdio.h>
4+
5+
class Base {
6+
public:
7+
void *first;
8+
void *second;
9+
void *third;
10+
};
11+
12+
class Derrived : public Base {};
13+
14+
Derrived derr;
15+
16+
int main() {
17+
derr.second = nullptr;
18+
printf("%p", derr.second);
19+
20+
return 0;
21+
}

0 commit comments

Comments
 (0)