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

[Vitaly Buka via llvm-commits]

Author: vitalybuka
Date: Wed Sep  7 18:37:15 2016
New Revision: 280883

URL: http://llvm.org/viewvc/llvm-project?rev=280883&view=rev
Log:
Revert "[asan] Avoid lifetime analysis for allocas with can be in ambiguous state"

Fails on Windows.

This reverts commit r280880.

Modified:
    llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
    llvm/trunk/test/Instrumentation/AddressSanitizer/lifetime.ll

Modified: llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp?rev=280883&r1=280882&r2=280883&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp (original)
+++ llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp Wed Sep  7 18:37:15 2016
@@ -833,76 +833,6 @@ struct FunctionStackPoisoner : public In
                      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))
-      if (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;
@@ -2180,8 +2110,6 @@ void FunctionStackPoisoner::processDynam
     return;
   }
 
-  removeAllocasWithAmbiguousLifetime(DynamicAllocaPoisonCallVec);
-
   // Insert poison calls for lifetime intrinsics for dynamic allocas.
   for (const auto &APC : DynamicAllocaPoisonCallVec) {
     assert(APC.InsBefore);
@@ -2209,8 +2137,6 @@ void FunctionStackPoisoner::processStati
     return;
   }
 
-  removeAllocasWithAmbiguousLifetime(StaticAllocaPoisonCallVec);
-
   int StackMallocIdx = -1;
   DebugLoc EntryDebugLocation;
   if (auto SP = F.getSubprogram())

Modified: llvm/trunk/test/Instrumentation/AddressSanitizer/lifetime.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/AddressSanitizer/lifetime.ll?rev=280883&r1=280882&r2=280883&view=diff
==============================================================================
--- llvm/trunk/test/Instrumentation/AddressSanitizer/lifetime.ll (original)
+++ llvm/trunk/test/Instrumentation/AddressSanitizer/lifetime.ll Wed Sep  7 18:37:15 2016
@@ -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)