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

[Bill Wendling via cfe-commits]

bwendling wrote:

> We've discussed this before, years ago. Previously, the sticking point was: how is LLVM going to know what the frontend considers the closest surrounding subobject to be? The LLVM type doesn't give you that information, and it seems like there's nowhere else that LLVM can get it from either, so this flag ends up not being useful and the best we can do is to give a conservatively-correct answer -- the complete object size if we want an upper bound, or 0 if we want a lower bound.

Right now we're giving a wrong answer (see below), so that's no good. If we're going to err on the side of caution, then we should return the default "can't calculate the size" value.

When you say that we can't detect what the front-end considers the "closest surrounding subobject" to be, is that mostly due to corner cases or is it a more general concern? (Note, we're really only interested in supporting this for C structs. C++ structs / classes would require therapy.)

> Clang does respect the "subobject" flag if it can symbolically evaluate the operand of `__builtin_object_size` sufficiently to determine which subobject is being referenced. Previously we've thought that that was the best we could do.

This is why the current behavior is wrong, in my opinion. The motivating example is below:

```
struct suspend_stats {
        int     success;
        int     fail;
        int     failed_freeze;
        int     failed_prepare;
        int     failed_suspend;
        int     failed_suspend_late;
        int     failed_suspend_noirq;
        int     failed_resume;
        int     failed_resume_early;
        int     failed_resume_noirq;
#define REC_FAILED_NUM  2
        int     last_failed_dev;
        char    failed_devs[REC_FAILED_NUM][40]; /* offsetof(struct suspend_stats, failed_devs) == 44 */
        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(sizeof(foo.failed_devs[1]));
    report(sizeof(foo.failed_devs[argc]));
    report(__builtin_dynamic_object_size(&foo.fail, 0));
    report(__builtin_dynamic_object_size(&foo.fail, 1));
    report(__builtin_dynamic_object_size(&foo.failed_freeze, 0));
    report(__builtin_dynamic_object_size(foo.failed_devs[1], 0));
    report(__builtin_dynamic_object_size(foo.failed_devs[1], 1));
    report(__builtin_dynamic_object_size(foo.failed_devs[argc], 0));
    report(__builtin_dynamic_object_size(foo.failed_devs[argc], 1));

    return 0;
}
```

The output with this change is now:

```
__builtin_dynamic_object_size(&foo.fail, 0): 128
__builtin_dynamic_object_size(&foo.fail, 1): 4
__builtin_dynamic_object_size(&foo.failed_freeze, 0): 124
__builtin_dynamic_object_size(foo.failed_devs[1], 0): 48
__builtin_dynamic_object_size(foo.failed_devs[1], 1): 40
__builtin_dynamic_object_size(foo.failed_devs[argc], 0): 48
__builtin_dynamic_object_size(foo.failed_devs[argc], 1): 40
```

Without the change, the last line is:

```
__builtin_dynamic_object_size(foo.failed_devs[argc], 1): 48
```

Which isn't correct according to GNU's documentation. So if we can't honor the TYPE bit, then we should return `-1 / 0` here, right?

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