Created attachment 51282 [details] struct layout changes bos1 behavior When bos1 is used on a member who is both an array and at the end of a structure, it fails to correctly resolve. This kind of behavior should only happen for flexible array members: struct trailing_array { int a; int b; unsigned char c[16]; }; struct middle_array { int a; unsigned char c[16]; int b; }; ok: sizeof(*working) == 24 ok: sizeof(working->c) == 16 ok: __builtin_object_size(working, 1) == -1 ok: __builtin_object_size(working->c, 1) == 16 ok: sizeof(*broken) == 24 ok: sizeof(broken->c) == 16 ok: __builtin_object_size(broken, 1) == -1 WAT: __builtin_object_size(broken->c, 1) == -1 (expected 16)
GCC treats all trailing arrays no matter what their size as flexible sized arrays. This is by design because of many code out there assumes that.
See PR 44386.
Eww. That means _FORTIFY_SOURCE doesn't work correctly. Can there please be a -fstrict-flex-arrays or something to turn off all the heuristics so a code base can declare it only uses flex arrays for dynamic trailing objects?
I'm not sure how feasible it is to change __builtin_object_size or to add an option to control this behavior but I agree that treating all trailing arrays as flexible array members is overly permissive and unhelpful (GCC warnings like -Warray-bounds are stricter and treat only zero and one-element arrays that way). Let me confirm this request and CC Siddhesh who just submitted a patch for __builtin_dynamic_object_size. Maybe that's a way toward something stricter.
Vim explicitly disables _FORTIFY_SOURCE to keep its use of 1-sized trailing arrays: https://github.com/vim/vim/issues/5581 so either they haven't tested with more recent gcc or they're hitting a corner case where __builtin_object_size does return the subscript value for the trailing member. I inherited the __builtin_object_size behaviour in __builtin_dynamic_object_size to remain consistent with current behaviour: ok: sizeof(*working) == 24 ok: sizeof(working->c) == 16 ok: __builtin_object_size(working, 1) == -1 ok: __builtin_object_size(working->c, 1) == 16 ok: __builtin_dynamic_object_size(working, 1) == -1 ok: __builtin_dynamic_object_size(working->c, 1) == 16 ok: sizeof(*broken) == 24 ok: sizeof(broken->c) == 16 ok: __builtin_object_size(broken, 1) == -1 WAT: __builtin_object_size(broken->c, 1) == -1 (expected 16) ok: __builtin_dynamic_object_size(broken, 1) == -1 WAT: __builtin_dynamic_object_size(broken->c, 1) == -1 (expected 16) However in theory if the pass can see the allocation, it should be able to build the right expression for object size. I'm updating the patchset to meld the two passes into one and I could add -fstrict-flex-arrays as one of the patches to make this stricter.
(In reply to Siddhesh Poyarekar from comment #5) > I'm updating the patchset to meld the two passes into one and I could add > -fstrict-flex-arrays as one of the patches to make this stricter. The work for __builtin_dynamic_object_size seems has been committed into upstream gcc already, however without -fstrict-flex-arrays. do you have plan to add this new option into gcc in stage1?
I couldn't work on -fstrict-flex-arrays then, sorry. I do have it in my plan for gcc 13, but I'll admit it's not on the very top of my list of things to do this year. If you or anyone else needs a stronger guarantee of this making it into gcc 13 and wants to work on it, I'll be happy to help with reviews.
(In reply to Siddhesh Poyarekar from comment #7) > I couldn't work on -fstrict-flex-arrays then, sorry. I do have it in my > plan for gcc 13, but I'll admit it's not on the very top of my list of > things to do this year. If you or anyone else needs a stronger guarantee of > this making it into gcc 13 and wants to work on it, I'll be happy to help > with reviews. thanks for the info. will study this a little bit more and hopefully make it into gcc13.
Just to clarify, __builtin_dynamic_object_size() shouldn't have anything to do with this. What's needed is something like -fstrict-flex-arrays so that all the "trailing array is a flex array" assumptions can be killed everywhere in GCC. Only an _actual_ flex array should be treated as such.
Here's a slightly reworked example: https://godbolt.org/z/EvehMax84
and with a flex array to compare: https://godbolt.org/z/s9nb4Y7q4
In the current tree-object-size.cc, "addr_object_size", it's clearly state the following: 607 /* For &X->fld, compute object size only if fld isn't the last 608 field, as struct { int i; char c[1]; } is often used instead 609 of flexible array member. */ and these part of codes were added back to 2009 with commit eb9ed98a951531f7fc40c69883b3285d58b168b2. it's reasonable to add a new option -fstrict-flex-arrays to remove the "trailing array is a flex array" assumptions in current GCC. and the following utility routine that is added in tree.[h|cc] in 2020 can be used to identify whether a trailing array member reference is a flexible array or not: /* Describes a "special" array member due to which component_ref_size returns null. */ enum struct special_array_member { none, /* Not a special array member. */ int_0, /* Interior array member with size zero. */ trail_0, /* Trailing array member with size zero. */ trail_1 /* Trailing array member with one element. */ }; /* Determines the size of the member referenced by the COMPONENT_REF REF, using its initializer expression if necessary in order to determine the size of an initialized flexible array member. If non-null, set *ARK when REF refers to an interior zero-length array or a trailing one-element array. Returns the size as sizetype (which might be zero for an object with an uninitialized flexible array member) or null if the size cannot be determined. */ tree component_ref_size (tree ref, special_array_member *sam /* = NULL */)
Maybe the enum needs to also be expanded so that [0] can be distinguished from []?
(In reply to Kees Cook from comment #13) > Maybe the enum needs to also be expanded so that [0] can be distinguished > from []? I believe that the IR for real flexible array [] is different from [0], it's already identified by the IR itself, no need for this enum to distinguish.
the following patch will fix the issue with this testing case: [opc@qinzhao-ol8u3-x86 gcc]$ git diff diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc index 5ca87ae3504..7df092346b9 100644 --- a/gcc/tree-object-size.cc +++ b/gcc/tree-object-size.cc @@ -604,9 +604,8 @@ addr_object_size (struct object_size_info *osi, const_tree ptr, else if (var != pt_var && TREE_CODE (pt_var) == MEM_REF) { tree v = var; - /* For &X->fld, compute object size only if fld isn't the last - field, as struct { int i; char c[1]; } is often used instead - of flexible array member. */ + /* For &X->fld, compute object size if fld isn't a flexible array + member. */ while (v && v != pt_var) switch (TREE_CODE (v)) { @@ -645,12 +644,19 @@ addr_object_size (struct object_size_info *osi, const_tree ptr, && TREE_CODE (TREE_TYPE (TREE_OPERAND (v, 0))) == RECORD_TYPE) { - tree fld_chain = DECL_CHAIN (TREE_OPERAND (v, 1)); - for (; fld_chain; fld_chain = DECL_CHAIN (fld_chain)) - if (TREE_CODE (fld_chain) == FIELD_DECL) - break; - - if (fld_chain) + bool is_flexible_array = false; + /* Set for accesses to special trailing arrays. */ + special_array_member sam{ }; + + tree refsize = component_ref_size (v, &sam); + /* if the array is a special trailing array, don't compute + * its size, otherwise, treat it as a normal array. */ + if (sam == special_array_member::trail_0 + || sam == special_array_member::trail_1 + || flexible_array_type_p (TREE_TYPE (TREE_OPERAND (v,0)))) + is_flexible_array = true; + + if (!is_flexible_array) { v = NULL_TREE;
additional work are needed in order to make this task complete: 1. add one more new gcc option: -fstrict-flex-arrays when it's on, only treat the following cases as flexing array: trailing array with size 0; trailing array with size 1; trailing flexible array; all other trailing arrays with size > 1 will be treated as normal arrays. 2. there a lot of places in GCC that currently assume all trailing arrays as flexible array, we might need to update all these places altogether to make GCC behavior consistently. As I checked, most of the places used an old routine array_at_struct_end_p, we might need to replace all the usage of "array_at_struct_end_p" with the new option + the more strict checking on flexing trailing array. let me know if you have any comments and suggestions.
(In reply to qinzhao from comment #16) > additional work are needed in order to make this task complete: > > 1. add one more new gcc option: > > -fstrict-flex-arrays > > when it's on, only treat the following cases as flexing array: > > trailing array with size 0; > trailing array with size 1; > trailing flexible array; > > all other trailing arrays with size > 1 will be treated as normal arrays. Under -fstrict-flex-arrays, arrays of size 0 and 1 should *not* be treated as flex arrays. Only "[]" should be a flexible array. Everything else should be treated as having the literal size given.
The zero size case exists (and is documented) solely as a substitute for flexible array members. Treating is as an ordinary array would disable that extension. It might be appropriate to provide a separate option to control it but conflating it with the other cases (one or more elements) doesn't seem like the robust design. As I mentioned in the review of the Clang change, https://reviews.llvm.org/D126864, so that code bases that use some larger number of elements than zero, such as one, and that can't easily change, can still benefit from the BOS enhancement for the remaining cases, it would be helpful for the new option to accept the minimum number of elements at which a trailing array ceases to be considered a poor-man's flexible array member.
(In reply to Martin Sebor from comment #18) > The zero size case exists (and is documented) solely as a substitute for > flexible array members. Treating is as an ordinary array would disable that > extension. It might be appropriate to provide a separate option to control > it but conflating it with the other cases (one or more elements) doesn't > seem like the robust design. > > As I mentioned in the review of the Clang change, > https://reviews.llvm.org/D126864, so that code bases that use some larger > number of elements than zero, such as one, and that can't easily change, can > still benefit from the BOS enhancement for the remaining cases, it would be > helpful for the new option to accept the minimum number of elements at which > a trailing array ceases to be considered a poor-man's flexible array member. I see your point about gaining the "trailing array" fix without breaking the older code bases, but that doesn't seem to fit the name (nor purpose) of -fstrict-flex-arrays, which should be considered a "complete" fix. To me it looks like -fstrict-flex-arrays should kill the [0] extension, the ancient [1] misuse, and the "anything trailing is flex" logic. If fixing _only_ the latter is desired, perhaps add an option for that, but no one is actually asking for it yet. :) The Linux kernel wants the "fully correct" mode.
Well, I just "asked" for such an option the same way you asked for -fstrict-flex-arrays in comment #3, because I believe it would be useful to make the BOS improvements you're looking for available even to code that can't do a whole-hog replacement of all trailing arrays with flexible array members. The spelling of the option names doesn't seem important to me (they could be separate options, or the same one with an argument).
(In reply to Martin Sebor from comment #20) > Well, I just "asked" for such an option the same way you asked for > -fstrict-flex-arrays in comment #3, because I believe it would be useful to > make the BOS improvements you're looking for available even to code that > can't do a whole-hog replacement of all trailing arrays with flexible array Right, sorry, I meant, "I have a project waiting to use this feature right now", where as other projects might, upon discovering this feature, decide they also only need "-fstrict-flex-arrays". e.g. what option would GCC itself use? > members. The spelling of the option names doesn't seem important to me > (they could be separate options, or the same one with an argument). How about "-fnot-flex-arrays=N" to mean "trailing arrays with N or more elements will NOT be treated like a flex array"? Then code with sockaddr can use "-fnot-flex-arrays=15", code with "[1]" arrays can use "-fnot-flex-arrays=2", code with only "[0]" arrays can use "-fnot-flex-arrays=1", and "-fstrict-flex-arrays" can be an alias for "-fnot-flex-arrays=0", which Linux would use.
(In reply to Kees Cook from comment #21) > How about "-fnot-flex-arrays=N" to mean "trailing arrays with N or more > elements will NOT be treated like a flex array"? > > Then code with sockaddr can use "-fnot-flex-arrays=15", code with "[1]" > arrays can use "-fnot-flex-arrays=2", code with only "[0]" arrays can use > "-fnot-flex-arrays=1", and "-fstrict-flex-arrays" can be an alias for > "-fnot-flex-arrays=0", which Linux would use. An arbitrary N will only make it abuse-friendly and potentially mask bugs. IMO if we choose to make multiple levels here it should only be -fstrict-flex-arrays={1,2} where 1 (the default) only allows "[]" and 2 allows "[0]", disabling all other size values. For anything else, -fno-strict-flex-arrays. My opinion on the default is not strong FWIW.
(In reply to Siddhesh Poyarekar from comment #22) > An arbitrary N will only make it abuse-friendly and potentially mask bugs. > IMO if we choose to make multiple levels here it should only be > -fstrict-flex-arrays={1,2} where 1 (the default) only allows "[]" and 2 > allows "[0]", disabling all other size values. For anything else, That could be ""[0]" or "[1]", disabling all other size values" if we want to build gcc and vim with -fstrict-flex-arrays and keep fortification enabled. Vim explicitly disables fortification right now for this reason. > -fno-strict-flex-arrays. My opinion on the default is not strong FWIW. Also I wonder if there should be an analogous -Wstrict-flex-arrays to issue warnings alongside changing codegen.
For the default, a complication is that standard C++ doesn't allow neither flexible array members nor zero sized arrays, so unless one uses extensions one can only write [1]. I think differentiating between only allowing [] as flex, or [] and [0], or [], [0] and [1], or any trailing array is useful.
So, based on all the discussion so far, how about the following: ** add the following gcc option: -fstrict-flex-arrays=[0|1|2|3] when -fstrict-flex-arrays=0: treat all trailing arrays as flexible arrays. the default behavior; when -fstrict-flex-arrays=1: Only treating [], [0], and [1] as flexible array; when -fstrict-flex-arrays=2: Only treating [] and [0] as flexible array; when -fstrict-flex-arrays=3: Only treating [] as flexible array; The strictest level. any comments?
(In reply to qinzhao from comment #25) > So, based on all the discussion so far, how about the following: > > ** add the following gcc option: > > -fstrict-flex-arrays=[0|1|2|3] > > when -fstrict-flex-arrays=0: > treat all trailing arrays as flexible arrays. the default behavior; Wouldn't this be -fno-strict-flex-arrays, i.e. the current behaviour? > when -fstrict-flex-arrays=1: > Only treating [], [0], and [1] as flexible array; > > when -fstrict-flex-arrays=2: > Only treating [] and [0] as flexible array; > > when -fstrict-flex-arrays=3: > Only treating [] as flexible array; The strictest level. If yes, then you end up having: -fstrict-flex-arrays=[1|2|3] with, I suppose, 1 as the default based on Jakub's comment about maximum compatibility support.
> On Jun 14, 2022, at 11:39 AM, siddhesh at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org> wrote: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 > > --- Comment #26 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> --- > (In reply to qinzhao from comment #25) >> So, based on all the discussion so far, how about the following: >> >> ** add the following gcc option: >> >> -fstrict-flex-arrays=[0|1|2|3] >> >> when -fstrict-flex-arrays=0: >> treat all trailing arrays as flexible arrays. the default behavior; > > Wouldn't this be -fno-strict-flex-arrays, i.e. the current behaviour? Yes, it’s the same. =0 is aliased with -fno-strict-flex-arrays. The point is, the larger the value of LEVEL, the stricter with treating the flexing array. i.e, 0 is the least strict, and 3 is the strictest mode. But we can delete the level 0 if not necessary. > >> when -fstrict-flex-arrays=1: >> Only treating [], [0], and [1] as flexible array; >> >> when -fstrict-flex-arrays=2: >> Only treating [] and [0] as flexible array; >> >> when -fstrict-flex-arrays=3: >> Only treating [] as flexible array; The strictest level. > > If yes, then you end up having: > > -fstrict-flex-arrays=[1|2|3] > > with, I suppose, 1 as the default based on Jakub's comment about maximum > compatibility support. Yes. And 3 is the one Kees requested for kernel usage.
(In reply to Qing Zhao from comment #27) > > Wouldn't this be -fno-strict-flex-arrays, i.e. the current behaviour? > > Yes, it’s the same. =0 is aliased with -fno-strict-flex-arrays. That is indeed what we do for many options, -fno-whatever is alias to -fwhatever=0 (or -fwhatever=something for options which take enums and not numbers).
(In reply to Jakub Jelinek from comment #28) > (In reply to Qing Zhao from comment #27) > > > Wouldn't this be -fno-strict-flex-arrays, i.e. the current behaviour? > > > > Yes, it’s the same. =0 is aliased with -fno-strict-flex-arrays. > > That is indeed what we do for many options, -fno-whatever is alias to > -fwhatever=0 (or -fwhatever=something for options which take enums and not > numbers). thank you for the info. could you point me an example of such option? then I can check to see how to implement this alias relationship between =0 and -fno-strict-flex-arrays?
grep shows: common.opt:Common Alias(Wattribute_alias=, 1, 0) Warning common.opt:Common Alias(Wimplicit-fallthrough=,3,0) Warning c-family/c.opt:C ObjC C++ ObjC++ Warning Alias(Warray-parameter=, 2, 0) c-family/c.opt:C++ ObjC++ Warning Alias(Wcatch-value=, 1, 0) c-family/c.opt:C ObjC C++ LTO ObjC++ Alias(Wdangling-pointer=, 2, 0) Warning c-family/c.opt:C ObjC C++ ObjC++ Warning Alias(Wformat=, 1, 0) c-family/c.opt:C ObjC C++ LTO ObjC++ Warning Alias(Wformat-overflow=, 1, 0) IntegerRange(0, 2) c-family/c.opt:C ObjC C++ LTO ObjC++ Warning Alias(Wformat-truncation=, 1, 0) c-family/c.opt:C ObjC C++ LTO ObjC++ Warning Alias(Wstringop-overflow=, 2, 0) c-family/c.opt:C++ Warning Alias(Wplacement-new=, 1, 0) c-family/c.opt:C ObjC C++ ObjC++ Warning Alias(Wshift-overflow=, 1, 0) c-family/c.opt:C ObjC C++ ObjC++ Warning Alias(Wunused-const-variable=, 2, 0) c-family/c.opt:C++ ObjC++ Alias(faligned-new=,1,0) fortran/lang.opt:Fortran Alias(ftail-call-workaround=,1,0)
It doesn't make sense to have a mode in which `int array[0]` is accepted but is not a flex array. Either that should be a compilation error (as the standard specifies), or it should be a flex array. Accepting it as an extension but having it do the wrong thing is not useful or helpful. Note that Clang has a dedicated warning flag for zero-length arrays: -Wzero-length-array, so anyone who wants to prohibit them may use -Werror=zero-length-array. It would be helpful for GCC could follow suit there. The other proposed modes: - Treat all trailing arrays as flexible arrays. the default behavior; - Only treating [], [0], and [1] as flexible array; - Only treating [] and [0] as flexible array; do make sense.
(In reply to James Y Knight from comment #31) > It doesn't make sense to have a mode in which `int array[0]` is accepted but > is not a flex array. > > Either that should be a compilation error (as the standard specifies), or it > should be a flex array. Accepting it as an extension but having it do the > wrong thing is not useful or helpful. > > Note that Clang has a dedicated warning flag for zero-length arrays: > -Wzero-length-array, so anyone who wants to prohibit them may use > -Werror=zero-length-array. It would be helpful for GCC could follow suit > there. there is a Bugzilla that has been filed for GCC to request the same warning for GCC: https://gcc.gnu.org/bugzilla//show_bug.cgi?id=94428 -Wzero-length-array As suggested by Siddhesh in comment#23, -Wstrict-flex-arrays might be necessary to be added too, and -Wzero-length-array will be an alias to -Wstrict-flex-arrays=3
(In reply to qinzhao from comment #32) > there is a Bugzilla that has been filed for GCC to request the same warning > for GCC: > https://gcc.gnu.org/bugzilla//show_bug.cgi?id=94428 > > -Wzero-length-array Great. Adding that flag, and eliminating the -fstrict-flex-arrays=3 option from this proposal would be good. > As suggested by Siddhesh in comment#23, -Wstrict-flex-arrays might be > necessary to be added too, and > -Wzero-length-array will be an alias to > -Wstrict-flex-arrays=3 I don't understand what the -Wstrict-flex-arrays warning and its multiple levels is proposed to actually do. Is it supposed to warn on the structs that change behavior in the corresponding -fstrict-flex-array=LEVEL? But that would mean -Wstrict-flex-arrays=1 would warn on any array at the end of a struct which has a size other than 0 or 1. That's clearly not going to be actually practical...so perhaps you had something else in mind?
-fstrict-flex-arrays=3 is still needed. (E.g. for proper FORTIFY coverage, etc.) I don't have an opinion about the -W options, though.(In reply to James Y Knight from comment #33) > (In reply to qinzhao from comment #32) > > there is a Bugzilla that has been filed for GCC to request the same warning > > for GCC: > > https://gcc.gnu.org/bugzilla//show_bug.cgi?id=94428 > > > > -Wzero-length-array > > Great. Adding that flag, and eliminating the -fstrict-flex-arrays=3 option > from this proposal would be good. Hmm? No, -fstrict-flex-arrays=3 is still needed (because it changes compiler _behavior_, e.g. for proper FORTIFY coverage or trailing arrays, etc). I don't have a strong opinion about the -W options; but they can't warn if they just see a struct declaration with a 0 or 1 element array: userspace will have those for years to come. Maybe it would warn if such a struct member is ever actually used in the code? That kind of behavior would be useful for the Linux kernel at least.
(In reply to James Y Knight from comment #33) > > I don't understand what the -Wstrict-flex-arrays warning and its multiple > levels is proposed to actually do. > > Is it supposed to warn on the structs that change behavior in the > corresponding -fstrict-flex-array=LEVEL? But that would mean > -Wstrict-flex-arrays=1 would warn on any array at the end of a struct which > has a size other than 0 or 1. That's clearly not going to be actually > practical...so perhaps you had something else in mind? I think that -Wstrict-flex-arrays will need to be cooperated with -fstrict-flex-arrays=N, i.e, only when -fstrict-flex-arrays=N is presenting, -Wstrict-flex-arrays will be valid and report the warnings when see a [0], or [1], or any trailing array based on N: when -fstrict-flex-arrays =0, -Wstrict-flex-arrays will NOT issue any warnings; =1, -Wstrict-flex-arrays will issue warnings when an array ref's index is larger than the upper bounds of a trailing array that is a regular trailing array; =2, -Wstrict-flex-arrays will issue warnings when an array ref's index is larger than the upper bounds of a trailing array that is a regular trailing array or [1]; =3, -Wstrict-flex-arrays will issue warnings when an array ref's index is larger than the upper bounds of a trailing array that is a regular trailing array or [1], or [0]. let me know if you have any comment and suggestion on this.
(In reply to Kees Cook from comment #34) > > Great. Adding that flag, and eliminating the -fstrict-flex-arrays=3 option > > from this proposal would be good. > > Hmm? No, -fstrict-flex-arrays=3 is still needed (because it changes compiler > _behavior_, e.g. for proper FORTIFY coverage or trailing arrays, etc). There is no purpose served by writing a struct member `int x[0];` other than to create a FAM. Zero-length arrays are not permitted by the C standard, but are a GCC compiler extension explicitly for the purpose of creating a FAM. This is entirely unlike `int x[1];` or `int x[10];` which of course have a primary meaning as a concrete array size... If the linux kernel doesn't want to allow `int x[0];` FAMs, then prohibit them entirely using -Werror=zero-length-array (once it's implemented).
(In reply to qinzhao from comment #35) > I think that -Wstrict-flex-arrays will need to be cooperated with > -fstrict-flex-arrays=N, i.e, only when -fstrict-flex-arrays=N is presenting, > -Wstrict-flex-arrays will be valid and report the warnings when see a [0], > or [1], or any trailing array based on N: When -fstrict-flex-arrays is used, I'd expect the existing -Warray-bounds warning to already emit diagnostics in the cases you list, because those cases should no longer be special-cased to act as a FAM, and thus no longer explicitly suppress it.
(In reply to James Y Knight from comment #37) > (In reply to qinzhao from comment #35) > > I think that -Wstrict-flex-arrays will need to be cooperated with > > -fstrict-flex-arrays=N, i.e, only when -fstrict-flex-arrays=N is presenting, > > -Wstrict-flex-arrays will be valid and report the warnings when see a [0], > > or [1], or any trailing array based on N: > > When -fstrict-flex-arrays is used, I'd expect the existing -Warray-bounds > warning to already emit diagnostics in the cases you list, because those > cases should no longer be special-cased to act as a FAM, and thus no longer > explicitly suppress it. yes, that's right. instead of adding a new -Wstrict-flex-arrays, we can also update the current -Warray-bounds to cooperate with the new -fstrict-flex-arrays to issue warnings according to the different level of strict-flex-arrays.
(In reply to Siddhesh Poyarekar from comment #23) > Also I wonder if there should be an analogous -Wstrict-flex-arrays to issue > warnings alongside changing codegen. please take a look at comments #32 and above to see whether the discussion of -Wstrict-flex-arrays is similar as what are in your mind?
(In reply to James Y Knight from comment #36) > (In reply to Kees Cook from comment #34) > > > Great. Adding that flag, and eliminating the -fstrict-flex-arrays=3 option > > > from this proposal would be good. > > > > Hmm? No, -fstrict-flex-arrays=3 is still needed (because it changes compiler > > _behavior_, e.g. for proper FORTIFY coverage or trailing arrays, etc). > > There is no purpose served by writing a struct member `int x[0];` other than > to create a FAM. Zero-length arrays are not permitted by the C standard, but > are a GCC compiler extension explicitly for the purpose of creating a FAM. > This is entirely unlike `int x[1];` or `int x[10];` which of course have a > primary meaning as a concrete array size... > > If the linux kernel doesn't want to allow `int x[0];` FAMs, then prohibit > them entirely using -Werror=zero-length-array (once it's implemented). [Kees, correct me if I'm wrong.] I don't think this satisfies what Kees initially asked for. The GCC extension that a trailing `[0]' array in a structure is causing FORTIFY to fail. It would be great to remove them all, but that's more-or-less a separate issue from making FORTIFY work in all instances. (From what I understand, removing the trailing `[0]' from Linux is an ongoing project.) The question then is if `-fstrict-flex-arrays=3' is used, what does a `[0]' at the end of a struct represent (assuming GCC no longer treats it as an FAM)?
(In reply to Bill Wendling from comment #40) > The question then is if `-fstrict-flex-arrays=3' is used, what does a `[0]' > at the end of a struct represent (assuming GCC no longer treats it as an > FAM)? It's a zero-sized object. The same as `struct { } foo;`
The master branch has been updated by Qing Zhao <qinzhao@gcc.gnu.org>: https://gcc.gnu.org/g:b9ad850e86b863c24f6f4f5acf08d49944cc7bbe commit r13-3171-gb9ad850e86b863c24f6f4f5acf08d49944cc7bbe Author: Qing Zhao <qing.zhao@oracle.com> Date: Fri Oct 7 14:59:01 2022 +0000 Use array_at_struct_end_p in __builtin_object_size [PR101836] Use array_at_struct_end_p to determine whether the trailing array of a structure is flexible array member in __builtin_object_size. gcc/ChangeLog: PR tree-optimization/101836 * tree-object-size.cc (addr_object_size): Use array_at_struct_end_p to determine a flexible array member reference. gcc/testsuite/ChangeLog: 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. * gcc.dg/strict-flex-array-2.c: New test. * gcc.dg/strict-flex-array-3.c: New test.
the related patch list for this work is (gcc13) 2a27ae32fabf85685ffff758459d7ec284ccb95a 710c9676520dfd38b4bfdcc937ce026ed89921d6 ace0ae09332bbc6b95e084c2c2b17c466339ff76 b9ad850e86b863c24f6f4f5acf08d49944cc7bbe 1879e48f3d8595bc9e7f583bbd12df3c6f5c42dc
fixed in gcc13 already