Skip to content

Commit 816a8da

Browse files
authored
[DevTSAN] Cleanup private shadow memory when function exit (#17735)
We need to skip the check for private memory, otherwise OpenCL CPU device may generate false positive reports due to stack re-use in different threads. However, SPIR-V builts 'ToPrivate' doesn't work as expected on OpenCL CPU device. So we need to manually cleanup private shadow before each function exit point.
1 parent 62ba344 commit 816a8da

File tree

5 files changed

+80
-4
lines changed

5 files changed

+80
-4
lines changed

libdevice/sanitizer/tsan_rtl.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ static const __SYCL_CONSTANT__ char __tsan_print_shadow_value[] =
2828
"[kernel] %p(%d) : {size: %d, access: %x, sid: %d, clock: %d, is_write: "
2929
"%d}\n";
3030

31+
static const __SYCL_CONSTANT__ char __tsan_print_cleanup_private[] =
32+
"[kernel] cleanup private shadow: %p ~ %p\n";
33+
3134
static const __SYCL_CONSTANT__ char __tsan_print_unsupport_device_type[] =
3235
"[kernel] Unsupport device type: %d\n";
3336

@@ -47,6 +50,10 @@ static const __SYCL_CONSTANT__ char __tsan_report_race[] =
4750

4851
namespace {
4952

53+
inline constexpr uptr RoundUpTo(uptr x, uptr boundary) {
54+
return (x + boundary - 1) & ~(boundary - 1);
55+
}
56+
5057
inline constexpr uptr RoundDownTo(uptr x, uptr boundary) {
5158
return x & ~(boundary - 1);
5259
}
@@ -334,4 +341,21 @@ TSAN_CHECK(write, true, 2)
334341
TSAN_CHECK(write, true, 4)
335342
TSAN_CHECK(write, true, 8)
336343

344+
DEVICE_EXTERN_C_NOINLINE void __tsan_cleanup_private(uptr addr, uint32_t size) {
345+
if (TsanLaunchInfo->DeviceTy != DeviceType::CPU)
346+
return;
347+
348+
if (size) {
349+
addr = RoundDownTo(addr, kShadowCell);
350+
size = RoundUpTo(size, kShadowCell);
351+
352+
RawShadow *Begin = MemToShadow_CPU(addr, 0);
353+
TSAN_DEBUG(__spirv_ocl_printf(
354+
__tsan_print_cleanup_private, Begin,
355+
(uptr)Begin + size / kShadowCell * kShadowCnt * kShadowSize - 1));
356+
for (uptr i = 0; i < size / kShadowCell * kShadowCnt; i++)
357+
Begin[i] = 0;
358+
}
359+
}
360+
337361
#endif // __SPIR__ || __SPIRV__

llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,9 @@ struct ThreadSanitizerOnSpirv {
120120

121121
void instrumentModule();
122122

123+
bool instrumentAllocInst(Function *F,
124+
SmallVectorImpl<Instruction *> &AllocaInsts);
125+
123126
void appendDebugInfoToArgs(Instruction *I, SmallVectorImpl<Value *> &Args);
124127

125128
private:
@@ -144,6 +147,7 @@ struct ThreadSanitizerOnSpirv {
144147

145148
// Accesses sizes are powers of two: 1, 2, 4, 8, 16.
146149
static const size_t kNumberOfAccessSizes = 5;
150+
FunctionCallee TsanCleanupPrivate;
147151
FunctionCallee TsanRead[kNumberOfAccessSizes];
148152
FunctionCallee TsanWrite[kNumberOfAccessSizes];
149153

@@ -261,6 +265,10 @@ void ThreadSanitizerOnSpirv::initialize() {
261265
Attr = Attr.addFnAttribute(C, Attribute::NoUnwind);
262266
Type *Int8PtrTy = IRB.getInt8PtrTy(kSpirOffloadConstantAS);
263267

268+
TsanCleanupPrivate =
269+
M.getOrInsertFunction("__tsan_cleanup_private", Attr, IRB.getVoidTy(),
270+
IntptrTy, IRB.getInt32Ty());
271+
264272
for (size_t i = 0; i < kNumberOfAccessSizes; ++i) {
265273
const unsigned ByteSize = 1U << i;
266274
std::string ByteSizeStr = utostr(ByteSize);
@@ -282,6 +290,28 @@ void ThreadSanitizerOnSpirv::initialize() {
282290
}
283291
}
284292

293+
bool ThreadSanitizerOnSpirv::instrumentAllocInst(
294+
Function *F, SmallVectorImpl<Instruction *> &AllocaInsts) {
295+
bool Changed = false;
296+
297+
EscapeEnumerator EE(*F, "tsan_cleanup", false);
298+
while (IRBuilder<> *AtExit = EE.Next()) {
299+
InstrumentationIRBuilder::ensureDebugInfo(*AtExit, *F);
300+
for (auto *Inst : AllocaInsts) {
301+
AllocaInst *AI = cast<AllocaInst>(Inst);
302+
if (auto AllocSize = AI->getAllocationSize(DL)) {
303+
AtExit->CreateCall(
304+
TsanCleanupPrivate,
305+
{AtExit->CreatePtrToInt(AI, IntptrTy),
306+
ConstantInt::get(AtExit->getInt32Ty(), *AllocSize)});
307+
Changed |= true;
308+
}
309+
}
310+
}
311+
312+
return Changed;
313+
}
314+
285315
void ThreadSanitizerOnSpirv::appendDebugInfoToArgs(
286316
Instruction *I, SmallVectorImpl<Value *> &Args) {
287317
auto &Loc = I->getDebugLoc();
@@ -793,6 +823,7 @@ bool ThreadSanitizer::sanitizeFunction(Function &F,
793823
SmallVector<Instruction*, 8> LocalLoadsAndStores;
794824
SmallVector<Instruction*, 8> AtomicAccesses;
795825
SmallVector<Instruction*, 8> MemIntrinCalls;
826+
SmallVector<Instruction *, 8> Allocas;
796827
bool Res = false;
797828
bool HasCalls = false;
798829
bool SanitizeFunction = F.hasFnAttribute(Attribute::SanitizeThread);
@@ -808,6 +839,9 @@ bool ThreadSanitizer::sanitizeFunction(Function &F,
808839
AtomicAccesses.push_back(&Inst);
809840
else if (isa<LoadInst>(Inst) || isa<StoreInst>(Inst))
810841
LocalLoadsAndStores.push_back(&Inst);
842+
else if (Spirv && isa<AllocaInst>(Inst) &&
843+
cast<AllocaInst>(Inst).getAllocatedType()->isSized())
844+
Allocas.push_back(&Inst);
811845
else if ((isa<CallInst>(Inst) && !isa<DbgInfoIntrinsic>(Inst)) ||
812846
isa<InvokeInst>(Inst)) {
813847
if (CallInst *CI = dyn_cast<CallInst>(&Inst))
@@ -850,6 +884,14 @@ bool ThreadSanitizer::sanitizeFunction(Function &F,
850884
InsertRuntimeIgnores(F);
851885
}
852886

887+
// FIXME: We need to skip the check for private memory, otherwise OpenCL CPU
888+
// device may generate false positive reports due to stack re-use in different
889+
// threads. However, SPIR-V builts 'ToPrivate' doesn't work as expected on
890+
// OpenCL CPU device. So we need to manually cleanup private shadow before
891+
// each function exit point.
892+
if (Spirv && !Allocas.empty())
893+
Res |= Spirv->instrumentAllocInst(&F, Allocas);
894+
853895
// Instrument function entry/exit points if there were instrumented accesses.
854896
if ((Res || HasCalls) && ClInstrumentFuncEntryExit) {
855897
InstrumentationIRBuilder IRB(&F.getEntryBlock(),
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
; RUN: opt < %s -passes='function(tsan),module(tsan-module)' -tsan-instrument-func-entry-exit=0 -tsan-instrument-memintrinsics=0 -S | FileCheck %s
2+
target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1"
3+
target triple = "spir64-unknown-unknown"
4+
5+
%"class.sycl::_V1::range" = type { %"class.sycl::_V1::detail::array" }
6+
%"class.sycl::_V1::detail::array" = type { [1 x i64] }
7+
8+
define spir_kernel void @test() {
9+
entry:
10+
%agg.tmp = alloca %"class.sycl::_V1::range", align 8
11+
; CHECK: [[REG1:%[0-9]+]] = ptrtoint ptr %agg.tmp to i64
12+
; CHECK-NEXT: call void @__tsan_cleanup_private(i64 [[REG1]], i32 8)
13+
ret void
14+
}

sycl/test-e2e/ThreadSanitizer/check_both_read.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// REQUIRES: linux, cpu || (gpu && level_zero)
22
// RUN: %{build} %device_tsan_flags -O0 -g -o %t1.out
33
// RUN: %{run} %t1.out 2>&1 | FileCheck %s
4-
// UNSUPPORTED: true
5-
// UNSUPPORTED-TRACKER: CMPLRLLVM-66203
64
#include "sycl/detail/core.hpp"
75
#include "sycl/usm.hpp"
86

sycl/test-e2e/ThreadSanitizer/check_no_race.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33
// RUN: %{run} %t1.out 2>&1 | FileCheck %s
44
// RUN: %{build} %device_tsan_flags -O2 -g -o %t2.out
55
// RUN: %{run} %t2.out 2>&1 | FileCheck %s
6-
// UNSUPPORTED: true
7-
// UNSUPPORTED-TRACKER: CMPLRLLVM-66203
86
#include "sycl/detail/core.hpp"
97
#include "sycl/usm.hpp"
108

0 commit comments

Comments
 (0)