> On Nov 25, 2024, at 16:46, Martin Uecker <uec...@tugraz.at> wrote:
> 
> 
> Hi Qing,
> 
> Am Montag, dem 25.11.2024 um 17:40 +0000 schrieb Qing Zhao:
>> Hi, Martin,
>> 
>> I didn’t go through all the details of your patch.
>> 
>> But I have one question:
>> 
>> Did you consider the effect of the option -fstrict-flex-array 
>> (https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/C-Dialect-Options.html#index-fstrict-flex-arrays)
>>  on how gcc treats the zero size trailing array, 1-element trailing array as 
>> flexible array member in the patch?
> 
> I used the function which was already there which
> does not take this into account.  For the new version
> of the patch this should not matter anymore.

Why it’s not matter anymore?

For the following testing case:

struct S{int x,y[1];}*a;
int main(void){
 struct S{int x,y[];};
}

With your latest patch,  the two structures are considered as compatible with 
-g;
However, if we add -fstrict-flex-array=2 or -fstrict-flex-array=3,  the 
trailing array y[1] is NOT treated
as FAM anymore, as a result, these two structure are NOT compatible too. 

Do I miss anything obvious? 

Thanks.

Qing
> 
> Martin
> 
> 
>> 
>> thanks.
>> 
>> Qing
>>> On Nov 23, 2024, at 14:45, Martin Uecker <uec...@tugraz.at> 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");
>>> 
>>> 
>> 
> 

Reply via email to