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 f168dd02ead1bf190454f49ed6807eeaca09903d..c8ba352e233145ada16e2f688c1d97d7d6322c2e 100644 (file)
--- 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<CallLifetimeEnd>(NormalEHLifetimeMarker,
-                                                    Alloca, Size);
-        else
-          pushFullExprCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker, Alloca,
-                                               Size);
+        pushCleanupAfterFullExpr<CallLifetimeEnd>(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<CallLifetimeEnd>(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 e08de61f9d41d720dfe39cabbfd18604700641f4..48c911788265945b0475c3c61d13d1b932c18a05 100644 (file)
--- 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 (file)
index 0000000..9ccb28c
--- /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();
+}