[CodeGen][ObjC] Destroy callee-destroyed arguments in the caller function when the receiver is nil

[ahatanak]

Summary

Callee-destroyed arguments to a method have to be destroyed in the caller function when the receiver is nil as the method doesn't get executed. This fixes PR48207.

rdar://71808391

[lanza]

Hey @ahatanak, I've ran into this problem in my companies projects and was wondering what the status of this patch is?

[rjmccall]

Functionally LGTM. Minor suggestion.

clang/lib/CodeGen/CGObjCMac.cpp

2169

Is this something that could reasonably just be a method on ParmVarDecl?

[ahatanak]

Make isParamDestroyedInCallee a method of ParmVarDecl.

clang/include/clang/AST/Decl.h

void setScopeInfo(unsigned scopeDepth, unsigned parameterIndex) {
 
     setParameterIndex(parameterIndex);
   }
 
   bool isObjCMethodParameter() const {
     return ParmVarDeclBits.IsObjCMethodParam;
   }
 
+  /// Determines whether this parameter is destroyed in the callee function.
+  bool isDestroyedInCallee() const;
+
   unsigned getFunctionScopeDepth() const {
     if (ParmVarDeclBits.IsObjCMethodParam) return 0;
     return ParmVarDeclBits.ScopeDepthOrObjCQuals;
   }
 
   static constexpr unsigned getMaxFunctionScopeDepth() {
     return (1u << NumScopeDepthOrObjCQualsBits) - 1;
   }

clang/lib/AST/Decl.cpp

SourceRange ParmVarDecl::getSourceRange() const {
   // DeclaratorDecl considers the range of postfix types as overlapping with the
   // declaration name, but this is not the case with parameters in ObjC methods.
   if (isa<ObjCMethodDecl>(getDeclContext()))
     return SourceRange(DeclaratorDecl::getBeginLoc(), getLocation());
 
   return DeclaratorDecl::getSourceRange();
 }
 
+bool ParmVarDecl::isDestroyedInCallee() const {
+  if (hasAttr<NSConsumedAttr>())
+    return true;
+
+  auto *RT = getType()->getAs<RecordType>();
+  if (RT && RT->getDecl()->isParamDestroyedInCallee())
+    return true;
+
+  return false;
+}
+
 Expr *ParmVarDecl::getDefaultArg() {
   assert(!hasUnparsedDefaultArg() && "Default argument is not yet parsed!");
   assert(!hasUninstantiatedDefaultArg() &&
          "Default argument is not yet instantiated!");
 
   Expr *Arg = getInit();
   if (auto *E = dyn_cast_or_null<FullExpr>(Arg))
     return E->getSubExpr();

clang/lib/CodeGen/CGObjCMac.cpp

if (Method) {
       for (ObjCMethodDecl::param_const_iterator i = Method->param_begin(),
            e = Method->param_end(); i != e; ++i, ++I) {
         const ParmVarDecl *ParamDecl = (*i);
         if (ParamDecl->hasAttr<NSConsumedAttr>()) {
           RValue RV = I->getRValue(CGF);
           assert(RV.isScalar() &&
                  "NullReturnState::complete - arg not on object");
           CGF.EmitARCRelease(RV.getScalarVal(), ARCImpreciseLifetime);
+        } else {
+          QualType QT = ParamDecl->getType();
+          auto *RT = QT->getAs<RecordType>();
+          if (RT && RT->getDecl()->isParamDestroyedInCallee()) {
+            RValue RV = I->getRValue(CGF);
+            QualType::DestructionKind DtorKind = QT.isDestructedType();
+            switch (DtorKind) {
+            case QualType::DK_cxx_destructor:
+              CGF.destroyCXXObject(CGF, RV.getAggregateAddress(), QT);
+              break;
+            case QualType::DK_nontrivial_c_struct:
+              CGF.destroyNonTrivialCStruct(CGF, RV.getAggregateAddress(), QT);
+              break;
+            default:
+              llvm_unreachable("unexpected dtor kind");
+              break;
+            }
+          }
         }
       }
     }
 
     // The phi code below assumes that we haven't needed any control flow yet.
     assert(CGF.Builder.GetInsertBlock() == NullBB);
 
     // If we've got a void return, just jump to the continuation block.
CGObjCCommonMac::EmitMessageSend(CodeGen::CodeGenFunction &CGF,
                                  ReturnValueSlot Return,
                                  QualType ResultType,
                                  Selector Sel,
                                  llvm::Value *Arg0,
                                  QualType Arg0Ty,
                                  bool IsSuper,
                                  const CallArgList &CallArgs,
                                  const ObjCMethodDecl *Method,
                                  const ObjCInterfaceDecl *ClassReceiver,
                                  const ObjCCommonTypesHelper &ObjCTypes) {
   CodeGenTypes &Types = CGM.getTypes();
   auto selTy = CGF.getContext().getObjCSelType();
   llvm::Value *SelValue;
 
   if (Method && Method->isDirectMethod()) {
     // Direct methods will synthesize the proper `_cmd` internally,
     // so just don't bother with setting the `_cmd` argument.
CGObjCCommonMac::EmitMessageSend(CodeGen::CodeGenFunction &CGF,
   // We don't need to emit a null check to zero out an indirect result if the
   // result is ignored.
   if (Return.isUnused())
     RequiresNullCheck = false;
 
   // Emit a null-check if there's a consumed argument other than the receiver.
   if (!RequiresNullCheck && CGM.getLangOpts().ObjCAutoRefCount && Method) {
     for (const auto *ParamDecl : Method->parameters()) {
-      if (ParamDecl->hasAttr<NSConsumedAttr>()) {
+      if (ParamDecl->isDestroyedInCallee()) {
         RequiresNullCheck = true;
         break;
       }
     }
   }
 
   NullReturnState nullReturn;
   if (RequiresNullCheck) {
messageRef = values.finishAndCreateGlobal(messageRefName,
                                         llvm::GlobalValue::WeakAnyLinkage);
     messageRef->setVisibility(llvm::GlobalValue::HiddenVisibility);
     messageRef->setSection(GetSectionName("__objc_msgrefs", "coalesced"));
   }
 
   bool requiresnullCheck = false;
   if (CGM.getLangOpts().ObjCAutoRefCount && method)
     for (const auto *ParamDecl : method->parameters()) {
-      if (ParamDecl->hasAttr<NSConsumedAttr>()) {
+      if (ParamDecl->isDestroyedInCallee()) {
         if (!nullReturn.NullBB)
           nullReturn.init(CGF, arg0);
         requiresnullCheck = true;
         break;
       }
     }
 
   Address mref =

clang/test/CodeGenObjC/objc-dispatch-null-check.m

+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fobjc-dispatch-method=non-legacy -fobjc-arc -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_STRONG:.*]] = type { i8* }
+
+typedef struct {
+  id x;
+} Strong;
+
+Strong getStrong(void);
+
+@interface I0
+- (void)passStrong:(Strong)a;
+@end
+
+// CHECK-LABEL: define void @test0(
+// CHECK: %[[AGG_TMP:.*]] = alloca %[[STRUCT_STRONG]], align 8
+// CHECK: %[[CALL:.*]] = call i8* @getStrong()
+// CHECK-NEXT: %[[COERCE_DIVE:.*]] = getelementptr inbounds %[[STRUCT_STRONG]], %[[STRUCT_STRONG]]* %[[AGG_TMP]], i32 0, i32 0
+// CHECK-NEXT: store i8* %[[CALL]], i8** %[[COERCE_DIVE]], align 8
+
+// CHECK: %[[MSGSEND_FN:.*]] = load i8*, i8**
+// CHECK: %[[V5:.*]] = bitcast i8* %[[MSGSEND_FN]] to void (i8*, i8*, i8*)*
+// CHECK: %[[COERCE_DIVE1:.*]] = getelementptr inbounds %[[STRUCT_STRONG]], %[[STRUCT_STRONG]]* %[[AGG_TMP]], i32 0, i32 0
+// CHECK: %[[V6:.*]] = load i8*, i8** %[[COERCE_DIVE1]], align 8
+// CHECK: call void %[[V5]]({{.*}}, i8* %[[V6]])
+// CHECK: br
+
+// CHECK: %[[V7:.*]] = bitcast %[[STRUCT_STRONG]]* %[[AGG_TMP]] to i8**
+// CHECK: call void @__destructor_8_s0(i8** %[[V7]])
+// CHECK: br
+
+void test0(I0 *a) {
+  [a passStrong:getStrong()];
+}

clang/test/CodeGenObjC/strong-in-c-struct.m

 StrongSmall getStrongSmall(void);
 StrongOuter getStrongOuter(void);
 StrongOuter2 getStrongOuter2(void);
 void calleeStrongSmall(StrongSmall);
 void func(Strong *);
 
 @interface C
 - (StrongSmall)getStrongSmall;
+- (void)m:(StrongSmall)s;
 + (StrongSmall)getStrongSmallClass;
 @end
 
 id g0;
 StrongSmall g1, g2;
 
 // CHECK: %[[STRUCT_STRONGSMALL:.*]] = type { i32, i8* }
 // CHECK: %[[STRUCT_STRONGOUTER:.*]] = type { %[[STRUCT_STRONG:.*]], i8*, double }

 
 // CHECK-LABEL: define i8* @test_assignment1(
 // CHECK-NOT: call void @__destructor
 
 id test_assignment1(void) {
   calleeStrongSmall(g2 = g1);
 }
 
+// CHECK-LABEL: define void @test_null_reveiver(
+// CHECK: %[[AGG_TMP:.*]] = alloca %[[STRUCT_STRONGSMALL]], align 8
+// CHECK: br i1
+
+// CHECK: %[[V7:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP]] to [2 x i64]*
+// CHECK: %[[V8:.*]] = load [2 x i64], [2 x i64]* %[[V7]], align 8
+// CHECK: call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to void ({{.*}}, [2 x i64] %[[V8]])
+// CHECK: br
+
+// CHECK: %[[V9:.*]] = bitcast %[[STRUCT_STRONGSMALL]]* %[[AGG_TMP]] to i8**
+// CHECK: call void @__destructor_8_s8(i8** %[[V9]]) #4
+// CHECK: br
+
+void test_null_reveiver(C *c) {
+  [c m:getStrongSmall()];
+}
+
 #endif /* USESTRUCT */

clang/test/CodeGenObjC/weak-in-c-struct.m

 // ARM64: %[[DST_ADDR:.*]] = alloca i8**, align 8
 // ARM64: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
 // ARM64: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
 // ARM64: %[[V1:.*]] = bitcast i8** %[[V0]] to i8*
 // ARM64: %[[V2:.*]] = getelementptr inbounds i8, i8* %[[V1]], i64 8
 // ARM64: %[[V3:.*]] = bitcast i8* %[[V2]] to i8**
 // ARM64: call void @llvm.objc.destroyWeak(i8** %[[V3]])
 
+@interface C
+- (void)m:(Weak)a;
+@end
+
 void test_constructor_destructor_Weak(void) {
   Weak t;
 }
 
 // ARM64: define void @test_copy_constructor_Weak(%[[STRUCT_WEAK]]* %{{.*}})
 // ARM64: call void @__copy_constructor_8_8_t0w4_w8(i8** %{{.*}}, i8** %{{.*}})
 // ARM64: call void @__destructor_8_w8(i8** %{{.*}})
 

 // COMMON: %[[V1:.*]] = bitcast %[[STRUCT_WEAK]]* %[[AGG_RESULT]] to i8**
 // COMMON: %[[V2:.*]] = bitcast %[[STRUCT_WEAK]]* %[[V0]] to i8**
 // COMMON: call void @__copy_constructor_{{.*}}(i8** %[[V1]], i8** %[[V2]])
 // COMMON: ret void
 
 Weak test_return_Weak(Weak *a) {
   return *a;
 }
+
+// COMMON-LABEL: define void @test_null_receiver(
+// COMMON: %[[AGG_TMP:.*]] = alloca %[[STRUCT_WEAK]]
+// COMMON: br i1
+
+// COMMON: call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to void (i8*, i8*, %[[STRUCT_WEAK]]*)*)({{.*}}, %[[STRUCT_WEAK]]* %[[AGG_TMP]])
+// COMMON: br
+
+// COMMON: %[[V6:.*]] = bitcast %[[STRUCT_WEAK]]* %[[AGG_TMP]] to i8**
+// COMMON: call void @__destructor_{{.*}}(i8** %[[V6]])
+// COMMON: br
+
+void test_null_receiver(C *c) {
+  [c m:getWeak()];
+}

clang/test/CodeGenObjCXX/objc-struct-cxx-abi.mm

 // RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fobjc-arc  -fobjc-weak -fobjc-runtime-has-weak -emit-llvm -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fobjc-arc  -fobjc-weak -fobjc-runtime-has-weak -fclang-abi-compat=4.0 -emit-llvm -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fobjc-arc  -fobjc-weak -fobjc-runtime-has-weak -emit-llvm -o - -DTRIVIALABI %s | FileCheck %s
 // RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fobjc-arc  -fobjc-weak -fobjc-runtime-has-weak -fclang-abi-compat=4.0 -emit-llvm -o - -DTRIVIALABI %s | FileCheck %s
 
 // Check that structs consisting solely of __strong or __weak pointer fields are
 // destructed in the callee function and structs consisting solely of __strong
 // pointer fields are passed directly.
 
 // CHECK: %[[STRUCT_STRONGWEAK:.*]] = type { i8*, i8* }
 // CHECK: %[[STRUCT_CONTAINSSTRONGWEAK:.*]] = type { %[[STRUCT_STRONGWEAK]] }
 // CHECK: %[[STRUCT_DERIVEDSTRONGWEAK:.*]] = type { %[[STRUCT_STRONGWEAK]] }
 // CHECK: %[[STRUCT_STRONG:.*]] = type { i8* }
 // CHECK: %[[STRUCT_S:.*]] = type { i8* }
 // CHECK: %[[STRUCT_CONTAINSNONTRIVIAL:.*]] = type { %{{.*}}, i8* }
+// CHECK: %[[STRUCT_NONTRIVIAL:.*]] = type { i32* }
 
 #ifdef TRIVIALABI
 struct __attribute__((trivial_abi)) StrongWeak {
 #else
 struct StrongWeak {
 #endif
   id fstrong;
   __weak id fweak;

 
 // This struct is not passed directly nor destructed in the callee because f0
 // has type NonTrivial.
 struct ContainsNonTrivial {
   NonTrivial f0;
   id f1;
 };
 
+@interface C
+- (void)passStrong:(Strong)a;
+- (void)passStrongWeak:(StrongWeak)a;
+- (void)passNonTrivial:(NonTrivial)a;
+@end
+
 // CHECK: define void @_Z19testParamStrongWeak10StrongWeak(%[[STRUCT_STRONGWEAK]]* %{{.*}})
 // CHECK: call %struct.StrongWeak* @_ZN10StrongWeakD1Ev(
 // CHECK-NEXT: ret void
 
 void testParamStrongWeak(StrongWeak a) {
 }
 
 // CHECK: define void @_Z18testCallStrongWeakP10StrongWeak(%[[STRUCT_STRONGWEAK]]* %[[A:.*]])

 
 struct D0 : B0, B1 {
   Strong m0() override;
 };
 
 Strong D0::m0() { return {}; }
 
 }
+
+namespace testNullReceiver {
+
+// CHECK-LABEL: define void @_ZN16testNullReceiver5test0EP1C(
+// CHECK: %[[AGG_TMP:.*]] = alloca %[[STRUCT_STRONG]], align 8
+// CHECK: br i1
+
+// CHECK: %[[COERCE_DIVE:.*]] = getelementptr inbounds %[[STRUCT_STRONG]], %[[STRUCT_STRONG]]* %[[AGG_TMP]], i32 0, i32 0
+// CHECK: %[[V7:.*]] = load i8*, i8** %[[COERCE_DIVE]], align 8
+// CHECK: %[[COERCE_VAL_PI:.*]] = ptrtoint i8* %[[V7]] to i64
+// CHECK: call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to void (i8*, i8*, i64)*)({{.*}}, i64 %[[COERCE_VAL_PI]])
+// CHECK: br
+
+// CHECK: %[[CALL1:.*]] = call %[[STRUCT_STRONG]]* @_ZN6StrongD1Ev(%[[STRUCT_STRONG]]* nonnull dereferenceable(8) %[[AGG_TMP]])
+// CHECK: br
+
+void test0(C *c) {
+  [c passStrong:Strong()];
+}
+
+// CHECK-LABEL: define void @_ZN16testNullReceiver5test1EP1C(
+// CHECK: %[[AGG_TMP:.*]] = alloca %[[STRUCT_STRONGWEAK]], align 8
+// CHECK: br i1
+
+// CHECK: call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to void ({{.*}}, %[[STRUCT_STRONGWEAK]]* %[[AGG_TMP]])
+// CHECK: br
+
+// CHECK: %[[CALL1:.*]] = call %[[STRUCT_STRONGWEAK]]* @_ZN10StrongWeakD1Ev(%[[STRUCT_STRONGWEAK]]* nonnull dereferenceable(16) %[[AGG_TMP]])
+// CHECK: br
+
+void test1(C *c) {
+  [c passStrongWeak:StrongWeak()];
+}
+
+// No null check needed.
+
+// CHECK-LABEL: define void @_ZN16testNullReceiver5test2EP1C(
+// CHECK: %[[AGG_TMP:.*]] = alloca %[[STRUCT_NONTRIVIAL]], align 8
+// CHECK: call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to void (i8*, i8*, %[[STRUCT_NONTRIVIAL]]*)*)({{.*}}, %[[STRUCT_NONTRIVIAL]]* %[[AGG_TMP]])
+// CHECK-NEXT: call %[[STRUCT_NONTRIVIAL]]* @_ZN10NonTrivialD1Ev(%[[STRUCT_NONTRIVIAL]]* nonnull dereferenceable(8) %[[AGG_TMP]])
+
+void test2(C *c) {
+  [c passNonTrivial:NonTrivial()];
+}
+
+}