llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.03k stars 11.57k forks source link

`__attribute__((cleanup))` does not respect `__block` qualifier #19695

Open davidchisnall opened 10 years ago

davidchisnall commented 10 years ago
Bugzilla Link 19321
Version trunk
OS All

Extended Description

The following simple test program:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <Block.h>

typedef int (^block_t)(void);

static void freestr(char **s)
{
    if (s)
    {
        printf("Freeing block: %s\n", *s);
        free(*s);
        *s = 0;
    }
}

block_t getCounter(char *name)
{
    __attribute__((cleanup(freestr)))
    __block char *n = strdup(name);
    __block int c=0;
    return Block_copy(^()
    {
        printf("%s called %d times\n", n, ++c);
        return c;
    });
}

int main(void)
{
    block_t counter = getCounter("A counter");
    counter();
    counter();
    counter();
    counter();
    Block_release(counter);
    return 0;
}

When run, produces:

Freeing block: A counter
(null) called 1 times
(null) called 2 times
(null) called 3 times
(null) called 4 times

The lifetime of the bound variable is the lifetime of the block, but the cleanup code runs sooner. The analogous C++ example (an object with a destructor) runs correctly.

CodingMarkus commented 1 year ago

I think the behavior of clang is correct. cleanup is documented to call the clean-up function as soon as the variable ceases to exist and the variable captured by the block is not the same variable as the one defined outside of the block, despite both using the same name. That's because n is a stack variable and thus ceases to exist when the function getCounter() returns. The variable captured inside the block is in fact a struct that looks like this:

struct Block_byref {
    void * isa;
    struct Block_byref * forwarding;
    int flags; /* refcount; */
    int size;
    void (* byref_keep)(struct Block_byref * dst, struct Block_byref * src);
    void (* byref_destroy)(struct Block_byref *);
    char * n; // <-- This is the variable value
};

This struct is initially on the stack and n points to it. When you call block copy, it is moved to the heap. Check out this code:

#include <stdio.h>
#include <stdlib.h>
#include <Block.h>

int main ( ) {
    __block int x = 10;
    printf("x is at location %p\n", &x);

    __auto_type block = ^{
        printf("x in block is at location %p\n", &x);
    };
    block();

    block = Block_copy(block);
    block();

    printf("x is at location %p\n", &x);
    return 0;
}

Output:

x is at location 0x16ef475f0
x in block is at location 0x16ef475f0
x in block is at location 0x60000365d118
x is at location 0x60000365d118

You can see how Block_copy() changes the address of the value. After Block_copy() has been called, the stack variable x and the block variable x both point to the same struct but when the current scope ends, the stack variable x ceases to exist and thus must have its clean-up function called. That the block also has a variable with that name is irrelevant, it's a different variable in a different scope.

Consider this example:

#include <stdlib.h>
#include <stdio.h>
#include <Block.h>

static
void freePtr ( int ** ptr )
{
    printf("freePtr: %p\n", *ptr);
    free(*ptr);
}

#define autofree __attribute__((cleanup(freePtr)))

static
void makePtr ( int ** outPtr )
{
    autofree int * intptr = malloc(sizeof(int));
    *intptr = 10;
    *outPtr = intptr;
}

int main ( )
{
    int * intptr = NULL;
    printf("Calling makePtr\n");
    makePtr(&intptr);
    printf("Returned from makePtr: %p\n", intptr);
    return 0;
}

Output:

freePtr: 0x60000265c030
Returned from makePtr: 0x60000265c030

The pointer is freed as soon as makePtr() returns, as that's where intptr goes out of scope and ceases to exist. The fact that the caller function (here main()) still has a reference to this memory location captured (even one with the same name) plays no role. cleanup does not support reference counting and it's not about memory pointers point to, it's about the variable itself that just holds the address value.

So I'd vote for closing this issue as I see the described behavior to match the behavior you'll get pretty much everywhere else as well: Variable goes out of scope, clean-up function is called. Blocks do not extend scope, blocks create their own independent scope.