r338945 - Avoid creating conditional cleanup blocks that contain only @llvm.lifetime.end calls

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 3 18:25:06 PDT 2018


Author: rsmith
Date: Fri Aug  3 18:25:06 2018
New Revision: 338945

URL: http://llvm.org/viewvc/llvm-project?rev=338945&view=rev
Log:
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

Added:
    cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp
Modified:
    cfe/trunk/lib/CodeGen/CGExpr.cpp
    cfe/trunk/test/CodeGenCXX/conditional-temporaries.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=338945&r1=338944&r2=338945&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Fri Aug  3 18:25:06 2018
@@ -498,18 +498,51 @@ EmitMaterializeTemporaryExpr(const Mater
   } 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;
     }

Modified: cfe/trunk/test/CodeGenCXX/conditional-temporaries.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/conditional-temporaries.cpp?rev=338945&r1=338944&r2=338945&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/conditional-temporaries.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/conditional-temporaries.cpp Fri Aug  3 18:25:06 2018
@@ -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);
+}

Added: cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp?rev=338945&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/lifetime-asan.cpp Fri Aug  3 18:25:06 2018
@@ -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();
+}




More information about the cfe-commits mailing list