[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