diff mbox series

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

Message ID 20230525152821.2989599-3-qing.zhao@oracle.com
State New
Headers
Series Accept and Handle the case when a structure including a FAM nested in another structure |

Commit Message

Qing Zhao May 25, 2023, 3:28 p.m. UTC 
  on a structure with a C99 flexible array member being nested in
another structure.

"The GCC extension accepts a structure containing an ISO C99 "flexible array
member", or a union containing such a structure (possibly recursively)
to be a member of a structure.

 There are two situations:

   * A structure containing a C99 flexible array member, or a union
     containing such a structure, is the last field of another structure,
     for example:

          struct flex  { int length; char data[]; };
          union union_flex { int others; struct flex f; };

          struct out_flex_struct { int m; struct flex flex_data; };
          struct out_flex_union { int n; union union_flex flex_data; };

     In the above, both 'out_flex_struct.flex_data.data[]' and
     'out_flex_union.flex_data.f.data[]' are considered as flexible
     arrays too.

   * A structure containing a C99 flexible array member, or a union
     containing such a structure, is not the last 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, accessing a member of the array 'mid_flex.flex_data.data[]'
     might have undefined behavior.  Compilers do not handle such a case
     consistently, Any code relying on this case should be modified to ensure
     that flexible array members only end up at the ends of structures.

     Please use the warning option '-Wflex-array-member-not-at-end' to
     identify all such cases in the source code and modify them.  This
     warning will be on by default starting from GCC 15.
"

gcc/c-family/ChangeLog:

	* c.opt: New option -Wflex-array-member-not-at-end.

gcc/c/ChangeLog:

	* c-decl.cc (finish_struct): Issue warnings for new option.

gcc/ChangeLog:

	* doc/extend.texi: Document GCC extension on a structure containing
	a flexible array member to be a member of another structure.

gcc/testsuite/ChangeLog:

	* gcc.dg/variable-sized-type-flex-array.c: New test.
---
 gcc/c-family/c.opt                            |  5 +++
 gcc/c/c-decl.cc                               |  9 ++++
 gcc/doc/extend.texi                           | 44 ++++++++++++++++++-
 .../gcc.dg/variable-sized-type-flex-array.c   | 31 +++++++++++++
 4 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c

Comments

Joseph Myers May 25, 2023, 8:51 p.m. UTC | #1 
The documentation in this case is OK, though claims about how a future 
version will behave have a poor track record (we tend to end up with such 
claims persisting in the documentation even though the change in question 
didn't get made and might sometimes no longer be considered desirable).
Qing Zhao May 25, 2023, 11:59 p.m. UTC | #2 
> On May 25, 2023, at 4:51 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> 
> The documentation in this case is OK, though claims about how a future 
> version will behave have a poor track record (we tend to end up with such 
> claims persisting in the documentation even though the change in question 
> didn't get made and might sometimes no longer be considered desirable).
Then, do you have any suggestions on this claim? Shall we delete it from the doc? Or keep it?

Qing
> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com
Joseph Myers May 26, 2023, 5:59 p.m. UTC | #3 
On Thu, 25 May 2023, Qing Zhao via Gcc-patches wrote:

> > On May 25, 2023, at 4:51 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> > 
> > The documentation in this case is OK, though claims about how a future 
> > version will behave have a poor track record (we tend to end up with such 
> > claims persisting in the documentation even though the change in question 
> > didn't get made and might sometimes no longer be considered desirable).
> Then, do you have any suggestions on this claim? Shall we delete it from 
> the doc? Or keep it?

My suggestion would be just to say the feature is deprecated without 
saying what a future version will do - also make sure to say it's 
deprecated in the GCC 14 release notes, and then if GCC 15 starts to warn, 
put something in the GCC 15 release notes as well.
Qing Zhao May 26, 2023, 7:07 p.m. UTC | #4 
Thank you for the suggestion.


> On May 26, 2023, at 1:59 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> 
> On Thu, 25 May 2023, Qing Zhao via Gcc-patches wrote:
> 
>>> On May 25, 2023, at 4:51 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>> 
>>> The documentation in this case is OK, though claims about how a future 
>>> version will behave have a poor track record (we tend to end up with such 
>>> claims persisting in the documentation even though the change in question 
>>> didn't get made and might sometimes no longer be considered desirable).
>> Then, do you have any suggestions on this claim? Shall we delete it from 
>> the doc? Or keep it?
> 
> My suggestion would be just to say the feature is deprecated without 
> saying what a future version will do - 

I will modify the doc as:

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 17ef80e75cc..e8a8be93ff1 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -1792,8 +1792,8 @@ consistently.  Any code relying on this case should be modified to ensure
 that flexible array members only end up at the ends of structures.
 
 Please use the warning option @option{-Wflex-array-member-not-at-end} to
-identify all such cases in the source code and modify them.  This warning
-will be on by default starting from GCC 15.
+identify all such cases in the source code and modify them.  This extension
+is now deprecated.
 @end itemize
 
Let me know if you have see any issue with the above.


> also make sure to say it's 
> deprecated in the GCC 14 release notes, and then if GCC 15 starts to warn, 
> put something in the GCC 15 release notes as well.

Okay, will add this to my todo list.

Another question:   is it better for me to rearrange the Patch 1/2 and Patch 2/2 a little bit, 
to put the FE , doc change and corresponding testing case together into one patch, (you have approved the FE part of change in Patch 1/2).
and then the mid-end change to tree-ojbect-size.cc and the corresponding testing cases to another patch?


Thank you!

Qing
> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com
Joseph Myers May 26, 2023, 8:12 p.m. UTC | #5 
On Fri, 26 May 2023, Qing Zhao via Gcc-patches wrote:

> Another question:   is it better for me to rearrange the Patch 1/2 and Patch 2/2 a little bit, 
> to put the FE , doc change and corresponding testing case together into one patch, (you have approved the FE part of change in Patch 1/2).
> and then the mid-end change to tree-ojbect-size.cc and the corresponding testing cases to another patch?

I don't really see this patch as needing to be split up into multiple 
parts at all.
Qing Zhao May 30, 2023, 3:29 p.m. UTC | #6 
> On May 26, 2023, at 4:12 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> 
> On Fri, 26 May 2023, Qing Zhao via Gcc-patches wrote:
> 
>> Another question:   is it better for me to rearrange the Patch 1/2 and Patch 2/2 a little bit, 
>> to put the FE , doc change and corresponding testing case together into one patch, (you have approved the FE part of change in Patch 1/2).
>> and then the mid-end change to tree-ojbect-size.cc and the corresponding testing cases to another patch?
> 
> I don't really see this patch as needing to be split up into multiple 
> parts at all.

Okay. 
Will update the documentation without mentioning the future version number and send out the webrev one more time.

Qing
> 
> -- 
> Joseph S. Myers
> joseph@codesourcery.com
diff mbox series

Patch

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 3333cddeece..c26d9801b63 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -737,6 +737,11 @@  Wformat-truncation=
 C ObjC C++ LTO ObjC++ Joined RejectNegative UInteger Var(warn_format_trunc) Warning LangEnabledBy(C ObjC C++ LTO ObjC++,Wformat=, warn_format >= 1, 0) IntegerRange(0, 2)
 Warn about calls to snprintf and similar functions that truncate output.
 
+Wflex-array-member-not-at-end
+C C++ Var(warn_flex_array_member_not_at_end) Warning
+Warn when a structure containing a C99 flexible array member as the last
+field is not at the end of another structure.
+
 Wif-not-aligned
 C ObjC C++ ObjC++ Var(warn_if_not_aligned) Init(1) Warning
 Warn when the field in a struct is not aligned.
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index e14f514cb6e..ecd10ebb69c 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9278,6 +9278,15 @@  finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
 	TYPE_INCLUDES_FLEXARRAY (t)
 	  = is_last_field && TYPE_INCLUDES_FLEXARRAY (TREE_TYPE (x));
 
+      if (warn_flex_array_member_not_at_end
+	  && !is_last_field
+	  && RECORD_OR_UNION_TYPE_P (TREE_TYPE (x))
+	  && TYPE_INCLUDES_FLEXARRAY (TREE_TYPE (x)))
+	warning_at (DECL_SOURCE_LOCATION (x),
+		    OPT_Wflex_array_member_not_at_end,
+		    "structure containing a flexible array member"
+		    " is not at the end of another structure");
+
       if (DECL_NAME (x)
 	  || RECORD_OR_UNION_TYPE_P (TREE_TYPE (x)))
 	saw_named_field = true;
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index f9d13b495ad..17ef80e75cc 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -1751,7 +1751,49 @@  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
+
+The GCC extension accepts a structure containing an ISO C99 @dfn{flexible array
+member}, or a union containing such a structure (possibly recursively)
+to be a member of a structure.
+
+There are two situations:
+
+@itemize @bullet
+@item
+A structure containing a C99 flexible array member, or a union containing
+such a structure, is the last field of another structure, for example:
+
+@smallexample
+struct flex  @{ int length; char data[]; @};
+union union_flex @{ int others; struct flex f; @};
+
+struct out_flex_struct @{ int m; struct flex flex_data; @};
+struct out_flex_union @{ int n; union union_flex flex_data; @};
+@end smallexample
+
+In the above, both @code{out_flex_struct.flex_data.data[]} and
+@code{out_flex_union.flex_data.f.data[]} are considered as flexible arrays too.
+
+@item
+A structure containing a C99 flexible array member, or a union containing
+such a structure, is not the last 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, accessing a member of the array @code{mid_flex.flex_data.data[]}
+might have undefined behavior.  Compilers do not handle such a case
+consistently.  Any code relying on this case should be modified to ensure
+that flexible array members only end up at the ends of structures.
+
+Please use the warning option @option{-Wflex-array-member-not-at-end} to
+identify all such cases in the source code and modify them.  This warning
+will be on by default starting from GCC 15.
 @end itemize
 
 Non-empty initialization of zero-length
diff --git a/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
new file mode 100644
index 00000000000..3924937bad4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/variable-sized-type-flex-array.c
@@ -0,0 +1,31 @@ 
+/* Test for -Wflex-array-member-not-at-end on structure/union with 
+   C99 flexible array members being embedded into another structure.  */
+/* { dg-do compile } */
+/* { dg-options "-Wflex-array-member-not-at-end" } */
+
+struct flex { int n; int data[]; };
+struct out_flex_end { int m; struct flex flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid { struct flex flex_data; int m; };  /* { dg-warning "structure containing a flexible array member is not at the end of another structure" } */
+/* since the warning has been issued for out_flex_mid, no need to
+   issue warning again when it is included in another structure/union.  */
+struct outer_flex_mid { struct out_flex_mid out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid { int a; struct outer_flex_mid b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+
+struct flex0 { int n; int data[0]; };
+struct out_flex_end0 { int m; struct flex0 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid0 { struct flex0 flex_data; int m; };  /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct outer_flex_mid0 { struct out_flex_mid0 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid0 { int a; struct outer_flex_mid0 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+struct flex1 { int n; int data[1]; };
+struct out_flex_end1 { int m; struct flex1 flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_mid1 { struct flex1 flex_data; int m; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */ 
+struct outer_flex_mid1 { struct out_flex_mid1 out_flex_data; int p; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_mid1 { int a; struct outer_flex_mid1 b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+
+struct flexn { int n; int data[8]; }; 
+struct out_flex_endn { int m; struct flexn flex_data; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */
+struct out_flex_midn { struct flexn flex_data; int m; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */ 
+struct outer_flex_midn { struct out_flex_midn out_flex_data; int p; }; /* { dg-bogus"structure containing a flexible array member is not at the end of another structure" } */
+union flex_union_midn { int a; struct outer_flex_midn b; }; /* { dg-bogus "structure containing a flexible array member is not at the end of another structure" } */