Hi, Richard,
> On Jun 28, 2022, at 3:16 AM, Richard Biener <richard.guent...@gmail.com> > wrote: > > On Mon, Jun 27, 2022 at 4:20 PM Qing Zhao via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> Hi, >> >> Per our discussion in the bug report, I came up with the following patch: >> >> ======= >> >> PR101836: Add a new option -fstrict-flex-array[=n] >> >> Add the new option and use it in __builtin_object_size. >> >> Treat the trailing array of a structure as a flexible array member in a >> stricter way. The value of 'n' controls the level of strictness. >> 'n'=0 is the least strict, all trailing arrays of structures are treated >> as flexible array members; This is the default behavior of GCC without >> specify >> this option. >> 'n'=3 is the strictest, only when the trailing array is declared as a >> flexible array member per C99 standard onwards ([]), it is treated as a >> flexible array member; >> There are two more levels in between 0 and 3, which are provided to support >> older codes that use GCC zero-length array extension ([0]), or one-size >> array as >> flexible array member ([1]): >> When 'n' is 1, the trailing array is treated as a flexible array member >> when it is declared as either [], [0], or [1]; >> When 'n' is 2, the trailing array is treated as a flexible array member >> when it is declared as either [], or [0]. >> >> There are other places in GCC that conservatively treat flexible array >> members. >> A follow-up patch will make -ftrict-flex-array option to control all these >> places consistently. >> >> Bootstrapped and regression tested on both X86 and aarch64, no issues. >> >> Any comment and suggestion? > > Since this aims at the C or C++ frontends but the middle-end eventually > consumes > this it would be much nicer to encode this in the types themselves. Yes, I agree. Let the C/C++ FE to decide whether the [0], [1], or [] trailing array field of a structure is a flex array member or not based on the option -fstrict-flex-array and then encode such info in the FIELD_DECL as flag DECL_NOT_FLEXARRAY. Later, the middle end just check the flag DECL_NOT_FLEXARRAY of the FIELD_DECL to decide whether the trailing array is flexible array or not. This will eliminate all the hacks in the middle-end (as you mentioned, array_at_struct_end_p, and “trailing_array”, etc, and there are quite some phases use this routine to query, and in an in-consistent way) > Since the least > strict reading is the default right now it would be a flag (on the > FIELD_DECL I suppose) > like DECL_NOT_FLEXARRAY or DECL_FIXED_SIZE? Alternatively the flag could > also be on the record type enclosing the trailing array member (but > type sharing might > make this more difficult in the end). > > There's also array_at_struct_end_p which is supposed to be the main > query interface > for this (but it seems people sneaked in more variants with eventually > different semantics ... :/) Yes, there are many places right now that query “array_at_struct_end_p”, I was planning a follow-up patchs to replace all “array_at_struct_end_p” with is_flexible_array_p. I guess that this follow-up patch will take quite some time to finish. So, my next step: 1. Update this current patch per your suggestion above, i.e, A. Add a new flag (DECL_NOT_FLEXARRAY) in FIELD_DECL, default is FALSE; B. In C/C++ FE, for a trailing array field of a structure, decide DECL_NOT_FLEXARRAY flag as following: Level 1: any trailing array that is NOT [0], [1], [], DECL_NOT_FLEXARRAY is TRUE; Level 2: any trailing array that is NOT [0], [], DECL_NOT_FLEXARRAY is TRUE; Level 3: any trailing array that is not [], DECL_NOT_FLEXARRAY is TRUE C. Use DECL_NOT_FLEXARRAY in tree-object-size.c for __builtin_object_size to resolve bug PR101836. 2. Then replace all “array_at_struct_end_p” with using DECL_NOT_FLEXARRAY in GCC, adding new testing cases For different phases with different level, resolving all regressions. I plan separate patches for 1 and 2. Commit 1 first to enable kernel work as soon as possible. Then continue working on 2 to make GCC consistent in gcc13. Let me know if you have any suggestion or comment. Thanks Qing > > Richard. > > > >> Okay for commit to Gcc13? >> >> thanks. >> >> Qing >> >> ======================= >> >> gcc/ >> >> PR tree-optimization/101836 >> * common.opt (fstrict-flex-array, fstrict-flex-array=): New options. >> * doc/invoke.texi (-fstrict-flex-array, -fstrict-flex-array=): >> Document. >> * tree-object-size.cc (addr_object_size): Call is_flexible_array_p to >> check whether an array is a flexible array. >> * tree.cc (special_array_member_type): New routine. >> (is_flexible_array_p): New routine. >> (component_ref_size): Call special_array_member_type to decide the >> type of special array member. >> * tree.h (enum struct special_array_member): Add is_vla, trail_flex. >> (special_array_member_type): New prototype. >> (is_flexible_array_p): New prototype. >> >> gcc/testsuite/ >> >> PR tree-optimization/101836 >> * gcc.dg/pr101836.c: New test. >> * gcc.dg/pr101836_1.c: New test. >> * gcc.dg/pr101836_2.c: New test. >> * gcc.dg/pr101836_3.c: New test. >> * gcc.dg/pr101836_4.c: New test. >> * gcc.dg/pr101836_5.c: New test. >> >> >> The complete patch is: