From: Richard Smith Date: Sat, 4 Aug 2018 01:25:06 +0000 (+0000) Subject: Avoid creating conditional cleanup blocks that contain only @llvm.lifetime.end calls X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6fc4075ad41865ef29f98ccb419044355745d342;p=clang Avoid creating conditional cleanup blocks that contain only @llvm.lifetime.end calls When a non-extended temporary object is created in a conditional branch, the lifetime of that temporary ends outside the conditional (at the end of the full-expression). If we're inserting lifetime markers, this means we could end up generating if (some_cond) { lifetime.start(&tmp); Tmp::Tmp(&tmp); } // ... if (some_cond) { lifetime.end(&tmp); } ... for a full-expression containing a subexpression of the form `some_cond ? Tmp().x : 0`. This patch moves the lifetime start for such a temporary out of the conditional branch so that we don't need to generate an additional basic block to hold the lifetime end marker. This is disabled if we want precise lifetime markers (for asan's stack-use-after-scope checks) or of the temporary has a non-trivial destructor (in which case we'd generate an extra basic block anyway to hold the destructor call). Differential Revision: https://reviews.llvm.org/D50286 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@338945 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp index f168dd02ea..c8ba352e23 100644 --- a/lib/CodeGen/CGExpr.cpp +++ b/lib/CodeGen/CGExpr.cpp @@ -498,18 +498,51 @@ EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *M) { } else { switch (M->getStorageDuration()) { case SD_Automatic: - case SD_FullExpression: if (auto *Size = EmitLifetimeStart( CGM.getDataLayout().getTypeAllocSize(Alloca.getElementType()), Alloca.getPointer())) { - if (M->getStorageDuration() == SD_Automatic) - pushCleanupAfterFullExpr(NormalEHLifetimeMarker, - Alloca, Size); - else - pushFullExprCleanup(NormalEHLifetimeMarker, Alloca, - Size); + pushCleanupAfterFullExpr(NormalEHLifetimeMarker, + Alloca, Size); } break; + + case SD_FullExpression: { + if (!ShouldEmitLifetimeMarkers) + break; + + // Avoid creating a conditional cleanup just to hold an llvm.lifetime.end + // marker. Instead, start the lifetime of a conditional temporary earlier + // so that it's unconditional. Don't do this in ASan's use-after-scope + // mode so that it gets the more precise lifetime marks. If the type has + // a non-trivial destructor, we'll have a cleanup block for it anyway, + // so this typically doesn't help; skip it in that case. + ConditionalEvaluation *OldConditional = nullptr; + CGBuilderTy::InsertPoint OldIP; + if (isInConditionalBranch() && !E->getType().isDestructedType() && + !CGM.getCodeGenOpts().SanitizeAddressUseAfterScope) { + OldConditional = OutermostConditional; + OutermostConditional = nullptr; + + OldIP = Builder.saveIP(); + llvm::BasicBlock *Block = OldConditional->getStartingBlock(); + Builder.restoreIP(CGBuilderTy::InsertPoint( + Block, llvm::BasicBlock::iterator(Block->back()))); + } + + if (auto *Size = EmitLifetimeStart( + CGM.getDataLayout().getTypeAllocSize(Alloca.getElementType()), + Alloca.getPointer())) { + pushFullExprCleanup(NormalEHLifetimeMarker, Alloca, + Size); + } + + if (OldConditional) { + OutermostConditional = OldConditional; + Builder.restoreIP(OldIP); + } + break; + } + default: break; } diff --git a/test/CodeGenCXX/conditional-temporaries.cpp b/test/CodeGenCXX/conditional-temporaries.cpp index e08de61f9d..48c9117882 100644 --- a/test/CodeGenCXX/conditional-temporaries.cpp +++ b/test/CodeGenCXX/conditional-temporaries.cpp @@ -1,6 +1,7 @@ // REQUIRES: amdgpu-registered-target -// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -O3 | FileCheck %s -// RUN: %clang_cc1 -emit-llvm %s -o - -triple=amdgcn-amd-amdhsa -O3 | FileCheck %s +// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -O2 -disable-llvm-passes | FileCheck %s --check-prefixes=CHECK,CHECK-NOOPT +// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-apple-darwin9 -O2 | FileCheck %s --check-prefixes=CHECK,CHECK-OPT +// RUN: %clang_cc1 -emit-llvm %s -o - -triple=amdgcn-amd-amdhsa -O2 | FileCheck %s --check-prefixes=CHECK,CHECK-OPT namespace { @@ -38,20 +39,167 @@ Checker c; } -// CHECK-LABEL: define i32 @_Z12getCtorCallsv() +// CHECK-OPT-LABEL: define i32 @_Z12getCtorCallsv() int getCtorCalls() { - // CHECK: ret i32 5 + // CHECK-OPT: ret i32 5 return ctorcalls; } -// CHECK-LABEL: define i32 @_Z12getDtorCallsv() +// CHECK-OPT-LABEL: define i32 @_Z12getDtorCallsv() int getDtorCalls() { - // CHECK: ret i32 5 + // CHECK-OPT: ret i32 5 return dtorcalls; } -// CHECK-LABEL: define zeroext i1 @_Z7successv() +// CHECK-OPT-LABEL: define zeroext i1 @_Z7successv() bool success() { - // CHECK: ret i1 true + // CHECK-OPT: ret i1 true return ctorcalls == dtorcalls; } + +struct X { ~X(); int f(); }; +int g(int, int, int); +// CHECK-LABEL: @_Z16lifetime_nontriv +int lifetime_nontriv(bool cond) { + // CHECK-NOOPT: store i1 false, + // CHECK-NOOPT: store i1 false, + // CHECK-NOOPT: store i1 false, + // CHECK-NOOPT: store i1 false, + // CHECK-NOOPT: store i1 false, + // CHECK-NOOPT: store i1 false, + // CHECK-NOOPT: br i1 + // + // CHECK-NOOPT: call void @llvm.lifetime.start + // CHECK-NOOPT: store i1 true, + // CHECK-NOOPT: store i1 true, + // CHECK-NOOPT: call i32 @_ZN1X1fEv( + // CHECK-NOOPT: call void @llvm.lifetime.start + // CHECK-NOOPT: store i1 true, + // CHECK-NOOPT: store i1 true, + // CHECK-NOOPT: call i32 @_ZN1X1fEv( + // CHECK-NOOPT: call void @llvm.lifetime.start + // CHECK-NOOPT: store i1 true, + // CHECK-NOOPT: store i1 true, + // CHECK-NOOPT: call i32 @_ZN1X1fEv( + // CHECK-NOOPT: call i32 @_Z1giii( + // CHECK-NOOPT: br label + // + // CHECK-NOOPT: call i32 @_Z1giii(i32 1, i32 2, i32 3) + // CHECK-NOOPT: br label + // + // CHECK-NOOPT: load i1, + // CHECK-NOOPT: br i1 + // CHECK-NOOPT: call void @_ZN1XD1Ev( + // CHECK-NOOPT: br label + // + // CHECK-NOOPT: load i1, + // CHECK-NOOPT: br i1 + // CHECK-NOOPT: call void @llvm.lifetime.end + // CHECK-NOOPT: br label + // + // CHECK-NOOPT: load i1, + // CHECK-NOOPT: br i1 + // CHECK-NOOPT: call void @_ZN1XD1Ev( + // CHECK-NOOPT: br label + // + // CHECK-NOOPT: load i1, + // CHECK-NOOPT: br i1 + // CHECK-NOOPT: call void @llvm.lifetime.end + // CHECK-NOOPT: br label + // + // CHECK-NOOPT: load i1, + // CHECK-NOOPT: br i1 + // CHECK-NOOPT: call void @_ZN1XD1Ev( + // CHECK-NOOPT: br label + // + // CHECK-NOOPT: load i1, + // CHECK-NOOPT: br i1 + // CHECK-NOOPT: call void @llvm.lifetime.end + // CHECK-NOOPT: br label + // + // CHECK-NOOPT: ret + + // CHECK-OPT: br i1 + // + // CHECK-OPT: call void @llvm.lifetime.start + // CHECK-OPT: call i32 @_ZN1X1fEv( + // CHECK-OPT: call void @llvm.lifetime.start + // CHECK-OPT: call i32 @_ZN1X1fEv( + // CHECK-OPT: call void @llvm.lifetime.start + // CHECK-OPT: call i32 @_ZN1X1fEv( + // CHECK-OPT: call i32 @_Z1giii( + // CHECK-OPT: call void @_ZN1XD1Ev( + // CHECK-OPT: call void @llvm.lifetime.end + // CHECK-OPT: call void @_ZN1XD1Ev( + // CHECK-OPT: call void @llvm.lifetime.end + // CHECK-OPT: call void @_ZN1XD1Ev( + // CHECK-OPT: call void @llvm.lifetime.end + // CHECK-OPT: br label + return cond ? g(X().f(), X().f(), X().f()) : g(1, 2, 3); +} + +struct Y { int f(); }; +int g(int, int, int); +// CHECK-LABEL: @_Z13lifetime_triv +int lifetime_triv(bool cond) { + // CHECK-NOOPT: call void @llvm.lifetime.start + // CHECK-NOOPT: call void @llvm.lifetime.start + // CHECK-NOOPT: call void @llvm.lifetime.start + // CHECK-NOOPT: br i1 + // + // CHECK-NOOPT: call i32 @_ZN1Y1fEv( + // CHECK-NOOPT: call i32 @_ZN1Y1fEv( + // CHECK-NOOPT: call i32 @_ZN1Y1fEv( + // CHECK-NOOPT: call i32 @_Z1giii( + // CHECK-NOOPT: br label + // + // CHECK-NOOPT: call i32 @_Z1giii(i32 1, i32 2, i32 3) + // CHECK-NOOPT: br label + // + // CHECK-NOOPT: call void @llvm.lifetime.end + // CHECK-NOOPT-NOT: br + // CHECK-NOOPT: call void @llvm.lifetime.end + // CHECK-NOOPT-NOT: br + // CHECK-NOOPT: call void @llvm.lifetime.end + // + // CHECK-NOOPT: ret + + // FIXME: LLVM isn't smart enough to remove the lifetime markers from the + // g(1, 2, 3) path here. + + // CHECK-OPT: call void @llvm.lifetime.start + // CHECK-OPT: call void @llvm.lifetime.start + // CHECK-OPT: call void @llvm.lifetime.start + // CHECK-OPT: br i1 + // + // CHECK-OPT: call i32 @_ZN1Y1fEv( + // CHECK-OPT: call i32 @_ZN1Y1fEv( + // CHECK-OPT: call i32 @_ZN1Y1fEv( + // CHECK-OPT: call i32 @_Z1giii( + // CHECK-OPT: br label + // + // CHECK-OPT: call void @llvm.lifetime.end + // CHECK-OPT: call void @llvm.lifetime.end + // CHECK-OPT: call void @llvm.lifetime.end + return cond ? g(Y().f(), Y().f(), Y().f()) : g(1, 2, 3); +} + +struct Z { ~Z() {} int f(); }; +int g(int, int, int); +// CHECK-LABEL: @_Z22lifetime_nontriv_empty +int lifetime_nontriv_empty(bool cond) { + // CHECK-OPT: br i1 + // + // CHECK-OPT: call void @llvm.lifetime.start + // CHECK-OPT: call i32 @_ZN1Z1fEv( + // CHECK-OPT: call void @llvm.lifetime.start + // CHECK-OPT: call i32 @_ZN1Z1fEv( + // CHECK-OPT: call void @llvm.lifetime.start + // CHECK-OPT: call i32 @_ZN1Z1fEv( + // CHECK-OPT: call i32 @_Z1giii( + // CHECK-OPT: call void @llvm.lifetime.end + // CHECK-OPT: call void @llvm.lifetime.end + // CHECK-OPT: call void @llvm.lifetime.end + // CHECK-OPT: br label + return cond ? g(Z().f(), Z().f(), Z().f()) : g(1, 2, 3); +} diff --git a/test/CodeGenCXX/lifetime-asan.cpp b/test/CodeGenCXX/lifetime-asan.cpp new file mode 100644 index 0000000000..9ccb28c876 --- /dev/null +++ b/test/CodeGenCXX/lifetime-asan.cpp @@ -0,0 +1,42 @@ +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 %s | FileCheck %s -check-prefixes=CHECK,CHECK-O0 --implicit-check-not=llvm.lifetime +// RUN: %clang -target x86_64-linux-gnu -S -emit-llvm -o - -fno-exceptions -O0 \ +// RUN: -fsanitize=address -fsanitize-address-use-after-scope %s | \ +// RUN: FileCheck %s -check-prefixes=CHECK,CHECK-ASAN-USE-AFTER-SCOPE + +extern int bar(char *A, int n); + +struct X { X(); ~X(); int *p; }; +struct Y { Y(); int *p; }; + +extern "C" void a(), b(), c(), d(); + +// CHECK-LABEL: @_Z3foo +void foo(int n) { + // CHECK: call void @a() + a(); + + // CHECK: call void @b() + // CHECK-ASAN-USE-AFTER-SCOPE: store i1 false + // CHECK-ASAN-USE-AFTER-SCOPE: store i1 false + // CHECK: br i1 + // + // CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.start + // CHECK-ASAN-USE-AFTER-SCOPE: store i1 true + // CHECK: call void @_ZN1XC + // CHECK: br label + // + // CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.start + // CHECK-ASAN-USE-AFTER-SCOPE: store i1 true + // CHECK: call void @_ZN1YC + // CHECK: br label + // + // CHECK: call void @c() + // CHECK-ASAN-USE-AFTER-SCOPE: br i1 + // CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.end + // CHECK-ASAN-USE-AFTER-SCOPE: br i1 + // CHECK-ASAN-USE-AFTER-SCOPE: @llvm.lifetime.end + b(), (n ? X().p : Y().p), c(); + + // CHECK: call void @d() + d(); +}