https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832
Gustavo A. R. Silva <gustavo at embeddedor dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |gustavo at embeddedor dot com
--- Comment #15 from Gustavo A. R. Silva <gustavo at embeddedor dot com> ---
Hi all,
While working on some Linux kernel code containing counted_by-annotated
flexible-array members and nested flexible-array structures, I revisited this
issue and found what appears to be a bug in GCC's handling of
__builtin_object_size() and __builtin_dynamic_object_size() for nested flexible
structures.
Back in 2021, when this issue was opened, we were still exploring what the
correct semantics should be for object sizes involving nested flexible
structures. Since then, counted_by has been implemented in both GCC and Clang
and adopted widely in the Linux kernel, which has improved our understanding of
how these built-ins are expected to behave for bounds checking.
Consider:
struct flex {
size_t count;
char a;
char fam[] __attribute__((counted_by(count)));
/* 7-byte tail padding */
};
struct nested_fam {
char hdr;
struct flex nested;
};
For a pointer to the nested flexible structure itself &p->nested, I believe the
expected behavior is that __bos/__bdos(&p->nested, 0) return the complete
reachable object size whenever enough information is available to determine it,
including the flexible-array tail. That is:
__builtin_object_size(&p->nested, 0) == sizeof(struct flex) + count *
sizeof(char)
__builtin_dynamic_object_size(&p->nested, 0) == sizeof(struct flex) + count *
sizeof(char)
while __bos/__bdos(&p->nested, 1) should return the size of the immediate
subobject. In this case:
__builtin_object_size(&p->nested, 1) == sizeof(struct flex)
__builtin_dynamic_object_size(&p->nested, 1) == sizeof(struct flex)
This is the behavior Clang implements today.
However, GCC currently returns:
__builtin_object_size(&p->nested, 1) == sizeof(struct flex) + count *
sizeof(char)
__builtin_dynamic_object_size(&p->nested, 1) == sizeof(struct flex) + count *
sizeof(char)
when the compiler can see the allocation, and
__builtin_object_size(&p->nested, 1) == -1
__builtin_dynamic_object_size(&p->nested, 0) == -1
__builtin_dynamic_object_size(&p->nested, 1) == -1
when it cannot, despite still being able to determine the flexible-array bound
via counted_by.
I believe the above results are inconsistent with the intended distinction
between mode 0 and mode 1 for the object size checking built-in functions.
Mode 0 describes the complete reachable object, while mode 1 describes the
immediate referenced subobject.
I have a reproducer[1] comparing GCC and Clang.
Clang produces:
__builtin_dynamic_object_size(&p->nested, 0) == 26
__builtin_dynamic_object_size(&p->nested, 1) == 16
__builtin_object_size(&p->nested, 0) == 26
__builtin_object_size(&p->nested, 1) == 16
which I believe is the expected behavior.
Current GCC (trunk) instead returns:
__builtin_dynamic_object_size(&p->nested, 1) == 26
__builtin_object_size(&p->nested, 1) == 26
for inline allocations, and
__builtin_dynamic_object_size(&p->nested, 0) == -1
__builtin_dynamic_object_size(&p->nested, 1) == -1
__builtin_object_size(&p->nested, 1) == -1
for non-inline allocations.
The issue appears to come from this code in addr_object_size():
if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (v)))
{
if (!TYPE_INCLUDES_FLEXARRAY (TREE_TYPE (v)))
{
v = NULL_TREE;
break;
}
else
{
v = TREE_OPERAND (v, 0);
break;
}
}
For record or union references that recursively contain a flexible-array
member,
GCC walks up to the enclosing object via:
v = TREE_OPERAND (v, 0);
instead of computing the object size directly from the referenced subobject.
This causes mode 1 queries to lose the distinction between:
&p->nested
and:
p
One practical consequence of the current behavior is that bounds-checking
mechanisms built on top of __builtin_dynamic_object_size() (fortified memcpy()
in the Linux kernel, for instance) cannot reliably distinguish between the
containing object and the nested flexible structure itself.
This distinction has become increasingly important as counted_by has been
adopted in the Linux kernel and compilers are expected to reason more precisely
about objects containing flexible-array members.
Once GCC walks up to the containing object, the resulting object size no longer
depends on the referenced subobject itself, but rather on the size of the
containing object.
I think the logic should instead always compute the size directly from the
referenced record/union:
- if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (v)))
- {
- if (!TYPE_INCLUDES_FLEXARRAY (TREE_TYPE (v)))
- {
- v = NULL_TREE;
- break;
- }
- else
- {
- v = TREE_OPERAND (v, 0);
- break;
- }
- }
+ if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (v)))
+ {
+ v = NULL_TREE;
+ break;
+ }
The testcase from this commit[2] should also be updated:
- expect (__builtin_object_size (&outer->a, 1), -1);
- expect (__builtin_object_size (&outest->b, 1), -1);
- expect (__builtin_object_size (&outest->b.a, 1), -1);
+ expect (__builtin_object_size (&outer->a, 1), sizeof(outer->a));
+ expect (__builtin_object_size (&outest->b, 1), sizeof(outest->b));
+ expect (__builtin_object_size (&outest->b.a, 1), sizeof(outest->b.a));
I intend to submit a proper patch to gcc-patches, but first, it'd be great to
hear your comments. :)
Thanks!
[1] https://godbolt.org/z/voMn4sGcP
[2]
https://gcc.gnu.org/cgit/gcc/commit/?id=e050ce7c3adf71eedd5482c29cf54b827e026642
