> 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"); >>> >>> >> >