[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

[JF Bastien via Phabricator via cfe-commits]

jfb created this revision.
jfb added a reviewer: ahatanak.
Herald added subscribers: cfe-commits, dexonsmith, jkorous.
Herald added a project: clang.

Blocks that capture themselves (and escape) after initialization currently codegen wrong because this:

  bool capturedByInit =
      Init && emission.IsEscapingByRef && isCapturedBy(D, Init);
  
  Address Loc =
      capturedByInit ? emission.Addr : emission.getObjectAddress(*this);

Already adjusts Loc from thr alloca to a GEP. This code:

  if (emission.IsEscapingByRef)
    Loc = emitBlockByrefAddress(Loc, &D, /*follow=*/false);

Was trying to do the same adjustment, and a GEP on a GEP (returning an int) triggers an assertion.

rdar://problem/47943027


Repository:
  rC Clang

https://reviews.llvm.org/D58218

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===================================================================
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -45,14 +45,35 @@
 // PATTERN:       %captured1 = getelementptr inbounds %struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 0, i32 4
 // PATTERN-NEXT:  store %struct.XYZ* inttoptr (i64 -6148914691236517206 to %struct.XYZ*), %struct.XYZ** %captured1, align 8
 // PATTERN:       %call = call %struct.XYZ* @create(
+using Block = void (^)();
+typedef struct XYZ {
+  Block block;
+} * xyz_t;
 void test_block_self_init() {
-  using Block = void (^)();
-  typedef struct XYZ {
-    Block block;
-  } * xyz_t;
   extern xyz_t create(Block block);
   __block xyz_t captured = create(^() {
-    (void)captured;
+    used(captured);
+  });
+}
+
+// Capturing with escape after initialization is also an edge case.
+//
+// UNINIT-LABEL:  test_block_captures_self_after_init(
+// ZERO-LABEL:    test_block_captures_self_after_init(
+// ZERO:          %block = alloca <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i8* }>, align 8
+// ZERO:          %captured1 = getelementptr inbounds %struct.__block_byref_captured.1, %struct.__block_byref_captured.1* %captured, i32 0, i32 4
+// ZERO-NEXT:     store %struct.XYZ* null, %struct.XYZ** %captured1, align 8
+// ZERO:          %call = call %struct.XYZ* @create(
+// PATTERN-LABEL: test_block_captures_self_after_init(
+// PATTERN:       %block = alloca <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i8* }>, align 8
+// PATTERN:       %captured1 = getelementptr inbounds %struct.__block_byref_captured.1, %struct.__block_byref_captured.1* %captured, i32 0, i32 4
+// PATTERN-NEXT:  store %struct.XYZ* inttoptr (i64 -6148914691236517206 to %struct.XYZ*), %struct.XYZ** %captured1, align 8
+// PATTERN:       %call = call %struct.XYZ* @create(
+void test_block_captures_self_after_init() {
+  extern xyz_t create(Block block);
+  __block xyz_t captured;
+  captured = create(^() {
+      used(captured);
   });
 }
 
Index: lib/CodeGen/CGDecl.cpp
===================================================================
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1637,7 +1637,7 @@
       return;
 
     // Only initialize a __block's storage: we always initialize the header.
-    if (emission.IsEscapingByRef)
+    if (emission.IsEscapingByRef && isa<llvm::AllocaInst>(Loc.getPointer()))
       Loc = emitBlockByrefAddress(Loc, &D, /*follow=*/false);
 
     CharUnits Size = getContext().getTypeSizeInChars(type);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D58218.186776.patch
Type: text/x-patch
Size: 2608 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190214/c04a1cea/attachment-0001.bin>