> On Dec 3, 2024, at 01:25, Martin Uecker <[email protected]> wrote:
>
> Am Montag, dem 02.12.2024 um 22:33 +0000 schrieb Qing Zhao:
>
> ....
>
>>>>
>>>> diff --git a/gcc/testsuite/gcc.dg/pr114014.c
>>>> b/gcc/testsuite/gcc.dg/pr114014.c
>>>> new file mode 100644
>>>> index 00000000000..ab783f4f85d
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.dg/pr114014.c
>>>> @@ -0,0 +1,14 @@
>>>> +/* PR c/114014
>>>> + * { dg-do compile }
>>>> + * { dg-options "-std=c23 -g" } */
>>>> +
>>>> +struct r {
>>>> + int a;
>>>> + char b[];
>>>> +};
>>>> +struct r {
>>>> + int a;
>>>> + char b[0];
>>>> +}; /* { dg-error "redefinition" } */
>>>> +
>>>> +
>>>>
>>>> Is the above testing case claiming that b[] and b[0] are compatible from a
>>>> language semantic point of view?
>>>
>>> It would test that we do not crash with checking.
>>>
>>> Semantically, in c23 if you redeclare a type in the same scope then
>>> it must not only be compatible but is also not allowed to differ.
>>> So a redeclaration in the same scope has stricter requirements than
>>> compatibility (this also true for typedefs for example).
>>
>> So, here does the “compatibility” mean “compatibility from a language
>> semantic point of view” or TBAA-compability?
>
> When we talk about what is allowed to be used and what warnings/errors
> appear in the test, it is about language semantics. What made the
> compiler crash was related to the TBAA semantics.
Okay.
>
>>>
>>> Whether we allow
>>>
>>> struct r {
>>> int a;
>>> char b[];
>>> };
>>>
>>> struct r {
>>> int a;
>>> char b[0];
>>> };
>>>
>>> depends on us because the [0] is an extension.
>>
>> [0] is an extension for representing FAM ONLY when -fstrict-flex-array<3,
>> when
>> -fstrict-flex-array=3 specified, [0] is NOT considered as an extension for
>> FAM anymore.
>> For [1], only when -fstrict-flex-array<2 spedified, it’s considered as an
>> extension for FAM.
>>
>> So, I still think that we should consider -fstrict-flex-array and its impact
>> on the GCC extensions [0] and [1].
>
> I do not understand what you mean by "consider
> -fstrict-flex-array"
So, my question is: what’s the behavior of compiling the following :
+struct r {
+ int a;
+ char b[];
+};
+struct r {
+ int a;
+ char b[0];
With "std=c23 -g -fstrict-flex-array=3"?
>
> Independent from this option, the types are always different
> types, e.g. sizeof(x->b) would still not be allowed for the
> first type because x->b is incomplete but allowed for the
> second, and return 1 if it were declared with char b[1].
Okay.
thanks.
Qing
>
>
> Martin
>
>>
>> Qing
>>> I would make it
>>> compatible but not allow redefinition as the types are different.
>>
>>
>>>
>>>
>>> Martin
>>>
>>>
>>>>
>>>> thanks.
>>>>
>>>> Qing
>>>>> even when we do not treat the later as FAM (i.e. still forbid
>>>>> out-of-bounds accesses).
>>>>>
>>>>> E.g. see Richard's comment:
>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114713#c2
>>>>>
>>>>>
>>>>> Martin
>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>> Qing
>>>>>>>
>>>>>>> Martin
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> thanks.
>>>>>>>>
>>>>>>>> Qing
>>>>>>>>> On Nov 23, 2024, at 14:45, Martin Uecker <[email protected]> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> This patch tries fixes the errors we have because of
>>>>>>>>> flexible array members. I am bit unsure about the exception
>>>>>>>>> for the mode.
>>>>>>>>>
>>>>>>>>> Bootstrapped and regression tested on x86_64.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Fix type compatibility for types with flexible array member
>>>>>>>>> [PR113688,PR114014,PR117724]
>>>>>>>>>
>>>>>>>>> verify_type checks the compatibility of TYPE_CANONICAL using
>>>>>>>>> gimple_canonical_types_compatible_p. But it is stricter than what
>>>>>>>>> the
>>>>>>>>> C standard requires and therefor inconsistent with how TYPE_CANONICAL
>>>>>>>>> is set
>>>>>>>>> in the C FE. Here, the logic is changed to ignore array size when
>>>>>>>>> one of the
>>>>>>>>> types is a flexible array member. To not get errors because of
>>>>>>>>> inconsistent
>>>>>>>>> number of members, zero-sized arrays are not ignored anymore when
>>>>>>>>> checking
>>>>>>>>> fields of a struct (which is stricter than what was done before).
>>>>>>>>> Finally, a exception is added that allows the TYPE_MODE of a type with
>>>>>>>>> flexible array member to differ from another compatible type.
>>>>>>>>>
>>>>>>>>> PR c/113688
>>>>>>>>> PR c/114014
>>>>>>>>> PR c/117724
>>>>>>>>>
>>>>>>>>> gcc/ChangeLog:
>>>>>>>>> * tree.cc (gimple_canonical_types_compatible_p): Revise
>>>>>>>>> logic for types with FAM.
>>>>>>>>> (verify_type): Add exception for mode for types with FAM.
>>>>>>>>>
>>>>>>>>> gcc/testsuite/ChangeLog:
>>>>>>>>> * gcc.dg/pr113688.c: New test.
>>>>>>>>> * gcc.dg/pr114014.c: New test.
>>>>>>>>> * gcc.dg/pr117724.c: New test.
>>>>>>>>>
>>>>>>>>> diff --git a/gcc/testsuite/gcc.dg/pr113688.c
>>>>>>>>> b/gcc/testsuite/gcc.dg/pr113688.c
>>>>>>>>> new file mode 100644
>>>>>>>>> index 00000000000..8dee8c86f1b
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/gcc/testsuite/gcc.dg/pr113688.c
>>>>>>>>> @@ -0,0 +1,8 @@
>>>>>>>>> +/* { dg-do compile } */
>>>>>>>>> +/* { dg-options "-g" } */
>>>>>>>>> +
>>>>>>>>> +struct S{int x,y[1];}*a;
>>>>>>>>> +int main(void){
>>>>>>>>> + struct S{int x,y[];};
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> diff --git a/gcc/testsuite/gcc.dg/pr114014.c
>>>>>>>>> b/gcc/testsuite/gcc.dg/pr114014.c
>>>>>>>>> new file mode 100644
>>>>>>>>> index 00000000000..ab783f4f85d
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/gcc/testsuite/gcc.dg/pr114014.c
>>>>>>>>> @@ -0,0 +1,14 @@
>>>>>>>>> +/* PR c/114014
>>>>>>>>> + * { dg-do compile }
>>>>>>>>> + * { dg-options "-std=c23 -g" } */
>>>>>>>>> +
>>>>>>>>> +struct r {
>>>>>>>>> + int a;
>>>>>>>>> + char b[];
>>>>>>>>> +};
>>>>>>>>> +struct r {
>>>>>>>>> + int a;
>>>>>>>>> + char b[0];
>>>>>>>>> +}; /* { dg-error "redefinition" } */
>>>>>>>>> +
>>>>>>>>> +
>>>>>>>>> diff --git a/gcc/testsuite/gcc.dg/pr117724.c
>>>>>>>>> b/gcc/testsuite/gcc.dg/pr117724.c
>>>>>>>>> new file mode 100644
>>>>>>>>> index 00000000000..d631daeb644
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/gcc/testsuite/gcc.dg/pr117724.c
>>>>>>>>> @@ -0,0 +1,16 @@
>>>>>>>>> +/* { dg-do compile } */
>>>>>>>>> +/* { dg-options "-g" } */
>>>>>>>>> +
>>>>>>>>> +struct {
>>>>>>>>> + unsigned long len;
>>>>>>>>> + unsigned long size;
>>>>>>>>> + char data[];
>>>>>>>>> +}; /* { dg-warning "unnamed struct" } */
>>>>>>>>> +struct {
>>>>>>>>> + struct {
>>>>>>>>> + unsigned long len;
>>>>>>>>> + unsigned long size;
>>>>>>>>> + char data[6];
>>>>>>>>> + };
>>>>>>>>> +}; /* { dg-warning "unnamed struct" } */
>>>>>>>>> +
>>>>>>>>> diff --git a/gcc/tree.cc b/gcc/tree.cc
>>>>>>>>> index 1da06c7d4e9..dbf6b180496 100644
>>>>>>>>> --- a/gcc/tree.cc
>>>>>>>>> +++ b/gcc/tree.cc
>>>>>>>>> @@ -13900,8 +13900,11 @@ gimple_canonical_types_compatible_p
>>>>>>>>> (const_tree t1, const_tree t2,
>>>>>>>>> || TREE_CODE (t1) == NULLPTR_TYPE)
>>>>>>>>> return true;
>>>>>>>>>
>>>>>>>>> - /* Can't be the same type if they have different mode. */
>>>>>>>>> - if (TYPE_MODE (t1) != TYPE_MODE (t2))
>>>>>>>>> + /* Can't be compatible types if they have different mode. We allow
>>>>>>>>> + mismatching modes for types with flexible array member. */
>>>>>>>>> + if (!flexible_array_type_p (t1)
>>>>>>>>> + && !flexible_array_type_p (t2)
>>>>>>>>> + && (TYPE_MODE (t1) != TYPE_MODE (t2)))
>>>>>>>>> return false;
>>>>>>>>>
>>>>>>>>> /* Non-aggregate types can be handled cheaply. */
>>>>>>>>> @@ -13952,7 +13955,7 @@ gimple_canonical_types_compatible_p
>>>>>>>>> (const_tree t1, const_tree t2,
>>>>>>>>> {
>>>>>>>>> case ARRAY_TYPE:
>>>>>>>>> /* Array types are the same if the element types are the same and
>>>>>>>>> - the number of elements are the same. */
>>>>>>>>> + minimum and maximum index are the same. */
>>>>>>>>> if (!gimple_canonical_types_compatible_p (TREE_TYPE (t1), TREE_TYPE
>>>>>>>>> (t2),
>>>>>>>>> trust_type_canonical)
>>>>>>>>>>> TYPE_STRING_FLAG (t1) != TYPE_STRING_FLAG (t2)
>>>>>>>>> @@ -14046,23 +14049,35 @@ gimple_canonical_types_compatible_p
>>>>>>>>> (const_tree t1, const_tree t2,
>>>>>>>>> f1 || f2;
>>>>>>>>> f1 = TREE_CHAIN (f1), f2 = TREE_CHAIN (f2))
>>>>>>>>> {
>>>>>>>>> - /* Skip non-fields and zero-sized fields. */
>>>>>>>>> - while (f1 && (TREE_CODE (f1) != FIELD_DECL
>>>>>>>>> - || (DECL_SIZE (f1)
>>>>>>>>> - && integer_zerop (DECL_SIZE (f1)))))
>>>>>>>>> + /* Skip non-fields. */
>>>>>>>>> + while (f1 && (TREE_CODE (f1) != FIELD_DECL))
>>>>>>>>> f1 = TREE_CHAIN (f1);
>>>>>>>>> - while (f2 && (TREE_CODE (f2) != FIELD_DECL
>>>>>>>>> - || (DECL_SIZE (f2)
>>>>>>>>> - && integer_zerop (DECL_SIZE (f2)))))
>>>>>>>>> + while (f2 && (TREE_CODE (f2) != FIELD_DECL))
>>>>>>>>> f2 = TREE_CHAIN (f2);
>>>>>>>>> if (!f1 || !f2)
>>>>>>>>> break;
>>>>>>>>> +
>>>>>>>>> + tree t1 = TREE_TYPE (f1);
>>>>>>>>> + tree t2 = TREE_TYPE (f2);
>>>>>>>>> +
>>>>>>>>> + /* Special case for flexible array members. */
>>>>>>>>> + if (TREE_CHAIN (f1) == NULL_TREE
>>>>>>>>> + && TREE_CHAIN (f2) == NULL_TREE
>>>>>>>>> + && TREE_CODE (t1) == ARRAY_TYPE
>>>>>>>>> + && TREE_CODE (t2) == ARRAY_TYPE
>>>>>>>>> + && (!DECL_NOT_FLEXARRAY (f1)
>>>>>>>>> + || !DECL_NOT_FLEXARRAY (f2))
>>>>>>>>> + && TYPE_REVERSE_STORAGE_ORDER (t1) == TYPE_REVERSE_STORAGE_ORDER
>>>>>>>>> (t2)
>>>>>>>>> + && TYPE_NONALIASED_COMPONENT (t1) == TYPE_NONALIASED_COMPONENT (t2)
>>>>>>>>> + && gimple_canonical_types_compatible_p
>>>>>>>>> + (TREE_TYPE (t1), TREE_TYPE (t2),
>>>>>>>>> + trust_type_canonical))
>>>>>>>>> + ;
>>>>>>>>> /* The fields must have the same name, offset and type. */
>>>>>>>>> - if (DECL_NONADDRESSABLE_P (f1) != DECL_NONADDRESSABLE_P (f2)
>>>>>>>>> + else if (DECL_NONADDRESSABLE_P (f1) != DECL_NONADDRESSABLE_P (f2)
>>>>>>>>>>> !gimple_compare_field_offset (f1, f2)
>>>>>>>>>>> !gimple_canonical_types_compatible_p
>>>>>>>>> - (TREE_TYPE (f1), TREE_TYPE (f2),
>>>>>>>>> - trust_type_canonical))
>>>>>>>>> + (t1, t2, trust_type_canonical))
>>>>>>>>> return false;
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> @@ -14206,6 +14221,9 @@ verify_type (const_tree t)
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> if (COMPLETE_TYPE_P (t) && TYPE_CANONICAL (t)
>>>>>>>>> + /* We allow a mismatch for flexible array members. */
>>>>>>>>> + && !flexible_array_type_p (t)
>>>>>>>>> + && !flexible_array_type_p (TYPE_CANONICAL (t))
>>>>>>>>> && TYPE_MODE (t) != TYPE_MODE (TYPE_CANONICAL (t)))
>>>>>>>>> {
>>>>>>>>> error ("%<TYPE_MODE%> of %<TYPE_CANONICAL%> is not compatible");
