Skip to content

Commit 07a991f

Browse files
committed
[TSan, SanitizerBinaryMetadata] Improve instrument for derived pointers via phis/selects
ThreadSanitizer.cpp and SanitizerBinaryMetadata.cpp previously used `getUnderlyingObject` to check if pointers originate from stack objects. However, `getUnderlyingObject()` by default only looks through linear chains, not selects/phis. In particular, this means that we miss cases involving involving pointer induction variables. This patch replaces `getUnderlyingObject` with `findAllocaForValue` which: 1. Properly tracks through PHINodes and select operations 2. Directly identifies if a pointer comes from a `AllocaInst` Performance impact: - Compilation: Moderate cost increase due to wider value tracing, but... - Runtime: Significant wins for code with pointer induction variables derived from stack allocas, especially for loop-heavy code, as instrumentation can now be safely omitted.
1 parent 058a4e8 commit 07a991f

File tree

3 files changed

+35
-4
lines changed

3 files changed

+35
-4
lines changed

llvm/lib/Transforms/Instrumentation/SanitizerBinaryMetadata.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,8 +393,8 @@ bool maybeSharedMutable(const Value *Addr) {
393393
if (!Addr)
394394
return true;
395395

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

400400
Addr = Addr->stripInBoundsOffsets();

llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -448,8 +448,8 @@ void ThreadSanitizer::chooseInstructionsToInstrument(
448448
}
449449
}
450450

451-
if (isa<AllocaInst>(getUnderlyingObject(Addr)) &&
452-
!PointerMayBeCaptured(Addr, /*ReturnCaptures=*/true)) {
451+
const AllocaInst *AI = findAllocaForValue(Addr);
452+
if (AI && !PointerMayBeCaptured(Addr, /*ReturnCaptures=*/true)) {
453453
// The variable is addressable but not captured, so it cannot be
454454
// referenced from a different thread and participate in a data race
455455
// (see llvm/Analysis/CaptureTracking.h for details).

llvm/test/Instrumentation/ThreadSanitizer/capture.ll

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,4 +88,35 @@ entry:
8888
; CHECK: __tsan_write
8989
; CHECK: ret void
9090

91+
define void @notcaptured3(i1 %cond) nounwind uwtable sanitize_thread {
92+
entry:
93+
%stkobj = alloca [2 x i32], align 8
94+
%derived = getelementptr inbounds i32, ptr %stkobj, i64 1
95+
%ptr = select i1 %cond, ptr %derived, ptr %stkobj
96+
store i32 42, ptr %ptr, align 4
97+
ret void
98+
}
99+
; CHECK-LABEL: define void @notcaptured3
100+
; CHECK-NOT: call void @__tsan_write4(ptr %ptr)
101+
; CHECK: ret void
91102

103+
define void @notcaptured4() nounwind uwtable sanitize_thread {
104+
entry:
105+
%stkobj = alloca [10 x i8], align 1
106+
br label %loop
107+
108+
exit:
109+
ret void
110+
111+
loop:
112+
%count = phi i32 [ 0, %entry ], [ %addone, %loop ]
113+
%derived = phi ptr [ %stkobj, %entry ], [ %ptraddone, %loop ]
114+
store i32 %count, ptr %derived, align 4
115+
%ptraddone = getelementptr inbounds i32, ptr %derived, i64 1
116+
%addone = add nuw nsw i32 %count, 1
117+
%eq10 = icmp eq i32 %addone, 10
118+
br i1 %eq10, label %exit, label %loop
119+
}
120+
; CHECK-LABEL: define void @notcaptured4
121+
; CHECK: ret void
122+
; CHECK-NOT: call void @__tsan_write4(ptr %derived)

0 commit comments

Comments
 (0)