[PATCH] D71431: Call objc_retainBlock before passing a block as a variadic argument

[Akira Hatanaka via Phabricator via cfe-commits]

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

[John McCall via Phabricator via cfe-commits]

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

[Akira Hatanaka via Phabricator via cfe-commits]

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

[John McCall via Phabricator via cfe-commits]

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

[Akira Hatanaka via Phabricator via cfe-commits]

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

[Akira Hatanaka via Phabricator via cfe-commits]

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