From 6c7a0bc3d964d4e0840f82db692e3b88c9eb1f33 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Fri, 16 Sep 2016 01:38:46 +0000 Subject: [PATCH] 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 --- .../Instrumentation/AddressSanitizer.cpp | 75 ------------------- .../AddressSanitizer/lifetime.ll | 49 +----------- 2 files changed, 4 insertions(+), 120 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp index 729dc7c55671..aef47aa37411 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -833,77 +833,6 @@ struct FunctionStackPoisoner : public InstVisitor { 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 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 InboundState; - bool Processed = false; - bool CollisionDetected = false; - - bool FindCollision(const std::pair &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 &PoisonCallVec) { - DenseMap 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()) diff --git a/llvm/test/Instrumentation/AddressSanitizer/lifetime.ll b/llvm/test/Instrumentation/AddressSanitizer/lifetime.ll index 9cf019aa6ccf..be72124f3ab6 100644 --- a/llvm/test/Instrumentation/AddressSanitizer/lifetime.ll +++ b/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 -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 -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-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)