Revert "[asan] Avoid lifetime analysis for allocas with can be in ambiguous state"

This approach is not good enough. Working on the new solution.

This reverts commit r280907.

llvm-svn: 281689

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

@@ -833,77 +833,6 @@ struct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {
                      Instruction *ThenTerm, Value *ValueIfFalse);
 };
 
-// Performs depth-first search on the control flow graph of block and checks if
-// we can get into the same block with different lifetime state.
-class AllocaLifetimeChecker {
-  // Contains values of the last lifetime intrinsics in the block.
-  // true: llvm.lifetime.start, false: llvm.lifetime.end
-  DenseMap<const BasicBlock *, bool> Markers;
-  // Contains the lifetime state we detected doing depth-first search on the
-  // control flow graph. We expect all future hits will have the same value.
-  // true: llvm.lifetime.start, false: llvm.lifetime.end
-  DenseMap<const BasicBlock *, bool> InboundState;
-  bool Processed = false;
-  bool CollisionDetected = false;
-
-  bool FindCollision(const std::pair<const BasicBlock *, bool> &BlockState) {
-    auto Ins = InboundState.insert(BlockState);
-    if (!Ins.second) {
-      // Already there. Return collision if they are different.
-      return BlockState.second != Ins.first->second;
-    }
-
-    // Use marker for successors if block contains any.
-    auto M = Markers.find(BlockState.first);
-    bool NewState = (M != Markers.end() ? M->second : BlockState.second);
-    for (const BasicBlock *SB : successors(BlockState.first))
-      // We may get into EHPad with any lifetime state, so ignore them.
-      if (!SB->isEHPad() && FindCollision({SB, NewState}))
-        return true;
-
-    return false;
-  }
-
-public:
-  // Assume that markers of the same block will be added in the same order as
-  // the order of corresponding intrinsics, so in the end we will keep only
-  // value of the last intrinsic.
-  void AddMarker(const BasicBlock *BB, bool start) {
-    assert(!Processed);
-    Markers[BB] = start;
-  }
-
-  bool HasAmbiguousLifetime() {
-    if (!Processed) {
-      Processed = true;
-      const Function *F = Markers.begin()->first->getParent();
-      CollisionDetected = FindCollision({&F->getEntryBlock(), false});
-    }
-    return CollisionDetected;
-  }
-};
-
-// Removes allocas for which exists at least one block simultaneously
-// reachable in both states: allocas is inside the scope, and alloca is outside
-// of the scope. We don't have enough information to validate access to such
-// variable, so we just remove such allocas from lifetime analysis.
-// This is workaround for PR28267.
-void removeAllocasWithAmbiguousLifetime(
-    SmallVectorImpl<FunctionStackPoisoner::AllocaPoisonCall> &PoisonCallVec) {
-  DenseMap<const AllocaInst *, AllocaLifetimeChecker> Checkers;
-  for (const auto &APC : PoisonCallVec)
-    Checkers[APC.AI].AddMarker(APC.InsBefore->getParent(), !APC.DoPoison);
-
-  auto IsAmbiguous =
-      [&Checkers](const FunctionStackPoisoner::AllocaPoisonCall &APC) {
-        return Checkers[APC.AI].HasAmbiguousLifetime();
-      };
-
-  PoisonCallVec.erase(
-      std::remove_if(PoisonCallVec.begin(), PoisonCallVec.end(), IsAmbiguous),
-      PoisonCallVec.end());
-}
-
 } // anonymous namespace
 
 char AddressSanitizer::ID = 0;
@@ -2191,8 +2120,6 @@ void FunctionStackPoisoner::processDynamicAllocas() {
     return;
   }
 
-  removeAllocasWithAmbiguousLifetime(DynamicAllocaPoisonCallVec);
-
   // Insert poison calls for lifetime intrinsics for dynamic allocas.
   for (const auto &APC : DynamicAllocaPoisonCallVec) {
     assert(APC.InsBefore);
@@ -2220,8 +2147,6 @@ void FunctionStackPoisoner::processStaticAllocas() {
     return;
   }
 
-  removeAllocasWithAmbiguousLifetime(StaticAllocaPoisonCallVec);
-
   int StackMallocIdx = -1;
   DebugLoc EntryDebugLocation;
   if (auto SP = F.getSubprogram())

llvm/test/Instrumentation/AddressSanitizer/lifetime.ll

@@ -1,6 +1,6 @@
 ; Test handling of llvm.lifetime intrinsics.
-; RUN: opt < %s -asan -asan-module -asan-use-after-scope -asan-use-after-return=0 -S | FileCheck %s
+; RUN: opt < %s -asan -asan-module -asan-use-after-scope -asan-use-after-return=0 -S | FileCheck %s --check-prefixes=CHECK,CHECK-DEFAULT
-; RUN: opt < %s -asan -asan-module -asan-use-after-scope -asan-use-after-return=0 -asan-instrument-dynamic-allocas=0 -S | FileCheck %s --check-prefix=CHECK-NO-DYNAMIC
+; RUN: opt < %s -asan -asan-module -asan-use-after-scope -asan-use-after-return=0 -asan-instrument-dynamic-allocas=0 -S | FileCheck %s --check-prefixes=CHECK,CHECK-NO-DYNAMIC
 
 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"
@@ -69,14 +69,14 @@ define void @lifetime() sanitize_address {
   %arr.ptr = bitcast [10 x i32]* %arr to i8*
 
   call void @llvm.lifetime.start(i64 40, i8* %arr.ptr)
-  ; CHECK: call void @__asan_unpoison_stack_memory(i64 %{{[^ ]+}}, i64 40)
+  ; CHECK-DEFAULT: call void @__asan_unpoison_stack_memory(i64 %{{[^ ]+}}, i64 40)
   ; CHECK-NO-DYNAMIC-NOT: call void @__asan_unpoison_stack_memory(i64 %{{[^ ]+}}, i64 40)
 
   store volatile i8 0, i8* %arr.ptr
   ; CHECK: store volatile
 
   call void @llvm.lifetime.end(i64 40, i8* %arr.ptr)
-  ; CHECK: call void @__asan_poison_stack_memory(i64 %{{[^ ]+}}, i64 40)
+  ; CHECK-DEFAULT: call void @__asan_poison_stack_memory(i64 %{{[^ ]+}}, i64 40)
   ; CHECK-NO-DYNAMIC-NOT: call void @__asan_poison_stack_memory(i64 %{{[^ ]+}}, i64 40)
 
   ; One more lifetime start/end for the same variable %i.
@@ -97,47 +97,6 @@ define void @lifetime() sanitize_address {
   ret void
 }
 
-; Case of lifetime depends on how we get into the block.
-define void @ambiguous_lifetime(i1 %x) sanitize_address {
-  ; CHECK-LABEL: define void @ambiguous_lifetime
-
-entry:
-  ; Regular variable lifetime intrinsics.
-  %i = alloca i8, align 4  ; Good
-  %j = alloca i8, align 4  ; Bad
-
-  call void @llvm.lifetime.start(i64 1, i8* %i)
-  ; CHECK: store i8 1, i8* %{{[0-9]+}}
-  ; CHECK-NEXT: call void @llvm.lifetime.start
-
-  br i1 %x, label %bb0, label %bb1
-
-bb0:
-  ; CHECK-LABEL: bb0:
-
-  call void @llvm.lifetime.start(i64 1, i8* %j)
-  ; CHECK-NOT: store i8 1, i8* %{{[0-9]+}}
-  ; CHECK-NEXT: call void @llvm.lifetime.start
-
-  br label %bb1
-
-bb1:
-  ; CHECK-LABEL: bb1:
-
-  store volatile i8 0, i8* %i
-  store volatile i8 0, i8* %j
-
-  call void @llvm.lifetime.end(i64 1, i8* %i)
-  ; CHECK: store i8 -8, i8* %{{[0-9]+}}
-  ; CHECK-NEXT: call void @llvm.lifetime.end
-
-  call void @llvm.lifetime.end(i64 1, i8* %j)
-  ; CHECK-NOT: store i8 -8, i8* %{{[0-9]+}}
-  ; CHECK-NEXT: call void @llvm.lifetime.end
-
-  ret void
-}
-
 ; Check that arguments of lifetime may come from phi nodes.
 define void @phi_args(i1 %x) sanitize_address {
   ; CHECK-LABEL: define void @phi_args(i1 %x)