Skip to content

Commit 1a3518a

Browse files
committed
[TSan, SanitizerBinaryMetadata] Analyze the capture status for alloca rather than arbitrary Addr
This commit addresses the limitation in `PointerMayBeCaptured` analysis when dealing with derived pointers (e.g. arr+1) as described in issue #132739. The current implementation may miss captures of the underlying alloca when analyzing derived pointers. Even when working correctly, backtracking to the original alloca during analysis causes redundancy to the outer's `findAllocaForValue`. Key changes: Directly analyze the capture status of the underlying alloca instead of derived pointers to ensure accurate capture detection This patch fixes some FNs of TSan, referring to the appending testcases for more details.
1 parent c274bbe commit 1a3518a

File tree

4 files changed

+39
-2
lines changed

4 files changed

+39
-2
lines changed

compiler-rt/test/tsan/stack_race3.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
2+
#include "test.h"
3+
4+
void *Thread(void *a) {
5+
barrier_wait(&barrier);
6+
((int *)a)[1] = 43;
7+
return 0;
8+
}
9+
10+
int main() {
11+
barrier_init(&barrier, 2);
12+
int Arr[2] = {41, 42};
13+
pthread_t t;
14+
pthread_create(&t, 0, Thread, &Arr[0]);
15+
Arr[1] = 43;
16+
barrier_wait(&barrier);
17+
pthread_join(t, 0);
18+
}
19+
20+
// CHECK: WARNING: ThreadSanitizer: data race
21+
// CHECK: Location is stack of main thread.

llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,7 @@ bool maybeSharedMutable(const Value *Addr) {
393393
return true;
394394

395395
const AllocaInst *AI = findAllocaForValue(Addr);
396-
if (AI && !PointerMayBeCaptured(Addr, /*ReturnCaptures=*/true))
396+
if (AI && !PointerMayBeCaptured(AI, /*ReturnCaptures=*/true))
397397
return false; // Object is on stack but does not escape.
398398

399399
Addr = Addr->stripInBoundsOffsets();

llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,8 @@ void ThreadSanitizer::chooseInstructionsToInstrument(
449449
}
450450

451451
const AllocaInst *AI = findAllocaForValue(Addr);
452-
if (AI && !PointerMayBeCaptured(Addr, /*ReturnCaptures=*/true)) {
452+
// Instead of Addr, we should check whether its base pointer is captured.
453+
if (AI && !PointerMayBeCaptured(AI, /*ReturnCaptures=*/true)) {
453454
// The variable is addressable but not captured, so it cannot be
454455
// referenced from a different thread and participate in a data race
455456
// (see llvm/Analysis/CaptureTracking.h for details).

llvm/test/Instrumentation/ThreadSanitizer/capture.ll

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,21 @@ entry:
4747
; CHECK: __tsan_write
4848
; CHECK: ret void
4949

50+
define void @captured3() nounwind uwtable sanitize_thread {
51+
entry:
52+
%stkobj = alloca [2 x i32], align 8
53+
; escapes due to store into global
54+
store ptr %stkobj, ptr @sink, align 8
55+
; derived is captured as its base object is captured
56+
%derived = getelementptr inbounds i32, ptr %stkobj, i64 1
57+
store i32 42, ptr %derived, align 4
58+
ret void
59+
}
60+
; CHECK-LABEL: define void @captured3
61+
; CHECK: __tsan_write
62+
; CHECK: __tsan_write
63+
; CHECK: ret void
64+
5065
define void @notcaptured0() nounwind uwtable sanitize_thread {
5166
entry:
5267
%ptr = alloca i32, align 4

0 commit comments

Comments
 (0)