From 0f2f1c2be164810da96f2bc7229ef56f54c7d137 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Tue, 6 Sep 2022 19:20:59 -0700 Subject: [PATCH] [sanitizers] Invalidate GlobalsAA GlobalsAA is considered stateless as usually transformations do not introduce new global accesses, and removed global access is not a problem for GlobalsAA users. Sanitizers introduce new global accesses: - Msan and Dfsan tracks origins and parameters with TLS, and to store stack origins. - Sancov uses global counters. HWAsan store tag state in TLS. - Asan modifies globals, but I am not sure if invalidation is required. I see no evidence that TSan needs invalidation. Reviewed By: aeubanks Differential Revision: https://reviews.llvm.org/D133394 --- .../Instrumentation/AddressSanitizer.cpp | 11 +++++++- .../Instrumentation/DataFlowSanitizer.cpp | 14 +++++++--- .../Instrumentation/HWAddressSanitizer.cpp | 13 +++++++--- .../Transforms/Instrumentation/MemorySanitizer.cpp | 12 ++++++++- .../Instrumentation/SanitizerCoverage.cpp | 13 +++++++--- .../MemorySanitizer/invalidate_global_aa.ll | 26 +++++++++++++++++++ .../MemorySanitizer/msan_invalidate.ll | 30 ++++++++++++++++++++++ 7 files changed, 107 insertions(+), 12 deletions(-) create mode 100644 llvm/test/Instrumentation/MemorySanitizer/invalidate_global_aa.ll create mode 100644 llvm/test/Instrumentation/MemorySanitizer/msan_invalidate.ll diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp index 37c6989..7c2b07f 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -26,6 +26,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/ADT/Triple.h" #include "llvm/ADT/Twine.h" +#include "llvm/Analysis/GlobalsModRef.h" #include "llvm/Analysis/MemoryBuiltins.h" #include "llvm/Analysis/StackSafetyAnalysis.h" #include "llvm/Analysis/TargetLibraryInfo.h" @@ -1143,7 +1144,15 @@ PreservedAnalyses AddressSanitizerPass::run(Module &M, Modified |= FunctionSanitizer.instrumentFunction(F, &TLI); } Modified |= ModuleSanitizer.instrumentModule(M); - return Modified ? PreservedAnalyses::none() : PreservedAnalyses::all(); + if (!Modified) + return PreservedAnalyses::all(); + + PreservedAnalyses PA = PreservedAnalyses::none(); + // GlobalsAA is considered stateless and does not get invalidated unless + // explicitly invalidated; PreservedAnalyses::none() is not enough. Sanitizers + // make changes that require GlobalsAA to be invalidated. + PA.abandon(); + return PA; } static size_t TypeSizeToSizeIndex(uint32_t TypeSize) { diff --git a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp index b6f0a124..a82ba71 100644 --- a/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp @@ -70,6 +70,7 @@ #include "llvm/ADT/StringSet.h" #include "llvm/ADT/Triple.h" #include "llvm/ADT/iterator.h" +#include "llvm/Analysis/GlobalsModRef.h" #include "llvm/Analysis/TargetLibraryInfo.h" #include "llvm/Analysis/ValueTracking.h" #include "llvm/IR/Argument.h" @@ -3366,8 +3367,13 @@ ModulePass *llvm::createDataFlowSanitizerLegacyPassPass( PreservedAnalyses DataFlowSanitizerPass::run(Module &M, ModuleAnalysisManager &AM) { - if (DataFlowSanitizer(ABIListFiles).runImpl(M, &AM)) { - return PreservedAnalyses::none(); - } - return PreservedAnalyses::all(); + if (!DataFlowSanitizer(ABIListFiles).runImpl(M, &AM)) + return PreservedAnalyses::all(); + + PreservedAnalyses PA = PreservedAnalyses::none(); + // GlobalsAA is considered stateless and does not get invalidated unless + // explicitly invalidated; PreservedAnalyses::none() is not enough. Sanitizers + // make changes that require GlobalsAA to be invalidated. + PA.abandon(); + return PA; } diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp index 2ffa4ee..17c36b8 100644 --- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp @@ -18,6 +18,7 @@ #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/Triple.h" +#include "llvm/Analysis/GlobalsModRef.h" #include "llvm/Analysis/PostDominators.h" #include "llvm/Analysis/StackSafetyAnalysis.h" #include "llvm/Analysis/ValueTracking.h" @@ -423,9 +424,15 @@ PreservedAnalyses HWAddressSanitizerPass::run(Module &M, auto &FAM = MAM.getResult(M).getManager(); for (Function &F : M) Modified |= HWASan.sanitizeFunction(F, FAM); - if (Modified) - return PreservedAnalyses::none(); - return PreservedAnalyses::all(); + if (!Modified) + return PreservedAnalyses::all(); + + PreservedAnalyses PA = PreservedAnalyses::none(); + // GlobalsAA is considered stateless and does not get invalidated unless + // explicitly invalidated; PreservedAnalyses::none() is not enough. Sanitizers + // make changes that require GlobalsAA to be invalidated. + PA.abandon(); + return PA; } void HWAddressSanitizerPass::printPipeline( raw_ostream &OS, function_ref MapClassName2PassName) { diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp index 3dc7317..66ac1ec 100644 --- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp @@ -152,6 +152,7 @@ #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/Triple.h" +#include "llvm/Analysis/GlobalsModRef.h" #include "llvm/Analysis/TargetLibraryInfo.h" #include "llvm/Analysis/ValueTracking.h" #include "llvm/IR/Argument.h" @@ -685,7 +686,16 @@ PreservedAnalyses MemorySanitizerPass::run(Module &M, Modified |= Msan.sanitizeFunction(F, FAM.getResult(F)); } - return Modified ? PreservedAnalyses::none() : PreservedAnalyses::all(); + + if (!Modified) + return PreservedAnalyses::all(); + + PreservedAnalyses PA = PreservedAnalyses::none(); + // GlobalsAA is considered stateless and does not get invalidated unless + // explicitly invalidated; PreservedAnalyses::none() is not enough. Sanitizers + // make changes that require GlobalsAA to be invalidated. + PA.abandon(); + return PA; } void MemorySanitizerPass::printPipeline( diff --git a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp index 7d71745..2eed3d7 100644 --- a/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp +++ b/llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp @@ -15,6 +15,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/Triple.h" #include "llvm/Analysis/EHPersonalities.h" +#include "llvm/Analysis/GlobalsModRef.h" #include "llvm/Analysis/PostDominators.h" #include "llvm/IR/Constant.h" #include "llvm/IR/DataLayout.h" @@ -291,9 +292,15 @@ PreservedAnalyses SanitizerCoveragePass::run(Module &M, auto PDTCallback = [&FAM](Function &F) -> const PostDominatorTree * { return &FAM.getResult(F); }; - if (ModuleSancov.instrumentModule(M, DTCallback, PDTCallback)) - return PreservedAnalyses::none(); - return PreservedAnalyses::all(); + if (!ModuleSancov.instrumentModule(M, DTCallback, PDTCallback)) + return PreservedAnalyses::all(); + + PreservedAnalyses PA = PreservedAnalyses::none(); + // GlobalsAA is considered stateless and does not get invalidated unless + // explicitly invalidated; PreservedAnalyses::none() is not enough. Sanitizers + // make changes that require GlobalsAA to be invalidated. + PA.abandon(); + return PA; } std::pair diff --git a/llvm/test/Instrumentation/MemorySanitizer/invalidate_global_aa.ll b/llvm/test/Instrumentation/MemorySanitizer/invalidate_global_aa.ll new file mode 100644 index 0000000..9a8de65 --- /dev/null +++ b/llvm/test/Instrumentation/MemorySanitizer/invalidate_global_aa.ll @@ -0,0 +1,26 @@ +; Check that sanitizers invalidate GlobalsAA + +; Msan and Dfsan use globals for origin tracking and TLS for parameters. +; RUN: opt < %s -S -passes='require,module(msan)' -debug-pass-manager 2>&1 | FileCheck %s +; RUN: opt < %s -S -passes='require,module(dfsan)' -debug-pass-manager 2>&1 | FileCheck %s + +; Some types of coverage use globals. +; RUN: opt < %s -S -passes='require,module(sancov-module)' -sanitizer-coverage-level=2 -debug-pass-manager 2>&1 | FileCheck %s + +; Uses TLS for tags. +; RUN: opt < %s -S -passes='require,module(hwasan)' -debug-pass-manager 2>&1 | FileCheck %s + +; Modifies globals. +; RUN: opt < %s -S -passes='require,module(asan)' -debug-pass-manager 2>&1 | FileCheck %s + +; CHECK: Running analysis: GlobalsAA on [module] +; CHECK: Running pass: {{.*}}Sanitizer{{.*}}Pass on [module] +; CHECK: Invalidating analysis: GlobalsAA on [module] + +target triple = "x86_64-unknown-linux" + +define i32 @test(ptr readonly %a) local_unnamed_addr sanitize_address sanitize_hwaddress { +entry: + %0 = load i32, ptr %a, align 4 + ret i32 %0 +} diff --git a/llvm/test/Instrumentation/MemorySanitizer/msan_invalidate.ll b/llvm/test/Instrumentation/MemorySanitizer/msan_invalidate.ll new file mode 100644 index 0000000..ae928fe --- /dev/null +++ b/llvm/test/Instrumentation/MemorySanitizer/msan_invalidate.ll @@ -0,0 +1,30 @@ +; Regression test for msan not invalidating GlobalsAA. +; RUN: opt < %s -S -passes='require,module(msan),require,early-cse' 2>&1 | FileCheck %s + +target triple = "x86_64-unknown-linux" + +define ptr @foo(ptr %p) local_unnamed_addr sanitize_memory { +entry: + ret ptr %p +} + +define i32 @test() local_unnamed_addr sanitize_memory { +entry: + ; CHECK-LABEL: define i32 @test() + + %x = alloca i32 + store i32 7, ptr %x + + ; CHECK: store i64 0, ptr @__msan_retval_tls + ; CHECK-NEXT: call ptr @foo( + + %call = call ptr @foo(ptr %x) + + ; If GlobalsAA is eliminated correctly, early-cse should not remove next load. + ; CHECK-NEXT: %[[MSRET:.*]] = load i64, ptr @__msan_retval_tls + ; CHECK-NEXT: %[[MSCMP:.*]] = icmp ne i64 %[[MSRET]], 0 + ; CHECK-NEXT: br i1 %[[MSCMP]], + + %ret = load i32, ptr %call + ret i32 %ret +} -- 2.7.4