Copying block that captures itself in its declaration leads to garbage

[Quuxplusone]

Bugzilla Link	PR8688
Status	REOPENED
Importance	P normal
Reported by	Kevin Ballard (kevin@sb.org)
Reported on	2010-11-24 16:58:05 -0800
Last modified on	2010-11-29 13:48:31 -0800
Version	unspecified
Hardware	Macintosh MacOS X
CC	fjahanian@apple.com, kremenek@apple.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments	copy.c (680 bytes, application/octet-stream)
Blocks
Blocked by
See also

Created attachment 5827
Test case

Calling Block_copy() during the declaration of a block variable that captures
itself results in a garbage value. The following snippet is all that's
necessary to reproduce:

    __block void (^broken)() = Block_copy(^{ (void)broken; });

I've attached a complete file that reproduces this issue when run.

This was reproduced in both Apple clang version 1.6 (tags/Apple/clang-70) and
Apple clang version 2.0 (tags/Apple/clang-121.4) (based on LLVM 2.9svn), in
both x86_64 and i386.

[Quuxplusone]

Attached copy.c (680 bytes, application/octet-stream): Test case

[Quuxplusone]

This is correct behavior, equivalent to:

  int x = x+1;

"don't do that".

-Chris

[Quuxplusone]

Either the block should capture its own variable in its closure, at which point
everything should work fine as the block variable is declared __block and
therefore the block, when run, should be using the "current value" of the
variable, or it shouldn't capture the variable, at which point the compiler
should complain that I'm using a variable that doesn't exist.

   int x = x+1;

is very different, as its declaring x, which currently has garbage data, then
using it, then stuffing the results back into x. That would be more akin to
something like

  __block void (^test)() = ^{ /* do something */ }();

e.g. calling the block before it's assigned to the variable. In this case, the
block is created, then assigned to the variable, and then when it's run, it
should be able to reference that __block variable just fine. As the variable
was declared with the __block qualifier, the block should not have made a const
copy of the garbage value at the time of creation.

[Quuxplusone]

I'm really not sure what proper etiquette is for reopening bugs, so I hope it's ok that I'm reopening this one. As I said in my previous comment, the current behavior is not, in fact, equivalent to that int x case. Either this should work properly, or the compiler should complain. The current behavior is simply not right.

[Quuxplusone]

Ah, I didn't notice that you had marked it __block.   Here's the larger example
from the testcase:

   __block void (^broken)() = Block_copy(^{
            printf("block 'broken': %p\n", (void *)broken);
        });
    broken();
    Block_release(broken);

The issue here is that block pointers are ref counted, and that when blocks are
copied, the refcount is increased.  Here, the block pointer is uninitialized
when the copy is made.

Fariborz, I think that the fix here is to emit the zero initialization of the
"byref alloca" earlier somehow.

[Quuxplusone]

(In reply to comment #4)
> Ah, I didn't notice that you had marked it __block.   Here's the larger
example
> from the testcase:
>
>
>    __block void (^broken)() = Block_copy(^{
>             printf("block 'broken': %p\n", (void *)broken);
>         });
>     broken();
>     Block_release(broken);
>
> The issue here is that block pointers are ref counted, and that when blocks
are
> copied, the refcount is increased.  Here, the block pointer is uninitialized
> when the copy is made.
>
> Fariborz, I think that the fix here is to emit the zero initialization of the
> "byref alloca" earlier somehow.

clang bug. After the call to Block_copy, "broken" need be initialized
indirectly through
"forwarding" field of the block copied as result of call to Block_copy.  This
is not happening,
in this particular use of blocks. Will investigate when I have a chance. Note
that llvm-gcc (and gcc) will not compile such uses of blocks (with internal
diagnostics).