[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

diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 37c6989..7c2b07f 100644 (file)
--- 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<GlobalsAA>();
+  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 b6f0a12..a82ba71 100644 (file)
--- 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<GlobalsAA>();
+  return PA;
 }

diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 2ffa4ee..17c36b8 100644 (file)
--- 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<FunctionAnalysisManagerModuleProxy>(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<GlobalsAA>();
+  return PA;
 }
 void HWAddressSanitizerPass::printPipeline(
     raw_ostream &OS, function_ref<StringRef(StringRef)> MapClassName2PassName) {

diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 3dc7317..66ac1ec 100644 (file)
--- 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<TargetLibraryAnalysis>(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<GlobalsAA>();
+  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 (file)
--- 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<PostDominatorTreeAnalysis>(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<GlobalsAA>();
+  return PA;
 }
 
 std::pair<Value *, Value *>

diff --git a/llvm/test/Instrumentation/MemorySanitizer/invalidate_global_aa.ll b/llvm/test/Instrumentation/MemorySanitizer/invalidate_global_aa.ll
new file mode 100644 (file)
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<globals-aa>,module(msan)' -debug-pass-manager 2>&1 | FileCheck %s
+; RUN: opt < %s -S -passes='require<globals-aa>,module(dfsan)' -debug-pass-manager 2>&1 | FileCheck %s
+
+; Some types of coverage use globals.
+; RUN: opt < %s -S -passes='require<globals-aa>,module(sancov-module)' -sanitizer-coverage-level=2 -debug-pass-manager 2>&1 | FileCheck %s
+
+; Uses TLS for tags.
+; RUN: opt < %s -S -passes='require<globals-aa>,module(hwasan)' -debug-pass-manager 2>&1 | FileCheck %s
+
+; Modifies globals.
+; RUN: opt < %s -S -passes='require<globals-aa>,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 (file)
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<globals-aa>,module(msan),require<globals-aa>,early-cse<memssa>' 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
+}