[clang] dc4e25d - [CodeGen][ObjC] Don't try to retain a __unsafe_unretained ARC pointer
Akira Hatanaka via cfe-commits
cfe-commits at lists.llvm.org
Wed May 6 12:47:38 PDT 2020
Author: Akira Hatanaka
Date: 2020-05-06T12:47:17-07:00
New Revision: dc4e25d4f238afb171aacb3774e1162c69574a0a
URL: https://github.com/llvm/llvm-project/commit/dc4e25d4f238afb171aacb3774e1162c69574a0a
DIFF: https://github.com/llvm/llvm-project/commit/dc4e25d4f238afb171aacb3774e1162c69574a0a.diff
LOG: [CodeGen][ObjC] Don't try to retain a __unsafe_unretained ARC pointer
passed to __builtin_os_log_format to extend its lifetime to the end of
its enclosing block
Extend only lifetimes of pointers returned by function calls or message
sends instead. In the long term, we should lifetime-extend pointers in
more complex expressions and non-ARC objects (e.g., C++ temporaries)
too.
rdar://problem/61846261
Added:
Modified:
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGenObjC/os_log.m
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index f5530aac6085..112a0ee7752f 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -1340,13 +1340,24 @@ RValue CodeGenFunction::emitBuiltinOSLogFormat(const CallExpr &E) {
} else if (const Expr *TheExpr = Item.getExpr()) {
ArgVal = EmitScalarExpr(TheExpr, /*Ignore*/ false);
- // 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 a temporary object that requires destruction after the full
+ // expression is passed, push a lifetime-extended cleanup to extend its
+ // lifetime to the end of the enclosing block scope.
+ auto LifetimeExtendObject = [&](const Expr *E) {
+ E = E->IgnoreParenCasts();
+ // Extend lifetimes of objects returned by function calls and message
+ // sends.
+
+ // FIXME: We should do this in other cases in which temporaries are
+ // created including arguments of non-ARC types (e.g., C++
+ // temporaries).
+ if (isa<CallExpr>(E) || isa<ObjCMessageExpr>(E))
+ return true;
+ return false;
+ };
+
if (TheExpr->getType()->isObjCRetainableType() &&
- getLangOpts().ObjCAutoRefCount) {
+ getLangOpts().ObjCAutoRefCount && LifetimeExtendObject(TheExpr)) {
assert(getEvaluationKind(TheExpr->getType()) == TEK_Scalar &&
"Only scalar can be a ObjC retainable type");
if (!isa<Constant>(ArgVal)) {
diff --git a/clang/test/CodeGenObjC/os_log.m b/clang/test/CodeGenObjC/os_log.m
index 4e70ef3e7574..6f9db76dcc9e 100644
--- a/clang/test/CodeGenObjC/os_log.m
+++ b/clang/test/CodeGenObjC/os_log.m
@@ -9,6 +9,11 @@
// rdar://problem/24528966
+ at interface C
+- (id)m0;
++ (id)m1;
+ at end
+
@class NSString;
extern __attribute__((visibility("default"))) NSString *GenString();
@@ -53,10 +58,10 @@
// 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:.*]] = bitcast %0* %[[V3]] to i8*
+ // CHECK-O0: %[[V4:.*]] = bitcast %{{.*}}* %[[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: %[[V6:.*]] = bitcast i8* %[[V5]] to %{{.*}}*
+ // CHECK-O0: %[[V7:.*]] = ptrtoint %{{.*}}* %[[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({{.*}}
@@ -94,8 +99,7 @@
// 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: %[[V6:.*]] = ptrtoint i8* %[[V0]] 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
@@ -104,8 +108,6 @@
// 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]])
@@ -113,27 +115,23 @@
// 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: %[[OS_LOG_ARG:.*]] = alloca %{{.*}}*, 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: %[[V1:.*]] = bitcast %{{.*}}* %[[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: %[[V3:.*]] = bitcast i8* %[[V2]] to %{{.*}}*
// 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: %[[V6:.*]] = bitcast i8* %[[V5]] to %{{.*}}*
// CHECK-O0: store %{{.*}}* %[[V6]], %{{.*}}** %[[OS_LOG_ARG]], align 8
-// CHECK-O0: %[[V7:.*]] = ptrtoint %[[V0]]* %[[V6]] to i64
+// CHECK-O0: %[[V7:.*]] = ptrtoint %{{.*}}* %[[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: %[[V10:.*]] = ptrtoint i8* %[[V8]] 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)
@@ -146,4 +144,66 @@ void test_builtin_os_log2(void *buf, id a) {
os_log_pack_send(buf);
}
+// CHECK-O0: define void @test_builtin_os_log3(
+// CHECK-O0-NOT: @llvm.objc.retain(
+
+void test_builtin_os_log3(void *buf, id __unsafe_unretained a) {
+ __builtin_os_log_format(buf, "capabilities: %@", a);
+ os_log_pack_send(buf);
+}
+
+// CHECK-NEWPM: define void @test_builtin_os_log4(
+// CHECK-NEWPM: %[[CALL:.*]] = tail call %{{.*}}* (...) @GenString()
+// CHECK-NEWPM: %[[V0:.*]] = bitcast %{{.*}}* %[[CALL]] to i8*
+// CHECK-NEWPM: %[[V1:.*]] = notail call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[V0]])
+// CHECK-NEWPM: %[[V2:.*]] = tail call i8* @llvm.objc.retain(i8* %[[V1]])
+// CHECK-NEWPM: tail call void @llvm.objc.release(i8* %[[V1]])
+// CHECK-NEWPM: tail call void @os_log_pack_send(
+// CHECK-NEWPM: tail call void (...) @llvm.objc.clang.arc.use(i8* %[[V2]])
+// CHECK-NEWPM: tail call void @llvm.objc.release(i8* %[[V2]])
+
+// CHECK-O0: define void @test_builtin_os_log4(
+
+void test_builtin_os_log4(void *buf) {
+ __builtin_os_log_format(buf, "capabilities: %@", (id)GenString());
+ os_log_pack_send(buf);
+}
+
+C *c;
+
+// CHECK-NEWPM: define void @test_builtin_os_log5(
+// CHECK-NEWPM: %[[CALL:.*]] = tail call {{.*}} @objc_msgSend
+// CHECK-NEWPM: %[[V3:.*]] = notail call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[CALL]])
+// CHECK-NEWPM: %[[V4:.*]] = tail call i8* @llvm.objc.retain(i8* %[[V3]])
+// CHECK-NEWPM: %[[CALL1:.*]] = tail call {{.*}} @objc_msgSend
+// CHECK-NEWPM: %[[V9:.*]] = notail call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[CALL1]])
+// CHECK-NEWPM: %[[V10:.*]] = tail call i8* @llvm.objc.retain(i8* %[[V9]])
+// CHECK-NEWPM: tail call void @llvm.objc.release(i8* %[[V9]])
+// CHECK-NEWPM: tail call void @llvm.objc.release(i8* %[[V3]])
+// CHECK-NEWPM: tail call void @os_log_pack_send(
+// CHECK-NEWPM: tail call void (...) @llvm.objc.clang.arc.use(i8* %[[V10]])
+// CHECK-NEWPM: tail call void @llvm.objc.release(i8* %[[V10]])
+// CHECK-NEWPM: tail call void (...) @llvm.objc.clang.arc.use(i8* %[[V4]])
+// CHECK-NEWPM: tail call void @llvm.objc.release(i8* %[[V4]])
+
+void test_builtin_os_log5(void *buf) {
+ __builtin_os_log_format(buf, "capabilities: %@ %@", [c m0], [C m1]);
+ os_log_pack_send(buf);
+}
+
+// FIXME: Lifetime of GenString's return should be extended in this case too.
+
+// CHECK-NEWPM: define void @test_builtin_os_log6(
+// CHECK-NEWPM: %[[CALL:.*]] = tail call %{{.*}} @GenString()
+// CHECK-NEWPM: %[[V0:.*]] = bitcast %[[V1]]* %[[CALL]] to i8*
+// CHECK-NEWPM: %[[V1:.*]] = notail call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %[[V0]])
+// CHECK-NEWPM: tail call void @llvm.objc.release(i8* %[[V1]])
+// CHECK-NEWPM: tail call void @os_log_pack_send(
+// CHECK-NEWPM-NOT: call void @llvm.objc.release(
+
+void test_builtin_os_log6(void *buf) {
+ __builtin_os_log_format(buf, "capabilities: %@", (0, GenString()));
+ os_log_pack_send(buf);
+}
+
#endif
More information about the cfe-commits
mailing list