[PATCH] D71431: Call objc_retainBlock before passing a block as a variadic argument
ahatanak created this revision. ahatanak added reviewers: rjmccall, arphaman. ahatanak added a project: clang. Herald added subscribers: ributzka, dexonsmith, jkorous. Copy the block to the heap before passing it to the callee in case the block escapes in the callee. rdar://problem/55683462 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D71431 Files: clang/lib/Sema/SemaExpr.cpp clang/test/CodeGenObjC/arc-blocks.m Index: clang/test/CodeGenObjC/arc-blocks.m === --- clang/test/CodeGenObjC/arc-blocks.m +++ clang/test/CodeGenObjC/arc-blocks.m @@ -730,5 +730,15 @@ test20_callee(^{ (void)x; }); } +// CHECK-LABEL: define void @test21( +// CHECK: %[[V6:.*]] = call i8* @llvm.objc.retainBlock( +// CHECK: %[[V7:.*]] = bitcast i8* %[[V6]] to void ()* +// CHECK: call void (i32, ...) @test21_callee(i32 1, void ()* %[[V7]]), + +void test21_callee(int n, ...); +void test21(id x) { + test21_callee(1, ^{ (void)x; }); +} + // CHECK: attributes [[NUW]] = { nounwind } // CHECK-UNOPT: attributes [[NUW]] = { nounwind } Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -5247,6 +5247,9 @@ for (Expr *A : Args.slice(ArgIx)) { ExprResult Arg = DefaultVariadicArgumentPromotion(A, CallType, FDecl); Invalid |= Arg.isInvalid(); +// Copy blocks to the heap. +if (A->getType()->isBlockPointerType()) + maybeExtendBlockObject(Arg); AllArgs.push_back(Arg.get()); } } Index: clang/test/CodeGenObjC/arc-blocks.m === --- clang/test/CodeGenObjC/arc-blocks.m +++ clang/test/CodeGenObjC/arc-blocks.m @@ -730,5 +730,15 @@ test20_callee(^{ (void)x; }); } +// CHECK-LABEL: define void @test21( +// CHECK: %[[V6:.*]] = call i8* @llvm.objc.retainBlock( +// CHECK: %[[V7:.*]] = bitcast i8* %[[V6]] to void ()* +// CHECK: call void (i32, ...) @test21_callee(i32 1, void ()* %[[V7]]), + +void test21_callee(int n, ...); +void test21(id x) { + test21_callee(1, ^{ (void)x; }); +} + // CHECK: attributes [[NUW]] = { nounwind } // CHECK-UNOPT: attributes [[NUW]] = { nounwind } Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -5247,6 +5247,9 @@ for (Expr *A : Args.slice(ArgIx)) { ExprResult Arg = DefaultVariadicArgumentPromotion(A, CallType, FDecl); Invalid |= Arg.isInvalid(); +// Copy blocks to the heap. +if (A->getType()->isBlockPointerType()) + maybeExtendBlockObject(Arg); AllArgs.push_back(Arg.get()); } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71431: Call objc_retainBlock before passing a block as a variadic argument
rjmccall added a comment. Patch looks good, but could you describe this behavior in the ARC documentation? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71431/new/ https://reviews.llvm.org/D71431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71431: Call objc_retainBlock before passing a block as a variadic argument
ahatanak updated this revision to Diff 233851. ahatanak added a comment. Explain why this change is needed in ARC documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71431/new/ https://reviews.llvm.org/D71431 Files: clang/docs/AutomaticReferenceCounting.rst clang/lib/Sema/SemaExpr.cpp clang/test/CodeGenObjC/arc-blocks.m Index: clang/test/CodeGenObjC/arc-blocks.m === --- clang/test/CodeGenObjC/arc-blocks.m +++ clang/test/CodeGenObjC/arc-blocks.m @@ -730,5 +730,15 @@ test20_callee(^{ (void)x; }); } +// CHECK-LABEL: define void @test21( +// CHECK: %[[V6:.*]] = call i8* @llvm.objc.retainBlock( +// CHECK: %[[V7:.*]] = bitcast i8* %[[V6]] to void ()* +// CHECK: call void (i32, ...) @test21_callee(i32 1, void ()* %[[V7]]), + +void test21_callee(int n, ...); +void test21(id x) { + test21_callee(1, ^{ (void)x; }); +} + // CHECK: attributes [[NUW]] = { nounwind } // CHECK-UNOPT: attributes [[NUW]] = { nounwind } Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -5247,6 +5247,9 @@ for (Expr *A : Args.slice(ArgIx)) { ExprResult Arg = DefaultVariadicArgumentPromotion(A, CallType, FDecl); Invalid |= Arg.isInvalid(); +// Copy blocks to the heap. +if (A->getType()->isBlockPointerType()) + maybeExtendBlockObject(Arg); AllArgs.push_back(Arg.get()); } } Index: clang/docs/AutomaticReferenceCounting.rst === --- clang/docs/AutomaticReferenceCounting.rst +++ clang/docs/AutomaticReferenceCounting.rst @@ -1863,6 +1863,12 @@ ``Block_copy``. The optimizer may remove such copies when it sees that the result is used only as an argument to a call. +When a block pointer type is converted to type ``id``, ``Block_copy`` is called. +This is necessary because a block allocated on the stack won't get copied to the +heap when the pointer converted to ``id`` escapes. The conversion to ``id`` +implicitly takes place when a block is passed to a function as a variadic +argument. + .. _arc.misc.exceptions: Exceptions Index: clang/test/CodeGenObjC/arc-blocks.m === --- clang/test/CodeGenObjC/arc-blocks.m +++ clang/test/CodeGenObjC/arc-blocks.m @@ -730,5 +730,15 @@ test20_callee(^{ (void)x; }); } +// CHECK-LABEL: define void @test21( +// CHECK: %[[V6:.*]] = call i8* @llvm.objc.retainBlock( +// CHECK: %[[V7:.*]] = bitcast i8* %[[V6]] to void ()* +// CHECK: call void (i32, ...) @test21_callee(i32 1, void ()* %[[V7]]), + +void test21_callee(int n, ...); +void test21(id x) { + test21_callee(1, ^{ (void)x; }); +} + // CHECK: attributes [[NUW]] = { nounwind } // CHECK-UNOPT: attributes [[NUW]] = { nounwind } Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -5247,6 +5247,9 @@ for (Expr *A : Args.slice(ArgIx)) { ExprResult Arg = DefaultVariadicArgumentPromotion(A, CallType, FDecl); Invalid |= Arg.isInvalid(); +// Copy blocks to the heap. +if (A->getType()->isBlockPointerType()) + maybeExtendBlockObject(Arg); AllArgs.push_back(Arg.get()); } } Index: clang/docs/AutomaticReferenceCounting.rst === --- clang/docs/AutomaticReferenceCounting.rst +++ clang/docs/AutomaticReferenceCounting.rst @@ -1863,6 +1863,12 @@ ``Block_copy``. The optimizer may remove such copies when it sees that the result is used only as an argument to a call. +When a block pointer type is converted to type ``id``, ``Block_copy`` is called. +This is necessary because a block allocated on the stack won't get copied to the +heap when the pointer converted to ``id`` escapes. The conversion to ``id`` +implicitly takes place when a block is passed to a function as a variadic +argument. + .. _arc.misc.exceptions: Exceptions ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71431: Call objc_retainBlock before passing a block as a variadic argument
rjmccall accepted this revision. rjmccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/docs/AutomaticReferenceCounting.rst:1866 +When a block pointer type is converted to type ``id``, ``Block_copy`` is called. +This is necessary because a block allocated on the stack won't get copied to the "to a non-block pointer type (such as ``id``)", please. Otherwise looks good. I didn't realize that this conversion behavior wasn't documented at all, so thank you for adding it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71431/new/ https://reviews.llvm.org/D71431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71431: Call objc_retainBlock before passing a block as a variadic argument
ahatanak marked an inline comment as done. ahatanak added inline comments. Comment at: clang/docs/AutomaticReferenceCounting.rst:1866 +When a block pointer type is converted to type ``id``, ``Block_copy`` is called. +This is necessary because a block allocated on the stack won't get copied to the rjmccall wrote: > "to a non-block pointer type (such as ``id``)", please. Otherwise looks > good. I didn't realize that this conversion behavior wasn't documented at > all, so thank you for adding it. I thought it was documented too and searched for it, but couldn't find it anywhere in the ARC doc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71431/new/ https://reviews.llvm.org/D71431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D71431: Call objc_retainBlock before passing a block as a variadic argument
This revision was automatically updated to reflect the committed changes. Closed by commit rGa0a670614a36: Call objc_retainBlock before passing a block as a variadic argument (authored by ahatanak). Changed prior to commit: https://reviews.llvm.org/D71431?vs=233851&id=233862#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71431/new/ https://reviews.llvm.org/D71431 Files: clang/docs/AutomaticReferenceCounting.rst clang/lib/Sema/SemaExpr.cpp clang/test/CodeGenObjC/arc-blocks.m Index: clang/test/CodeGenObjC/arc-blocks.m === --- clang/test/CodeGenObjC/arc-blocks.m +++ clang/test/CodeGenObjC/arc-blocks.m @@ -730,5 +730,15 @@ test20_callee(^{ (void)x; }); } +// CHECK-LABEL: define void @test21( +// CHECK: %[[V6:.*]] = call i8* @llvm.objc.retainBlock( +// CHECK: %[[V7:.*]] = bitcast i8* %[[V6]] to void ()* +// CHECK: call void (i32, ...) @test21_callee(i32 1, void ()* %[[V7]]), + +void test21_callee(int n, ...); +void test21(id x) { + test21_callee(1, ^{ (void)x; }); +} + // CHECK: attributes [[NUW]] = { nounwind } // CHECK-UNOPT: attributes [[NUW]] = { nounwind } Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -5247,6 +5247,9 @@ for (Expr *A : Args.slice(ArgIx)) { ExprResult Arg = DefaultVariadicArgumentPromotion(A, CallType, FDecl); Invalid |= Arg.isInvalid(); +// Copy blocks to the heap. +if (A->getType()->isBlockPointerType()) + maybeExtendBlockObject(Arg); AllArgs.push_back(Arg.get()); } } Index: clang/docs/AutomaticReferenceCounting.rst === --- clang/docs/AutomaticReferenceCounting.rst +++ clang/docs/AutomaticReferenceCounting.rst @@ -1863,6 +1863,12 @@ ``Block_copy``. The optimizer may remove such copies when it sees that the result is used only as an argument to a call. +When a block pointer type is converted to a non-block pointer type (such as +``id``), ``Block_copy`` is called. This is necessary because a block allocated +on the stack won't get copied to the heap when the non-block pointer escapes. +A block pointer is implicitly converted to ``id`` when it is passed to a +function as a variadic argument. + .. _arc.misc.exceptions: Exceptions Index: clang/test/CodeGenObjC/arc-blocks.m === --- clang/test/CodeGenObjC/arc-blocks.m +++ clang/test/CodeGenObjC/arc-blocks.m @@ -730,5 +730,15 @@ test20_callee(^{ (void)x; }); } +// CHECK-LABEL: define void @test21( +// CHECK: %[[V6:.*]] = call i8* @llvm.objc.retainBlock( +// CHECK: %[[V7:.*]] = bitcast i8* %[[V6]] to void ()* +// CHECK: call void (i32, ...) @test21_callee(i32 1, void ()* %[[V7]]), + +void test21_callee(int n, ...); +void test21(id x) { + test21_callee(1, ^{ (void)x; }); +} + // CHECK: attributes [[NUW]] = { nounwind } // CHECK-UNOPT: attributes [[NUW]] = { nounwind } Index: clang/lib/Sema/SemaExpr.cpp === --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -5247,6 +5247,9 @@ for (Expr *A : Args.slice(ArgIx)) { ExprResult Arg = DefaultVariadicArgumentPromotion(A, CallType, FDecl); Invalid |= Arg.isInvalid(); +// Copy blocks to the heap. +if (A->getType()->isBlockPointerType()) + maybeExtendBlockObject(Arg); AllArgs.push_back(Arg.get()); } } Index: clang/docs/AutomaticReferenceCounting.rst === --- clang/docs/AutomaticReferenceCounting.rst +++ clang/docs/AutomaticReferenceCounting.rst @@ -1863,6 +1863,12 @@ ``Block_copy``. The optimizer may remove such copies when it sees that the result is used only as an argument to a call. +When a block pointer type is converted to a non-block pointer type (such as +``id``), ``Block_copy`` is called. This is necessary because a block allocated +on the stack won't get copied to the heap when the non-block pointer escapes. +A block pointer is implicitly converted to ``id`` when it is passed to a +function as a variadic argument. + .. _arc.misc.exceptions: Exceptions ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits