Author: Akira Hatanaka Date: 2020-03-06T16:46:50-08:00 New Revision: f4d791f8332c2bb7e89849d0fe4ef48cb0a23229
URL: https://github.com/llvm/llvm-project/commit/f4d791f8332c2bb7e89849d0fe4ef48cb0a23229 DIFF: https://github.com/llvm/llvm-project/commit/f4d791f8332c2bb7e89849d0fe4ef48cb0a23229.diff LOG: [CodeGen][ObjC] Extend lifetime of ObjC pointers passed to calls to __builtin_os_log_format This is needed to keep all the objects, including temporaries returned by function calls, written to the buffer alive until os_log_pack_send is called. rdar://problem/60105410 Added: Modified: clang/lib/CodeGen/CGBuiltin.cpp clang/lib/Sema/SemaChecking.cpp clang/test/CodeGenObjC/os_log.m Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp index 417b308d7a22..952cc3f0c9b8 100644 --- a/clang/lib/CodeGen/CGBuiltin.cpp +++ b/clang/lib/CodeGen/CGBuiltin.cpp @@ -1320,14 +1320,30 @@ RValue CodeGenFunction::emitBuiltinOSLogFormat(const CallExpr &E) { } else if (const Expr *TheExpr = Item.getExpr()) { ArgVal = EmitScalarExpr(TheExpr, /*Ignore*/ false); - // Check if this is a retainable type. + // If this is a retainable type, push a lifetime-extended cleanup to + // ensure the lifetime of the argument is extended to the end of the + // enclosing block scope. + // FIXME: We only have to do this if the argument is a temporary, which + // gets released after the full expression. if (TheExpr->getType()->isObjCRetainableType()) { assert(getEvaluationKind(TheExpr->getType()) == TEK_Scalar && "Only scalar can be a ObjC retainable type"); - // Check if the object is constant, if not, save it in - // RetainableOperands. - if (!isa<Constant>(ArgVal)) - RetainableOperands.push_back(ArgVal); + if (!isa<Constant>(ArgVal)) { + CleanupKind Cleanup = getARCCleanupKind(); + QualType Ty = TheExpr->getType(); + Address Alloca = Address::invalid(); + Address Addr = CreateMemTemp(Ty, "os.log.arg", &Alloca); + ArgVal = EmitARCRetain(Ty, ArgVal); + Builder.CreateStore(ArgVal, Addr); + pushLifetimeExtendedDestroy(Cleanup, Alloca, Ty, + CodeGenFunction::destroyARCStrongPrecise, + Cleanup & EHCleanup); + + // Push a clang.arc.use call to ensure ARC optimizer knows that the + // argument has to be alive. + if (CGM.getCodeGenOpts().OptimizationLevel != 0) + pushCleanupAfterFullExpr<CallObjCArcUse>(Cleanup, ArgVal); + } } } else { ArgVal = Builder.getInt32(Item.getConstValue().getQuantity()); @@ -1349,18 +1365,6 @@ RValue CodeGenFunction::emitBuiltinOSLogFormat(const CallExpr &E) { llvm::Function *F = CodeGenFunction(CGM).generateBuiltinOSLogHelperFunction( Layout, BufAddr.getAlignment()); EmitCall(FI, CGCallee::forDirect(F), ReturnValueSlot(), Args); - - // Push a clang.arc.use cleanup for each object in RetainableOperands. The - // cleanup will cause the use to appear after the final log call, keeping - // the object valid while it’s held in the log buffer. Note that if there’s - // a release cleanup on the object, it will already be active; since - // cleanups are emitted in reverse order, the use will occur before the - // object is released. - if (!RetainableOperands.empty() && getLangOpts().ObjCAutoRefCount && - CGM.getCodeGenOpts().OptimizationLevel != 0) - for (llvm::Value *Object : RetainableOperands) - pushFullExprCleanup<CallObjCArcUse>(getARCCleanupKind(), Object); - return RValue::get(BufAddr.getPointer()); } diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index cda6910364e5..106e90f4c44c 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -1843,6 +1843,8 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID, return ExprError(); break; case Builtin::BI__builtin_os_log_format: + Cleanup.setExprNeedsCleanups(true); + LLVM_FALLTHROUGH; case Builtin::BI__builtin_os_log_format_buffer_size: if (SemaBuiltinOSLogFormat(TheCall)) return ExprError(); diff --git a/clang/test/CodeGenObjC/os_log.m b/clang/test/CodeGenObjC/os_log.m index d41a4ce346db..1dd39eebf383 100644 --- a/clang/test/CodeGenObjC/os_log.m +++ b/clang/test/CodeGenObjC/os_log.m @@ -24,7 +24,8 @@ // CHECK: %[[V0:.*]] = bitcast %[[TY0]]* %[[CALL]] to i8* // CHECK: %[[V1:.*]] = notail call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[V0]]) // CHECK-LEGACY: %[[V2:.*]] = ptrtoint %[[TY0]]* %[[CALL]] to i64 - // CHECK-NEWPM: %[[V2:.*]] = ptrtoint i8* %[[V1]] to i64 + // CHECK-NEWPM: %[[RETAINED:.*]] = tail call i8* @llvm.objc.retain(i8* %[[V1]]) + // CHECK-NEWPM: %[[V2:.*]] = ptrtoint i8* %[[RETAINED]] to i64 // CHECK: store i8 2, i8* %[[BUF]], align 1 // CHECK: %[[NUMARGS_I:.*]] = getelementptr i8, i8* %[[BUF]], i64 1 // CHECK: store i8 1, i8* %[[NUMARGS_I]], align 1 @@ -37,8 +38,8 @@ // CHECK: store i64 %[[V2]], i64* %[[ARGDATACAST_I]], align 1 // CHECK-LEGACY: tail call void (...) @llvm.objc.clang.arc.use(%[[TY0]]* %[[CALL]]) // CHECK-LEGACY: tail call void @llvm.objc.release(i8* %[[V0]]) - // CHECK-NEWPM: tail call void (...) @llvm.objc.clang.arc.use(i8* %[[V1]]) - // CHECK-NEWPM: tail call void @llvm.objc.release(i8* %[[V1]]) + // CHECK-NEWPM: tail call void (...) @llvm.objc.clang.arc.use(i8* %[[RETAINED]]) + // CHECK-NEWPM: tail call void @llvm.objc.release(i8* %[[RETAINED]]) // CHECK: ret i8* %[[BUF]] // clang.arc.use is used and removed in IR optimizations. At O0, we should not @@ -51,8 +52,11 @@ // CHECK-O0: %[[V1:.*]] = bitcast %[[TY0]]* %[[CALL]] to i8* // CHECK-O0: %[[V2:.*]] = notail call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[V1]]) // CHECK-O0: %[[V3:.*]] = bitcast i8* %[[V2]] to %[[TY0]]* - // CHECK-O0: %[[V4:.*]] = ptrtoint %[[TY0]]* %[[V3]] to i64 - // CHECK-O0: call void @__os_log_helper_1_2_1_8_64(i8* %[[V0]], i64 %[[V4]]) + // CHECK-O0: %[[V4:.*]] = bitcast %0* %[[V3]] to i8* + // CHECK-O0: %[[V5:.*]] = call i8* @llvm.objc.retain(i8* %[[V4]]) + // CHECK-O0: %[[V6:.*]] = bitcast i8* %[[V5]] to %0* + // CHECK-O0: %[[V7:.*]] = ptrtoint %0* %[[V6]] to i64 + // CHECK-O0: call void @__os_log_helper_1_2_1_8_64(i8* %[[V0]], i64 %[[V7]]) // CHECK-O0: %[[V5:.*]] = bitcast %[[TY0]]* %[[V3]] to i8* // CHECK-O0-NOT: call void (...) @llvm.objc.clang.arc.use({{.*}} // CHECK-O0: call void @llvm.objc.release(i8* %[[V5]]) @@ -80,4 +84,62 @@ // CHECK-O0: %[[V0:.*]] = load i64, i64* %[[ARG0_ADDR]], align 8 // CHECK-O0: store i64 %[[V0]], i64* %[[ARGDATACAST]], align 1 +void os_log_pack_send(void *); + +// CHECK-NEWPM: define void @test_builtin_os_log2(i8* %[[BUF:.*]], i8* %[[A:.*]]) +// CHECK-NEWPM: %[[V0:.*]] = tail call i8* @llvm.objc.retain(i8* %[[A]]) +// CHECK-NEWPM: %[[CALL:.*]] = tail call %{{.*}}* (...) @GenString() +// CHECK-NEWPM: %[[V1:.*]] = bitcast %{{.*}}* %[[CALL]] to i8* +// CHECK-NEWPM: %[[V2:.*]] = notail call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[V1]]) +// CHECK-NEWPM: %[[V3:.*]] = tail call i8* @llvm.objc.retain(i8* %[[V2]]) +// CHECK-NEWPM: %[[V4:.*]] = ptrtoint i8* %[[V3]] to i64 +// CHECK-NEWPM: %[[V5:.*]] = tail call i8* @llvm.objc.retain(i8* %[[V0]]) +// CHECK-NEWPM: %[[V6:.*]] = ptrtoint i8* %[[V5]] to i64 +// CHECK-NEWPM: %[[ARGDATA_I:.*]] = getelementptr i8, i8* %[[BUF]], i64 4 +// CHECK-NEWPM: %[[ARGDATACAST_I:.*]] = bitcast i8* %[[ARGDATA_I]] to i64* +// CHECK-NEWPM: store i64 %[[V4]], i64* %[[ARGDATACAST_I]], align 1 +// CHECK-NEWPM: %[[ARGDATA3_I:.*]] = getelementptr i8, i8* %[[BUF]], i64 14 +// CHECK-NEWPM: %[[ARGDATACAST4_I:.*]] = bitcast i8* %[[ARGDATA3_I]] to i64* +// CHECK-NEWPM: store i64 %[[V6]], i64* %[[ARGDATACAST4_I]], align 1 +// CHECK-NEWPM: tail call void @llvm.objc.release(i8* %[[V2]]) +// CHECK-NEWPM: tail call void @os_log_pack_send(i8* nonnull %[[BUF]]) +// CHECK-NEWPM: tail call void (...) @llvm.objc.clang.arc.use(i8* %[[V5]]) +// CHECK-NEWPM: tail call void @llvm.objc.release(i8* %[[V5]]) +// CHECK-NEWPM: tail call void (...) @llvm.objc.clang.arc.use(i8* %[[V3]]) +// CHECK-NEWPM: tail call void @llvm.objc.release(i8* %[[V3]]) +// CHECK-NEWPM: tail call void @llvm.objc.release(i8* %[[V0]]) + +// CHECK-O0: define void @test_builtin_os_log2(i8* %{{.*}}, i8* %[[A:.*]]) +// CHECK-O0: alloca i8*, align 8 +// CHECK-O0: %[[A_ADDR:.*]] = alloca i8*, align 8 +// CHECK-O0: %[[OS_LOG_ARG:.*]] = alloca %[[V0]]*, align 8 +// CHECK-O0: %[[OS_LOG_ARG1:.*]] = alloca i8*, align 8 +// CHECK-O0: call void @llvm.objc.storeStrong(i8** %[[A_ADDR]], i8* %[[A]]) +// CHECK-O0: %[[CALL:.*]] = call %{{.*}}* (...) @GenString() +// CHECK-O0: %[[V1:.*]] = bitcast %[[V0]]* %[[CALL]] to i8* +// CHECK-O0: %[[V2:.*]] = notail call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[V1]]) #2 +// CHECK-O0: %[[V3:.*]] = bitcast i8* %[[V2]] to %[[V0]]* +// CHECK-O0: %[[V4:.*]] = bitcast %{{.*}}* %[[V3]] to i8* +// CHECK-O0: %[[V5:.*]] = call i8* @llvm.objc.retain(i8* %[[V4]]) #2 +// CHECK-O0: %[[V6:.*]] = bitcast i8* %[[V5]] to %[[V0]]* +// CHECK-O0: store %{{.*}}* %[[V6]], %{{.*}}** %[[OS_LOG_ARG]], align 8 +// CHECK-O0: %[[V7:.*]] = ptrtoint %[[V0]]* %[[V6]] to i64 +// CHECK-O0: %[[V8:.*]] = load i8*, i8** %[[A_ADDR]], align 8 +// CHECK-O0: %[[V9:.*]] = call i8* @llvm.objc.retain(i8* %[[V8]]) #2 +// CHECK-O0: store i8* %[[V9]], i8** %[[OS_LOG_ARG1]], align 8 +// CHECK-O0: %[[V10:.*]] = ptrtoint i8* %[[V9]] to i64 +// CHECK-O0: call void @__os_log_helper_1_2_2_8_64_8_64(i8* %{{.*}}, i64 %[[V7]], i64 %[[V10]]) +// CHECK-O0: %[[V11:.*]] = bitcast %{{.*}}* %[[V3]] to i8* +// CHECK-O0: call void @llvm.objc.release(i8* %[[V11]]) +// CHECK-O0: call void @os_log_pack_send( +// CHECK-O0: call void @llvm.objc.storeStrong(i8** %[[OS_LOG_ARG1]], i8* null) +// CHECK-O0: %[[V13:.*]] = bitcast %{{.*}}** %[[OS_LOG_ARG]] to i8** +// CHECK-O0: call void @llvm.objc.storeStrong(i8** %[[V13]], i8* null) +// CHECK-O0: call void @llvm.objc.storeStrong(i8** %[[A_ADDR]], i8* null) + +void test_builtin_os_log2(void *buf, id a) { + __builtin_os_log_format(buf, "capabilities: %@ %@", GenString(), a); + os_log_pack_send(buf); +} + #endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
