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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
