[llvm] [clang] [Clang] Correct __builtin_dynamic_object_size for subobject types (PR #78526)

Bill Wendling via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 19 13:46:24 PST 2024


bwendling wrote:

> @bwendling I think you are reading the GCC docs too pedantically. In particular, they also say

> > If there are multiple objects ptr can point to and all of them are known at compile time, the returned number is the maximum of remaining byte counts in those objects if type & 2 is 0 and minimum if nonzero.

Isn't this referring to pointing to an object in a union? That's only way this makes sense (in C).

> which makes it abundantly clear that what you get is an upper bound or lower bound, respectively. -1 and 0 are just the upper and lower bounds if you have no useful information at all. If you want to check whether the size is _exactly_ known, you'll have to compare both bounds. Outside of doing that, you can never assume that the bound is precise.

I'm still not convinced that it's abundantly clear... but if we want to compare max vs. min, things get worse because we return `0` when the `type` is `3` (https://godbolt.org/z/4f5onM3W7). So even with checking upper and lower bounds, we come away knowing nothing about the size of the object (probably a deficiency in Clang).

> Btw, it looks like your initial example gets 48 for both modes on GCC as well? https://c.godbolt.org/z/EfGWv4Wrh

If you remove the `argc = 1;`, you get the difference. Your example might be a GCC bug.

> > All of these are explicit in the LLVM IR. Is the worry that they've been changed from some transformations? Or are there others I'm missing?
> 
> Apart from the fact that what you are doing is simply illegal under our IR semantics, a practical case where this will likely compute incorrect results are unions.

?? I think it's a bit much to say that this is "illegal." We may not get every instance of something, but we can certainly determine the size of a struct. If by "illegal" you mean determining what a sub-object is, I don't agree but if it's so there might be a way to retain whatever information is needed to determine what a sub-object is.

> For unions, clang will use the type of the union member with the largest size as the alloca type, regardless of which union member is active. I haven't tried, but your patch will probably compute the subobject size based on that arbitrarily picked member, rather than the one being accessed.

We don't seem to support unions correctly now:

```
$ cat ~/llvm/__bdos3.c
struct suspend_stats {
        int     failed_resume_noirq;
        int     last_failed_dev;
        union {
          char    failed_devs[2][40];
          char    failed_devs2[38];
        };
        int     last_failed_errno;
        int     bar;
};

#define report(x) __builtin_printf(#x ": %zu\n", x)

int main(int argc, char *argv[])
{
    struct suspend_stats foo;

    report(__builtin_dynamic_object_size(&foo.failed_devs2[argc], 0));
    report(__builtin_dynamic_object_size(&foo.failed_devs2[argc], 1));

    report(__builtin_dynamic_object_size(&foo.failed_devs2[argc], 2));
    report(__builtin_dynamic_object_size(&foo.failed_devs2[argc], 3));


    return 0;
}
$ clang -O2 ~/llvm/__bdos3.c && ./a.out
__builtin_dynamic_object_size(&foo.failed_devs2[argc], 0): 87
__builtin_dynamic_object_size(&foo.failed_devs2[argc], 1): 87
__builtin_dynamic_object_size(&foo.failed_devs2[argc], 2): 87
__builtin_dynamic_object_size(&foo.failed_devs2[argc], 3): 0
$ gcc -O2 ~/llvm/__bdos3.c && ./a.out
__builtin_dynamic_object_size(&foo.failed_devs2[argc], 0): 87
__builtin_dynamic_object_size(&foo.failed_devs2[argc], 1): 37
__builtin_dynamic_object_size(&foo.failed_devs2[argc], 2): 87
__builtin_dynamic_object_size(&foo.failed_devs2[argc], 3): 37
```

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


More information about the cfe-commits mailing list