* [V7][PATCH 0/2]Accept and Handle the case when a structure including a FAM nested in another structure @ 2023-05-19 20:49 Qing Zhao 2023-05-19 20:49 ` [V7][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao 2023-05-19 20:49 ` [V7][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650] Qing Zhao 0 siblings, 2 replies; 10+ messages in thread From: Qing Zhao @ 2023-05-19 20:49 UTC (permalink / raw) To: joseph, richard.guenther, jakub, gcc-patches Cc: keescook, siddhesh, uecker, Qing Zhao Hi, This is the 7th version of the patch, which rebased on the latest trunk. This is an important patch needed by Linux Kernel security project. We already have an extensive discussion on this issue and I have went through 6 revisions of the patches based on the discussion and resolved all the comments and suggestions raised during the discussion; compared to the 6th version, the major change are: 1. update the documentation to replace the mentioning of GCC14 with GCC15. 2. update the documentation to replace the following wording: "A structure or a union with a C99 flexible array member" with: "A structure containing a C99 flexible array member, or a union containing such a structure," All others are the same as 6th version. the 6th version are here: https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616312.html https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616313.html https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616314.html Kees has tested the 6th version of the patch with Linux kernel, and everything is good. relsolved many false positives for bounds checking. Notes for the review history of these patches (2 patches) 1.The patch 1/2: Handle component_ref to a structre/union field including flexible array member [PR101832] The C front-end part has been approved by Joseph. For the middle-end, most of the change has been reviewed by Richard (and modified based on his comments and suggestions), except the change in tree-object-size.cc. 2.The patch 2/2: Update documentation to clarify a GCC extension This is basically a C FE and documentation change, I have updated it based on previous comments and suggestions. Joseph, could you review it to see whether this version is ready to go? bootstrapped and regression tested on aarch64 and x86. Okay for commit? thanks a lot. Qing (for more details on the review history, I listed other important notes below: A. Richard Biener has reviewed the middle-end part of the first patch and raised some comments for the 4th version: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/613643.html I updated it with his suggestion and Sandra’s comments as 5th version: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614100.html B. The comments for the 5th version: https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614511.html (In this one, Joseph approved the C FE change of the first patch). https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614514.html (In this one, Joseph raised two comments on the documentation wordings for the 2nd patch. And I updated based on his comment in the 6th version) ) ^ permalink raw reply [flat|nested] 10+ messages in thread
* [V7][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] 2023-05-19 20:49 [V7][PATCH 0/2]Accept and Handle the case when a structure including a FAM nested in another structure Qing Zhao @ 2023-05-19 20:49 ` Qing Zhao 2023-05-19 23:11 ` Bernhard Reutner-Fischer 2023-05-19 20:49 ` [V7][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650] Qing Zhao 1 sibling, 1 reply; 10+ messages in thread From: Qing Zhao @ 2023-05-19 20:49 UTC (permalink / raw) To: joseph, richard.guenther, jakub, gcc-patches Cc: keescook, siddhesh, uecker, Qing Zhao 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 diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc index b5b491cf2da..2c620b681d9 100644 --- a/gcc/c/c-decl.cc +++ b/gcc/c/c-decl.cc @@ -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; diff --git a/gcc/lto/lto-common.cc b/gcc/lto/lto-common.cc index 537570204b3..35827aab075 100644 --- a/gcc/lto/lto-common.cc +++ b/gcc/lto/lto-common.cc @@ -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); diff --git a/gcc/print-tree.cc b/gcc/print-tree.cc index ccecd3dc6a7..aaded53b1b1 100644 --- a/gcc/print-tree.cc +++ b/gcc/print-tree.cc @@ -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) diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c new file mode 100644 index 00000000000..60078e11634 --- /dev/null +++ b/gcc/testsuite/gcc.dg/builtin-object-size-pr101832.c @@ -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; +} diff --git a/gcc/tree-core.h b/gcc/tree-core.h index 9d44c04bf03..6f33ea0db97 100644 --- a/gcc/tree-core.h +++ b/gcc/tree-core.h @@ -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; diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc index 9a936a91983..1619d144ecd 100644 --- a/gcc/tree-object-size.cc +++ b/gcc/tree-object-size.cc @@ -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))) diff --git a/gcc/tree-streamer-in.cc b/gcc/tree-streamer-in.cc index c803800862c..1b11e5a1bae 100644 --- a/gcc/tree-streamer-in.cc +++ b/gcc/tree-streamer-in.cc @@ -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 diff --git a/gcc/tree-streamer-out.cc b/gcc/tree-streamer-out.cc index 5751f77273b..ce1b143e1ea 100644 --- a/gcc/tree-streamer-out.cc +++ b/gcc/tree-streamer-out.cc @@ -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)); } 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) /* In an IDENTIFIER_NODE, this means that assemble_name was called with this string as an argument. */ -- 2.31.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [V7][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] 2023-05-19 20:49 ` [V7][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao @ 2023-05-19 23:11 ` Bernhard Reutner-Fischer 2023-05-24 14:09 ` Qing Zhao 0 siblings, 1 reply; 10+ messages in thread From: Bernhard Reutner-Fischer @ 2023-05-19 23:11 UTC (permalink / raw) To: Qing Zhao via Gcc-patches Cc: Bernhard Reutner-Fischer, Qing Zhao, joseph, richard.guenther, jakub, keescook, siddhesh, uecker 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. */ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [V7][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] 2023-05-19 23:11 ` Bernhard Reutner-Fischer @ 2023-05-24 14:09 ` Qing Zhao 2023-05-25 5:41 ` Bernhard Reutner-Fischer 0 siblings, 1 reply; 10+ messages in thread From: Qing Zhao @ 2023-05-24 14:09 UTC (permalink / raw) To: Bernhard Reutner-Fischer Cc: Qing Zhao via Gcc-patches, joseph, richard.guenther, jakub, keescook, siddhesh, uecker 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. */ > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [V7][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] 2023-05-24 14:09 ` Qing Zhao @ 2023-05-25 5:41 ` Bernhard Reutner-Fischer 2023-05-25 15:21 ` Qing Zhao 0 siblings, 1 reply; 10+ messages in thread From: Bernhard Reutner-Fischer @ 2023-05-25 5:41 UTC (permalink / raw) To: Qing Zhao Cc: Qing Zhao via Gcc-patches, joseph, richard.guenther, jakub, keescook, siddhesh, uecker 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. */ >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [V7][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] 2023-05-25 5:41 ` Bernhard Reutner-Fischer @ 2023-05-25 15:21 ` Qing Zhao 0 siblings, 0 replies; 10+ messages in thread From: Qing Zhao @ 2023-05-25 15:21 UTC (permalink / raw) To: Bernhard Reutner-Fischer Cc: Qing Zhao via Gcc-patches, joseph, richard.guenther, jakub, keescook, siddhesh, uecker > 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. */ ^ permalink raw reply [flat|nested] 10+ messages in thread
* [V7][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650] 2023-05-19 20:49 [V7][PATCH 0/2]Accept and Handle the case when a structure including a FAM nested in another structure Qing Zhao 2023-05-19 20:49 ` [V7][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao @ 2023-05-19 20:49 ` Qing Zhao 2023-05-19 21:12 ` Joseph Myers 1 sibling, 1 reply; 10+ messages in thread From: Qing Zhao @ 2023-05-19 20:49 UTC (permalink / raw) To: joseph, richard.guenther, jakub, gcc-patches Cc: keescook, siddhesh, uecker, Qing Zhao on a structure with a C99 flexible array member being nested in another structure. "GCC extension accepts a structure containing an ISO C99 "flexible array member", or a union containing such a structure (possibly recursively) to be a member of a structure. There are two situations: * A structure containing a C99 flexible array member, or a union containing such a structure, is the last field of another structure, for example: struct flex { int length; char data[]; }; union union_flex { int others; struct flex f; }; struct out_flex_struct { int m; struct flex flex_data; }; struct out_flex_union { int n; union union_flex flex_data; }; In the above, both 'out_flex_struct.flex_data.data[]' and 'out_flex_union.flex_data.f.data[]' are considered as flexible arrays too. * A structure containing a C99 flexible array member, or a union containing such a structure, is the middle field of another structure, for example: struct flex { int length; char data[]; }; struct mid_flex { int m; struct flex flex_data; int n; }; In the above, 'mid_flex.flex_data.data[]' has undefined behavior. Compilers do not handle such case consistently, Any code relying on such case should be modified to ensure that flexible array members only end up at the ends of structures. Please use warning option '-Wflex-array-member-not-at-end' to identify all such cases in the source code and modify them. This warning will be on by default starting from GCC 15. " gcc/c-family/ChangeLog: * c.opt: New option -Wflex-array-member-not-at-end. gcc/c/ChangeLog: * c-decl.cc (finish_struct): Issue warnings for new option. gcc/ChangeLog: * doc/extend.texi: Document GCC extension on a structure containing a flexible array member to be a member of another structure. gcc/testsuite/ChangeLog: * gcc.dg/variable-sized-type-flex-array.c: New test. --- gcc/c-family/c.opt | 5 +++ gcc/c/c-decl.cc | 9 ++++ gcc/doc/extend.texi | 45 ++++++++++++++++++- .../gcc.dg/variable-sized-type-flex-array.c | 31 +++++++++++++ 4 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 3333cddeece..c26d9801b63 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -737,6 +737,11 @@ Wformat-truncation= C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2) Warn about calls to snprintf and similar functions that truncate output. +Wflex-array-member-not-at-end +C C++ Var(warn_flex_array_member_not_at_end) Warning +Warn when a structure containing a C99 flexible array member as the last +field is not at the end of another structure. + Wif-not-aligned C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning Warn when the field in a struct is not aligned. diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc index 2c620b681d9..9a48f28788d 100644 --- a/gcc/c/c-decl.cc +++ b/gcc/c/c-decl.cc @@ -9293,6 +9293,15 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes, TYPE_INCLUDE_FLEXARRAY (t) = is_last_field && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x)); + if (warn_flex_array_member_not_at_end + && !is_last_field + && RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)) + && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x))) + warning_at (DECL_SOURCE_LOCATION (x), + OPT_Wflex_array_member_not_at_end, + "structure containing a flexible array member" + " is not at the end of another structure"); + if (DECL_NAME (x) || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x))) saw_named_field = true; diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index ed8b9c8a87b..6425ba57e88 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -1751,7 +1751,50 @@ Flexible array members may only appear as the last member of a A structure containing a flexible array member, or a union containing such a structure (possibly recursively), may not be a member of a structure or an element of an array. (However, these uses are -permitted by GCC as extensions.) +permitted by GCC as extensions, see details below.) +@end itemize + +GCC extension accepts a structure containing an ISO C99 @dfn{flexible array +member}, or a union containing such a structure (possibly recursively) +to be a member of a structure. + +There are two situations: + +@itemize @bullet +@item +A structure containing a C99 flexible array member, or a union containing +such a structure, is the last field of another structure, for example: + +@smallexample +struct flex @{ int length; char data[]; @}; +union union_flex @{ int others; struct flex f; @}; + +struct out_flex_struct @{ int m; struct flex flex_data; @}; +struct out_flex_union @{ int n; union union_flex flex_data; @}; +@end smallexample + +In the above, both @code{out_flex_struct.flex_data.data[]} and +@code{out_flex_union.flex_data.f.data[]} are considered as flexible arrays too. + + +@item +A structure containing a C99 flexible array member, or a union containing +such a structure, is the middle field of another structure, for example: + +@smallexample +struct flex @{ int length; char data[]; @}; + +struct mid_flex @{ int m; struct flex flex_data; int n; @}; +@end smallexample + +In the above, @code{mid_flex.flex_data.data[]} has undefined behavior. +Compilers do not handle such case consistently, Any code relying on +such case should be modified to ensure that flexible array members +only end up at the ends of structures. + +Please use warning option @option{-Wflex-array-member-not-at-end} to +identify all such cases in the source code and modify them. This warning +will be on by default starting from GCC 15. @end itemize Non-empty initialization of zero-length diff --git a/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c new file mode 100644 index 00000000000..3924937bad4 --- /dev/null +++ b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c @@ -0,0 +1,31 @@ +/* Test for -Wflex-array-member-not-at-end on structure/union with + C99 flexible array members being embedded into another structure. */ +/* { dg-do compile } */ +/* { dg-options "-Wflex-array-member-not-at-end" } */ + +struct flex { int n; int data[]; }; +struct out_flex_end { int m; struct flex flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ +struct out_flex_mid { struct flex flex_data; int m; }; /* { dg-warning "structure containing a flexible array member is not at the end of another structure" } */ +/* since the warning has been issued for out_flex_mid, no need to + issue warning again when it is included in another structure/union. */ +struct outer_flex_mid { struct out_flex_mid out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ +union flex_union_mid { int a; struct outer_flex_mid b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ + + +struct flex0 { int n; int data[0]; }; +struct out_flex_end0 { int m; struct flex0 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ +struct out_flex_mid0 { struct flex0 flex_data; int m; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ +struct outer_flex_mid0 { struct out_flex_mid0 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ +union flex_union_mid0 { int a; struct outer_flex_mid0 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ + +struct flex1 { int n; int data[1]; }; +struct out_flex_end1 { int m; struct flex1 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ +struct out_flex_mid1 { struct flex1 flex_data; int m; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ +struct outer_flex_mid1 { struct out_flex_mid1 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ +union flex_union_mid1 { int a; struct outer_flex_mid1 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ + +struct flexn { int n; int data[8]; }; +struct out_flex_endn { int m; struct flexn flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ +struct out_flex_midn { struct flexn flex_data; int m; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */ +struct outer_flex_midn { struct out_flex_midn out_flex_data; int p; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */ +union flex_union_midn { int a; struct outer_flex_midn b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ -- 2.31.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [V7][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650] 2023-05-19 20:49 ` [V7][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650] Qing Zhao @ 2023-05-19 21:12 ` Joseph Myers 2023-05-24 13:05 ` Qing Zhao 2023-05-24 16:57 ` Qing Zhao 0 siblings, 2 replies; 10+ messages in thread From: Joseph Myers @ 2023-05-19 21:12 UTC (permalink / raw) To: Qing Zhao Cc: richard.guenther, jakub, gcc-patches, keescook, siddhesh, uecker On Fri, 19 May 2023, Qing Zhao via Gcc-patches wrote: > +GCC extension accepts a structure containing an ISO C99 @dfn{flexible array "The GCC extension" or "A GCC extension". > +@item > +A structure containing a C99 flexible array member, or a union containing > +such a structure, is the middle field of another structure, for example: There might be more than one middle field, and I think this case also includes where it's the *first* field - any field other than the last. > +@smallexample > +struct flex @{ int length; char data[]; @}; > + > +struct mid_flex @{ int m; struct flex flex_data; int n; @}; > +@end smallexample > + > +In the above, @code{mid_flex.flex_data.data[]} has undefined behavior. And it's not literally mid_flex.flex_data.data[] that has undefined behavior, but trying to access a member of that array. > +Compilers do not handle such case consistently, Any code relying on "such a case", and "," should be "." at the end of a sentence. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [V7][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650] 2023-05-19 21:12 ` Joseph Myers @ 2023-05-24 13:05 ` Qing Zhao 2023-05-24 16:57 ` Qing Zhao 1 sibling, 0 replies; 10+ messages in thread From: Qing Zhao @ 2023-05-24 13:05 UTC (permalink / raw) To: Joseph Myers Cc: richard.guenther, jakub, gcc-patches, keescook, siddhesh, uecker Joseph, Thanks a lot for the review. And sorry for my late reply (just came back from a short vacation). > On May 19, 2023, at 5:12 PM, Joseph Myers <joseph@codesourcery.com> wrote: > > On Fri, 19 May 2023, Qing Zhao via Gcc-patches wrote: > >> +GCC extension accepts a structure containing an ISO C99 @dfn{flexible array > > "The GCC extension" or "A GCC extension". Okay. > >> +@item >> +A structure containing a C99 flexible array member, or a union containing >> +such a structure, is the middle field of another structure, for example: > > There might be more than one middle field, and I think this case also > includes where it's the *first* field - any field other than the last. Good point. Will fix this. > >> +@smallexample >> +struct flex @{ int length; char data[]; @}; >> + >> +struct mid_flex @{ int m; struct flex flex_data; int n; @}; >> +@end smallexample >> + >> +In the above, @code{mid_flex.flex_data.data[]} has undefined behavior. > > And it's not literally mid_flex.flex_data.data[] that has undefined > behavior, but trying to access a member of that array. Yes, you are right. Will fix this. > >> +Compilers do not handle such case consistently, Any code relying on > > "such a case", and "," should be "." at the end of a sentence. Okay, will fix this. Thanks Qing > > -- > Joseph S. Myers > joseph@codesourcery.com ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [V7][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650] 2023-05-19 21:12 ` Joseph Myers 2023-05-24 13:05 ` Qing Zhao @ 2023-05-24 16:57 ` Qing Zhao 1 sibling, 0 replies; 10+ messages in thread From: Qing Zhao @ 2023-05-24 16:57 UTC (permalink / raw) To: Joseph Myers Cc: richard.guenther, jakub, gcc-patches, keescook, siddhesh, uecker Hi, Joseph, I modified the gcc/doc/extend.texi per your suggestion as following: Let me know if you have further comment and suggestion on this patch. I will send out the V8 of the patch after some testing. Thanks. Qing. ============================================ diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index 6425ba57e88..9aedaa802e0 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -1754,7 +1754,7 @@ structure or an element of an array. (However, these uses are permitted by GCC as extensions, see details below.) @end itemize -GCC extension accepts a structure containing an ISO C99 @dfn{flexible array +The GCC extension accepts a structure containing an ISO C99 @dfn{flexible array member}, or a union containing such a structure (possibly recursively) to be a member of a structure. @@ -1776,10 +1776,9 @@ struct out_flex_union @{ int n; union union_flex flex_data; @}; In the above, both @code{out_flex_struct.flex_data.data[]} and @code{out_flex_union.flex_data.f.data[]} are considered as flexible arrays too. - @item A structure containing a C99 flexible array member, or a union containing -such a structure, is the middle field of another structure, for example: +such a structure, is not the last field of another structure, for example: @smallexample struct flex @{ int length; char data[]; @}; @@ -1787,12 +1786,12 @@ struct flex @{ int length; char data[]; @}; struct mid_flex @{ int m; struct flex flex_data; int n; @}; @end smallexample -In the above, @code{mid_flex.flex_data.data[]} has undefined behavior. -Compilers do not handle such case consistently, Any code relying on -such case should be modified to ensure that flexible array members -only end up at the ends of structures. +In the above, accessing a member of the array @code{mid_flex.flex_data.data[]} +might have undefined behavior. Compilers do not handle such a case +consistently. Any code relying on this case should be modified to ensure +that flexible array members only end up at the ends of structures. -Please use warning option @option{-Wflex-array-member-not-at-end} to +Please use the warning option @option{-Wflex-array-member-not-at-end} to identify all such cases in the source code and modify them. This warning will be on by default starting from GCC 15. @end itemize > On May 19, 2023, at 5:12 PM, Joseph Myers <joseph@codesourcery.com> wrote: > > On Fri, 19 May 2023, Qing Zhao via Gcc-patches wrote: > >> +GCC extension accepts a structure containing an ISO C99 @dfn{flexible array > > "The GCC extension" or "A GCC extension". > >> +@item >> +A structure containing a C99 flexible array member, or a union containing >> +such a structure, is the middle field of another structure, for example: > > There might be more than one middle field, and I think this case also > includes where it's the *first* field - any field other than the last. > >> +@smallexample >> +struct flex @{ int length; char data[]; @}; >> + >> +struct mid_flex @{ int m; struct flex flex_data; int n; @}; >> +@end smallexample >> + >> +In the above, @code{mid_flex.flex_data.data[]} has undefined behavior. > > And it's not literally mid_flex.flex_data.data[] that has undefined > behavior, but trying to access a member of that array. > >> +Compilers do not handle such case consistently, Any code relying on > > "such a case", and "," should be "." at the end of a sentence. > > -- > Joseph S. Myers > joseph@codesourcery.com ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-05-25 15:21 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-19 20:49 [V7][PATCH 0/2]Accept and Handle the case when a structure including a FAM nested in another structure Qing Zhao 2023-05-19 20:49 ` [V7][PATCH 1/2] Handle component_ref to a structre/union field including flexible array member [PR101832] Qing Zhao 2023-05-19 23:11 ` Bernhard Reutner-Fischer 2023-05-24 14:09 ` Qing Zhao 2023-05-25 5:41 ` Bernhard Reutner-Fischer 2023-05-25 15:21 ` Qing Zhao 2023-05-19 20:49 ` [V7][PATCH 2/2] Update documentation to clarify a GCC extension [PR77650] Qing Zhao 2023-05-19 21:12 ` Joseph Myers 2023-05-24 13:05 ` Qing Zhao 2023-05-24 16:57 ` Qing Zhao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).