diff mbox series

Improve -Wflex-array-member-not-at-end changes.html wording |Plus: and warning bug? (was: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.)

Message ID fc0d61bb-e787-4bd5-b257-3d7db610bece@codesourcery.com
State New
Headers show
Series Improve -Wflex-array-member-not-at-end changes.html wording |Plus: and warning bug? (was: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.) | expand

Commit Message

Tobias Burnus Sept. 25, 2023, 6:24 p.m. UTC
Hi all,

I stumbled over this as I found the wording in the release notes rather unclear.is.


First, the following gives only a -pedantic warning and not a -Wflex-array-member-not-at-end:

   struct t { int b; int x[]; };
   struct q { int b; struct t a[2]; int c; };

warning: invalid use of structure with flexible array member [-Wpedantic]

If I remove the "[2]", it shows additionally:
   warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

It seems as if it should print latter warning also inside the struct.

Qing? Joseph? Thoughts?

* * *

Secondly, if this is deprecated, shouldn't then the warning enabled by, e.g., -Wall or made
otherwise more prominent? (-std=?) - Currently, one either has to find the new flag or use
-pedantic.

Or is this not really regarded as deprecated? But then (IMHO) we should not really claim so and just
add the warning without deprecation.

BTW; clang-15 prints the -Wgnu-variable-sized-type-not-at-end warning by default.

Joseph, all: Thoughts?

* * *

Cross ref: The patch adding the new warning is r14-2197-g070a6bf0bdc6761
https://gcc.gnu.org/pipermail/gcc-cvs/2023-June/385730.html (cf. previously in this thread)


* * *

Regarding the changes.html wording:

On 07.08.23 16:22, Qing Zhao via Gcc-patches wrote:

> Comparing to the 1st version, the only change is to address Richard's
> comment on refering a warning option for diagnosing deprecated behavior.
...
> +++ b/htdocs/gcc-14/changes.html
> @@ -30,7 +30,18 @@ a work-in-progress.</p>
>   <!-- .................................................................. -->
>   <h2>Caveats</h2>
>   <ul>
> -  <li>...</li>
> +  <li><strong>C:</strong>
> +      Support for the GCC extension, a structure containing a C99 flexible array
> +      member, or a union containing such a structure, is not the last field of
> +      another structure, is deprecated. Refer to
> +      <a href="https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html">
> +      Zero Length Arrays</a>.

...

I find the first sentence difficult to read. What do you think of the following?
(It is hard to come up with some good wording.)



Tobias

PS:  C17 has:
"A structure or union shall not contain a member with incomplete or function type (hence, a structure
  shall not contain an instance of itself, but may contain a pointer to an instance of itself), except that
  the last member of a structure with more than one named member may have incomplete array type;
  such a structure (and any union containing, possibly recursively, a member that is such a structure)
  shall not be a member of a structure or an element of an array."

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

Comments

Richard Biener Sept. 26, 2023, 6:49 a.m. UTC | #1
On Mon, 25 Sep 2023, Tobias Burnus wrote:

> Hi all,
> 
> I stumbled over this as I found the wording in the release notes rather
> unclear.is.
> 
> 
> First, the following gives only a -pedantic warning and not a
> -Wflex-array-member-not-at-end:
> 
>   struct t { int b; int x[]; };
>   struct q { int b; struct t a[2]; int c; };
> 
> warning: invalid use of structure with flexible array member [-Wpedantic]
> 
> If I remove the "[2]", it shows additionally:
>   warning: structure containing a flexible array member is not at the end of
>   another structure [-Wflex-array-member-not-at-end]
> 
> It seems as if it should print latter warning also inside the struct.
> 
> Qing? Joseph? Thoughts?

I think an array element with a flex array is invalid and this is what
is diagnosed here.  Maybe it should say ' as array element'

> * * *
> 
> Secondly, if this is deprecated, shouldn't then the warning enabled by, e.g.,
> -Wall or made
> otherwise more prominent? (-std=?) - Currently, one either has to find the new
> flag or use
> -pedantic.
> 
> Or is this not really regarded as deprecated? But then (IMHO) we should not
> really claim so and just
> add the warning without deprecation.
> 
> BTW; clang-15 prints the -Wgnu-variable-sized-type-not-at-end warning by
> default.
> 
> Joseph, all: Thoughts?
> 
> * * *
> 
> Cross ref: The patch adding the new warning is r14-2197-g070a6bf0bdc6761
> https://gcc.gnu.org/pipermail/gcc-cvs/2023-June/385730.html (cf. previously in
> this thread)
> 
> 
> * * *
> 
> Regarding the changes.html wording:
> 
> On 07.08.23 16:22, Qing Zhao via Gcc-patches wrote:
> 
> > Comparing to the 1st version, the only change is to address Richard's
> > comment on refering a warning option for diagnosing deprecated behavior.
> ...
> > +++ b/htdocs/gcc-14/changes.html
> > @@ -30,7 +30,18 @@ a work-in-progress.</p>
> >   <!-- ..................................................................
> >   -->
> >   <h2>Caveats</h2>
> >   <ul>
> > -  <li>...</li>
> > +  <li><strong>C:</strong>
> > +      Support for the GCC extension, a structure containing a C99 flexible
> > array
> > +      member, or a union containing such a structure, is not the last field
> > of
> > +      another structure, is deprecated. Refer to
> > +      <a href="https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html">
> > +      Zero Length Arrays</a>.
> 
> ...
> 
> I find the first sentence difficult to read. What do you think of the
> following?
> (It is hard to come up with some good wording.)
> 
> --- a/htdocs/gcc-14/changes.html
> +++ b/htdocs/gcc-14/changes.html
> @@ -31,9 +31,10 @@ a work-in-progress.</p>
>  <h2>Caveats</h2>
>  <ul>
>    <li><strong>C:</strong>
> -      Support for the GCC extension, a structure containing a C99 flexible
> array
> -      member, or a union containing such a structure, is not the last field
> of
> -      another structure, is deprecated. Refer to
> +      Support for the GCC extension that a structure containing a C99
> flexible
> +      array (and any union containing a member of such structure) can be a
> +      member of a structure has been deprecated for the case that it is not
> +      the last member. Refer to
>        <a href="https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html">
>        Zero Length Arrays</a>.
>        Any code relying on this extension should be modifed to ensure that
> 
> 
> Tobias
> 
> PS:  C17 has:
> "A structure or union shall not contain a member with incomplete or function
> type (hence, a structure
>  shall not contain an instance of itself, but may contain a pointer to an
>  instance of itself), except that
>  the last member of a structure with more than one named member may have
>  incomplete array type;
>  such a structure (and any union containing, possibly recursively, a member
>  that is such a structure)
>  shall not be a member of a structure or an element of an array."
> 
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra?e 201, 80634
> M?nchen; Gesellschaft mit beschr?nkter Haftung; Gesch?ftsf?hrer: Thomas
> Heurung, Frank Th?rauf; Sitz der Gesellschaft: M?nchen; Registergericht
> M?nchen, HRB 106955
> 
>
Tobias Burnus Sept. 26, 2023, 7:26 a.m. UTC | #2
Hi Richard,

On 26.09.23 08:49, Richard Biener wrote:
> On Mon, 25 Sep 2023, Tobias Burnus wrote:
>> First, the following gives only a -pedantic warning and not a
>> -Wflex-array-member-not-at-end:
>>
>>    struct t { int b; int x[]; };
>>    struct q { int b; struct t a[2]; int c; };
>>
>> warning: invalid use of structure with flexible array member [-Wpedantic]
>>
>> If I remove the "[2]", it shows additionally:
>>    warning: structure containing a flexible array member is not at the end of
>>    another structure [-Wflex-array-member-not-at-end]
>>
>> It seems as if it should print latter warning also inside the struct.
> I think an array element with a flex array is invalid and this is what
> is diagnosed here.  Maybe it should say ' as array element'

My issue is not that it is invalid – but that it is *not* diagnosed
by Qing's -Wflex-array-member-not-at-end

And I believe it should be diagnosed.

(The example is diagnosed when changing 'struct t c[2]' to 'struct t c'
(i.e. array→scalar); it is also diagnosed by "-pedantic", but that diagnoses too much.)

* * *

Additionally in previous email:

* RFC whether -Wflex-array-member-not-at-end should be enabled by, e.g. -Wall,
   if we want to deprecate this feature

* Proposed wording improvement for the associated entry in the release notes (gcc-14/changes.html)

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
Qing Zhao Oct. 2, 2023, 3:58 a.m. UTC | #3
Hi, Tobias,

Sorry for the late reply.

I has been on vacation after Cauldron, and will be back to work in the mid of Oct. will look at this issue at that time.

Qing

> On Sep 25, 2023, at 2:24 PM, Tobias Burnus <tobias@codesourcery.com> wrote:
> 
> Hi all,
> 
> I stumbled over this as I found the wording in the release notes rather unclear.is.
> 
> 
> First, the following gives only a -pedantic warning and not a -Wflex-array-member-not-at-end:
> 
>  struct t { int b; int x[]; };
>  struct q { int b; struct t a[2]; int c; };
> 
> warning: invalid use of structure with flexible array member [-Wpedantic]
> 
> If I remove the "[2]", it shows additionally:
>  warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> 
> It seems as if it should print latter warning also inside the struct.
> 
> Qing? Joseph? Thoughts?
> 
> * * *
> 
> Secondly, if this is deprecated, shouldn't then the warning enabled by, e.g., -Wall or made
> otherwise more prominent? (-std=?) - Currently, one either has to find the new flag or use
> -pedantic.
> 
> Or is this not really regarded as deprecated? But then (IMHO) we should not really claim so and just
> add the warning without deprecation.
> 
> BTW; clang-15 prints the -Wgnu-variable-sized-type-not-at-end warning by default.
> 
> Joseph, all: Thoughts?
> 
> * * *
> 
> Cross ref: The patch adding the new warning is r14-2197-g070a6bf0bdc6761
> https://gcc.gnu.org/pipermail/gcc-cvs/2023-June/385730.html (cf. previously in this thread)
> 
> 
> * * *
> 
> Regarding the changes.html wording:
> 
> On 07.08.23 16:22, Qing Zhao via Gcc-patches wrote:
> 
>> Comparing to the 1st version, the only change is to address Richard's
>> comment on refering a warning option for diagnosing deprecated behavior.
> ...
>> +++ b/htdocs/gcc-14/changes.html
>> @@ -30,7 +30,18 @@ a work-in-progress.</p>
>>  <!-- .................................................................. -->
>>  <h2>Caveats</h2>
>>  <ul>
>> -  <li>...</li>
>> +  <li><strong>C:</strong>
>> +      Support for the GCC extension, a structure containing a C99 flexible array
>> +      member, or a union containing such a structure, is not the last field of
>> +      another structure, is deprecated. Refer to
>> +      <a href="https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html">
>> +      Zero Length Arrays</a>.
> 
> ...
> 
> I find the first sentence difficult to read. What do you think of the following?
> (It is hard to come up with some good wording.)
> 
> --- a/htdocs/gcc-14/changes.html
> +++ b/htdocs/gcc-14/changes.html
> @@ -31,9 +31,10 @@ a work-in-progress.</p>
> <h2>Caveats</h2>
> <ul>
>   <li><strong>C:</strong>
> -      Support for the GCC extension, a structure containing a C99 flexible array
> -      member, or a union containing such a structure, is not the last field of
> -      another structure, is deprecated. Refer to
> +      Support for the GCC extension that a structure containing a C99 flexible
> +      array (and any union containing a member of such structure) can be a
> +      member of a structure has been deprecated for the case that it is not
> +      the last member. Refer to
>       <a href="https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html">
>       Zero Length Arrays</a>.
>       Any code relying on this extension should be modifed to ensure that
> 
> 
> Tobias
> 
> PS:  C17 has:
> "A structure or union shall not contain a member with incomplete or function type (hence, a structure
> shall not contain an instance of itself, but may contain a pointer to an instance of itself), except that
> the last member of a structure with more than one named member may have incomplete array type;
> such a structure (and any union containing, possibly recursively, a member that is such a structure)
> shall not be a member of a structure or an element of an array."
> 
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
Qing Zhao Oct. 19, 2023, 8:49 p.m. UTC | #4
Hi, Tobias,

Sorry for the late reply (just came back from a long vacation after Cauldron).

And thank you for reporting this issue.

Please see my reply embedded below:

> On Sep 25, 2023, at 2:24 PM, Tobias Burnus <tobias@codesourcery.com> wrote:
> 
> Hi all,
> 
> I stumbled over this as I found the wording in the release notes rather unclear.is.
> 
> 
> First, the following gives only a -pedantic warning and not a -Wflex-array-member-not-at-end:
> 
>  struct t { int b; int x[]; };
>  struct q { int b; struct t a[2]; int c; };
> 
> warning: invalid use of structure with flexible array member [-Wpedantic]
> 
> If I remove the "[2]", it shows additionally:
>  warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

I think that the above behavior is correct as Richard mentioned previously.  -:)

First, by C99, a structure with flexible array member cannot be an element of an array, 
Therefore, struct t a[2] is an incorrect usage, the warning:

warning: invalid use of structure with flexible array member [-Wpedantic]

is complaining about this standard violation.  Though the diagnositic  message might need to be more specific like the following:

Warning: invalid use of structure with flexible array member as array element [-Wpedantic]

Then, after fixing this standard violation issue, the testing case is as following:

 struct t { int b; int x[]; };
 struct q { int b; struct t a; int c; };

adding -Wflex-array-member-not-at-end will report the expecting message:

/home/opc/Install/latest/bin/gcc -O  -Wflex-array-member-not-at-end t.c -S
t.c:2:29: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
    2 |  struct q { int b; struct t a; int c; };
      |                             ^

So, I think that GCC’s behavior is correct. 

However, we might need to make the diagnostic message more accurate here (I can submit a small patch to improve this). 

> 
> It seems as if it should print latter warning also inside the struct.
> 
> Qing? Joseph? Thoughts?
> 
> * * *
> 
> Secondly, if this is deprecated, shouldn't then the warning enabled by, e.g., -Wall or made
> otherwise more prominent? (-std=?) - Currently, one either has to find the new flag or use
> -pedantic.

Yes, agreed, However, I think that it might be better to delay this to next GCC release by giving users plenty time to fix all the -Wflex-array-member-not-at-end warnings.  As I know, linux kernel exposed a lot of warnings when adding -Wflex-array-member-not-at-end, and kernel people are trying to fix all these warnings in the source base.  

> 
> Or is this not really regarded as deprecated? But then (IMHO) we should not really claim so and just
> add the warning without deprecation.

I think that our final goal is to deprecate this ambiguous extension from GCC completely, but we need time to mitigate users step by step. 
> 
> BTW; clang-15 prints the -Wgnu-variable-sized-type-not-at-end warning by default.
> 
> Joseph, all: Thoughts?
> 
> * * *
> 
> Cross ref: The patch adding the new warning is r14-2197-g070a6bf0bdc6761
> https://gcc.gnu.org/pipermail/gcc-cvs/2023-June/385730.html (cf. previously in this thread)
> 
> 
> * * *
> 
> Regarding the changes.html wording:
> 
> On 07.08.23 16:22, Qing Zhao via Gcc-patches wrote:
> 
>> Comparing to the 1st version, the only change is to address Richard's
>> comment on refering a warning option for diagnosing deprecated behavior.
> ...
>> +++ b/htdocs/gcc-14/changes.html
>> @@ -30,7 +30,18 @@ a work-in-progress.</p>
>>  <!-- .................................................................. -->
>>  <h2>Caveats</h2>
>>  <ul>
>> -  <li>...</li>
>> +  <li><strong>C:</strong>
>> +      Support for the GCC extension, a structure containing a C99 flexible array
>> +      member, or a union containing such a structure, is not the last field of
>> +      another structure, is deprecated. Refer to
>> +      <a href="https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html">
>> +      Zero Length Arrays</a>.
> 
> ...
> 
> I find the first sentence difficult to read. What do you think of the following?
> (It is hard to come up with some good wording.)
> 
> --- a/htdocs/gcc-14/changes.html
> +++ b/htdocs/gcc-14/changes.html
> @@ -31,9 +31,10 @@ a work-in-progress.</p>
> <h2>Caveats</h2>
> <ul>
>   <li><strong>C:</strong>
> -      Support for the GCC extension, a structure containing a C99 flexible array
> -      member, or a union containing such a structure, is not the last field of
> -      another structure, is deprecated. Refer to
> +      Support for the GCC extension that a structure containing a C99 flexible
> +      array (and any union containing a member of such structure) can be a
> +      member of a structure has been deprecated for the case that it is not
> +      the last member. Refer to
>       <a href="https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html">
>       Zero Length Arrays</a>.
>       Any code relying on this extension should be modifed to ensure that

I  felt the above is more difficult to understand… how about below:

> +      Support for the GCC extension that a structure containing a C99 flexible
> +      array, or a union containing a member of such structure, is not the last field 
> +      of another structure, has been deprecated. Refer to
> 

thanks.

Qing

> Tobias
> 
> PS:  C17 has:
> "A structure or union shall not contain a member with incomplete or function type (hence, a structure
> shall not contain an instance of itself, but may contain a pointer to an instance of itself), except that
> the last member of a structure with more than one named member may have incomplete array type;
> such a structure (and any union containing, possibly recursively, a member that is such a structure)
> shall not be a member of a structure or an element of an array."
> 
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
Kees Cook Oct. 19, 2023, 11:29 p.m. UTC | #5
On Thu, Oct 19, 2023 at 08:49:00PM +0000, Qing Zhao wrote:
> > On Sep 25, 2023, at 2:24 PM, Tobias Burnus <tobias@codesourcery.com> wrote:
> > Secondly, if this is deprecated, shouldn't then the warning enabled by, e.g., -Wall or made
> > otherwise more prominent? (-std=?) - Currently, one either has to find the new flag or use
> > -pedantic.
> 
> Yes, agreed, However, I think that it might be better to delay this to next GCC release by giving users plenty time to fix all the -Wflex-array-member-not-at-end warnings.  As I know, linux kernel exposed a lot of warnings when adding -Wflex-array-member-not-at-end, and kernel people are trying to fix all these warnings in the source base.  

Yeah, for the code bases that use flexible arrays (C99 or GNU
Extension), there are cases where they're used as intentional implicit
unions, and the refactoring to make them unambiguous is non-trivial. I
think it may need to be some time before -Wflex-array-member-not-at-end
ends up in -Wall.

I gave some examples of this code pattern (and potential solutions)
here:
https://lore.kernel.org/lkml/202310161249.43FB42A6@keescook
diff mbox series

Patch

--- a/htdocs/gcc-14/changes.html
+++ b/htdocs/gcc-14/changes.html
@@ -31,9 +31,10 @@  a work-in-progress.</p>
  <h2>Caveats</h2>
  <ul>
    <li><strong>C:</strong>
-      Support for the GCC extension, a structure containing a C99 flexible array
-      member, or a union containing such a structure, is not the last field of
-      another structure, is deprecated. Refer to
+      Support for the GCC extension that a structure containing a C99 flexible
+      array (and any union containing a member of such structure) can be a
+      member of a structure has been deprecated for the case that it is not
+      the last member. Refer to
        <a href="https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html">
        Zero Length Arrays</a>.
        Any code relying on this extension should be modifed to ensure that