[Bug c/101836] New: __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[Bug c/101836] New: __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

            Bug ID: 101836
           Summary: __builtin_object_size(P->M, 1) where M is an array and
                    the last member of a struct fails
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: kees at outflux dot net
  Target Milestone: ---

Created attachment 51282
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51282&action=edit
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)

[Bug c/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
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.

[Bug c/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
See PR 44386.

[Bug c/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #3 from Kees Cook <kees at outflux dot net> ---
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?

[Bug c/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-10-13
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
                 CC|                            |msebor at gcc dot gnu.org,
                   |                            |siddhesh at redhat dot com

--- Comment #4 from Martin Sebor <msebor at gcc dot gnu.org> ---
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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
          Component|c                           |middle-end

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[siddhesh at gotplt dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

Siddhesh Poyarekar <siddhesh at gotplt dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |siddhesh at gotplt dot org

--- Comment #5 from Siddhesh Poyarekar <siddhesh at gotplt dot org> ---
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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

qinzhao at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |qinzhao at gcc dot gnu.org

--- Comment #6 from qinzhao at gcc dot gnu.org ---
(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?

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[siddhesh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #7 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #8 from qinzhao at gcc dot gnu.org ---
(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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #9 from Kees Cook <kees at outflux dot net> ---
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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #10 from Kees Cook <kees at outflux dot net> ---
Here's a slightly reworked example:
https://godbolt.org/z/EvehMax84

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #11 from Kees Cook <kees at outflux dot net> ---
and with a flex array to compare:
https://godbolt.org/z/s9nb4Y7q4

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #12 from qinzhao at gcc dot gnu.org ---
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 */)

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #13 from Kees Cook <kees at outflux dot net> ---
Maybe the enum needs to also be expanded so that [0] can be distinguished from
[]?

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #14 from qinzhao at gcc dot gnu.org ---
(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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #15 from qinzhao at gcc dot gnu.org ---
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;

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #16 from qinzhao at gcc dot gnu.org ---
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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #17 from Kees Cook <kees at outflux dot net> ---
(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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #18 from Martin Sebor <msebor at gcc dot gnu.org> ---
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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #19 from Kees Cook <kees at outflux dot net> ---
(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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #20 from Martin Sebor <msebor at gcc dot gnu.org> ---
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).

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #21 from Kees Cook <kees at outflux dot net> ---
(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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[siddhesh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #22 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
(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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[siddhesh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #23 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
(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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #24 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #25 from qinzhao at gcc dot gnu.org ---
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?

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[siddhesh at gcc dot gnu.org]

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?

> 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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qing.zhao at oracle dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #27 from Qing Zhao <qing.zhao at oracle dot com> ---
> 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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #28 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(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).

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #29 from qinzhao at gcc dot gnu.org ---
(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?

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #30 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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)

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[foom at fuhm dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

James Y Knight <foom at fuhm dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |foom at fuhm dot net

--- Comment #31 from James Y Knight <foom at fuhm dot net> ---
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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #32 from qinzhao at gcc dot gnu.org ---
(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

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[foom at fuhm dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #33 from James Y Knight <foom at fuhm dot net> ---
(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?

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #34 from Kees Cook <kees at outflux dot net> ---
-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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #35 from qinzhao at gcc dot gnu.org ---
(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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[foom at fuhm dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #36 from James Y Knight <foom at fuhm dot net> ---
(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).

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[foom at fuhm dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #37 from James Y Knight <foom at fuhm dot net> ---
(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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #38 from qinzhao at gcc dot gnu.org ---
(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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #39 from qinzhao at gcc dot gnu.org ---
(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?

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[isanbard at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

Bill Wendling <isanbard at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |isanbard at gmail dot com

--- Comment #40 from Bill Wendling <isanbard at gmail dot com> ---
(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)?

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[kees at outflux dot net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #41 from Kees Cook <kees at outflux dot net> ---
(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;`

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #42 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
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.

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

--- Comment #43 from qinzhao at gcc dot gnu.org ---
the related patch list for this work is (gcc13)

2a27ae32fabf85685ffff758459d7ec284ccb95a
710c9676520dfd38b4bfdcc937ce026ed89921d6
ace0ae09332bbc6b95e084c2c2b17c466339ff76
b9ad850e86b863c24f6f4f5acf08d49944cc7bbe
1879e48f3d8595bc9e7f583bbd12df3c6f5c42dc

[Bug middle-end/101836] __builtin_object_size(P->M, 1) where M is an array and the last member of a struct fails

[qinzhao at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

qinzhao at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #44 from qinzhao at gcc dot gnu.org ---
fixed in gcc13 already