From: Nico Weber Date: Fri, 11 Oct 2019 02:44:20 +0000 (+0000) Subject: Revert 374481 "[tsan,msan] Insert module constructors in a module pass" X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e6d329dbcbeafd37730fc535b34455a92e3f4e2d;p=llvm Revert 374481 "[tsan,msan] Insert module constructors in a module pass" CodeGen/sanitizer-module-constructor.c fails on mac and windows, see e.g. http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/11424 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@374503 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/Transforms/Instrumentation/MemorySanitizer.h b/include/llvm/Transforms/Instrumentation/MemorySanitizer.h index 01a86ee3f1f..71d0fa5f595 100644 --- a/include/llvm/Transforms/Instrumentation/MemorySanitizer.h +++ b/include/llvm/Transforms/Instrumentation/MemorySanitizer.h @@ -40,7 +40,6 @@ struct MemorySanitizerPass : public PassInfoMixin { MemorySanitizerPass(MemorySanitizerOptions Options) : Options(Options) {} PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM); - PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM); private: MemorySanitizerOptions Options; diff --git a/include/llvm/Transforms/Instrumentation/ThreadSanitizer.h b/include/llvm/Transforms/Instrumentation/ThreadSanitizer.h index ce0e46745ab..b4e7d9924ff 100644 --- a/include/llvm/Transforms/Instrumentation/ThreadSanitizer.h +++ b/include/llvm/Transforms/Instrumentation/ThreadSanitizer.h @@ -27,8 +27,6 @@ FunctionPass *createThreadSanitizerLegacyPassPass(); /// yet, the pass inserts the declarations. Otherwise the existing globals are struct ThreadSanitizerPass : public PassInfoMixin { PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM); - PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM); }; - } // namespace llvm #endif /* LLVM_TRANSFORMS_INSTRUMENTATION_THREADSANITIZER_H */ diff --git a/lib/Passes/PassRegistry.def b/lib/Passes/PassRegistry.def index eb350cb665f..4cd05ee11f6 100644 --- a/lib/Passes/PassRegistry.def +++ b/lib/Passes/PassRegistry.def @@ -86,8 +86,6 @@ MODULE_PASS("synthetic-counts-propagation", SyntheticCountsPropagation()) MODULE_PASS("wholeprogramdevirt", WholeProgramDevirtPass(nullptr, nullptr)) MODULE_PASS("verify", VerifierPass()) MODULE_PASS("asan-module", ModuleAddressSanitizerPass(/*CompileKernel=*/false, false, true, false)) -MODULE_PASS("msan-module", MemorySanitizerPass({})) -MODULE_PASS("tsan-module", ThreadSanitizerPass()) MODULE_PASS("kasan-module", ModuleAddressSanitizerPass(/*CompileKernel=*/true, false, true, false)) MODULE_PASS("sancov-module", ModuleSanitizerCoveragePass()) MODULE_PASS("poison-checking", PoisonCheckingPass()) diff --git a/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/lib/Transforms/Instrumentation/MemorySanitizer.cpp index 69c9020e060..cf93de6f759 100644 --- a/lib/Transforms/Instrumentation/MemorySanitizer.cpp +++ b/lib/Transforms/Instrumentation/MemorySanitizer.cpp @@ -587,25 +587,9 @@ private: /// An empty volatile inline asm that prevents callback merge. InlineAsm *EmptyAsm; -}; -void insertModuleCtor(Module &M) { - getOrCreateSanitizerCtorAndInitFunctions( - M, kMsanModuleCtorName, kMsanInitName, - /*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, FunctionCallee) { - if (!ClWithComdat) { - appendToGlobalCtors(M, Ctor, 0); - return; - } - Comdat *MsanCtorComdat = M.getOrInsertComdat(kMsanModuleCtorName); - Ctor->setComdat(MsanCtorComdat); - appendToGlobalCtors(M, Ctor, 0, Ctor); - }); -} + Function *MsanCtorFunction; +}; /// A legacy function pass for msan instrumentation. /// @@ -651,14 +635,6 @@ PreservedAnalyses MemorySanitizerPass::run(Function &F, return PreservedAnalyses::all(); } -PreservedAnalyses MemorySanitizerPass::run(Module &M, - ModuleAnalysisManager &AM) { - if (Options.Kernel) - return PreservedAnalyses::all(); - insertModuleCtor(M); - return PreservedAnalyses::none(); -} - char MemorySanitizerLegacyPass::ID = 0; INITIALIZE_PASS_BEGIN(MemorySanitizerLegacyPass, "msan", @@ -944,6 +920,23 @@ void MemorySanitizer::initializeModule(Module &M) { OriginStoreWeights = MDBuilder(*C).createBranchWeights(1, 1000); if (!CompileKernel) { + std::tie(MsanCtorFunction, std::ignore) = + getOrCreateSanitizerCtorAndInitFunctions( + M, kMsanModuleCtorName, kMsanInitName, + /*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, FunctionCallee) { + if (!ClWithComdat) { + appendToGlobalCtors(M, Ctor, 0); + return; + } + Comdat *MsanCtorComdat = M.getOrInsertComdat(kMsanModuleCtorName); + Ctor->setComdat(MsanCtorComdat); + appendToGlobalCtors(M, Ctor, 0, Ctor); + }); + if (TrackOrigins) M.getOrInsertGlobal("__msan_track_origins", IRB.getInt32Ty(), [&] { return new GlobalVariable( @@ -961,8 +954,6 @@ void MemorySanitizer::initializeModule(Module &M) { } bool MemorySanitizerLegacyPass::doInitialization(Module &M) { - if (!Options.Kernel) - insertModuleCtor(M); MSan.emplace(M, Options); return true; } @@ -4587,9 +4578,8 @@ static VarArgHelper *CreateVarArgHelper(Function &Func, MemorySanitizer &Msan, } bool MemorySanitizer::sanitizeFunction(Function &F, TargetLibraryInfo &TLI) { - if (!CompileKernel && F.getName() == kMsanModuleCtorName) + if (!CompileKernel && (&F == MsanCtorFunction)) return false; - MemorySanitizerVisitor Visitor(F, *this, TLI); // Clear out readonly/readnone attributes. diff --git a/lib/Transforms/Instrumentation/ThreadSanitizer.cpp b/lib/Transforms/Instrumentation/ThreadSanitizer.cpp index ac274a155a8..643a03783e7 100644 --- a/lib/Transforms/Instrumentation/ThreadSanitizer.cpp +++ b/lib/Transforms/Instrumentation/ThreadSanitizer.cpp @@ -92,10 +92,11 @@ namespace { /// 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: - void initialize(Module &M); + void initializeCallbacks(Module &M); bool instrumentLoadOrStore(Instruction *I, const DataLayout &DL); bool instrumentAtomic(Instruction *I, const DataLayout &DL); bool instrumentMemIntrinsic(Instruction *I); @@ -107,6 +108,8 @@ private: void InsertRuntimeIgnores(Function &F); Type *IntptrTy; + IntegerType *OrdTy; + // Callbacks to run-time library are computed in doInitialization. FunctionCallee TsanFuncEntry; FunctionCallee TsanFuncExit; FunctionCallee TsanIgnoreBegin; @@ -127,6 +130,7 @@ private: FunctionCallee TsanVptrUpdate; FunctionCallee TsanVptrLoad; FunctionCallee MemmoveFn, MemcpyFn, MemsetFn; + Function *TsanCtorFunction; }; struct ThreadSanitizerLegacyPass : FunctionPass { @@ -139,32 +143,16 @@ struct ThreadSanitizerLegacyPass : FunctionPass { private: Optional TSan; }; - -void insertModuleCtor(Module &M) { - 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, FunctionCallee) { appendToGlobalCtors(M, Ctor, 0); }); -} - } // namespace PreservedAnalyses ThreadSanitizerPass::run(Function &F, FunctionAnalysisManager &FAM) { - ThreadSanitizer TSan; + ThreadSanitizer TSan(*F.getParent()); if (TSan.sanitizeFunction(F, FAM.getResult(F))) return PreservedAnalyses::none(); return PreservedAnalyses::all(); } -PreservedAnalyses ThreadSanitizerPass::run(Module &M, - ModuleAnalysisManager &MAM) { - insertModuleCtor(M); - return PreservedAnalyses::none(); -} - char ThreadSanitizerLegacyPass::ID = 0; INITIALIZE_PASS_BEGIN(ThreadSanitizerLegacyPass, "tsan", "ThreadSanitizer: detects data races.", false, false) @@ -181,8 +169,7 @@ void ThreadSanitizerLegacyPass::getAnalysisUsage(AnalysisUsage &AU) const { } bool ThreadSanitizerLegacyPass::doInitialization(Module &M) { - insertModuleCtor(M); - TSan.emplace(); + TSan.emplace(M); return true; } @@ -196,10 +183,7 @@ FunctionPass *llvm::createThreadSanitizerLegacyPassPass() { return new ThreadSanitizerLegacyPass(); } -void ThreadSanitizer::initialize(Module &M) { - const DataLayout &DL = M.getDataLayout(); - IntptrTy = DL.getIntPtrType(M.getContext()); - +void ThreadSanitizer::initializeCallbacks(Module &M) { IRBuilder<> IRB(M.getContext()); AttributeList Attr; Attr = Attr.addAttribute(M.getContext(), AttributeList::FunctionIndex, @@ -213,7 +197,7 @@ void ThreadSanitizer::initialize(Module &M) { IRB.getVoidTy()); TsanIgnoreEnd = M.getOrInsertFunction("__tsan_ignore_thread_end", Attr, IRB.getVoidTy()); - IntegerType *OrdTy = IRB.getInt32Ty(); + OrdTy = IRB.getInt32Ty(); for (size_t i = 0; i < kNumberOfAccessSizes; ++i) { const unsigned ByteSize = 1U << i; const unsigned BitSize = ByteSize * 8; @@ -296,6 +280,20 @@ void ThreadSanitizer::initialize(Module &M) { IRB.getInt8PtrTy(), IRB.getInt32Ty(), IntptrTy); } +ThreadSanitizer::ThreadSanitizer(Module &M) { + const DataLayout &DL = M.getDataLayout(); + IntptrTy = DL.getIntPtrType(M.getContext()); + 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, FunctionCallee) { + appendToGlobalCtors(M, Ctor, 0); + }); +} + static bool isVtableAccess(Instruction *I) { if (MDNode *Tag = I->getMetadata(LLVMContext::MD_tbaa)) return Tag->isTBAAVtableAccess(); @@ -438,9 +436,9 @@ 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.getName() == kTsanModuleCtorName) + if (&F == TsanCtorFunction) return false; - initialize(*F.getParent()); + initializeCallbacks(*F.getParent()); SmallVector AllLoadsAndStores; SmallVector LocalLoadsAndStores; SmallVector AtomicAccesses; diff --git a/test/Instrumentation/MemorySanitizer/msan_basic.ll b/test/Instrumentation/MemorySanitizer/msan_basic.ll index f56233d1342..499de14aea3 100644 --- a/test/Instrumentation/MemorySanitizer/msan_basic.ll +++ b/test/Instrumentation/MemorySanitizer/msan_basic.ll @@ -1,9 +1,10 @@ -; RUN: opt < %s -msan-check-access-address=0 -S -passes='module(msan-module),function(msan)' 2>&1 | FileCheck -allow-deprecated-dag-overlap %s -; RUN: opt < %s --passes='module(msan-module),function(msan)' -msan-check-access-address=0 -S | FileCheck -allow-deprecated-dag-overlap %s -; RUN: opt < %s -msan-check-access-address=0 -msan-track-origins=1 -S -passes='module(msan-module),function(msan)' 2>&1 | \ -; RUN: FileCheck -allow-deprecated-dag-overlap -check-prefixes=CHECK,CHECK-ORIGINS %s -; RUN: opt < %s -passes='module(msan-module),function(msan)' -msan-check-access-address=0 -msan-track-origins=1 -S | \ -; RUN: FileCheck -allow-deprecated-dag-overlap -check-prefixes=CHECK,CHECK-ORIGINS %s +; RUN: opt < %s -msan-check-access-address=0 -S -passes=msan 2>&1 | FileCheck \ +; RUN: -allow-deprecated-dag-overlap %s +; RUN: opt < %s -msan -msan-check-access-address=0 -S | FileCheck -allow-deprecated-dag-overlap %s +; RUN: opt < %s -msan-check-access-address=0 -msan-track-origins=1 -S \ +; RUN: -passes=msan 2>&1 | FileCheck -allow-deprecated-dag-overlap \ +; RUN: -check-prefix=CHECK -check-prefix=CHECK-ORIGINS %s +; RUN: opt < %s -msan -msan-check-access-address=0 -msan-track-origins=1 -S | FileCheck -allow-deprecated-dag-overlap -check-prefix=CHECK -check-prefix=CHECK-ORIGINS %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-S128" target triple = "x86_64-unknown-linux-gnu" diff --git a/test/Instrumentation/ThreadSanitizer/tsan_basic.ll b/test/Instrumentation/ThreadSanitizer/tsan_basic.ll index 953ab8ed8dc..8b85d7b8bdd 100644 --- a/test/Instrumentation/ThreadSanitizer/tsan_basic.ll +++ b/test/Instrumentation/ThreadSanitizer/tsan_basic.ll @@ -1,5 +1,5 @@ ; RUN: opt < %s -tsan -S | FileCheck %s -; RUN: opt < %s -passes='function(tsan),module(tsan-module)' -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"