Skip to content

Commit bf6986f

Browse files
authored
[TSan, SanitizerBinaryMetadata] Improve instrument for derived pointers via phis/selects (#132752)
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 pointer induction variables. For instance, ```llvm %stkobj = alloca [2 x i32], align 8 ; getUnderlyingObject(%derived) = %derived %derived = getelementptr inbounds i32, ptr %stkobj, i64 1 ``` This will result in redundant instrumentation of TSan, resulting in greater performance costs, especially when there are loops, referring to this [godbolt page](https://godbolt.org/z/eaT1fPjTW) for details. ```cpp char loop(int x) { char buf[10]; char *p = buf; for (int i = 0; i < x && i < 10; i++) { // Should not instrument, as its base object is a non-captured stack // variable. // However, currectly, it is instrumented due to %p = %phi ... *p++ = i; } // Use buf to prevent it from being eliminated by optimization return buf[9]; } ``` There are TWO APIs `getUnderlyingObjectAggressive` and `findAllocaForValue` that can backtrack the pointer via tree traversal, supporting phis/selects. 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 f351172 commit bf6986f

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
@@ -392,8 +392,8 @@ bool maybeSharedMutable(const Value *Addr) {
392392
if (!Addr)
393393
return true;
394394

395-
if (isa<AllocaInst>(getUnderlyingObject(Addr)) &&
396-
!PointerMayBeCaptured(Addr, /*ReturnCaptures=*/true))
395+
const AllocaInst *AI = findAllocaForValue(Addr);
396+
if (AI && !PointerMayBeCaptured(Addr, /*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 & 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)