[cfe-commits] r133620 - in /cfe/trunk: lib/CodeGen/CGExpr.cpp lib/CodeGen/CGObjC.cpp lib/CodeGen/CodeGenFunction.h lib/Sema/SemaInit.cpp test/CodeGenObjCXX/arc-references.mm

Douglas Gregor dgregor at apple.com
Wed Jun 22 09:12:01 PDT 2011


Author: dgregor
Date: Wed Jun 22 11:12:01 2011
New Revision: 133620

URL: http://llvm.org/viewvc/llvm-project?rev=133620&view=rev
Log:
When binding a reference to an Automatic Reference Counting temporary,
retain/release the temporary object appropriately. Previously, we
would only perform the retain/release operations when the reference
would extend the lifetime of the temporary, but this does the wrong
thing across calls.

Modified:
    cfe/trunk/lib/CodeGen/CGExpr.cpp
    cfe/trunk/lib/CodeGen/CGObjC.cpp
    cfe/trunk/lib/CodeGen/CodeGenFunction.h
    cfe/trunk/lib/Sema/SemaInit.cpp
    cfe/trunk/test/CodeGenObjCXX/arc-references.mm

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=133620&r1=133619&r2=133620&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Wed Jun 22 11:12:01 2011
@@ -205,11 +205,21 @@
                             const CXXDestructorDecl *&ReferenceTemporaryDtor,
                             QualType &ObjCARCReferenceLifetimeType,
                             const NamedDecl *InitializedDecl) {
-  ObjCARCReferenceLifetimeType = QualType();
-  
   // Look through expressions for materialized temporaries (for now).
-  if (isa<MaterializeTemporaryExpr>(E))
-    E = cast<MaterializeTemporaryExpr>(E)->GetTemporaryExpr();
+  if (const MaterializeTemporaryExpr *M 
+                                      = dyn_cast<MaterializeTemporaryExpr>(E)) {
+    // Objective-C++ ARC:
+    //   If we are binding a reference to a temporary that has ownership, we 
+    //   need to perform retain/release operations on the temporary.
+    if (CGF.getContext().getLangOptions().ObjCAutoRefCount &&        
+        E->getType()->isObjCLifetimeType() &&
+        (E->getType().getObjCLifetime() == Qualifiers::OCL_Strong ||
+         E->getType().getObjCLifetime() == Qualifiers::OCL_Weak ||
+         E->getType().getObjCLifetime() == Qualifiers::OCL_Autoreleasing))
+      ObjCARCReferenceLifetimeType = E->getType();
+    
+    E = M->GetTemporaryExpr();
+  }
 
   if (const CXXDefaultArgExpr *DAE = dyn_cast<CXXDefaultArgExpr>(E))
     E = DAE->getExpr();
@@ -243,6 +253,57 @@
     // We have to load the lvalue.
     RV = CGF.EmitLoadOfLValue(LV, E->getType());
   } else {
+    if (!ObjCARCReferenceLifetimeType.isNull()) {
+      ReferenceTemporary = CreateReferenceTemporary(CGF, 
+                                                  ObjCARCReferenceLifetimeType, 
+                                                    InitializedDecl);
+      
+      
+      LValue RefTempDst = CGF.MakeAddrLValue(ReferenceTemporary, 
+                                             ObjCARCReferenceLifetimeType);
+
+      CGF.EmitScalarInit(E, dyn_cast_or_null<ValueDecl>(InitializedDecl),
+                         RefTempDst, false);
+      
+      bool ExtendsLifeOfTemporary = false;
+      if (const VarDecl *Var = dyn_cast_or_null<VarDecl>(InitializedDecl)) {
+        if (Var->extendsLifetimeOfTemporary())
+          ExtendsLifeOfTemporary = true;
+      } else if (InitializedDecl && isa<FieldDecl>(InitializedDecl)) {
+        ExtendsLifeOfTemporary = true;
+      }
+      
+      if (!ExtendsLifeOfTemporary) {
+        // Since the lifetime of this temporary isn't going to be extended,
+        // we need to clean it up ourselves at the end of the full expression.
+        switch (ObjCARCReferenceLifetimeType.getObjCLifetime()) {
+        case Qualifiers::OCL_None:
+        case Qualifiers::OCL_ExplicitNone:
+        case Qualifiers::OCL_Autoreleasing:
+          break;
+            
+        case Qualifiers::OCL_Strong:
+          CGF.PushARCReleaseCleanup(CGF.getARCCleanupKind(), 
+                                    ObjCARCReferenceLifetimeType, 
+                                    ReferenceTemporary,
+                                    /*Precise lifetime=*/false, 
+                                    /*For full expression=*/true);
+          break;
+          
+        case Qualifiers::OCL_Weak:
+          CGF.PushARCWeakReleaseCleanup(NormalAndEHCleanup, 
+                                        ObjCARCReferenceLifetimeType, 
+                                        ReferenceTemporary,
+                                        /*For full expression=*/true);
+          break;
+        }
+        
+        ObjCARCReferenceLifetimeType = QualType();
+      }
+      
+      return ReferenceTemporary;
+    }
+    
     llvm::SmallVector<SubobjectAdjustment, 2> Adjustments;
     while (true) {
       E = E->IgnoreParens();
@@ -298,34 +359,6 @@
         if (!ClassDecl->hasTrivialDestructor())
           ReferenceTemporaryDtor = ClassDecl->getDestructor();
       }
-      else if (CGF.getContext().getLangOptions().ObjCAutoRefCount) {
-        if (const ValueDecl *InitVD = dyn_cast<ValueDecl>(InitializedDecl)) {
-          if (const ReferenceType *RefType
-                                  = InitVD->getType()->getAs<ReferenceType>()) {
-            QualType PointeeType = RefType->getPointeeType();
-            if (PointeeType->isObjCLifetimeType() &&
-                PointeeType.getObjCLifetime() != Qualifiers::OCL_ExplicitNone) {
-              // Objective-C++ ARC: We're binding a reference to 
-              // lifetime-qualified type to a temporary, so we need to extend 
-              // the lifetime of the temporary with appropriate retain/release/
-              // autorelease calls.
-              ObjCARCReferenceLifetimeType = PointeeType;
-              
-              // Create a temporary variable that we can bind the reference to.
-              ReferenceTemporary = CreateReferenceTemporary(CGF, PointeeType, 
-                                                            InitializedDecl);
-
-              unsigned Alignment =
-                CGF.getContext().getTypeAlignInChars(PointeeType).getQuantity();
-              LValue lvalue =
-                CGF.MakeAddrLValue(ReferenceTemporary, PointeeType, Alignment);
-
-              CGF.EmitScalarInit(E, InitVD, lvalue, false);
-              return ReferenceTemporary;
-            }
-          }
-        }
-      }
     }
 
     RV = CGF.EmitAnyExpr(E, AggSlot);

Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=133620&r1=133619&r2=133620&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp Wed Jun 22 11:12:01 2011
@@ -1985,6 +1985,21 @@
     CallReleaseForObject(QualType type, llvm::Value *addr, bool precise)
       : ObjCReleasingCleanup(type, addr), precise(precise) {}
 
+    using ObjCReleasingCleanup::Emit;
+    static void Emit(CodeGenFunction &CGF, bool IsForEH,
+                     QualType type, llvm::Value *addr, bool precise) {
+      // EHScopeStack::Cleanup objects can never have their destructors called,
+      // so use placement new to construct our temporary object.
+      union {
+        void* align;
+        char data[sizeof(CallReleaseForObject)];
+      };
+      
+      CallReleaseForObject *Object
+        = new (&align) CallReleaseForObject(type, addr, precise);
+      Object->Emit(CGF, IsForEH);
+    }
+
     void release(CodeGenFunction &CGF, QualType type, llvm::Value *addr) {
       llvm::Value *ptr = CGF.Builder.CreateLoad(addr, "tmp");
       CGF.EmitARCRelease(ptr, precise);
@@ -2033,6 +2048,21 @@
     CallWeakReleaseForObject(QualType type, llvm::Value *addr)
       : ObjCReleasingCleanup(type, addr) {}
 
+    using ObjCReleasingCleanup::Emit;
+    static void Emit(CodeGenFunction &CGF, bool IsForEH,
+                     QualType type, llvm::Value *addr) {
+      // EHScopeStack::Cleanup objects can never have their destructors called,
+      // so use placement new to construct our temporary object.
+      union {
+        void* align;
+        char data[sizeof(CallWeakReleaseForObject)];
+      };
+      
+      CallWeakReleaseForObject *Object
+        = new (&align) CallWeakReleaseForObject(type, addr);
+      Object->Emit(CGF, IsForEH);
+    }
+    
     void release(CodeGenFunction &CGF, QualType type, llvm::Value *addr) {
       CGF.EmitARCDestroyWeak(addr);
     }
@@ -2099,16 +2129,24 @@
 void CodeGenFunction::PushARCReleaseCleanup(CleanupKind cleanupKind,
                                             QualType type,
                                             llvm::Value *addr,
-                                            bool precise) {
-  EHStack.pushCleanup<CallReleaseForObject>(cleanupKind, type, addr, precise);
+                                            bool precise,
+                                            bool forFullExpr) {
+  if (forFullExpr)
+    pushFullExprCleanup<CallReleaseForObject>(cleanupKind, type, addr, precise);
+  else
+    EHStack.pushCleanup<CallReleaseForObject>(cleanupKind, type, addr, precise);
 }
 
 /// PushARCWeakReleaseCleanup - Enter a cleanup to perform a weak
 /// release on the given object or array of objects.
 void CodeGenFunction::PushARCWeakReleaseCleanup(CleanupKind cleanupKind,
                                                 QualType type,
-                                                llvm::Value *addr) {
-  EHStack.pushCleanup<CallWeakReleaseForObject>(cleanupKind, type, addr);
+                                                llvm::Value *addr,
+                                                bool forFullExpr) {
+  if (forFullExpr)
+    pushFullExprCleanup<CallWeakReleaseForObject>(cleanupKind, type, addr);
+  else
+    EHStack.pushCleanup<CallWeakReleaseForObject>(cleanupKind, type, addr);
 }
 
 /// PushARCReleaseCleanup - Enter a cleanup to perform a release on a

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=133620&r1=133619&r2=133620&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Wed Jun 22 11:12:01 2011
@@ -223,6 +223,16 @@
     }
   };
 
+  template <class T, class A0, class A1, class A2>
+  class UnconditionalCleanup3 : public Cleanup {
+    A0 a0; A1 a1; A2 a2;
+  public:
+    UnconditionalCleanup3(A0 a0, A1 a1, A2 a2) : a0(a0), a1(a1), a2(a2) {}
+    void Emit(CodeGenFunction &CGF, bool IsForEHCleanup) {
+      T::Emit(CGF, IsForEHCleanup, a0, a1, a2);
+    }
+  };
+
   /// ConditionalCleanupN stores the saved form of its N parameters,
   /// then restores them and performs the cleanup.
   template <class T, class A0>
@@ -258,6 +268,27 @@
       : a0_saved(a0), a1_saved(a1) {}
   };
 
+  template <class T, class A0, class A1, class A2>
+  class ConditionalCleanup3 : public Cleanup {
+    typedef typename DominatingValue<A0>::saved_type A0_saved;
+    typedef typename DominatingValue<A1>::saved_type A1_saved;
+    typedef typename DominatingValue<A2>::saved_type A2_saved;
+    A0_saved a0_saved;
+    A1_saved a1_saved;
+    A2_saved a2_saved;
+    
+    void Emit(CodeGenFunction &CGF, bool IsForEHCleanup) {
+      A0 a0 = DominatingValue<A0>::restore(CGF, a0_saved);
+      A1 a1 = DominatingValue<A1>::restore(CGF, a1_saved);
+      A2 a2 = DominatingValue<A2>::restore(CGF, a2_saved);
+      T::Emit(CGF, IsForEHCleanup, a0, a1, a2);
+    }
+    
+  public:
+    ConditionalCleanup3(A0_saved a0, A1_saved a1, A2_saved a2)
+    : a0_saved(a0), a1_saved(a1), a2_saved(a2) {}
+  };
+
 private:
   // The implementation for this class is in CGException.h and
   // CGException.cpp; the definition is here because it's used as a
@@ -697,6 +728,27 @@
     initFullExprCleanup();
   }
 
+  /// pushFullExprCleanup - Push a cleanup to be run at the end of the
+  /// current full-expression.  Safe against the possibility that
+  /// we're currently inside a conditionally-evaluated expression.
+  template <class T, class A0, class A1, class A2>
+  void pushFullExprCleanup(CleanupKind kind, A0 a0, A1 a1, A2 a2) {
+    // If we're not in a conditional branch, or if none of the
+    // arguments requires saving, then use the unconditional cleanup.
+    if (!isInConditionalBranch()) {
+      typedef EHScopeStack::UnconditionalCleanup3<T, A0, A1, A2> CleanupType;
+      return EHStack.pushCleanup<CleanupType>(kind, a0, a1, a2);
+    }
+    
+    typename DominatingValue<A0>::saved_type a0_saved = saveValueInCond(a0);
+    typename DominatingValue<A1>::saved_type a1_saved = saveValueInCond(a1);
+    typename DominatingValue<A2>::saved_type a2_saved = saveValueInCond(a2);
+    
+    typedef EHScopeStack::ConditionalCleanup3<T, A0, A1, A2> CleanupType;
+    EHStack.pushCleanup<CleanupType>(kind, a0_saved, a1_saved, a2_saved);
+    initFullExprCleanup();
+  }
+
   /// PushDestructorCleanup - Push a cleanup to call the
   /// complete-object destructor of an object of the given type at the
   /// given address.  Does nothing if T is not a C++ class type with a
@@ -2036,9 +2088,10 @@
   llvm::Value *EmitARCRetainAutoreleaseScalarExpr(const Expr *expr);
 
   void PushARCReleaseCleanup(CleanupKind kind, QualType type,
-                             llvm::Value *addr, bool precise);
+                             llvm::Value *addr, bool precise,
+                             bool forFullExpr = false);
   void PushARCWeakReleaseCleanup(CleanupKind kind, QualType type,
-                                 llvm::Value *addr);
+                                 llvm::Value *addr, bool forFullExpr = false);
   void PushARCFieldReleaseCleanup(CleanupKind cleanupKind,
                                   const FieldDecl *Field);
   void PushARCFieldWeakReleaseCleanup(CleanupKind cleanupKind,

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=133620&r1=133619&r2=133620&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Wed Jun 22 11:12:01 2011
@@ -4084,6 +4084,13 @@
                                          Entity.getType().getNonReferenceType(),
                                                          CurInit.get(),
                                      Entity.getType()->isLValueReferenceType());
+
+      // If we're binding to an Objective-C object that has lifetime, we
+      // need cleanups.
+      if (S.getLangOptions().ObjCAutoRefCount &&
+          CurInit.get()->getType()->isObjCLifetimeType())
+        S.ExprNeedsCleanups = true;
+            
       break;
 
     case SK_ExtraneousCopyToTemporary:

Modified: cfe/trunk/test/CodeGenObjCXX/arc-references.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/arc-references.mm?rev=133620&r1=133619&r2=133620&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenObjCXX/arc-references.mm (original)
+++ cfe/trunk/test/CodeGenObjCXX/arc-references.mm Wed Jun 22 11:12:01 2011
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-nonfragile-abi -fblocks -fobjc-arc -O2 -disable-llvm-optzns -o - %s | FileCheck %s
 
+ at interface A
+ at end
+
 id getObject();
 void callee();
 
@@ -44,6 +47,33 @@
   // CHECK-NEXT: ret void
 }
 
+// CHECK: define void @_Z5test4RU8__strongP11objc_object
+void test4(__strong id &x) {
+  // CHECK: call i8* @objc_retain
+  __strong A* const &ar = x;
+  // CHECK: store i32 17, i32*
+  int i = 17;
+  // CHECK: call void @objc_release(
+  // CHECK: ret void
+}
+
+void sink(__strong A* &&);
+
+// CHECK: define void @_Z5test5RU8__strongP11objc_object
+void test5(__strong id &x) {
+  // CHECK: [[OBJ_ID:%[a-zA-Z0-9]+]] = call i8* @objc_retain
+  // CHECK-NEXT: [[OBJ_A:%[a-zA-Z0-9]+]] = bitcast i8* [[OBJ_ID]] to [[A:%[a-zA-Z0-9]+]]*
+  // CHECK-NEXT: store [[A]]* [[OBJ_A]], [[A]]** [[REFTMP:%[a-zA-Z0-9]+]]
+  // CHECK-NEXT: call void @_Z4sinkOU8__strongP1A
+  sink(x);  
+  // CHECK-NEXT: [[OBJ_A:%[a-zA-Z0-9]+]] = load [[A]]** [[REFTMP]]
+  // CHECK-NEXT: [[OBJ_ID:%[a-zA-Z0-9]+]] = bitcast [[A]]* [[OBJ_A]] to i8*
+  // CHECK-NEXT: call void @objc_release
+  // CHECK-NEXT: store i32 17, i32
+  int i = 17;
+  // CHECK-NEXT: ret void
+}
+
 // CHECK: define internal void @__cxx_global_var_init(
 // CHECK: call i8* @_Z9getObjectv
 // CHECK-NEXT: call i8* @objc_retainAutoreleasedReturnValue





More information about the cfe-commits mailing list