]> granicus.if.org Git - clang/commitdiff
When emitting a new-expression inside a conditional expression,
authorJohn McCall <rjmccall@apple.com>
Fri, 17 Sep 2010 00:50:28 +0000 (00:50 +0000)
committerJohn McCall <rjmccall@apple.com>
Fri, 17 Sep 2010 00:50:28 +0000 (00:50 +0000)
the cleanup might not be dominated by the allocation code.
In this case, we have to store aside all the delete arguments
in case we need them later.  There's room for optimization here
in cases where we end up not actually needing the cleanup in
different branches (or being able to pop it after the
initialization code).

Also make sure we only call this operator delete along the path
where we actually allocated something.

Fixes rdar://problem/8439196.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@114145 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/CGExprCXX.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
lib/Sema/SemaTemplateInstantiateDecl.cpp
test/CodeGenCXX/exceptions.cpp

index 584f6c099b75ca27334b59db8f8196386d266c66..4dbfa7ee4fa5eceae444e91e886e4b24fa9a471f 100644 (file)
@@ -675,6 +675,105 @@ static void EmitNewInitializer(CodeGenFunction &CGF, const CXXNewExpr *E,
   StoreAnyExprIntoOneUnit(CGF, E, NewPtr);
 }
 
+/// A utility class for saving an rvalue.
+class SavedRValue {
+public:
+  enum Kind { ScalarLiteral, ScalarAddress,
+              AggregateLiteral, AggregateAddress,
+              Complex };
+
+private:
+  llvm::Value *Value;
+  Kind K;
+
+  SavedRValue(llvm::Value *V, Kind K) : Value(V), K(K) {}
+
+public:
+  SavedRValue() {}
+
+  static SavedRValue forScalarLiteral(llvm::Value *V) {
+    return SavedRValue(V, ScalarLiteral);
+  }
+
+  static SavedRValue forScalarAddress(llvm::Value *Addr) {
+    return SavedRValue(Addr, ScalarAddress);
+  }
+
+  static SavedRValue forAggregateLiteral(llvm::Value *V) {
+    return SavedRValue(V, AggregateLiteral);
+  }
+
+  static SavedRValue forAggregateAddress(llvm::Value *Addr) {
+    return SavedRValue(Addr, AggregateAddress);
+  }
+
+  static SavedRValue forComplexAddress(llvm::Value *Addr) {
+    return SavedRValue(Addr, Complex);
+  }
+
+  Kind getKind() const { return K; }
+  llvm::Value *getValue() const { return Value; }
+};
+
+/// Given an r-value, perform the code necessary to make sure that a
+/// future RestoreRValue will be able to load the value without
+/// domination concerns.
+static SavedRValue SaveRValue(CodeGenFunction &CGF, RValue RV) {
+  if (RV.isScalar()) {
+    llvm::Value *V = RV.getScalarVal();
+
+    // These automatically dominate and don't need to be saved.
+    if (isa<llvm::Constant>(V) || isa<llvm::AllocaInst>(V))
+      return SavedRValue::forScalarLiteral(V);
+
+    // Everything else needs an alloca.
+    llvm::Value *Addr = CGF.CreateTempAlloca(V->getType(), "saved-rvalue");
+    CGF.Builder.CreateStore(V, Addr);
+    return SavedRValue::forScalarAddress(Addr);
+  }
+
+  if (RV.isComplex()) {
+    CodeGenFunction::ComplexPairTy V = RV.getComplexVal();
+    const llvm::Type *ComplexTy =
+      llvm::StructType::get(CGF.getLLVMContext(),
+                            V.first->getType(), V.second->getType(),
+                            (void*) 0);
+    llvm::Value *Addr = CGF.CreateTempAlloca(ComplexTy, "saved-complex");
+    CGF.StoreComplexToAddr(V, Addr, /*volatile*/ false);
+    return SavedRValue::forComplexAddress(Addr);
+  }
+
+  assert(RV.isAggregate());
+  llvm::Value *V = RV.getAggregateAddr(); // TODO: volatile?
+  if (isa<llvm::Constant>(V) || isa<llvm::AllocaInst>(V))
+    return SavedRValue::forAggregateLiteral(V);
+
+  llvm::Value *Addr = CGF.CreateTempAlloca(V->getType(), "saved-rvalue");
+  CGF.Builder.CreateStore(V, Addr);
+  return SavedRValue::forAggregateAddress(Addr);
+}
+
+/// Given a saved r-value produced by SaveRValue, perform the code
+/// necessary to restore it to usability at the current insertion
+/// point.
+static RValue RestoreRValue(CodeGenFunction &CGF, SavedRValue RV) {
+  switch (RV.getKind()) {
+  case SavedRValue::ScalarLiteral:
+    return RValue::get(RV.getValue());
+  case SavedRValue::ScalarAddress:
+    return RValue::get(CGF.Builder.CreateLoad(RV.getValue()));
+  case SavedRValue::AggregateLiteral:
+    return RValue::getAggregate(RV.getValue());
+  case SavedRValue::AggregateAddress:
+    return RValue::getAggregate(CGF.Builder.CreateLoad(RV.getValue()));
+  case SavedRValue::Complex:
+    return RValue::getComplex(CGF.LoadComplexFromAddr(RV.getValue(), false));
+  }
+
+  llvm_unreachable("bad saved r-value kind");
+  return RValue();
+}
+
 namespace {
   /// A cleanup to call the given 'operator delete' function upon
   /// abnormal exit from a new expression.
@@ -729,6 +828,104 @@ namespace {
                    ReturnValueSlot(), DeleteArgs, OperatorDelete);
     }
   };
+
+  /// A cleanup to call the given 'operator delete' function upon
+  /// abnormal exit from a new expression when the new expression is
+  /// conditional.
+  class CallDeleteDuringConditionalNew : public EHScopeStack::Cleanup {
+    size_t NumPlacementArgs;
+    const FunctionDecl *OperatorDelete;
+    SavedRValue Ptr;
+    SavedRValue AllocSize;
+
+    SavedRValue *getPlacementArgs() {
+      return reinterpret_cast<SavedRValue*>(this+1);
+    }
+
+  public:
+    static size_t getExtraSize(size_t NumPlacementArgs) {
+      return NumPlacementArgs * sizeof(SavedRValue);
+    }
+
+    CallDeleteDuringConditionalNew(size_t NumPlacementArgs,
+                                   const FunctionDecl *OperatorDelete,
+                                   SavedRValue Ptr,
+                                   SavedRValue AllocSize) 
+      : NumPlacementArgs(NumPlacementArgs), OperatorDelete(OperatorDelete),
+        Ptr(Ptr), AllocSize(AllocSize) {}
+
+    void setPlacementArg(unsigned I, SavedRValue Arg) {
+      assert(I < NumPlacementArgs && "index out of range");
+      getPlacementArgs()[I] = Arg;
+    }
+
+    void Emit(CodeGenFunction &CGF, bool IsForEH) {
+      const FunctionProtoType *FPT
+        = OperatorDelete->getType()->getAs<FunctionProtoType>();
+      assert(FPT->getNumArgs() == NumPlacementArgs + 1 ||
+             (FPT->getNumArgs() == 2 && NumPlacementArgs == 0));
+
+      CallArgList DeleteArgs;
+
+      // The first argument is always a void*.
+      FunctionProtoType::arg_type_iterator AI = FPT->arg_type_begin();
+      DeleteArgs.push_back(std::make_pair(RestoreRValue(CGF, Ptr), *AI++));
+
+      // A member 'operator delete' can take an extra 'size_t' argument.
+      if (FPT->getNumArgs() == NumPlacementArgs + 2) {
+        RValue RV = RestoreRValue(CGF, AllocSize);
+        DeleteArgs.push_back(std::make_pair(RV, *AI++));
+      }
+
+      // Pass the rest of the arguments, which must match exactly.
+      for (unsigned I = 0; I != NumPlacementArgs; ++I) {
+        RValue RV = RestoreRValue(CGF, getPlacementArgs()[I]);
+        DeleteArgs.push_back(std::make_pair(RV, *AI++));
+      }
+
+      // Call 'operator delete'.
+      CGF.EmitCall(CGF.CGM.getTypes().getFunctionInfo(DeleteArgs, FPT),
+                   CGF.CGM.GetAddrOfFunction(OperatorDelete),
+                   ReturnValueSlot(), DeleteArgs, OperatorDelete);
+    }
+  };
+}
+
+/// Enter a cleanup to call 'operator delete' if the initializer in a
+/// new-expression throws.
+static void EnterNewDeleteCleanup(CodeGenFunction &CGF,
+                                  const CXXNewExpr *E,
+                                  llvm::Value *NewPtr,
+                                  llvm::Value *AllocSize,
+                                  const CallArgList &NewArgs) {
+  // If we're not inside a conditional branch, then the cleanup will
+  // dominate and we can do the easier (and more efficient) thing.
+  if (!CGF.isInConditionalBranch()) {
+    CallDeleteDuringNew *Cleanup = CGF.EHStack
+      .pushCleanupWithExtra<CallDeleteDuringNew>(EHCleanup,
+                                                 E->getNumPlacementArgs(),
+                                                 E->getOperatorDelete(),
+                                                 NewPtr, AllocSize);
+    for (unsigned I = 0, N = E->getNumPlacementArgs(); I != N; ++I)
+      Cleanup->setPlacementArg(I, NewArgs[I+1].first);
+
+    return;
+  }
+
+  // Otherwise, we need to save all this stuff.
+  SavedRValue SavedNewPtr = SaveRValue(CGF, RValue::get(NewPtr));
+  SavedRValue SavedAllocSize = SaveRValue(CGF, RValue::get(AllocSize));
+
+  CallDeleteDuringConditionalNew *Cleanup = CGF.EHStack
+    .pushCleanupWithExtra<CallDeleteDuringConditionalNew>(InactiveEHCleanup,
+                                                 E->getNumPlacementArgs(),
+                                                 E->getOperatorDelete(),
+                                                 SavedNewPtr,
+                                                 SavedAllocSize);
+  for (unsigned I = 0, N = E->getNumPlacementArgs(); I != N; ++I)
+    Cleanup->setPlacementArg(I, SaveRValue(CGF, NewArgs[I+1].first));
+
+  CGF.ActivateCleanupBlock(CGF.EHStack.stable_begin());
 }
 
 llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
@@ -827,13 +1024,7 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
   // exception is thrown.
   EHScopeStack::stable_iterator CallOperatorDelete;
   if (E->getOperatorDelete()) {
-    CallDeleteDuringNew *Cleanup = CGF.EHStack
-      .pushCleanupWithExtra<CallDeleteDuringNew>(EHCleanup,
-                                                 E->getNumPlacementArgs(),
-                                                 E->getOperatorDelete(),
-                                                 NewPtr, AllocSize);
-    for (unsigned I = 0, N = E->getNumPlacementArgs(); I != N; ++I)
-      Cleanup->setPlacementArg(I, NewArgs[I+1].first);
+    EnterNewDeleteCleanup(*this, E, NewPtr, AllocSize, NewArgs);
     CallOperatorDelete = EHStack.stable_begin();
   }
 
index 0dbc6120ed0b19f825ea804aaf6d910701cafb1f..aae8ecf66703377d32e0f1a2bd322d1ccd661e13 100644 (file)
@@ -1431,9 +1431,11 @@ static void SetupCleanupBlockActivation(CodeGenFunction &CGF,
                                         EHScopeStack::stable_iterator C,
                                         ForActivation_t Kind) {
   EHCleanupScope &Scope = cast<EHCleanupScope>(*CGF.EHStack.find(C));
-  assert(!Scope.getActiveFlag() && "scope already has activation flag");
 
-  bool NeedFlag = false;
+  // We always need the flag if we're activating the cleanup, because
+  // we have to assume that the current location doesn't necessarily
+  // dominate all future uses of the cleanup.
+  bool NeedFlag = (Kind == ForActivation);
 
   // Calculate whether the cleanup was used:
 
@@ -1452,16 +1454,17 @@ static void SetupCleanupBlockActivation(CodeGenFunction &CGF,
   // If it hasn't yet been used as either, we're done.
   if (!NeedFlag) return;
 
-  llvm::AllocaInst *Var = CGF.CreateTempAlloca(CGF.Builder.getInt1Ty());
-  Scope.setActiveFlag(Var);
+  llvm::AllocaInst *Var = Scope.getActiveFlag();
+  if (!Var) {
+    Var = CGF.CreateTempAlloca(CGF.Builder.getInt1Ty(), "cleanup.isactive");
+    Scope.setActiveFlag(Var);
 
-  if (Kind == ForActivation) {
-    CGF.InitTempAlloca(Var, CGF.Builder.getFalse());
-    CGF.Builder.CreateStore(CGF.Builder.getTrue(), Var);
-  } else {
-    CGF.InitTempAlloca(Var, CGF.Builder.getTrue());
-    CGF.Builder.CreateStore(CGF.Builder.getFalse(), Var);
+    // Initialize to true or false depending on whether it was
+    // active up to this point.
+    CGF.InitTempAlloca(Var, CGF.Builder.getInt1(Kind == ForDeactivation));
   }
+
+  CGF.Builder.CreateStore(CGF.Builder.getInt1(Kind == ForActivation), Var);
 }
 
 /// Activate a cleanup that was created in an inactivated state.
@@ -1479,7 +1482,7 @@ void CodeGenFunction::ActivateCleanupBlock(EHScopeStack::stable_iterator C) {
 void CodeGenFunction::DeactivateCleanupBlock(EHScopeStack::stable_iterator C) {
   assert(C != EHStack.stable_end() && "deactivating bottom of stack?");
   EHCleanupScope &Scope = cast<EHCleanupScope>(*EHStack.find(C));
-  assert(Scope.isActive() && "double activation");
+  assert(Scope.isActive() && "double deactivation");
 
   // If it's the top of the stack, just pop it.
   if (C == EHStack.stable_begin()) {
index 26b262ccb1ce5d6e26c50ab65228df80e85a5534..a77044324a0bdf1718d76fd267508ca16518f5ae 100644 (file)
@@ -674,6 +674,10 @@ public:
     --ConditionalBranchLevel;
   }
 
+  /// isInConditionalBranch - Return true if we're currently emitting
+  /// one branch or the other of a conditional expression.
+  bool isInConditionalBranch() const { return ConditionalBranchLevel != 0; }
+
 private:
   CGDebugInfo *DebugInfo;
 
index c97a8788ef205c037c1b1e99594c7a3b1401ca3b..f9ff20463abaeec49202dd8874330f3d6863a443 100644 (file)
@@ -1035,7 +1035,8 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(FunctionDecl *D,
 
   // Attach the parameters
   for (unsigned P = 0; P < Params.size(); ++P)
-    Params[P]->setOwningFunction(Function);
+    if (Params[P])
+      Params[P]->setOwningFunction(Function);
   Function->setParams(Params.data(), Params.size());
 
   SourceLocation InstantiateAtPOI;
index 2337eee071f0a79185604a079551c89e28b71cd0..c60f2a1b45d04bda8134f99aec6e9406cd0d7256 100644 (file)
@@ -164,26 +164,78 @@ namespace test2 {
 
 namespace test3 {
   struct A {
-    A(int); A(int, int); ~A();
+    A(int); A(int, int); A(const A&); ~A();
     void *p;
-    void *operator new(size_t, void*, void*);
-    void operator delete(void*, void*, void*);
+    void *operator new(size_t, void*, double);
+    void operator delete(void*, void*, double);
   };
 
+  void *foo();
+  double bar();
+  A makeA(), *makeAPtr();
+
   A *a() {
     // CHECK:    define [[A:%.*]]* @_ZN5test31aEv()
     // CHECK:      [[FOO:%.*]] = call i8* @_ZN5test33fooEv()
-    // CHECK:      [[BAR:%.*]] = call i8* @_ZN5test33barEv()
-    // CHECK:      [[NEW:%.*]] = call i8* @_ZN5test31AnwEmPvS1_(i64 8, i8* [[FOO]], i8* [[BAR]])
+    // CHECK:      [[BAR:%.*]] = call double @_ZN5test33barEv()
+    // CHECK:      [[NEW:%.*]] = call i8* @_ZN5test31AnwEmPvd(i64 8, i8* [[FOO]], double [[BAR]])
     // CHECK-NEXT: [[CAST:%.*]] = bitcast i8* [[NEW]] to [[A]]*
     // CHECK-NEXT: invoke void @_ZN5test31AC1Ei([[A]]* [[CAST]], i32 5)
     // CHECK:      ret [[A]]* [[CAST]]
-    // CHECK:      invoke void @_ZN5test31AdlEPvS1_S1_(i8* [[NEW]], i8* [[FOO]], i8* [[BAR]])
+    // CHECK:      invoke void @_ZN5test31AdlEPvS1_d(i8* [[NEW]], i8* [[FOO]], double [[BAR]])
     // CHECK:      call void @_ZSt9terminatev()
-    extern void *foo(), *bar();
-
     return new(foo(),bar()) A(5);
   }
+
+  // rdar://problem/8439196
+  A *b(bool cond) {
+
+    // CHECK:    define [[A:%.*]]* @_ZN5test31bEb(i1 zeroext
+    // CHECK:      [[SAVED0:%.*]] = alloca i8*
+    // CHECK-NEXT: [[SAVED1:%.*]] = alloca i8*
+    // CHECK-NEXT: [[CLEANUPACTIVE:%.*]] = alloca i1
+    // CHECK-NEXT: [[TMP:%.*]] = alloca [[A]], align 8
+    // CHECK:      [[TMPACTIVE:%.*]] = alloca i1
+    // CHECK-NEXT: store i1 false, i1* [[TMPACTIVE]]
+    // CHECK-NEXT: store i1 false, i1* [[CLEANUPACTIVE]]
+
+    // CHECK:      [[COND:%.*]] = trunc i8 {{.*}} to i1
+    // CHECK-NEXT: br i1 [[COND]]
+    return (cond ?
+
+    // CHECK:      [[FOO:%.*]] = call i8* @_ZN5test33fooEv()
+    // CHECK-NEXT: [[NEW:%.*]] = call i8* @_ZN5test31AnwEmPvd(i64 8, i8* [[FOO]], double [[CONST:.*]])
+    // CHECK-NEXT: store i8* [[NEW]], i8** [[SAVED0]]
+    // CHECK-NEXT: store i8* [[FOO]], i8** [[SAVED1]]
+    // CHECK-NEXT: store i1 true, i1* [[CLEANUPACTIVE]]
+    // CHECK-NEXT: [[CAST:%.*]] = bitcast i8* [[NEW]] to [[A]]*
+    // CHECK-NEXT: invoke void @_ZN5test35makeAEv([[A]]* sret [[TMP]])
+    // CHECK:      store i1 true, i1* [[TMPACTIVE]]
+    // CHECK-NEXT: invoke void @_ZN5test31AC1ERKS0_([[A]]* [[CAST]], [[A]]* [[TMP]])
+    // CHECK:      store i1 false, i1* [[CLEANUPACTIVE]]
+    // CHECK-NEXT: br label
+    //   -> cond.end
+            new(foo(),10.0) A(makeA()) :
+
+    // CHECK:      [[MAKE:%.*]] = invoke [[A]]* @_ZN5test38makeAPtrEv()
+    // CHECK:      br label
+    //   -> cond.end
+            makeAPtr());
+
+    // cond.end:
+    // CHECK:      [[RESULT:%.*]] = phi [[A]]* {{.*}}[[CAST]]{{.*}}[[MAKE]]
+    // CHECK-NEXT: [[ISACTIVE:%.*]] = load i1* [[TMPACTIVE]]
+    // CHECK-NEXT: br i1 [[ISACTIVE]]
+    // CHECK:      invoke void @_ZN5test31AD1Ev
+    // CHECK:      ret [[A]]* [[RESULT]]
+
+    // in the EH path:
+    // CHECK:      [[ISACTIVE:%.*]] = load i1* [[CLEANUPACTIVE]]
+    // CHECK-NEXT: br i1 [[ISACTIVE]]
+    // CHECK:      [[V0:%.*]] = load i8** [[SAVED0]]
+    // CHECK-NEXT: [[V1:%.*]] = load i8** [[SAVED1]]
+    // CHECK-NEXT: invoke void @_ZN5test31AdlEPvS1_d(i8* [[V0]], i8* [[V1]], double [[CONST]])
+  }
 }
 
 namespace test4 {
@@ -206,5 +258,4 @@ namespace test4 {
 
     return new(foo(),bar()) A(5);
   }
-
 }