[clang] [Clang] Fix __builtin_dynamic_object_size off by 4 (PR #111015)

Kees Cook via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 4 16:33:01 PDT 2024


kees wrote:

> This is why I believe the gcc behavior is correct. When it knows the size given to `malloc` it uses that. When it doesn't know that it simply returns INT_MAX. When you ask gcc for the `__bdos` of the FAM it will use the `count` to calculate the size.

(nit: `SIZE_MAX`, not `INT_MAX`)

AIUI, GCC considers this to be a bug in the face of `counted_by`. The reason GCC returns "unknown" (`SIZE_MAX`) in the case of a pointer like that is because (prior to `counted_by`) if the pointer wasn't to local storage, it could no know if it was a singleton or not. i.e. it may be pointing into a larger array of objects, so it cannot know the size. (This is most clear for `char *` variables, for example.)

Take a look at this: https://godbolt.org/z/b1ev4eP9G

```
struct foo {
    int counter;
    int array[] __attribute__((counted_by(counter)));
};

struct bar {
    int counter;
    int array[];
};

void __attribute__((noinline)) emit_length(size_t length)
{
    printf("%zu\n", length);
}

// GCC and Clang agree: we cannot know the storage `SIZE_MAX` (correct)
void arg_from_space(struct bar *p)
{
    emit_length(__builtin_dynamic_object_size(p, 1));
} 

// Clang performs the `counted_by` calculation (correct)
// GCC thinks this isn't a singleton, returns `SIZE_MAX` (wrong)
void where_did_my_argument_come_from(struct foo *p)
{
    emit_length(__builtin_dynamic_object_size(p, 1));
}

// GCC and Clang agree: 4 (correct)
void local_storage_zero(void)
{
    struct foo local = { .counter = 0 };
    struct foo *p = &local;

    emit_length(__builtin_dynamic_object_size(p, 1));
}

// GCC says 4 (correct)
// Clang say 8 (wrong)
void local_storage_lies(void)
{
    struct foo local = { .counter = 1 };
    struct foo *p = &local;

    emit_length(__builtin_dynamic_object_size(p, 1));
}
```

This shows how GCC will adjust the `__bdos` when it is certain of the object being a singleton, but it still missing the "counted_by requires a singleton" logic. (See also `-Wflexible-array-member-not-at-end` in GCC). It also shows that Clang's `__bdos` can be lied to.

https://github.com/llvm/llvm-project/pull/111015


More information about the cfe-commits mailing list