Skip to content

Commit

Permalink
[CodeGen][ObjC] Don't try to retain a __unsafe_unretained ARC pointer
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@ahatanaka
ahatanaka committed May 6, 2020
1 parent f03b6e7 commit dc4e25d
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 23 deletions.
23 changes: 17 additions & 6 deletions clang/lib/CodeGen/CGBuiltin.cpp
View file
Expand Up @@ -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)) {
Expand Down
94 changes: 77 additions & 17 deletions clang/test/CodeGenObjC/os_log.m
View file
Expand Up @@ -9,6 +9,11 @@

// rdar://problem/24528966

@interface C
- (id)m0;
+ (id)m1;
@end

@class NSString;
extern __attribute__((visibility("default"))) NSString *GenString();

Expand Down Expand Up @@ -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({{.*}}
Expand Down Expand Up @@ -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
Expand All @@ -104,36 +108,30 @@
// 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: %[[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)
Expand All @@ -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

0 comments on commit dc4e25d

Please sign in to comment.