From 6dbe225afee23c2c19fb4ae5ee688d9824be525d Mon Sep 17 00:00:00 2001 From: Philip Pfaffe Date: Wed, 16 Jan 2019 09:28:01 +0000 Subject: [PATCH] [NewPM][TSan] Reiterate the TSan port Summary: Second iteration of D56433 which got reverted in rL350719. The problem in the previous version was that we dropped the thunk calling the tsan init function. The new version keeps the thunk which should appease dyld, but is not actually OK wrt. the current semantics of function passes. Hence, add a helper to insert the functions only on the first time. The helper allows hooking into the insertion to be able to append them to the global ctors list. Reviewers: chandlerc, vitalybuka, fedor.sergeev, leonardchan Subscribers: hiraditya, bollu, llvm-commits Differential Revision: https://reviews.llvm.org/D56538 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@351314 91177308-0d34-0410-b5e6-96231b3b80d8 --- bindings/go/llvm/InstrumentationBindings.cpp | 3 +- include/llvm/InitializePasses.h | 2 +- include/llvm/Transforms/Instrumentation.h | 3 - .../Instrumentation/ThreadSanitizer.h | 33 +++++++ include/llvm/Transforms/Utils/ModuleUtils.h | 13 +++ lib/Passes/PassBuilder.cpp | 1 + lib/Passes/PassRegistry.def | 1 + .../Instrumentation/Instrumentation.cpp | 2 +- .../Instrumentation/ThreadSanitizer.cpp | 95 ++++++++++++------- lib/Transforms/Utils/ModuleUtils.cpp | 22 +++++ .../ThreadSanitizer/tsan_basic.ll | 2 +- 11 files changed, 137 insertions(+), 40 deletions(-) create mode 100644 include/llvm/Transforms/Instrumentation/ThreadSanitizer.h diff --git a/bindings/go/llvm/InstrumentationBindings.cpp b/bindings/go/llvm/InstrumentationBindings.cpp index 6ce43db620a..c3b4f2eaf50 100644 --- a/bindings/go/llvm/InstrumentationBindings.cpp +++ b/bindings/go/llvm/InstrumentationBindings.cpp @@ -17,6 +17,7 @@ #include "llvm/IR/Module.h" #include "llvm/Transforms/Instrumentation.h" #include "llvm/Transforms/Instrumentation/MemorySanitizer.h" +#include "llvm/Transforms/Instrumentation/ThreadSanitizer.h" using namespace llvm; @@ -29,7 +30,7 @@ void LLVMAddAddressSanitizerModulePass(LLVMPassManagerRef PM) { } void LLVMAddThreadSanitizerPass(LLVMPassManagerRef PM) { - unwrap(PM)->add(createThreadSanitizerPass()); + unwrap(PM)->add(createThreadSanitizerLegacyPassPass()); } void LLVMAddMemorySanitizerLegacyPassPass(LLVMPassManagerRef PM) { diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h index c4946a7dd24..037c0dbb56e 100644 --- a/include/llvm/InitializePasses.h +++ b/include/llvm/InitializePasses.h @@ -392,7 +392,7 @@ void initializeTailDuplicatePass(PassRegistry&); void initializeTargetLibraryInfoWrapperPassPass(PassRegistry&); void initializeTargetPassConfigPass(PassRegistry&); void initializeTargetTransformInfoWrapperPassPass(PassRegistry&); -void initializeThreadSanitizerPass(PassRegistry&); +void initializeThreadSanitizerLegacyPassPass(PassRegistry&); void initializeTwoAddressInstructionPassPass(PassRegistry&); void initializeTypeBasedAAWrapperPassPass(PassRegistry&); void initializeUnifyFunctionExitNodesPass(PassRegistry&); diff --git a/include/llvm/Transforms/Instrumentation.h b/include/llvm/Transforms/Instrumentation.h index 34b52072012..017cab0a775 100644 --- a/include/llvm/Transforms/Instrumentation.h +++ b/include/llvm/Transforms/Instrumentation.h @@ -155,9 +155,6 @@ ModulePass *createAddressSanitizerModulePass(bool CompileKernel = false, FunctionPass *createHWAddressSanitizerPass(bool CompileKernel = false, bool Recover = false); -// Insert ThreadSanitizer (race detection) instrumentation -FunctionPass *createThreadSanitizerPass(); - // Insert DataFlowSanitizer (dynamic data flow analysis) instrumentation ModulePass *createDataFlowSanitizerPass( const std::vector &ABIListFiles = std::vector(), diff --git a/include/llvm/Transforms/Instrumentation/ThreadSanitizer.h b/include/llvm/Transforms/Instrumentation/ThreadSanitizer.h new file mode 100644 index 00000000000..701e2e6ec89 --- /dev/null +++ b/include/llvm/Transforms/Instrumentation/ThreadSanitizer.h @@ -0,0 +1,33 @@ +//===- Transforms/Instrumentation/MemorySanitizer.h - TSan Pass -----------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file defines the thread sanitizer pass. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_THREADSANITIZER_H +#define LLVM_TRANSFORMS_INSTRUMENTATION_THREADSANITIZER_H + +#include "llvm/IR/PassManager.h" +#include "llvm/Pass.h" + +namespace llvm { +// Insert ThreadSanitizer (race detection) instrumentation +FunctionPass *createThreadSanitizerLegacyPassPass(); + +/// A function pass for tsan instrumentation. +/// +/// Instruments functions to detect race conditions reads. This function pass +/// inserts calls to runtime library functions. If the functions aren't declared +/// yet, the pass inserts the declarations. Otherwise the existing globals are +struct ThreadSanitizerPass : public PassInfoMixin { + PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM); +}; +} // namespace llvm +#endif /* LLVM_TRANSFORMS_INSTRUMENTATION_THREADSANITIZER_H */ diff --git a/include/llvm/Transforms/Utils/ModuleUtils.h b/include/llvm/Transforms/Utils/ModuleUtils.h index f77b3b20a67..fee492be2a9 100644 --- a/include/llvm/Transforms/Utils/ModuleUtils.h +++ b/include/llvm/Transforms/Utils/ModuleUtils.h @@ -58,6 +58,19 @@ std::pair createSanitizerCtorAndInitFunctions( ArrayRef InitArgTypes, ArrayRef InitArgs, StringRef VersionCheckName = StringRef()); +/// Creates sanitizer constructor function lazily. If a constructor and init +/// function already exist, this function returns it. Otherwise it calls \c +/// createSanitizerCtorAndInitFunctions. The FunctionsCreatedCallback is invoked +/// in that case, passing the new Ctor and Init function. +/// +/// \return Returns pair of pointers to constructor, and init functions +/// respectively. +std::pair getOrCreateSanitizerCtorAndInitFunctions( + Module &M, StringRef CtorName, StringRef InitName, + ArrayRef InitArgTypes, ArrayRef InitArgs, + function_ref FunctionsCreatedCallback, + StringRef VersionCheckName = StringRef()); + // Creates and returns a sanitizer init function without argument if it doesn't // exist, and adds it to the global constructors list. Otherwise it returns the // existing function. diff --git a/lib/Passes/PassBuilder.cpp b/lib/Passes/PassBuilder.cpp index 576393533aa..5ec94ea6f40 100644 --- a/lib/Passes/PassBuilder.cpp +++ b/lib/Passes/PassBuilder.cpp @@ -95,6 +95,7 @@ #include "llvm/Transforms/Instrumentation/GCOVProfiler.h" #include "llvm/Transforms/Instrumentation/InstrProfiling.h" #include "llvm/Transforms/Instrumentation/MemorySanitizer.h" +#include "llvm/Transforms/Instrumentation/ThreadSanitizer.h" #include "llvm/Transforms/Instrumentation/PGOInstrumentation.h" #include "llvm/Transforms/Scalar/ADCE.h" #include "llvm/Transforms/Scalar/AlignmentFromAssumptions.h" diff --git a/lib/Passes/PassRegistry.def b/lib/Passes/PassRegistry.def index 209a8811c10..771d2f5b212 100644 --- a/lib/Passes/PassRegistry.def +++ b/lib/Passes/PassRegistry.def @@ -232,6 +232,7 @@ FUNCTION_PASS("view-cfg", CFGViewerPass()) FUNCTION_PASS("view-cfg-only", CFGOnlyViewerPass()) FUNCTION_PASS("transform-warning", WarnMissedTransformationsPass()) FUNCTION_PASS("msan", MemorySanitizerPass()) +FUNCTION_PASS("tsan", ThreadSanitizerPass()) #undef FUNCTION_PASS #ifndef FUNCTION_PASS_WITH_PARAMS diff --git a/lib/Transforms/Instrumentation/Instrumentation.cpp b/lib/Transforms/Instrumentation/Instrumentation.cpp index 5828019a03a..c3e323613c7 100644 --- a/lib/Transforms/Instrumentation/Instrumentation.cpp +++ b/lib/Transforms/Instrumentation/Instrumentation.cpp @@ -113,7 +113,7 @@ void llvm::initializeInstrumentation(PassRegistry &Registry) { initializeInstrProfilingLegacyPassPass(Registry); initializeMemorySanitizerLegacyPassPass(Registry); initializeHWAddressSanitizerPass(Registry); - initializeThreadSanitizerPass(Registry); + initializeThreadSanitizerLegacyPassPass(Registry); initializeSanitizerCoverageModulePass(Registry); initializeDataFlowSanitizerPass(Registry); initializeEfficiencySanitizerPass(Registry); diff --git a/lib/Transforms/Instrumentation/ThreadSanitizer.cpp b/lib/Transforms/Instrumentation/ThreadSanitizer.cpp index fa1e5a157a0..077364e15c4 100644 --- a/lib/Transforms/Instrumentation/ThreadSanitizer.cpp +++ b/lib/Transforms/Instrumentation/ThreadSanitizer.cpp @@ -19,6 +19,7 @@ // The rest is handled by the run-time library. //===----------------------------------------------------------------------===// +#include "llvm/Transforms/Instrumentation/ThreadSanitizer.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" @@ -86,15 +87,16 @@ static const char *const kTsanInitName = "__tsan_init"; namespace { /// ThreadSanitizer: instrument the code in module to find races. -struct ThreadSanitizer : public FunctionPass { - ThreadSanitizer() : FunctionPass(ID) {} - StringRef getPassName() const override; - void getAnalysisUsage(AnalysisUsage &AU) const override; - bool runOnFunction(Function &F) override; - bool doInitialization(Module &M) override; - static char ID; // Pass identification, replacement for typeid. +/// +/// Instantiating ThreadSanitizer inserts the tsan runtime library API function +/// declarations into the module if they don't exist already. Instantiating +/// ensures the __tsan_init function is in the list of global constructors for +/// the module. +struct ThreadSanitizer { + ThreadSanitizer(Module &M); + bool sanitizeFunction(Function &F, const TargetLibraryInfo &TLI); - private: +private: void initializeCallbacks(Module &M); bool instrumentLoadOrStore(Instruction *I, const DataLayout &DL); bool instrumentAtomic(Instruction *I, const DataLayout &DL); @@ -130,27 +132,55 @@ struct ThreadSanitizer : public FunctionPass { Function *MemmoveFn, *MemcpyFn, *MemsetFn; Function *TsanCtorFunction; }; + +struct ThreadSanitizerLegacyPass : FunctionPass { + ThreadSanitizerLegacyPass() : FunctionPass(ID) {} + StringRef getPassName() const override; + void getAnalysisUsage(AnalysisUsage &AU) const override; + bool runOnFunction(Function &F) override; + bool doInitialization(Module &M) override; + static char ID; // Pass identification, replacement for typeid. +private: + Optional TSan; +}; } // namespace -char ThreadSanitizer::ID = 0; -INITIALIZE_PASS_BEGIN( - ThreadSanitizer, "tsan", - "ThreadSanitizer: detects data races.", - false, false) +PreservedAnalyses ThreadSanitizerPass::run(Function &F, + FunctionAnalysisManager &FAM) { + ThreadSanitizer TSan(*F.getParent()); + if (TSan.sanitizeFunction(F, FAM.getResult(F))) + return PreservedAnalyses::none(); + return PreservedAnalyses::all(); +} + +char ThreadSanitizerLegacyPass::ID = 0; +INITIALIZE_PASS_BEGIN(ThreadSanitizerLegacyPass, "tsan", + "ThreadSanitizer: detects data races.", false, false) INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass) -INITIALIZE_PASS_END( - ThreadSanitizer, "tsan", - "ThreadSanitizer: detects data races.", - false, false) +INITIALIZE_PASS_END(ThreadSanitizerLegacyPass, "tsan", + "ThreadSanitizer: detects data races.", false, false) -StringRef ThreadSanitizer::getPassName() const { return "ThreadSanitizer"; } +StringRef ThreadSanitizerLegacyPass::getPassName() const { + return "ThreadSanitizerLegacyPass"; +} -void ThreadSanitizer::getAnalysisUsage(AnalysisUsage &AU) const { +void ThreadSanitizerLegacyPass::getAnalysisUsage(AnalysisUsage &AU) const { AU.addRequired(); } -FunctionPass *llvm::createThreadSanitizerPass() { - return new ThreadSanitizer(); +bool ThreadSanitizerLegacyPass::doInitialization(Module &M) { + TSan.emplace(M); + return true; +} + +bool ThreadSanitizerLegacyPass::runOnFunction(Function &F) { + auto &TLI = getAnalysis().getTLI(); + TSan->sanitizeFunction(F, TLI); + return true; +} + +FunctionPass *llvm::createThreadSanitizerLegacyPassPass() { + return new ThreadSanitizerLegacyPass(); } void ThreadSanitizer::initializeCallbacks(Module &M) { @@ -252,16 +282,16 @@ void ThreadSanitizer::initializeCallbacks(Module &M) { IRB.getInt32Ty(), IntptrTy)); } -bool ThreadSanitizer::doInitialization(Module &M) { +ThreadSanitizer::ThreadSanitizer(Module &M) { const DataLayout &DL = M.getDataLayout(); IntptrTy = DL.getIntPtrType(M.getContext()); - std::tie(TsanCtorFunction, std::ignore) = createSanitizerCtorAndInitFunctions( - M, kTsanModuleCtorName, kTsanInitName, /*InitArgTypes=*/{}, - /*InitArgs=*/{}); - - appendToGlobalCtors(M, TsanCtorFunction, 0); - - return true; + std::tie(TsanCtorFunction, std::ignore) = + getOrCreateSanitizerCtorAndInitFunctions( + M, kTsanModuleCtorName, kTsanInitName, /*InitArgTypes=*/{}, + /*InitArgs=*/{}, + // This callback is invoked when the functions are created the first + // time. Hook them into the global ctors list in that case: + [&](Function *Ctor, Function *) { appendToGlobalCtors(M, Ctor, 0); }); } static bool isVtableAccess(Instruction *I) { @@ -402,7 +432,8 @@ void ThreadSanitizer::InsertRuntimeIgnores(Function &F) { } } -bool ThreadSanitizer::runOnFunction(Function &F) { +bool ThreadSanitizer::sanitizeFunction(Function &F, + const TargetLibraryInfo &TLI) { // This is required to prevent instrumenting call to __tsan_init from within // the module constructor. if (&F == TsanCtorFunction) @@ -416,8 +447,6 @@ bool ThreadSanitizer::runOnFunction(Function &F) { bool HasCalls = false; bool SanitizeFunction = F.hasFnAttribute(Attribute::SanitizeThread); const DataLayout &DL = F.getParent()->getDataLayout(); - const TargetLibraryInfo *TLI = - &getAnalysis().getTLI(); // Traverse all instructions, collect loads/stores/returns, check for calls. for (auto &BB : F) { @@ -428,7 +457,7 @@ bool ThreadSanitizer::runOnFunction(Function &F) { LocalLoadsAndStores.push_back(&Inst); else if (isa(Inst) || isa(Inst)) { if (CallInst *CI = dyn_cast(&Inst)) - maybeMarkSanitizerLibraryCallNoBuiltin(CI, TLI); + maybeMarkSanitizerLibraryCallNoBuiltin(CI, &TLI); if (isa(Inst)) MemIntrinCalls.push_back(&Inst); HasCalls = true; diff --git a/lib/Transforms/Utils/ModuleUtils.cpp b/lib/Transforms/Utils/ModuleUtils.cpp index 8040cc7d09c..ae5e72ea4d3 100644 --- a/lib/Transforms/Utils/ModuleUtils.cpp +++ b/lib/Transforms/Utils/ModuleUtils.cpp @@ -174,6 +174,28 @@ std::pair llvm::createSanitizerCtorAndInitFunctions( return std::make_pair(Ctor, InitFunction); } +std::pair +llvm::getOrCreateSanitizerCtorAndInitFunctions( + Module &M, StringRef CtorName, StringRef InitName, + ArrayRef InitArgTypes, ArrayRef InitArgs, + function_ref FunctionsCreatedCallback, + StringRef VersionCheckName) { + assert(!CtorName.empty() && "Expected ctor function name"); + + if (Function *Ctor = M.getFunction(CtorName)) + // FIXME: Sink this logic into the module, similar to the handling of + // globals. This will make moving to a concurrent model much easier. + if (Ctor->arg_size() == 0 || + Ctor->getReturnType() == Type::getVoidTy(M.getContext())) + return {Ctor, declareSanitizerInitFunction(M, InitName, InitArgTypes)}; + + Function *Ctor, *InitFunction; + std::tie(Ctor, InitFunction) = llvm::createSanitizerCtorAndInitFunctions( + M, CtorName, InitName, InitArgTypes, InitArgs, VersionCheckName); + FunctionsCreatedCallback(Ctor, InitFunction); + return std::make_pair(Ctor, InitFunction); +} + Function *llvm::getOrCreateInitFunction(Module &M, StringRef Name) { assert(!Name.empty() && "Expected init function name"); if (Function *F = M.getFunction(Name)) { diff --git a/test/Instrumentation/ThreadSanitizer/tsan_basic.ll b/test/Instrumentation/ThreadSanitizer/tsan_basic.ll index 69d4117399b..8b85d7b8bdd 100644 --- a/test/Instrumentation/ThreadSanitizer/tsan_basic.ll +++ b/test/Instrumentation/ThreadSanitizer/tsan_basic.ll @@ -1,4 +1,5 @@ ; RUN: opt < %s -tsan -S | FileCheck %s +; RUN: opt < %s -passes=tsan -S | FileCheck %s target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64" target triple = "x86_64-unknown-linux-gnu" @@ -77,6 +78,5 @@ define void @SwiftErrorCall(i8** swifterror) sanitize_thread { call void @SwiftError(i8** %0) ret void } - ; CHECK: define internal void @tsan.module_ctor() ; CHECK: call void @__tsan_init()