From 6ae05777b8c331b793a40c7af24990be16b34efd Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Tue, 16 Apr 2019 07:54:20 +0000 Subject: [PATCH] Asan use-after-scope: don't poison allocas if there were untraced lifetime intrinsics in the function (PR41481) If there are any intrinsics that cannot be traced back to an alloca, we might have missed the start of a variable's scope, leading to false error reports if the variable is poisoned at function entry. Instead, if there are some intrinsics that can't be traced, fail safe and don't poison the variables in that function. Differential revision: https://reviews.llvm.org/D60686 llvm-svn: 358478 --- .../Instrumentation/AddressSanitizer.cpp | 15 +++++++- .../stack-poisoning-and-lifetime.ll | 36 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp index 4c827aca20ec..d73907c5aca9 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -884,6 +884,7 @@ struct FunctionStackPoisoner : public InstVisitor { }; SmallVector DynamicAllocaPoisonCallVec; SmallVector StaticAllocaPoisonCallVec; + bool HasUntracedLifetimeIntrinsic = false; SmallVector DynamicAllocaVec; SmallVector StackRestoreVec; @@ -918,6 +919,14 @@ struct FunctionStackPoisoner : public InstVisitor { initializeCallbacks(*F.getParent()); + if (HasUntracedLifetimeIntrinsic) { + // If there are lifetime intrinsics which couldn't be traced back to an + // alloca, we may not know exactly when a variable enters scope, and + // therefore should "fail safe" by not poisoning them. + StaticAllocaPoisonCallVec.clear(); + DynamicAllocaPoisonCallVec.clear(); + } + processDynamicAllocas(); processStaticAllocas(); @@ -1040,8 +1049,12 @@ struct FunctionStackPoisoner : public InstVisitor { // Find alloca instruction that corresponds to llvm.lifetime argument. AllocaInst *AI = llvm::findAllocaForValue(II.getArgOperand(1), AllocaForValue); + if (!AI) { + HasUntracedLifetimeIntrinsic = true; + return; + } // We're interested only in allocas we can handle. - if (!AI || !ASan.isInterestingAlloca(*AI)) + if (!ASan.isInterestingAlloca(*AI)) return; bool DoPoison = (ID == Intrinsic::lifetime_end); AllocaPoisonCall APC = {&II, AI, SizeValue, DoPoison}; diff --git a/llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll b/llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll index 56a63110a84c..5523da63d373 100644 --- a/llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll +++ b/llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll @@ -209,6 +209,42 @@ entry: ; CHECK: ret void } +declare void @foo(i32*) +define void @PR41481(i1 %b) sanitize_address { +; CHECK-LABEL: @PR41481 +entry: + %p1 = alloca i32 + %p2 = alloca i32 + %q1 = bitcast i32* %p1 to i8* + %q2 = bitcast i32* %p2 to i8* + br label %bb1 + + ; Since we cannot account for all lifetime intrinsics in this function, we + ; might have missed a lifetime.start one and therefore shouldn't poison the + ; allocas at function entry. + ; ENTRY: store i64 -935356719533264399 + ; ENTRY-UAS: store i64 -935356719533264399 + +bb1: + %p = select i1 %b, i32* %p1, i32* %p2 + %q = select i1 %b, i8* %q1, i8* %q2 + call void @llvm.lifetime.start.p0i8(i64 4, i8* %q) + call void @foo(i32* %p) + br i1 %b, label %bb2, label %bb3 + +bb2: + call void @llvm.lifetime.end.p0i8(i64 4, i8* %q1) + br label %end + +bb3: + call void @llvm.lifetime.end.p0i8(i64 4, i8* %q2) + br label %end + +end: + ret void +} + + declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture) declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture)