This is an archive of the discontinued LLVM Phabricator instance.

Differential D22661

Unpoison stack before resume instruction
ClosedPublic

Authored by vitalybuka on Jul 21 2016, 7:47 PM.

Details

Reviewers
kcc
eugenis
Commits
rGe3a032a7408c: Unpoison stack before resume instruction
rL276480: Unpoison stack before resume instruction
Summary

Clang inserts cleanup code before resume similar way as before return instruction.
This makes asan poison local variables causing false use-after-scope reports.

__asan_handle_no_return does not help here as it was executed before
llvm.lifetime.end inserted into resume block.

To avoid false report we need to unpoison stack for resume same way as for return.

cleanupret similar to resume but it's used only on windows.

PR27453

Diff Detail

Repository
rL LLVM

Event Timeline

vitalybuka updated this revision to Diff 65015.Jul 21 2016, 7:47 PM
vitalybuka retitled this revision from to Unpoison stack before resume instruction.
vitalybuka updated this object.
vitalybuka added reviewers: kcc, eugenis.
vitalybuka updated this revision to Diff 65127.Jul 22 2016, 1:09 PM

Rename

eugenis edited edge metadata.Jul 22 2016, 1:21 PM

I'm not exactly up-to-date on the exception handling IR. I see instructions like "catchret" and "cleanupret" in the LangRef document. Do we need to handle them as well?

eugenis added inline comments.Jul 22 2016, 1:38 PM
lib/Transforms/Instrumentation/AddressSanitizer.cpp
1796 ↗(On Diff #65127)

Why do you rename this?
I kind of like the current name more.

test/Instrumentation/AddressSanitizer/lifetime-throw.ll
23 ↗(On Diff #65127)

"#3" it is not defined anywhere.
How does this work?

vitalybuka updated this revision to Diff 65163.Jul 22 2016, 2:44 PM
vitalybuka marked an inline comment as done.
vitalybuka edited edge metadata.

Handle cleanupret

vitalybuka updated this object.Jul 22 2016, 2:45 PM
vitalybuka added inline comments.
test/Instrumentation/AddressSanitizer/lifetime-throw.ll
23 ↗(On Diff #65127)

It just ignores undefined attributes, I see the same in other tests.

eugenis accepted this revision.Jul 22 2016, 2:52 PM
eugenis edited edge metadata.

LGTM
I'm a bit unhappy about the tests effectively counting the number of __asan_poison_stack_memory calls, and not verifying their placement. This is probably good enough, but a few extra checks would not hurt.

test/Instrumentation/AddressSanitizer/lifetime-throw-win.ll
1 ↗(On Diff #65163)

typo: handling here and below

This revision is now accepted and ready to land.Jul 22 2016, 2:52 PM
vitalybuka updated this revision to Diff 65171.Jul 22 2016, 3:07 PM
vitalybuka edited edge metadata.

Make Evgeniy a bit happier

Closed by commit rL276480: Unpoison stack before resume instruction (authored by vitalybuka). · Explain WhyJul 22 2016, 3:12 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
trunk/
lib/
Transforms/
Instrumentation/
6 lines
test/
Instrumentation/
AddressSanitizer/
106 lines
2 lines

Diff 65175

llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp

Show First 20 Lines • Show All 670 Lines • ▼ Show 20 Linesstruct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {
void poisonStack(); void poisonStack();
void createDynamicAllocasInitStorage(); void createDynamicAllocasInitStorage();
// ----------------------- Visitors. // ----------------------- Visitors.
/// \brief Collect all Ret instructions. /// \brief Collect all Ret instructions.
void visitReturnInst(ReturnInst &RI) { RetVec.push_back(&RI); } void visitReturnInst(ReturnInst &RI) { RetVec.push_back(&RI); }
/// \brief Collect all Resume instructions.
void visitResumeInst(ResumeInst &RI) { RetVec.push_back(&RI); }
/// \brief Collect all CatchReturnInst instructions.
void visitCleanupReturnInst(CleanupReturnInst &CRI) { RetVec.push_back(&CRI); }
void unpoisonDynamicAllocasBeforeInst(Instruction *InstBefore, void unpoisonDynamicAllocasBeforeInst(Instruction *InstBefore,
Value *SavedStack) { Value *SavedStack) {
IRBuilder<> IRB(InstBefore); IRBuilder<> IRB(InstBefore);
Value *DynamicAreaPtr = IRB.CreatePtrToInt(SavedStack, IntptrTy); Value *DynamicAreaPtr = IRB.CreatePtrToInt(SavedStack, IntptrTy);
// When we insert _asan_allocas_unpoison before @llvm.stackrestore, we // When we insert _asan_allocas_unpoison before @llvm.stackrestore, we
// need to adjust extracted SP to compute the address of the most recent // need to adjust extracted SP to compute the address of the most recent
// alloca. We have a special @llvm.get.dynamic.area.offset intrinsic for // alloca. We have a special @llvm.get.dynamic.area.offset intrinsic for
// this purpose. // this purpose.
▲ Show 20 Lines • Show All 1,689 Lines • Show Last 20 Lines

llvm/trunk/test/Instrumentation/AddressSanitizer/lifetime-throw.ll

; Test handling of llvm.lifetime intrinsics with C++ exceptions.
; RUN: opt < %s -asan -asan-module -asan-use-after-scope -asan-use-after-return=0 -S | FileCheck %s
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
%struct.ABC = type { i32 }
$_ZN3ABCD2Ev = comdat any
$_ZTS3ABC = comdat any
$_ZTI3ABC = comdat any
@_ZTVN10__cxxabiv117__class_type_infoE = external global i8*
@_ZTS3ABC = linkonce_odr constant [5 x i8] c"3ABC\00", comdat
@_ZTI3ABC = linkonce_odr constant { i8*, i8* } { i8* bitcast (i8** getelementptr inbounds (i8*, i8** @_ZTVN10__cxxabiv117__class_type_infoE, i64 2) to i8*), i8* getelementptr inbounds ([5 x i8], [5 x i8]* @_ZTS3ABC, i32 0, i32 0) }, comdat
define void @Throw() sanitize_address personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
; CHECK-LABEL: define void @Throw()
entry:
%x = alloca %struct.ABC, align 4
%0 = bitcast %struct.ABC* %x to i8*
call void @llvm.lifetime.start(i64 4, i8* %0)
; CHECK: call void @__asan_unpoison_stack_memory
; CHECK-NEXT: @llvm.lifetime.start
%exception = call i8* @__cxa_allocate_exception(i64 4)
invoke void @__cxa_throw(i8* %exception, i8* bitcast ({ i8*, i8* }* @_ZTI3ABC to i8*), i8* bitcast (void (%struct.ABC*)* @_ZN3ABCD2Ev to i8*)) noreturn
to label %unreachable unwind label %lpad
; CHECK: call void @__asan_handle_no_return
; CHECK-NEXT: @__cxa_throw
lpad:
%1 = landingpad { i8*, i32 }
cleanup
call void @_ZN3ABCD2Ev(%struct.ABC* nonnull %x)
call void @llvm.lifetime.end(i64 4, i8* %0)
; CHECK: call void @__asan_poison_stack_memory
; CHECK-NEXT: @llvm.lifetime.end
resume { i8*, i32 } %1
; CHECK: call void @__asan_unpoison_stack_memory
; CHECK-NEXT: resume
unreachable:
unreachable
}
%rtti.TypeDescriptor9 = type { i8**, i8*, [10 x i8] }
%eh.CatchableType = type { i32, i32, i32, i32, i32, i32, i32 }
%eh.CatchableTypeArray.1 = type { i32, [1 x i32] }
%eh.ThrowInfo = type { i32, i32, i32, i32 }
$"\01??1ABC@@QEAA@XZ" = comdat any
$"\01??_R0?AUABC@@@8" = comdat any
$"_CT??_R0?AUABC@@@84" = comdat any
$"_CTA1?AUABC@@" = comdat any
$"_TI1?AUABC@@" = comdat any
@"\01??_7type_info@@6B@" = external constant i8*
@"\01??_R0?AUABC@@@8" = linkonce_odr global %rtti.TypeDescriptor9 { i8** @"\01??_7type_info@@6B@", i8* null, [10 x i8] c".?AUABC@@\00" }, comdat
@__ImageBase = external constant i8
@"_CT??_R0?AUABC@@@84" = linkonce_odr unnamed_addr constant %eh.CatchableType { i32 0, i32 trunc (i64 sub nuw nsw (i64 ptrtoint (%rtti.TypeDescriptor9* @"\01??_R0?AUABC@@@8" to i64), i64 ptrtoint (i8* @__ImageBase to i64)) to i32), i32 0, i32 -1, i32 0, i32 4, i32 0 }, section ".xdata", comdat
@"_CTA1?AUABC@@" = linkonce_odr unnamed_addr constant %eh.CatchableTypeArray.1 { i32 1, [1 x i32] [i32 trunc (i64 sub nuw nsw (i64 ptrtoint (%eh.CatchableType* @"_CT??_R0?AUABC@@@84" to i64), i64 ptrtoint (i8* @__ImageBase to i64)) to i32)] }, section ".xdata", comdat
@"_TI1?AUABC@@" = linkonce_odr unnamed_addr constant %eh.ThrowInfo { i32 0, i32 trunc (i64 sub nuw nsw (i64 ptrtoint (void (%struct.ABC*)* @"\01??1ABC@@QEAA@XZ" to i64), i64 ptrtoint (i8* @__ImageBase to i64)) to i32), i32 0, i32 trunc (i64 sub nuw nsw (i64 ptrtoint (%eh.CatchableTypeArray.1* @"_CTA1?AUABC@@" to i64), i64 ptrtoint (i8* @__ImageBase to i64)) to i32) }, section ".xdata", comdat
define void @ThrowWin() sanitize_address personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
; CHECK-LABEL: define void @ThrowWin()
entry:
%x = alloca %struct.ABC, align 4
%tmp = alloca %struct.ABC, align 4
%0 = bitcast %struct.ABC* %x to i8*
call void @llvm.lifetime.start(i64 4, i8* %0)
; CHECK: call void @__asan_unpoison_stack_memory
; CHECK-NEXT: @llvm.lifetime.start
%1 = bitcast %struct.ABC* %tmp to i8*
invoke void @_CxxThrowException(i8* %1, %eh.ThrowInfo* nonnull @"_TI1?AUABC@@") noreturn
to label %unreachable unwind label %ehcleanup
; CHECK: call void @__asan_handle_no_return
; CHECK-NEXT: @_CxxThrowException
ehcleanup:
%2 = cleanuppad within none []
call void @"\01??1ABC@@QEAA@XZ"(%struct.ABC* nonnull %x) [ "funclet"(token %2) ]
call void @llvm.lifetime.end(i64 4, i8* %0)
; CHECK: call void @__asan_poison_stack_memory
; CHECK-NEXT: @llvm.lifetime.end
cleanupret from %2 unwind to caller
; CHECK: call void @__asan_unpoison_stack_memory
; CHECK-NEXT: cleanupret
unreachable:
unreachable
}
declare i32 @__gxx_personality_v0(...)
declare void @llvm.lifetime.start(i64, i8* nocapture)
declare void @llvm.lifetime.end(i64, i8* nocapture)
declare void @__cxa_throw(i8*, i8*, i8*) local_unnamed_addr
declare i8* @__cxa_allocate_exception(i64) local_unnamed_addr
declare void @_ZN3ABCD2Ev(%struct.ABC* %this) unnamed_addr
declare void @"\01??1ABC@@QEAA@XZ"(%struct.ABC* %this)
declare void @_CxxThrowException(i8*, %eh.ThrowInfo*)
declare i32 @__CxxFrameHandler3(...)

llvm/trunk/test/Instrumentation/AddressSanitizer/lifetime.ll

; Test hanlding of llvm.lifetime intrinsics. ; 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
; RUN: opt < %s -asan -asan-module -asan-use-after-scope -asan-use-after-return=0 -asan-instrument-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-allocas=0 -S | FileCheck %s --check-prefix=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 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" target triple = "x86_64-unknown-linux-gnu"
declare void @llvm.lifetime.start(i64, i8* nocapture) nounwind declare void @llvm.lifetime.start(i64, i8* nocapture) nounwind
declare void @llvm.lifetime.end(i64, i8* nocapture) nounwind declare void @llvm.lifetime.end(i64, i8* nocapture) nounwind
▲ Show 20 Lines • Show All 120 Lines • Show Last 20 Lines