Author: Akira Hatanaka Date: 2021-01-21T17:38:46-08:00 New Revision: 3d349ed7e1108686271a09314dafaa356df4006d
URL: https://github.com/llvm/llvm-project/commit/3d349ed7e1108686271a09314dafaa356df4006d DIFF: https://github.com/llvm/llvm-project/commit/3d349ed7e1108686271a09314dafaa356df4006d.diff LOG: [CodeGen][ObjC] Fix broken IR generated when there is a nil receiver check This patch fixes a bug in emitARCOperationAfterCall where it inserts the fall-back call after a bitcast instruction and then replaces the bitcast's operand with the result of the fall-back call. The generated IR without this patch looks like this: msgSend.call: ; preds = %entry %call = call i8* bitcast (i8* (i8*, i8*, ...)* @objc_msgSend br label %msgSend.cont msgSend.null-receiver: ; preds = %entry call void @llvm.objc.release(i8* %4) br label %msgSend.cont msgSend.cont: %8 = phi i8* [ %call, %msgSend.call ], [ null, %msgSend.null-receiver ] %9 = bitcast i8* %10 to %0* %10 = call i8* @llvm.objc.retain(i8* %8) Notice that `%9 = bitcast i8* %10` to %0* is taking operand %10 which is defined after it. To fix the bug, this patch modifies the insert point to point to the bitcast instruction so that the fall-back call is inserted before the bitcast. In addition, it teaches the function to look at phi instructions that are generated when there is a check for a null receiver and insert the retainRV/claimRV instruction right after the call instead of inserting a fall-back call right after the phi instruction. rdar://73360225 Differential Revision: https://reviews.llvm.org/D95181 Added: Modified: clang/lib/CodeGen/CGObjC.cpp clang/test/CodeGenObjC/ns_consume_null_check.m Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CGObjC.cpp b/clang/lib/CodeGen/CGObjC.cpp index bdb9f4002f3c..3f930c76fe0a 100644 --- a/clang/lib/CodeGen/CGObjC.cpp +++ b/clang/lib/CodeGen/CGObjC.cpp @@ -2894,45 +2894,57 @@ typedef llvm::function_ref<llvm::Value *(CodeGenFunction &CGF, ValueTransform; /// Insert code immediately after a call. + +// FIXME: We should find a way to emit the runtime call immediately +// after the call is emitted to eliminate the need for this function. static llvm::Value *emitARCOperationAfterCall(CodeGenFunction &CGF, llvm::Value *value, ValueTransform doAfterCall, ValueTransform doFallback) { - if (llvm::CallInst *call = dyn_cast<llvm::CallInst>(value)) { - CGBuilderTy::InsertPoint ip = CGF.Builder.saveIP(); + CGBuilderTy::InsertPoint ip = CGF.Builder.saveIP(); + if (llvm::CallInst *call = dyn_cast<llvm::CallInst>(value)) { // Place the retain immediately following the call. CGF.Builder.SetInsertPoint(call->getParent(), ++llvm::BasicBlock::iterator(call)); value = doAfterCall(CGF, value); - - CGF.Builder.restoreIP(ip); - return value; } else if (llvm::InvokeInst *invoke = dyn_cast<llvm::InvokeInst>(value)) { - CGBuilderTy::InsertPoint ip = CGF.Builder.saveIP(); - // Place the retain at the beginning of the normal destination block. llvm::BasicBlock *BB = invoke->getNormalDest(); CGF.Builder.SetInsertPoint(BB, BB->begin()); value = doAfterCall(CGF, value); - CGF.Builder.restoreIP(ip); - return value; - // Bitcasts can arise because of related-result returns. Rewrite // the operand. } else if (llvm::BitCastInst *bitcast = dyn_cast<llvm::BitCastInst>(value)) { + // Change the insert point to avoid emitting the fall-back call after the + // bitcast. + CGF.Builder.SetInsertPoint(bitcast->getParent(), bitcast->getIterator()); llvm::Value *operand = bitcast->getOperand(0); operand = emitARCOperationAfterCall(CGF, operand, doAfterCall, doFallback); bitcast->setOperand(0, operand); - return bitcast; - - // Generic fall-back case. + value = bitcast; } else { - // Retain using the non-block variant: we never need to do a copy - // of a block that's been returned to us. - return doFallback(CGF, value); + auto *phi = dyn_cast<llvm::PHINode>(value); + if (phi && phi->getNumIncomingValues() == 2 && + isa<llvm::ConstantPointerNull>(phi->getIncomingValue(1)) && + isa<llvm::CallBase>(phi->getIncomingValue(0))) { + // Handle phi instructions that are generated when it's necessary to check + // whether the receiver of a message is null. + llvm::Value *inVal = phi->getIncomingValue(0); + inVal = emitARCOperationAfterCall(CGF, inVal, doAfterCall, doFallback); + phi->setIncomingValue(0, inVal); + value = phi; + } else { + // Generic fall-back case. + // Retain using the non-block variant: we never need to do a copy + // of a block that's been returned to us. + value = doFallback(CGF, value); + } } + + CGF.Builder.restoreIP(ip); + return value; } /// Given that the given expression is some sort of call (which does diff --git a/clang/test/CodeGenObjC/ns_consume_null_check.m b/clang/test/CodeGenObjC/ns_consume_null_check.m index 3a0aa6f7c596..e02654d4e21b 100644 --- a/clang/test/CodeGenObjC/ns_consume_null_check.m +++ b/clang/test/CodeGenObjC/ns_consume_null_check.m @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -fobjc-dispatch-method=mixed -fobjc-runtime-has-weak -fexceptions -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -fobjc-dispatch-method=mixed -fobjc-runtime-has-weak -fexceptions -fobjc-exceptions -o - %s | FileCheck %s @interface NSObject - (id) new; @@ -7,6 +7,7 @@ - (id) new; @interface MyObject : NSObject - (char)isEqual:(id) __attribute__((ns_consumed)) object; - (_Complex float) asComplexWithArg: (id) __attribute__((ns_consumed)) object; ++(instancetype)m0:(id) __attribute__((ns_consumed)) object; @end MyObject *x; @@ -82,4 +83,27 @@ void test1(void) { // CHECK: landingpad // CHECK: call void @llvm.objc.destroyWeak(i8** [[WEAKOBJ]]) [[NUW]] +void test2(id a) { + id obj = [MyObject m0:a]; +} + +// CHECK-LABEL: define{{.*}} void @test2( +// CHECK: %[[CALL:.*]] = call i8* bitcast (i8* (i8*, i8*, ...)* @objc_msgSend +// CHECK-NEXT: %[[V6:.*]] = {{.*}}call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[CALL]]) + +// CHECK: phi i8* [ %[[V6]], %{{.*}} ], [ null, %{{.*}} ] + +void test3(id a) { + @try { + id obj = [MyObject m0:a]; + } @catch (id x) { + } +} + +// CHECK-LABEL: define{{.*}} void @test3( +// CHECK: %[[CALL:.*]] = invoke i8* bitcast (i8* (i8*, i8*, ...)* @objc_msgSend +// CHECK: %[[V6:.*]] = {{.*}}call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[CALL]]) + +// CHECK: phi i8* [ %[[V6]], %{{.*}} ], [ null, %{{.*}} ] + // CHECK: attributes [[NUW]] = { nounwind } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits