[V4,2/2] Update documentation to clarify a GCC extension

  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:

   * The structure with a C99 flexible array member 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.

   * The structure with a C99 flexible array member 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[]' is allowed to be extended
     flexibly to the padding.  E.g, up to 4 elements.

     However, relying on space in struct padding is a bad programming
     practice, compilers do not handle such extension consistently, Any
     code relying on this behavior should be modified to ensure that
     flexible array members only end up at the ends of structures.

     Please use warning option '-Wgnu-variable-sized-type-not-at-end' to
     identify all such cases in the source code and modify them.  This
     extension will be deprecated from gcc in the next release.
"

gcc/c-family/ChangeLog:

	* c.opt: New option -Wgnu-variable-sized-type-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                               |  7 +++
 gcc/doc/extend.texi                           | 48 ++++++++++++++++++-
 .../gcc.dg/variable-sized-type-flex-array.c   | 31 ++++++++++++
 4 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
  

Ping.

Qing

> On Feb 24, 2023, at 1:35 PM, Qing Zhao <qing.zhao@oracle.com> wrote:
> 
> 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:
> 
>   * The structure with a C99 flexible array member 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.
> 
>   * The structure with a C99 flexible array member 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[]' is allowed to be extended
>     flexibly to the padding.  E.g, up to 4 elements.
> 
>     However, relying on space in struct padding is a bad programming
>     practice, compilers do not handle such extension consistently, Any
>     code relying on this behavior should be modified to ensure that
>     flexible array members only end up at the ends of structures.
> 
>     Please use warning option '-Wgnu-variable-sized-type-not-at-end' to
>     identify all such cases in the source code and modify them.  This
>     extension will be deprecated from gcc in the next release.
> "
> 
> gcc/c-family/ChangeLog:
> 
> 	* c.opt: New option -Wgnu-variable-sized-type-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                               |  7 +++
> gcc/doc/extend.texi                           | 48 ++++++++++++++++++-
> .../gcc.dg/variable-sized-type-flex-array.c   | 31 ++++++++++++
> 4 files changed, 90 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..660ac07f3d4 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.
> 
> +Wgnu-variable-sized-type-not-at-end
> +C C++ Var(warn_variable_sized_type_not_at_end) Warning
> +Warn about structures or unions with C99 flexible array members are not
> +at the end of a 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 f589a2f5192..c5b54f07965 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -9296,6 +9296,13 @@ finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
> 	       && is_last_field)
> 	TYPE_INCLUDE_FLEXARRAY (t) = true;
> 
> +      if (warn_variable_sized_type_not_at_end
> +	  && !is_last_field
> +	  && TYPE_INCLUDE_FLEXARRAY (TREE_TYPE (x)))
> +	warning_at (DECL_SOURCE_LOCATION (x),
> +		    OPT_Wgnu_variable_sized_type_not_at_end,
> +		    "variable sized type not at the end of a struct");
> +
>       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 c1122916255..e278148c332 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -1748,7 +1748,53 @@ 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
> +The structure with a C99 flexible array member 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
> +The structure with a C99 flexible array member 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[]} is allowed to be extended
> +flexibly to the padding.  E.g, up to 4 elements.
> +
> +However, relying on space in struct padding is a bad programming practice,
> +compilers do not handle such extension consistently, Any code relying on
> +this behavior should be modified to ensure that flexible array members
> +only end up at the ends of structures.
> +
> +Please use warning option  @option{-Wgnu-variable-sized-type-not-at-end} to
> +identify all such cases in the source code and modify them.  This extension
> +will be deprecated from gcc in the next release.
> @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..e3f65c5ed07
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
> @@ -0,0 +1,31 @@
> +/* Test for -Wgnu-variable-sized-type-not-at-end on structure/union with 
> +   C99 flexible array members being embedded into another structure.  */
> +/* { dg-do compile } */
> +/* { dg-options "-Wgnu-variable-sized-type-not-at-end" } */
> +
> +struct flex { int n; int data[]; };
> +struct out_flex_end { int m; struct flex flex_data; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +struct out_flex_mid { struct flex flex_data; int m; };  /* { dg-warning "variable sized type not at the end of a struct" } */
> +/* 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 "variable sized type not at the end of a struct" } */
> +union flex_union_mid { int a; struct outer_flex_mid b; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +
> +
> +struct flex0 { int n; int data[0]; };
> +struct out_flex_end0 { int m; struct flex0 flex_data; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +struct out_flex_mid0 { struct flex0 flex_data; int m; };  /* { dg-bogus "variable sized type not at the end of a struct" } */
> +struct outer_flex_mid0 { struct out_flex_mid0 out_flex_data; int p; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +union flex_union_mid0 { int a; struct outer_flex_mid0 b; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +
> +struct flex1 { int n; int data[1]; };
> +struct out_flex_end1 { int m; struct flex1 flex_data; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +struct out_flex_mid1 { struct flex1 flex_data; int m; }; /* { dg-bogus "variable sized type not at the end of a struct" } */ 
> +struct outer_flex_mid1 { struct out_flex_mid1 out_flex_data; int p; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +union flex_union_mid1 { int a; struct outer_flex_mid1 b; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +
> +struct flexn { int n; int data[8]; }; 
> +struct out_flex_endn { int m; struct flexn flex_data; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> +struct out_flex_midn { struct flexn flex_data; int m; }; /* { dg-bogus"variable sized type not at the end of a struct" } */ 
> +struct outer_flex_midn { struct out_flex_midn out_flex_data; int p; }; /* { dg-bogus"variable sized type not at the end of a struct" } */
> +union flex_union_midn { int a; struct outer_flex_midn b; }; /* { dg-bogus "variable sized type not at the end of a struct" } */
> -- 
> 2.31.1
>
  

On 3/2/23 17:03, Qing Zhao via Gcc-patches wrote:
> Ping.

It looks to me like there is an associated code patch (for PR101832) 
that is still under technical discussion?  Or is this documentation 
patch independent of that change?

-Sandra
  

> On Mar 12, 2023, at 7:14 PM, Sandra Loosemore <sandra@codesourcery.com> wrote:
> 
> On 3/2/23 17:03, Qing Zhao via Gcc-patches wrote:
>> Ping.
> 
> It looks to me like there is an associated code patch (for PR101832) that is still under technical discussion?  

Yes, the 1st patch in this serie is the patch for PR101832.  Which is under review.
This 2nd patch is the associated doc change.
> Or is this documentation patch independent of that change?

These two patches are better to be associated together. But I think the patch for PR101832 might be
 put in first into GCC13 if people think the doc change is risky at this moment. 

Qing
> 
> -Sandra
  

On 2/24/23 11:35, Qing Zhao via Gcc-patches wrote:

> gcc/c-family/ChangeLog:
> 
> 	* c.opt: New option -Wgnu-variable-sized-type-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.

I'm only a documentation (and nios2) maintainer so I cannot approve 
adding a new option or warning.  I was going to review the documentation 
parts, at least, but I think this proposal is technically flawed because 
it is trying to document something that is undefined behavior in ways 
that it doesn't actually behave on all targets.

> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index c1122916255..e278148c332 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -1748,7 +1748,53 @@ 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
> +The structure with a C99 flexible array member 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
> +The structure with a C99 flexible array member 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[]} is allowed to be extended
> +flexibly to the padding.  E.g, up to 4 elements.

I think this paragraph is incorrect; how GCC lays out this structure 
depends on the target and ABI.  Looking at output from a GCC 12 
nios2-elf build I have handy, I see it is in fact laying out mid_flex so 
that member n is at the same address at offset 8 as flex_data.data[0], 
which is not useful at all.

> +However, relying on space in struct padding is a bad programming practice,
> +compilers do not handle such extension consistently, Any code relying on
> +this behavior should be modified to ensure that flexible array members
> +only end up at the ends of structures.
> +
> +Please use warning option  @option{-Wgnu-variable-sized-type-not-at-end} to
> +identify all such cases in the source code and modify them.  This extension
> +will be deprecated from gcc in the next release.

My suggestion would be to make this a hard error instead of a warning, 
unless there is some real body of code out there that depends on this 
feature on a target that actually does insert padding.  If it's a 
warning, it ought to be enabled by default.  And, rather than trying to 
document the behavior, the manual should just say it's undefined.

-Sandra
  

Hi, Sandra,

Thanks a lot for your review and comment.

Yes, the issue you raised in below was  a really tough one that I didn’t feel very comfortable to handle it well…

This documentation change is mainly to fix: PR77650 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77650).

The real user case for it was the old glibc headers. (As Joshph mentioned in the comment #1:
 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77650#c1)

And from my understanding, the code in glibc has already been fixed. 
https://gcc.gnu.org/pipermail/gcc-patches/2023-February/611220.html

But I am not sure in addition to Glibc, whether there are other use cases out there currently.

That’s the reason we cannot simply reject this case as an error at this moment. And the new warning -Wgnu-variable-sized-type-not-at-end
Is added to catch such case in user case to encourage user to update them. And then later we can completely delete this case from GCC support.

Right now, GCC’s implementation cannot handle such case consistently. So, how to document this case is really hard.

> On Mar 14, 2023, at 11:26 PM, Sandra Loosemore <sandra@codesourcery.com> wrote:
> 
> On 2/24/23 11:35, Qing Zhao via Gcc-patches wrote:
> 
>> gcc/c-family/ChangeLog:
>> 	* c.opt: New option -Wgnu-variable-sized-type-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.
> 
> I'm only a documentation (and nios2) maintainer so I cannot approve adding a new option or warning.  I was going to review the documentation parts, at least, but I think this proposal is technically flawed because it is trying to document something that is undefined behavior in ways that it doesn't actually behave on all targets.

So, should we mention it in the documentation at all?
Of mention it but specify the possible undefined behavior, the potential risk to use this case, and then warn the user to get rid of this usage with the new warning?

> 
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index c1122916255..e278148c332 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -1748,7 +1748,53 @@ 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
>> +The structure with a C99 flexible array member 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
>> +The structure with a C99 flexible array member 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[]} is allowed to be extended
>> +flexibly to the padding.  E.g, up to 4 elements.
> 
> I think this paragraph is incorrect; how GCC lays out this structure depends on the target and ABI.  Looking at output from a GCC 12 nios2-elf build I have handy, I see it is in fact laying out mid_flex so that member n is at the same address at offset 8 as flex_data.data[0], which is not useful at all.

I think that this behavior you mentioned is consistent with what’s the expected. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77650#c5. 
But not sure whether we should document it or not?

>> +However, relying on space in struct padding is a bad programming practice,
>> +compilers do not handle such extension consistently, Any code relying on
>> +this behavior should be modified to ensure that flexible array members
>> +only end up at the ends of structures.
>> +
>> +Please use warning option  @option{-Wgnu-variable-sized-type-not-at-end} to
>> +identify all such cases in the source code and modify them.  This extension
>> +will be deprecated from gcc in the next release.
> 
> My suggestion would be to make this a hard error instead of a warning, unless there is some real body of code out there that depends on this feature on a target that actually does insert padding.  If it's a warning, it ought to be enabled by default.  And, rather than trying to document the behavior, the manual should just say it's undefined.

As I mentioned above, the warning was added mainly for unknown user cases, to warn them about this case and encourage them to update the code to prepare GCC deprecating this misfeature. 

Yes, I think “rather than trying to document the behavior, just say it’s undefined” might be more reasonable, I will update the doc based on this suggestion.

Thanks a lot.

Qing
> 
> -Sandra