Index: llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp =================================================================== --- llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -833,6 +833,76 @@ 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)) + 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 &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; @@ -2110,6 +2180,8 @@ return; } + removeAllocasWithAmbiguousLifetime(DynamicAllocaPoisonCallVec); + // Insert poison calls for lifetime intrinsics for dynamic allocas. for (const auto &APC : DynamicAllocaPoisonCallVec) { assert(APC.InsBefore); @@ -2137,6 +2209,8 @@ return; } + removeAllocasWithAmbiguousLifetime(StaticAllocaPoisonCallVec); + int StackMallocIdx = -1; DebugLoc EntryDebugLocation; if (auto SP = F.getSubprogram()) Index: llvm/trunk/test/Instrumentation/AddressSanitizer/lifetime.ll =================================================================== --- llvm/trunk/test/Instrumentation/AddressSanitizer/lifetime.ll +++ llvm/trunk/test/Instrumentation/AddressSanitizer/lifetime.ll @@ -97,6 +97,47 @@ 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)