[clang] f4d791f - [CodeGen][ObjC] Extend lifetime of ObjC pointers passed to calls to
via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 6 16:47:19 PST 2020
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
More information about the cfe-commits
mailing list