[V7,1/2] Handle component_ref to a structre/union field including flexible array member [PR101832]
Checks
Commit Message
GCC extension accepts the case when a struct with a flexible array member
is embedded into another struct or union (possibly recursively).
__builtin_object_size should treat such struct as flexible size.
gcc/c/ChangeLog:
PR tree-optimization/101832
* c-decl.cc (finish_struct): Set TYPE_INCLUDE_FLEXARRAY for
struct/union type.
gcc/lto/ChangeLog:
PR tree-optimization/101832
* lto-common.cc (compare_tree_sccs_1): Compare bit
TYPE_NO_NAMED_ARGS_STDARG_P or TYPE_INCLUDE_FLEXARRAY properly
for its corresponding type.
gcc/ChangeLog:
PR tree-optimization/101832
* print-tree.cc (print_node): Print new bit type_include_flexarray.
* tree-core.h (struct tree_type_common): Use bit no_named_args_stdarg_p
as type_include_flexarray for RECORD_TYPE or UNION_TYPE.
* tree-object-size.cc (addr_object_size): Handle structure/union type
when it has flexible size.
* tree-streamer-in.cc (unpack_ts_type_common_value_fields): Stream
in bit no_named_args_stdarg_p properly for its corresponding type.
* tree-streamer-out.cc (pack_ts_type_common_value_fields): Stream
out bit no_named_args_stdarg_p properly for its corresponding type.
* tree.h (TYPE_INCLUDE_FLEXARRAY): New macro TYPE_INCLUDE_FLEXARRAY.
gcc/testsuite/ChangeLog:
PR tree-optimization/101832
* gcc.dg/builtin-object-size-pr101832.c: New test.
---
gcc/c/c-decl.cc | 11 ++
gcc/lto/lto-common.cc | 5 +-
gcc/print-tree.cc | 5 +
.../gcc.dg/builtin-object-size-pr101832.c | 134 ++++++++++++++++++
gcc/tree-core.h | 2 +
gcc/tree-object-size.cc | 23 ++-
gcc/tree-streamer-in.cc | 5 +-
gcc/tree-streamer-out.cc | 5 +-
gcc/tree.h | 7 +-
9 files changed, 192 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c
Comments
On Fri, 19 May 2023 20:49:47 +0000
Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> GCC extension accepts the case when a struct with a flexible array member
> is embedded into another struct or union (possibly recursively).
Do you mean TYPE_TRAILING_FLEXARRAY()?
> diff --git a/gcc/tree.h b/gcc/tree.h
> index 0b72663e6a1..237644e788e 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -786,7 +786,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
> (...) prototype, where arguments can be accessed with va_start and
> va_arg), as opposed to an unprototyped function. */
> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
> - (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
> + (FUNC_OR_METHOD_CHECK (NODE)->type_common.no_named_args_stdarg_p)
> +
> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
> + at the last field recursively. */
> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
> + (RECORD_OR_UNION_CHECK (NODE)->type_common.no_named_args_stdarg_p)
Until i read the description above i read TYPE_INCLUDE_FLEXARRAY as an
option to include or not include something. The description hints more
at TYPE_INCLUDES_FLEXARRAY (with an S) to be a type which has at least
one member which has a trailing flexible array or which itself has a
trailing flexible array.
>
> /* In an IDENTIFIER_NODE, this means that assemble_name was called with
> this string as an argument. */
Bernhard,
Thanks a lot for your comments.
> On May 19, 2023, at 7:11 PM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>
> On Fri, 19 May 2023 20:49:47 +0000
> Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
>> GCC extension accepts the case when a struct with a flexible array member
>> is embedded into another struct or union (possibly recursively).
>
> Do you mean TYPE_TRAILING_FLEXARRAY()?
The following might be more accurate description:
GCC extension accepts the case when a struct with a flexible array member
is embedded into another struct or union (possibly recursively) as the last field.
>
>> diff --git a/gcc/tree.h b/gcc/tree.h
>> index 0b72663e6a1..237644e788e 100644
>> --- a/gcc/tree.h
>> +++ b/gcc/tree.h
>> @@ -786,7 +786,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
>> (...) prototype, where arguments can be accessed with va_start and
>> va_arg), as opposed to an unprototyped function. */
>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>> - (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>> + (FUNC_OR_METHOD_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>> +
>> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>> + at the last field recursively. */
>> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
>> + (RECORD_OR_UNION_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>
> Until i read the description above i read TYPE_INCLUDE_FLEXARRAY as an
> option to include or not include something. The description hints more
> at TYPE_INCLUDES_FLEXARRAY (with an S) to be a type which has at least
> one member which has a trailing flexible array or which itself has a
> trailing flexible array.
Yes, TYPE_INCLUDES_FLEXARRAY (maybe with a S is a better name) means the structure/union TYPE includes a flexible array member or includes a struct with a flexible array member as the last field.
Hope this is clear.
thanks.
Qing
>
>>
>> /* In an IDENTIFIER_NODE, this means that assemble_name was called with
>> this string as an argument. */
>
On 24 May 2023 16:09:21 CEST, Qing Zhao <qing.zhao@oracle.com> wrote:
>Bernhard,
>
>Thanks a lot for your comments.
>
>> On May 19, 2023, at 7:11 PM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>>
>> On Fri, 19 May 2023 20:49:47 +0000
>> Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>
>>> GCC extension accepts the case when a struct with a flexible array member
>>> is embedded into another struct or union (possibly recursively).
>>
>> Do you mean TYPE_TRAILING_FLEXARRAY()?
>
>The following might be more accurate description:
>
>GCC extension accepts the case when a struct with a flexible array member
> is embedded into another struct or union (possibly recursively) as the last field.
>
>
>
>>
>>> diff --git a/gcc/tree.h b/gcc/tree.h
>>> index 0b72663e6a1..237644e788e 100644
>>> --- a/gcc/tree.h
>>> +++ b/gcc/tree.h
>>> @@ -786,7 +786,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
>>> (...) prototype, where arguments can be accessed with va_start and
>>> va_arg), as opposed to an unprototyped function. */
>>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>> - (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>> + (FUNC_OR_METHOD_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>> +
>>> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>> + at the last field recursively. */
>>> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>> + (RECORD_OR_UNION_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>
>> Until i read the description above i read TYPE_INCLUDE_FLEXARRAY as an
>> option to include or not include something. The description hints more
>> at TYPE_INCLUDES_FLEXARRAY (with an S) to be a type which has at least
>> one member which has a trailing flexible array or which itself has a
>> trailing flexible array.
>
>Yes, TYPE_INCLUDES_FLEXARRAY (maybe with a S is a better name) means the structure/union TYPE includes a flexible array member or includes a struct with a flexible array member as the last field.
>
So ANY_TRAILING_FLEXARRAY or TYPE_CONTAINS_FLEXARRAY, TYPE_INCLUDES_FLEXARRAY or something like that would be more clear, i don't know.
I'd probably use the first, but that's enough bike shedding for me now. Let's see what others think.
thanks,
>Hope this is clear.
>thanks.
>
>Qing
>>
>>>
>>> /* In an IDENTIFIER_NODE, this means that assemble_name was called with
>>> this string as an argument. */
>>
>
> On May 25, 2023, at 1:41 AM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>
> On 24 May 2023 16:09:21 CEST, Qing Zhao <qing.zhao@oracle.com> wrote:
>> Bernhard,
>>
>> Thanks a lot for your comments.
>>
>>> On May 19, 2023, at 7:11 PM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>>>
>>> On Fri, 19 May 2023 20:49:47 +0000
>>> Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>>
>>>> GCC extension accepts the case when a struct with a flexible array member
>>>> is embedded into another struct or union (possibly recursively).
>>>
>>> Do you mean TYPE_TRAILING_FLEXARRAY()?
>>
>> The following might be more accurate description:
>>
>> GCC extension accepts the case when a struct with a flexible array member
>> is embedded into another struct or union (possibly recursively) as the last field.
>>
>>
>>
>>>
>>>> diff --git a/gcc/tree.h b/gcc/tree.h
>>>> index 0b72663e6a1..237644e788e 100644
>>>> --- a/gcc/tree.h
>>>> +++ b/gcc/tree.h
>>>> @@ -786,7 +786,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
>>>> (...) prototype, where arguments can be accessed with va_start and
>>>> va_arg), as opposed to an unprototyped function. */
>>>> #define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
>>>> - (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>>> + (FUNC_OR_METHOD_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>>> +
>>>> +/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
>>>> + at the last field recursively. */
>>>> +#define TYPE_INCLUDE_FLEXARRAY(NODE) \
>>>> + (RECORD_OR_UNION_CHECK (NODE)->type_common.no_named_args_stdarg_p)
>>>
>>> Until i read the description above i read TYPE_INCLUDE_FLEXARRAY as an
>>> option to include or not include something. The description hints more
>>> at TYPE_INCLUDES_FLEXARRAY (with an S) to be a type which has at least
>>> one member which has a trailing flexible array or which itself has a
>>> trailing flexible array.
>>
>> Yes, TYPE_INCLUDES_FLEXARRAY (maybe with a S is a better name) means the structure/union TYPE includes a flexible array member or includes a struct with a flexible array member as the last field.
>>
>
> So ANY_TRAILING_FLEXARRAY or TYPE_CONTAINS_FLEXARRAY, TYPE_INCLUDES_FLEXARRAY or something like that would be more clear, i don't know.
> I'd probably use the first, but that's enough bike shedding for me now. Let's see what others think.
Thanks. I changed it to TYPE_INCLUDES_FLEXARRAY.
Qing
>
> thanks,
>
>> Hope this is clear.
>> thanks.
>>
>> Qing
>>>
>>>>
>>>> /* In an IDENTIFIER_NODE, this means that assemble_name was called with
>>>> this string as an argument. */
@@ -9282,6 +9282,17 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
/* Set DECL_NOT_FLEXARRAY flag for FIELD_DECL x. */
DECL_NOT_FLEXARRAY (x) = !is_flexible_array_member_p (is_last_field, x);
+ /* Set TYPE_INCLUDE_FLEXARRAY for the context of x, t.
+ when x is an array and is the last field. */
+ if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE)
+ TYPE_INCLUDE_FLEXARRAY (t)
+ = is_last_field && flexible_array_member_type_p (TREE_TYPE (x));
+ /* Recursively set TYPE_INCLUDE_FLEXARRAY for the context of x, t
+ when x is an union or record and is the last field. */
+ else if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
+ TYPE_INCLUDE_FLEXARRAY (t)
+ = is_last_field && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x));
+
if (DECL_NAME (x)
|| RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
saw_named_field = true;
@@ -1275,7 +1275,10 @@ compare_tree_sccs_1 (tree t1, tree t2, tree **map)
if (AGGREGATE_TYPE_P (t1))
compare_values (TYPE_TYPELESS_STORAGE);
compare_values (TYPE_EMPTY_P);
- compare_values (TYPE_NO_NAMED_ARGS_STDARG_P);
+ if (FUNC_OR_METHOD_TYPE_P (t1))
+ compare_values (TYPE_NO_NAMED_ARGS_STDARG_P);
+ if (RECORD_OR_UNION_TYPE_P (t1))
+ compare_values (TYPE_INCLUDE_FLEXARRAY);
compare_values (TYPE_PACKED);
compare_values (TYPE_RESTRICT);
compare_values (TYPE_USER_ALIGN);
@@ -632,6 +632,11 @@ print_node (FILE *file, const char *prefix, tree node, int indent,
&& TYPE_CXX_ODR_P (node))
fputs (" cxx-odr-p", file);
+ if ((code == RECORD_TYPE
+ || code == UNION_TYPE)
+ && TYPE_INCLUDE_FLEXARRAY (node))
+ fputs (" include-flexarray", file);
+
/* The transparent-union flag is used for different things in
different nodes. */
if ((code == UNION_TYPE || code == RECORD_TYPE)
new file mode 100644
@@ -0,0 +1,134 @@
+/* PR 101832:
+ GCC extension accepts the case when a struct with a C99 flexible array
+ member is embedded into another struct (possibly recursively).
+ __builtin_object_size will treat such struct as flexible size.
+ However, when a structure with non-C99 flexible array member, i.e, trailing
+ [0], [1], or [4], is embedded into anther struct, the stucture will not
+ be treated as flexible size. */
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include "builtin-object-size-common.h"
+
+#define expect(p, _v) do { \
+ size_t v = _v; \
+ if (p == v) \
+ __builtin_printf ("ok: %s == %zd\n", #p, p); \
+ else {\
+ __builtin_printf ("WAT: %s == %zd (expected %zd)\n", #p, p, v); \
+ FAIL (); \
+ } \
+} while (0);
+
+
+struct A {
+ int n;
+ char data[];
+};
+
+struct B {
+ int m;
+ struct A a;
+};
+
+struct C {
+ int q;
+ struct B b;
+};
+
+struct A0 {
+ int n;
+ char data[0];
+};
+
+struct B0 {
+ int m;
+ struct A0 a;
+};
+
+struct C0 {
+ int q;
+ struct B0 b;
+};
+
+struct A1 {
+ int n;
+ char data[1];
+};
+
+struct B1 {
+ int m;
+ struct A1 a;
+};
+
+struct C1 {
+ int q;
+ struct B1 b;
+};
+
+struct An {
+ int n;
+ char data[8];
+};
+
+struct Bn {
+ int m;
+ struct An a;
+};
+
+struct Cn {
+ int q;
+ struct Bn b;
+};
+
+volatile void *magic1, *magic2;
+
+int main (int argc, char *argv[])
+{
+ struct B *outer;
+ struct C *outest;
+
+ /* Make sure optimization can't find some other object size. */
+ outer = (void *)magic1;
+ outest = (void *)magic2;
+
+ 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);
+
+ struct B0 *outer0;
+ struct C0 *outest0;
+
+ /* Make sure optimization can't find some other object size. */
+ outer0 = (void *)magic1;
+ outest0 = (void *)magic2;
+
+ expect (__builtin_object_size (&outer0->a, 1), sizeof (outer0->a));
+ expect (__builtin_object_size (&outest0->b, 1), sizeof (outest0->b));
+ expect (__builtin_object_size (&outest0->b.a, 1), sizeof (outest0->b.a));
+
+ struct B1 *outer1;
+ struct C1 *outest1;
+
+ /* Make sure optimization can't find some other object size. */
+ outer1 = (void *)magic1;
+ outest1 = (void *)magic2;
+
+ expect (__builtin_object_size (&outer1->a, 1), sizeof (outer1->a));
+ expect (__builtin_object_size (&outest1->b, 1), sizeof (outest1->b));
+ expect (__builtin_object_size (&outest1->b.a, 1), sizeof (outest1->b.a));
+
+ struct Bn *outern;
+ struct Cn *outestn;
+
+ /* Make sure optimization can't find some other object size. */
+ outern = (void *)magic1;
+ outestn = (void *)magic2;
+
+ expect (__builtin_object_size (&outern->a, 1), sizeof (outern->a));
+ expect (__builtin_object_size (&outestn->b, 1), sizeof (outestn->b));
+ expect (__builtin_object_size (&outestn->b.a, 1), sizeof (outestn->b.a));
+
+ DONE ();
+ return 0;
+}
@@ -1712,6 +1712,8 @@ struct GTY(()) tree_type_common {
unsigned typeless_storage : 1;
unsigned empty_flag : 1;
unsigned indivisible_p : 1;
+ /* TYPE_NO_NAMED_ARGS_STDARG_P for a stdarg function.
+ Or TYPE_INCLUDE_FLEXARRAY for RECORD_TYPE and UNION_TYPE. */
unsigned no_named_args_stdarg_p : 1;
unsigned spare : 1;
@@ -633,11 +633,32 @@ addr_object_size (struct object_size_info *osi, const_tree ptr,
v = NULL_TREE;
break;
case COMPONENT_REF:
- if (TREE_CODE (TREE_TYPE (v)) != ARRAY_TYPE)
+ /* When the ref is not to an aggregate type, i.e, an array,
+ a record or a union, it will not have flexible size,
+ compute the object size directly. */
+ if (!AGGREGATE_TYPE_P (TREE_TYPE (v)))
{
v = NULL_TREE;
break;
}
+ /* if the ref is to a record or union type, but the type
+ does not include a flexible array recursively, compute
+ the object size directly. */
+ if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (v)))
+ {
+ if (!TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (v)))
+ {
+ v = NULL_TREE;
+ break;
+ }
+ else
+ {
+ v = TREE_OPERAND (v, 0);
+ break;
+ }
+ }
+ /* Now the ref is to an array type. */
+ gcc_assert (TREE_CODE (TREE_TYPE (v)) == ARRAY_TYPE);
is_flexible_array_mem_ref = array_ref_flexible_size_p (v);
while (v != pt_var && TREE_CODE (v) == COMPONENT_REF)
if (TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0)))
@@ -386,7 +386,10 @@ unpack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
if (AGGREGATE_TYPE_P (expr))
TYPE_TYPELESS_STORAGE (expr) = (unsigned) bp_unpack_value (bp, 1);
TYPE_EMPTY_P (expr) = (unsigned) bp_unpack_value (bp, 1);
- TYPE_NO_NAMED_ARGS_STDARG_P (expr) = (unsigned) bp_unpack_value (bp, 1);
+ if (FUNC_OR_METHOD_TYPE_P (expr))
+ TYPE_NO_NAMED_ARGS_STDARG_P (expr) = (unsigned) bp_unpack_value (bp, 1);
+ if (RECORD_OR_UNION_TYPE_P (expr))
+ TYPE_INCLUDE_FLEXARRAY (expr) = (unsigned) bp_unpack_value (bp, 1);
TYPE_PRECISION (expr) = bp_unpack_var_len_unsigned (bp);
SET_TYPE_ALIGN (expr, bp_unpack_var_len_unsigned (bp));
#ifdef ACCEL_COMPILER
@@ -355,7 +355,10 @@ pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
if (AGGREGATE_TYPE_P (expr))
bp_pack_value (bp, TYPE_TYPELESS_STORAGE (expr), 1);
bp_pack_value (bp, TYPE_EMPTY_P (expr), 1);
- bp_pack_value (bp, TYPE_NO_NAMED_ARGS_STDARG_P (expr), 1);
+ if (FUNC_OR_METHOD_TYPE_P (expr))
+ bp_pack_value (bp, TYPE_NO_NAMED_ARGS_STDARG_P (expr), 1);
+ if (RECORD_OR_UNION_TYPE_P (expr))
+ bp_pack_value (bp, TYPE_INCLUDE_FLEXARRAY (expr), 1);
bp_pack_var_len_unsigned (bp, TYPE_PRECISION (expr));
bp_pack_var_len_unsigned (bp, TYPE_ALIGN (expr));
}
@@ -786,7 +786,12 @@ extern void omp_clause_range_check_failed (const_tree, const char *, int,
(...) prototype, where arguments can be accessed with va_start and
va_arg), as opposed to an unprototyped function. */
#define TYPE_NO_NAMED_ARGS_STDARG_P(NODE) \
- (TYPE_CHECK (NODE)->type_common.no_named_args_stdarg_p)
+ (FUNC_OR_METHOD_CHECK (NODE)->type_common.no_named_args_stdarg_p)
+
+/* True if this RECORD_TYPE or UNION_TYPE includes a flexible array member
+ at the last field recursively. */
+#define TYPE_INCLUDE_FLEXARRAY(NODE) \
+ (RECORD_OR_UNION_CHECK (NODE)->type_common.no_named_args_stdarg_p)
/* In an IDENTIFIER_NODE, this means that assemble_name was called with
this string as an argument. */