diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp --- 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 @@ 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 --- 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 @@ 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 --- 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 @@ 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 --- 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 @@ 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 --- 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 @@ 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 --- /dev/null +++ b/llvm/test/Instrumentation/MemorySanitizer/invalidate_global_aa.ll @@ -0,0 +1,30 @@ +; 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 datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define dso_local noundef ptr @_Z10max_by_ptrPiS_(ptr noundef readonly %a, ptr noundef readonly %b) local_unnamed_addr sanitize_address sanitize_hwaddress { +entry: + %0 = load i32, ptr %a, align 4 + %1 = load i32, ptr %b, align 4 + %cmp = icmp slt i32 %0, %1 + %cond = select i1 %cmp, ptr %b, ptr %a + ret ptr %cond +} diff --git a/llvm/test/Instrumentation/MemorySanitizer/msan_invalidate.ll b/llvm/test/Instrumentation/MemorySanitizer/msan_invalidate.ll new file mode 100644 --- /dev/null +++ b/llvm/test/Instrumentation/MemorySanitizer/msan_invalidate.ll @@ -0,0 +1,45 @@ +; Regression test for msan not invalidating GlobalsAA. +; RUN: opt < %s -S -passes='require,module(msan),require,early-cse' 2>&1 | FileCheck %s + +target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +define dso_local noundef ptr @_Z10max_by_ptrPiS_(ptr noundef readonly %a, ptr noundef readonly %b) local_unnamed_addr sanitize_memory { +entry: + %0 = load i32, ptr %a, align 4 + %1 = load i32, ptr %b, align 4 + %cmp = icmp slt i32 %0, %1 + %cond = select i1 %cmp, ptr %b, ptr %a + ret ptr %cond +} + +define dso_local noundef i32 @main() local_unnamed_addr sanitize_memory { +entry: + ; CHECK-LABEL: define dso_local noundef i32 @main() + ; CHECK: store i64 0, ptr @__msan_retval_tls, align 8 + ; CHECK-NEXT: call noundef ptr @_Z10max_by_ptrPiS_(ptr noundef %px.0.px.0.px.0., ptr noundef nonnull %y) + ; If GlobalsAA is eliminated correctly, early-cse should not remove next load. + ; CHECK-NEXT: %[[MSRET:.*]] = load i64, ptr @__msan_retval_tls, align 8 + ; CHECK-NEXT: %[[MSCMP:.*]] = icmp ne i64 %[[MSRET]], 0 + ; CHECK-NEXT: br i1 %[[MSCMP]], + + %x = alloca i32, align 4 + %px = alloca ptr, align 8 + %y = alloca i32, align 4 + call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %x) + call void @llvm.lifetime.start.p0(i64 8, ptr nonnull %px) + store volatile ptr %x, ptr %px, align 8 + call void @llvm.lifetime.start.p0(i64 4, ptr nonnull %y) + store i32 43, ptr %y, align 4 + %px.0.px.0.px.0. = load volatile ptr, ptr %px, align 8 + %call = call noundef ptr @_Z10max_by_ptrPiS_(ptr noundef %px.0.px.0.px.0., ptr noundef nonnull %y) + %0 = load i32, ptr %call, align 4 + call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %y) + call void @llvm.lifetime.end.p0(i64 8, ptr nonnull %px) + call void @llvm.lifetime.end.p0(i64 4, ptr nonnull %x) + ret i32 %0 +} + +declare void @llvm.lifetime.start.p0(i64 immarg, ptr nocapture) + +declare void @llvm.lifetime.end.p0(i64 immarg, ptr nocapture)