public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
@ 2023-08-07 14:22 Qing Zhao
  2023-08-08  7:55 ` Richard Biener
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Qing Zhao @ 2023-08-07 14:22 UTC (permalink / raw)
  To: rguenther, gcc-patches; +Cc: joseph, Qing Zhao

Hi,

This is the 2nd version of the patch.
Comparing to the 1st version, the only change is to address Richard's
comment on refering a warning option for diagnosing deprecated behavior.


Okay for committing?

thanks.

Qing

======

*htdocs/gcc-14/changes.html (Caveats): Add notice about deprecating a C
extension about flexible array members.
---
 htdocs/gcc-14/changes.html | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html
index dad1ba53..eae25f1a 100644
--- a/htdocs/gcc-14/changes.html
+++ 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>.
+      Any code relying on this extension should be modifed to ensure that
+      C99 flexible array members only end up at the ends of structures.
+      Please use the warning option
+      <a href="https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wflex-array-member-not-at-end"><code>-Wflex-array-member-not-at-end</code></a> to
+      identify all such cases in the source code and modify them.
+  </li>
 </ul>
 
 
-- 
2.31.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
  2023-08-07 14:22 [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members Qing Zhao
@ 2023-08-08  7:55 ` Richard Biener
  2023-09-25 18:24 ` 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.) Tobias Burnus
  2024-05-04 11:11 ` [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members Sebastian Huber
  2 siblings, 0 replies; 20+ messages in thread
From: Richard Biener @ 2023-08-08  7:55 UTC (permalink / raw)
  To: Qing Zhao; +Cc: gcc-patches, joseph

On Mon, 7 Aug 2023, Qing Zhao wrote:

> Hi,
> 
> This is the 2nd version of the patch.
> Comparing to the 1st version, the only change is to address Richard's
> comment on refering a warning option for diagnosing deprecated behavior.
> 
> 
> Okay for committing?

OK.

> thanks.
> 
> Qing
> 
> ======
> 
> *htdocs/gcc-14/changes.html (Caveats): Add notice about deprecating a C
> extension about flexible array members.
> ---
>  htdocs/gcc-14/changes.html | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html
> index dad1ba53..eae25f1a 100644
> --- a/htdocs/gcc-14/changes.html
> +++ 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>.
> +      Any code relying on this extension should be modifed to ensure that
> +      C99 flexible array members only end up at the ends of structures.
> +      Please use the warning option
> +      <a href="https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wflex-array-member-not-at-end"><code>-Wflex-array-member-not-at-end</code></a> to
> +      identify all such cases in the source code and modify them.
> +  </li>
>  </ul>
>  
>  
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* 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.)
  2023-08-07 14:22 [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members Qing Zhao
  2023-08-08  7:55 ` Richard Biener
@ 2023-09-25 18:24 ` Tobias Burnus
  2023-09-26  6:49   ` Richard Biener
                     ` (2 more replies)
  2024-05-04 11:11 ` [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members Sebastian Huber
  2 siblings, 3 replies; 20+ messages in thread
From: Tobias Burnus @ 2023-09-25 18:24 UTC (permalink / raw)
  To: Qing Zhao, rguenther, gcc-patches; +Cc: joseph

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 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.)
  2023-09-25 18:24 ` 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.) Tobias Burnus
@ 2023-09-26  6:49   ` Richard Biener
  2023-09-26  7:26     ` Improve -Wflex-array-member-not-at-end changes.html wording |Plus: and warning bug? Tobias Burnus
  2023-10-02  3:58   ` 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.) Qing Zhao
  2023-10-19 20:49   ` Qing Zhao
  2 siblings, 1 reply; 20+ messages in thread
From: Richard Biener @ 2023-09-26  6:49 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: Qing Zhao, gcc-patches, joseph

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

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Improve -Wflex-array-member-not-at-end changes.html wording |Plus: and warning bug?
  2023-09-26  6:49   ` Richard Biener
@ 2023-09-26  7:26     ` Tobias Burnus
  0 siblings, 0 replies; 20+ messages in thread
From: Tobias Burnus @ 2023-09-26  7:26 UTC (permalink / raw)
  To: Richard Biener; +Cc: Qing Zhao, gcc-patches, joseph

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 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.)
  2023-09-25 18:24 ` 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.) Tobias Burnus
  2023-09-26  6:49   ` Richard Biener
@ 2023-10-02  3:58   ` Qing Zhao
  2023-10-19 20:49   ` Qing Zhao
  2 siblings, 0 replies; 20+ messages in thread
From: Qing Zhao @ 2023-10-02  3:58 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: Richard Biener, gcc Patches, joseph

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 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.)
  2023-09-25 18:24 ` 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.) Tobias Burnus
  2023-09-26  6:49   ` Richard Biener
  2023-10-02  3:58   ` 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.) Qing Zhao
@ 2023-10-19 20:49   ` Qing Zhao
  2023-10-19 23:29     ` Kees Cook
  2 siblings, 1 reply; 20+ messages in thread
From: Qing Zhao @ 2023-10-19 20:49 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: Richard Biener, GCC Patches, Joseph Myers, Kees Cook

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: 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.)
  2023-10-19 20:49   ` Qing Zhao
@ 2023-10-19 23:29     ` Kees Cook
  0 siblings, 0 replies; 20+ messages in thread
From: Kees Cook @ 2023-10-19 23:29 UTC (permalink / raw)
  To: Qing Zhao; +Cc: Tobias Burnus, Richard Biener, GCC Patches, Joseph Myers

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

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
  2023-08-07 14:22 [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members Qing Zhao
  2023-08-08  7:55 ` Richard Biener
  2023-09-25 18:24 ` 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.) Tobias Burnus
@ 2024-05-04 11:11 ` Sebastian Huber
  2024-05-06  7:08   ` Richard Biener
  2 siblings, 1 reply; 20+ messages in thread
From: Sebastian Huber @ 2024-05-04 11:11 UTC (permalink / raw)
  To: Qing Zhao, rguenther, gcc-patches; +Cc: joseph

On 07.08.23 16:22, Qing Zhao via Gcc-patches wrote:
> Hi,
> 
> This is the 2nd version of the patch.
> Comparing to the 1st version, the only change is to address Richard's
> comment on refering a warning option for diagnosing deprecated behavior.
> 
> 
> Okay for committing?
> 
> thanks.
> 
> Qing
> 
> ======
> 
> *htdocs/gcc-14/changes.html (Caveats): Add notice about deprecating a C
> extension about flexible array members.
> ---
>   htdocs/gcc-14/changes.html | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html
> index dad1ba53..eae25f1a 100644
> --- a/htdocs/gcc-14/changes.html
> +++ 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>.
> +      Any code relying on this extension should be modifed to ensure that
> +      C99 flexible array members only end up at the ends of structures.
> +      Please use the warning option
> +      <a href="https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wflex-array-member-not-at-end"><code>-Wflex-array-member-not-at-end</code></a> to
> +      identify all such cases in the source code and modify them.
> +  </li>
>   </ul>

I have a question with respect to the static initialization of flexible 
array members. According to the documentation this is supported by GCC:

https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html

"GCC allows static initialization of flexible array members. This is 
equivalent to defining a new structure containing the original structure 
followed by an array of sufficient size to contain the data. E.g. in the 
following, f1 is constructed as if it were declared like f2.

struct f1 {
   int x; int y[];
} f1 = { 1, { 2, 3, 4 } };

struct f2 {
   struct f1 f1; int data[3];
} f2 = { { 1 }, { 2, 3, 4 } };
"

However, when I compile this code, I get a warning like this:

flex-array.c:6:13: warning: structure containing a flexible array member 
is not at the end of another structure [-Wflex-array-member-not-at-end]
     6 |   struct f1 f1; int data[3];
       |

In general, I agree that flexible array members should be at the end, 
however the support for static initialization is quite important from my 
point of view especially for applications for embedded systems. Here, 
dynamic allocations may not be allowed or feasible.

-- 
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
  2024-05-04 11:11 ` [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members Sebastian Huber
@ 2024-05-06  7:08   ` Richard Biener
  2024-05-06 13:29     ` Sebastian Huber
  0 siblings, 1 reply; 20+ messages in thread
From: Richard Biener @ 2024-05-06  7:08 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: Qing Zhao, gcc-patches, joseph

On Sat, 4 May 2024, Sebastian Huber wrote:

> On 07.08.23 16:22, Qing Zhao via Gcc-patches wrote:
> > Hi,
> > 
> > This is the 2nd version of the patch.
> > Comparing to the 1st version, the only change is to address Richard's
> > comment on refering a warning option for diagnosing deprecated behavior.
> > 
> > 
> > Okay for committing?
> > 
> > thanks.
> > 
> > Qing
> > 
> > ======
> > 
> > *htdocs/gcc-14/changes.html (Caveats): Add notice about deprecating a C
> > extension about flexible array members.
> > ---
> >   htdocs/gcc-14/changes.html | 13 ++++++++++++-
> >   1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html
> > index dad1ba53..eae25f1a 100644
> > --- a/htdocs/gcc-14/changes.html
> > +++ 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>.
> > +      Any code relying on this extension should be modifed to ensure that
> > +      C99 flexible array members only end up at the ends of structures.
> > +      Please use the warning option
> > +      <a
> > href="https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wflex-array-member-not-at-end"><code>-Wflex-array-member-not-at-end</code></a>
> > to
> > +      identify all such cases in the source code and modify them.
> > +  </li>
> >   </ul>
> 
> I have a question with respect to the static initialization of flexible array
> members. According to the documentation this is supported by GCC:
> 
> https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> 
> "GCC allows static initialization of flexible array members. This is
> equivalent to defining a new structure containing the original structure
> followed by an array of sufficient size to contain the data. E.g. in the
> following, f1 is constructed as if it were declared like f2.
> 
> struct f1 {
>   int x; int y[];
> } f1 = { 1, { 2, 3, 4 } };
> 
> struct f2 {
>   struct f1 f1; int data[3];
> } f2 = { { 1 }, { 2, 3, 4 } };
> "
> 
> However, when I compile this code, I get a warning like this:
> 
> flex-array.c:6:13: warning: structure containing a flexible array member is
> not at the end of another structure [-Wflex-array-member-not-at-end]
>     6 |   struct f1 f1; int data[3];
>       |
> 
> In general, I agree that flexible array members should be at the end, however
> the support for static initialization is quite important from my point of view
> especially for applications for embedded systems. Here, dynamic allocations
> may not be allowed or feasible.

I do not get a diagnostic for this on trunk?  And I agree there shouldn't
be any.

Richard.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
  2024-05-06  7:08   ` Richard Biener
@ 2024-05-06 13:29     ` Sebastian Huber
  2024-05-06 14:20       ` Qing Zhao
  0 siblings, 1 reply; 20+ messages in thread
From: Sebastian Huber @ 2024-05-06 13:29 UTC (permalink / raw)
  To: Richard Biener; +Cc: Qing Zhao, gcc-patches

On 06.05.24 09:08, Richard Biener wrote:
> On Sat, 4 May 2024, Sebastian Huber wrote:
> 
>> On 07.08.23 16:22, Qing Zhao via Gcc-patches wrote:
>>> Hi,
>>>
>>> This is the 2nd version of the patch.
>>> Comparing to the 1st version, the only change is to address Richard's
>>> comment on refering a warning option for diagnosing deprecated behavior.
>>>
>>>
>>> Okay for committing?
>>>
>>> thanks.
>>>
>>> Qing
>>>
>>> ======
>>>
>>> *htdocs/gcc-14/changes.html (Caveats): Add notice about deprecating a C
>>> extension about flexible array members.
>>> ---
>>>    htdocs/gcc-14/changes.html | 13 ++++++++++++-
>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html
>>> index dad1ba53..eae25f1a 100644
>>> --- a/htdocs/gcc-14/changes.html
>>> +++ 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>.
>>> +      Any code relying on this extension should be modifed to ensure that
>>> +      C99 flexible array members only end up at the ends of structures.
>>> +      Please use the warning option
>>> +      <a
>>> href="https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wflex-array-member-not-at-end"><code>-Wflex-array-member-not-at-end</code></a>
>>> to
>>> +      identify all such cases in the source code and modify them.
>>> +  </li>
>>>    </ul>
>>
>> I have a question with respect to the static initialization of flexible array
>> members. According to the documentation this is supported by GCC:
>>
>> https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
>>
>> "GCC allows static initialization of flexible array members. This is
>> equivalent to defining a new structure containing the original structure
>> followed by an array of sufficient size to contain the data. E.g. in the
>> following, f1 is constructed as if it were declared like f2.
>>
>> struct f1 {
>>    int x; int y[];
>> } f1 = { 1, { 2, 3, 4 } };
>>
>> struct f2 {
>>    struct f1 f1; int data[3];
>> } f2 = { { 1 }, { 2, 3, 4 } };
>> "
>>
>> However, when I compile this code, I get a warning like this:
>>
>> flex-array.c:6:13: warning: structure containing a flexible array member is
>> not at the end of another structure [-Wflex-array-member-not-at-end]
>>      6 |   struct f1 f1; int data[3];
>>        |
>>
>> In general, I agree that flexible array members should be at the end, however
>> the support for static initialization is quite important from my point of view
>> especially for applications for embedded systems. Here, dynamic allocations
>> may not be allowed or feasible.
> 
> I do not get a diagnostic for this on trunk?  And I agree there shouldn't
> be any.

It seems that this warning is not enabled by -Wall and -Wextra. I tried 
this:

gcc -Wflex-array-member-not-at-end -S -o - flex-array.c
         .file   "flex-array.c"
flex-array.c:6:13: warning: structure containing a flexible array member 
is not at the end of another structure [-Wflex-array-member-not-at-end]
     6 |   struct f1 f1; int data[3];
       |             ^~
         .text
         .globl  f1
         .data
         .align 4
         .type   f1, @object
         .size   f1, 16
f1:
         .long   1
         .long   2
         .long   3
         .long   4
         .globl  f2
         .align 16
         .type   f2, @object
         .size   f2, 16
f2:
         .long   1
         .long   2
         .long   3
         .long   4
         .ident  "GCC: (GNU) 15.0.0 20240506 (experimental) [master 
ec1cdad89af]"
         .section        .note.GNU-stack,"",@progbits

-- 
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
  2024-05-06 13:29     ` Sebastian Huber
@ 2024-05-06 14:20       ` Qing Zhao
  2024-05-07 13:15         ` Sebastian Huber
  0 siblings, 1 reply; 20+ messages in thread
From: Qing Zhao @ 2024-05-06 14:20 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: Richard Biener, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 4562 bytes --]

Hi, Sebastian,

Looks like that the behavior you described is correct.
What’s your major concern? ( a little confused).

Qing

On May 6, 2024, at 09:29, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote:

On 06.05.24 09:08, Richard Biener wrote:
On Sat, 4 May 2024, Sebastian Huber wrote:
On 07.08.23 16:22, Qing Zhao via Gcc-patches wrote:
Hi,

This is the 2nd version of the patch.
Comparing to the 1st version, the only change is to address Richard's
comment on refering a warning option for diagnosing deprecated behavior.


Okay for committing?

thanks.

Qing

======

*htdocs/gcc-14/changes.html (Caveats): Add notice about deprecating a C
extension about flexible array members.
---
  htdocs/gcc-14/changes.html | 13 ++++++++++++-
  1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html
index dad1ba53..eae25f1a 100644
--- a/htdocs/gcc-14/changes.html
+++ 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>.
+      Any code relying on this extension should be modifed to ensure that
+      C99 flexible array members only end up at the ends of structures.
+      Please use the warning option
+      <a
href="https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wflex-array-member-not-at-end"><code>-Wflex-array-member-not-at-end</code></a>
to
+      identify all such cases in the source code and modify them.
+  </li>
  </ul>

I have a question with respect to the static initialization of flexible array
members. According to the documentation this is supported by GCC:

https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html

"GCC allows static initialization of flexible array members. This is
equivalent to defining a new structure containing the original structure
followed by an array of sufficient size to contain the data. E.g. in the
following, f1 is constructed as if it were declared like f2.

struct f1 {
  int x; int y[];
} f1 = { 1, { 2, 3, 4 } };

struct f2 {
  struct f1 f1; int data[3];
} f2 = { { 1 }, { 2, 3, 4 } };
"

However, when I compile this code, I get a warning like this:

flex-array.c:6:13: warning: structure containing a flexible array member is
not at the end of another structure [-Wflex-array-member-not-at-end]
    6 |   struct f1 f1; int data[3];
      |

In general, I agree that flexible array members should be at the end, however
the support for static initialization is quite important from my point of view
especially for applications for embedded systems. Here, dynamic allocations
may not be allowed or feasible.
I do not get a diagnostic for this on trunk?  And I agree there shouldn't
be any.

It seems that this warning is not enabled by -Wall and -Wextra. I tried this:

gcc -Wflex-array-member-not-at-end -S -o - flex-array.c
       .file   "flex-array.c"
flex-array.c:6:13: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
   6 |   struct f1 f1; int data[3];
     |             ^~
       .text
       .globl  f1
       .data
       .align 4
       .type   f1, @object
       .size   f1, 16
f1:
       .long   1
       .long   2
       .long   3
       .long   4
       .globl  f2
       .align 16
       .type   f2, @object
       .size   f2, 16
f2:
       .long   1
       .long   2
       .long   3
       .long   4
       .ident  "GCC: (GNU) 15.0.0 20240506 (experimental) [master ec1cdad89af]"
       .section        .note.GNU-stack,"",@progbits

--
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de<mailto:sebastian.huber@embedded-brains.de>
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
  2024-05-06 14:20       ` Qing Zhao
@ 2024-05-07 13:15         ` Sebastian Huber
  2024-05-07 14:26           ` Qing Zhao
  0 siblings, 1 reply; 20+ messages in thread
From: Sebastian Huber @ 2024-05-07 13:15 UTC (permalink / raw)
  To: Qing Zhao; +Cc: Richard Biener, gcc-patches

On 06.05.24 16:20, Qing Zhao wrote:
> Hi, Sebastian,
> 
> Looks like that the behavior you described is correct.
> What’s your major concern? ( a little confused).

I am concerned that the static initialization of structures with 
flexible array members no longer works. In the RTEMS open source 
real-time operating system, we use flexible array members in some parts. 
One example is the thread control block which is used to manage a thread:

struct _Thread_Control {
   /** This field is the object management structure for each thread. */
   Objects_Control          Object;

[...]

   /**
    * @brief Variable length array of user extension pointers.
    *
    * The length is defined by the application via <rtems/confdefs.h>.
    */
   void                                 *extensions[];
};

In a static configuration of the operating system we have something like 
this:

struct Thread_Configured_control {
/*
  * This was added to address the following warning.
  * warning: invalid use of structure with flexible array member
  */
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wpedantic"
   Thread_Control Control;
#pragma GCC diagnostic pop

   #if CONFIGURE_MAXIMUM_USER_EXTENSIONS > 0
     void *extensions[ CONFIGURE_MAXIMUM_USER_EXTENSIONS + 1 ];
   #endif
   Configuration_Scheduler_node Scheduler_nodes[ 
_CONFIGURE_SCHEDULER_COUNT ];
   RTEMS_API_Control API_RTEMS;
   #ifdef RTEMS_POSIX_API
     POSIX_API_Control API_POSIX;
   #endif
   #if CONFIGURE_MAXIMUM_THREAD_NAME_SIZE > 1
     char name[ CONFIGURE_MAXIMUM_THREAD_NAME_SIZE ];
   #endif
   #if defined(_CONFIGURE_ENABLE_NEWLIB_REENTRANCY) && \
     !defined(_REENT_THREAD_LOCAL)
     struct _reent Newlib;
   #endif
};

This is used to define a table of thread control blocks:

Thread_Configured_control \
name##_Objects[ _Objects_Maximum_per_allocation( max ) ]; \
static RTEMS_SECTION( ".noinit.rtems.content.objects." #name ) \

I would like no know which consequences the deprecation this GCC 
extension has.

-- 
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
  2024-05-07 13:15         ` Sebastian Huber
@ 2024-05-07 14:26           ` Qing Zhao
  2024-05-07 17:57             ` Sebastian Huber
  0 siblings, 1 reply; 20+ messages in thread
From: Qing Zhao @ 2024-05-07 14:26 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: Richard Biener, gcc-patches

Hi, Sebastian,

Thanks for your explanation.

Our goal is to deprecate the GCC extension on  structure containing a flexible array member not at the end of another structure. In order to achieve this goal, we provided the warning option -Wflex-array-member-not-at-end for the users to
locate all such cases in their source code and update the source code to eliminate such cases.

The static initialization of structures with flexible array members will still work as long as the flexible array members are at
the end of the structures.

My question: is it possible to update your source code to move the structure with flexible array member to the end of the
containing structure?

i.e, in your example, in the struct Thread_Configured_control, move the field “Thread_Control Control” to the end of the structure?

Thanks

Qing

> On May 7, 2024, at 09:15, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote:
> 
> On 06.05.24 16:20, Qing Zhao wrote:
>> Hi, Sebastian,
>> Looks like that the behavior you described is correct.
>> What’s your major concern? ( a little confused).
> 
> I am concerned that the static initialization of structures with flexible array members no longer works. In the RTEMS open source real-time operating system, we use flexible array members in some parts. One example is the thread control block which is used to manage a thread:
> 
> struct _Thread_Control {
>  /** This field is the object management structure for each thread. */
>  Objects_Control          Object;
> 
> [...]
> 
>  /**
>   * @brief Variable length array of user extension pointers.
>   *
>   * The length is defined by the application via <rtems/confdefs.h>.
>   */
>  void                                 *extensions[];
> };
> 
> In a static configuration of the operating system we have something like this:
> 
> struct Thread_Configured_control {
> /*
> * This was added to address the following warning.
> * warning: invalid use of structure with flexible array member
> */
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wpedantic"
>  Thread_Control Control;
> #pragma GCC diagnostic pop
> 
>  #if CONFIGURE_MAXIMUM_USER_EXTENSIONS > 0
>    void *extensions[ CONFIGURE_MAXIMUM_USER_EXTENSIONS + 1 ];
>  #endif
>  Configuration_Scheduler_node Scheduler_nodes[ _CONFIGURE_SCHEDULER_COUNT ];
>  RTEMS_API_Control API_RTEMS;
>  #ifdef RTEMS_POSIX_API
>    POSIX_API_Control API_POSIX;
>  #endif
>  #if CONFIGURE_MAXIMUM_THREAD_NAME_SIZE > 1
>    char name[ CONFIGURE_MAXIMUM_THREAD_NAME_SIZE ];
>  #endif
>  #if defined(_CONFIGURE_ENABLE_NEWLIB_REENTRANCY) && \
>    !defined(_REENT_THREAD_LOCAL)
>    struct _reent Newlib;
>  #endif
> };
> 
> This is used to define a table of thread control blocks:
> 
> Thread_Configured_control \
> name##_Objects[ _Objects_Maximum_per_allocation( max ) ]; \
> static RTEMS_SECTION( ".noinit.rtems.content.objects." #name ) \
> 
> I would like no know which consequences the deprecation this GCC extension has.
> 
> -- 
> embedded brains GmbH & Co. KG
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: sebastian.huber@embedded-brains.de
> phone: +49-89-18 94 741 - 16
> fax:   +49-89-18 94 741 - 08
> 
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
  2024-05-07 14:26           ` Qing Zhao
@ 2024-05-07 17:57             ` Sebastian Huber
  2024-05-07 18:34               ` Qing Zhao
  2024-05-07 20:14               ` Qing Zhao
  0 siblings, 2 replies; 20+ messages in thread
From: Sebastian Huber @ 2024-05-07 17:57 UTC (permalink / raw)
  To: Qing Zhao; +Cc: Richard Biener, gcc-patches

On 07.05.24 16:26, Qing Zhao wrote:
> Hi, Sebastian,
> 
> Thanks for your explanation.
> 
> Our goal is to deprecate the GCC extension on  structure containing a flexible array member not at the end of another structure. In order to achieve this goal, we provided the warning option -Wflex-array-member-not-at-end for the users to
> locate all such cases in their source code and update the source code to eliminate such cases.

What is the benefit of deprecating this GCC extension? If GCC extensions 
are removed, then it would be nice to enable the associated warnings by 
default.

> 
> The static initialization of structures with flexible array members will still work as long as the flexible array members are at
> the end of the structures.

Removing the support for flexible array members in the middle of 
compounds will make the static initialization practically infeasible.

> 
> My question: is it possible to update your source code to move the structure with flexible array member to the end of the
> containing structure?
> 
> i.e, in your example, in the struct Thread_Configured_control, move the field “Thread_Control Control” to the end of the structure?

If we move the Thread_Control to the end, how would I add a 
configuration defined number of elements at the end?

-- 
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
  2024-05-07 17:57             ` Sebastian Huber
@ 2024-05-07 18:34               ` Qing Zhao
  2024-05-07 21:52                 ` Kees Cook
  2024-05-07 20:14               ` Qing Zhao
  1 sibling, 1 reply; 20+ messages in thread
From: Qing Zhao @ 2024-05-07 18:34 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: Richard Biener, gcc-patches, Kees Cook

[-- Attachment #1: Type: text/plain, Size: 2631 bytes --]



On May 7, 2024, at 13:57, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote:

On 07.05.24 16:26, Qing Zhao wrote:
Hi, Sebastian,
Thanks for your explanation.
Our goal is to deprecate the GCC extension on  structure containing a flexible array member not at the end of another structure. In order to achieve this goal, we provided the warning option -Wflex-array-member-not-at-end for the users to
locate all such cases in their source code and update the source code to eliminate such cases.

What is the benefit of deprecating this GCC extension? If GCC extensions are removed, then it would be nice to enable the associated warnings by default.

We had a long discussion before deciding to deprecating this GCC extension. Please see details here:

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

Yes, we do plan to enable this warning by default before final deprecation.  (Might consider to enable this warning by default in GCC15… and then deprecate it in the next release)

Right now, there is an ongoing work in Linux kernel to get rid of all such cases. Kees might have more information on this.


The static initialization of structures with flexible array members will still work as long as the flexible array members are at
the end of the structures.

Removing the support for flexible array members in the middle of compounds will make the static initialization practically infeasible.

 If the flexible array member is moved to the end of the compounds, the static initialization still work. What’s the issue here?

My question: is it possible to update your source code to move the structure with flexible array member to the end of the
containing structure?
i.e, in your example, in the struct Thread_Configured_control, move the field “Thread_Control Control” to the end of the structure?

If we move the Thread_Control to the end, how would I add a configuration defined number of elements at the end?
Don’t understand this, why moving the Thread_Control Control” to the end of the containing structure will make this a problem?
Could you please explain this with a simplified example?

Thanks.

Qing

--
embedded brains GmbH & Co. KG
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
  2024-05-07 17:57             ` Sebastian Huber
  2024-05-07 18:34               ` Qing Zhao
@ 2024-05-07 20:14               ` Qing Zhao
  1 sibling, 0 replies; 20+ messages in thread
From: Qing Zhao @ 2024-05-07 20:14 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: Richard Biener, gcc-patches, kees Cook

(Resend since my previous email in HTML and inline quoting wasn’t work, I changed the mail setting, hopefully this time it’s good). Sorry for the inconvenience.


> On May 7, 2024, at 13:57, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote:
> 
> On 07.05.24 16:26, Qing Zhao wrote:
>> Hi, Sebastian,
>> Thanks for your explanation.
>> Our goal is to deprecate the GCC extension on  structure containing a flexible array member not at the end of another structure. In order to achieve this goal, we provided the warning option -Wflex-array-member-not-at-end for the users to
>> locate all such cases in their source code and update the source code to eliminate such cases.
> 
> What is the benefit of deprecating this GCC extension? If GCC extensions are removed, then it would be nice to enable the associated warnings by default.

We had a long discussion before deciding to deprecating this GCC extension. Please see details here:

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

Yes, we do plan to enable this warning by default before final deprecation.  (Might consider to enable this warning by default in GCC15… and then deprecate it in the next release)

Right now, there is an ongoing work in Linux kernel to get rid of all such cases. Kees might have more information on this.

> 
>> The static initialization of structures with flexible array members will still work as long as the flexible array members are at
>> the end of the structures.
> 
> Removing the support for flexible array members in the middle of compounds will make the static initialization practically infeasible.
 If the flexible array member is moved to the end of the compounds, the static initialization still work. What’s the issue here?
> 
>> My question: is it possible to update your source code to move the structure with flexible array member to the end of the
>> containing structure?
>> i.e, in your example, in the struct Thread_Configured_control, move the field “Thread_Control Control” to the end of the structure?
> 
> If we move the Thread_Control to the end, how would I add a configuration defined number of elements at the end?

Don’t understand this, why moving the Thread_Control Control” to the end of the containing structure will make this a problem? 
Could you please explain this with a simplified example? 

Thanks.

Qing
> 
> -- 
> embedded brains GmbH & Co. KG
> Herr Sebastian HUBER
> Dornierstr. 4
> 82178 Puchheim
> Germany
> email: sebastian.huber@embedded-brains.de
> phone: +49-89-18 94 741 - 16
> fax:   +49-89-18 94 741 - 08
> 
> Registergericht: Amtsgericht München
> Registernummer: HRB 157899
> Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
> Unsere Datenschutzerklärung finden Sie hier:
> https://embedded-brains.de/datenschutzerklaerung/


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
  2024-05-07 18:34               ` Qing Zhao
@ 2024-05-07 21:52                 ` Kees Cook
  2024-05-08  7:25                   ` Richard Biener
  2024-05-08 19:28                   ` Qing Zhao
  0 siblings, 2 replies; 20+ messages in thread
From: Kees Cook @ 2024-05-07 21:52 UTC (permalink / raw)
  To: Sebastian Huber; +Cc: Qing Zhao, Richard Biener, gcc-patches

On Tue, May 07, 2024 at 06:34:19PM +0000, Qing Zhao wrote:
> On May 7, 2024, at 13:57, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote:
> > On 07.05.24 16:26, Qing Zhao wrote:
> > > Hi, Sebastian,
> > > Thanks for your explanation.
> > > Our goal is to deprecate the GCC extension on  structure
> > > containing a flexible array member not at the end of another
> > > structure. In order to achieve this goal, we provided the warning option
> > > -Wflex-array-member-not-at-end for the users to locate all such
> > > cases in their source code and update the source code to eliminate
> > > such cases.
> >
> > What is the benefit of deprecating this GCC extension? If GCC
> > extensions are removed, then it would be nice to enable the associated
> > warnings by default.

The goal of all of the recent array bounds and flexible array work is to
make sizing information unambiguous (e.g. via __builtin_object_size(),
__builtin_dynamic_object_size(), and the array-bounds sanitizer). For
the compiler to be able to deterministically report size information
on arrays, we needed to deprecate this case even though it had been
supported in the past. (Though we also _added_ extensions to support
for other things, like flexible arrays in unions, and the coming
__counted_by attribute.)

For example:

struct flex     { int length; char data[]; };
struct mid_flex { int m; struct flex flex_data; int n; int o; };

#define SZ(p)	__builtin_dynamic_object_size(p, 1)

void foo(struct flex *f, struct mid_flex *m)
{
	printf("%zu\n", SZ(f));
	printf("%zu\n", SZ(m->flex_data));
}

int main(void)
{
        struct mid_flex m = { .flex_data.length = 8 };
	foo(&m->flex_data, &m);
	return 0;
}

This is printing the size of the same object. But the desired results
are ambiguous. Does m->flex_data have an unknown size (i.e. SIZE_MAX)
because it's a flex array, or does it contain 8 bytes, since it overlaps
with the other structure's trailing 2 ints?

The answer from GCC 13 was neither:

18446744073709551615
4

It considered flex_data to be only the size of it's non-flex-array
members, but only when there was semantic context that it was part of
another structure. (Yet more ambiguity.)

In GCC 14, this is "resolved" to be unknown since it is a flex array
which has no sizing info, and context doesn't matter:

18446744073709551615
18446744073709551615

But this paves the way for the coming 'counted_by' attribute which will
allow for struct flex above to be defined as:

struct flex { int length; char data[] __attribute__((counted_by(length))); };

At which point GCC can deterministically report the object size.

Hopefully I've captured this all correctly -- Qing can correct me. :)

> >
> > > We had a long discussion before deciding to deprecating this GCC
> > > extension. Please see details here:
> > >
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832
> > >
> > > Yes, we do plan to enable this warning by default before final
> > > deprecation.  (Might consider to enable this warning by default in
> > > GCC15… and then deprecate it in the next release)
> > >
> > > Right now, there is an ongoing work in Linux kernel to get rid of
> > > all such cases. Kees might have more information on this.
> > >
> > >
> > > The static initialization of structures with flexible array members
> > > will still work as long as the flexible array members are at the end of
> > > the structures.
> >
> > Removing the support for flexible array members in the middle of
> > compounds will make the static initialization practically infeasible.
>
> If the flexible array member is moved to the end of the compounds,
> the static initialization still work. What’s the issue here?
>
> > > My question: is it possible to update your source code to move
> > > the structure with flexible array member to the end of the containing
> > > structure?
> > >
> > > i.e, in your example, in the struct Thread_Configured_control,
> > > move the field “Thread_Control Control” to the end of the structure?
> >
> > If we move the Thread_Control to the end, how would I add a
> > configuration defined number of elements at the end?
>
> Don’t understand this, why moving the Thread_Control Control” to
> the end of the containing structure will make this a problem?
> Could you please explain this with a simplified example?

I found your example at [2] and tried to trim/summarize it here:


struct _Thread_Control {
    Objects_Control Object;
    ...
    void            *extensions[];
};
typedef struct _Thread_Control Thread_Control;

struct Thread_Configured_control {
  Thread_Control Control;

  #if CONFIGURE_MAXIMUM_USER_EXTENSIONS > 0
    void *extensions[ CONFIGURE_MAXIMUM_USER_EXTENSIONS + 1 ];
  #endif
  Configuration_Scheduler_node Scheduler_nodes[ _CONFIGURE_SCHEDULER_COUNT ];
  RTEMS_API_Control API_RTEMS;
  #ifdef RTEMS_POSIX_API
    POSIX_API_Control API_POSIX;
  #endif
  #if CONFIGURE_MAXIMUM_THREAD_NAME_SIZE > 1
    char name[ CONFIGURE_MAXIMUM_THREAD_NAME_SIZE ];
  #endif
  #if defined(_CONFIGURE_ENABLE_NEWLIB_REENTRANCY) && \
    !defined(_REENT_THREAD_LOCAL)
    struct _reent Newlib;
  #endif
};

#define THREAD_INFORMATION_DEFINE( name, api, cls, max ) \
...
static ... \
Thread_Configured_control \
name##_Objects[ _Objects_Maximum_per_allocation( max ) ]; \
...


I don't see any static initialization of struct _Thread_Control::extensions
nor any member initialization of the name##_Objects, and even then that
is all legal in any arrangement:

truct flex      { int length; char data[]; };
struct mid_flex { int m; struct flex flex_data; int n; int o; };
struct end_flex { int m; int n; struct flex flex_data; };

struct flex f = { .length = 2 };
struct mid_flex m = { .m = 5 };
struct end_flex e = { .m = 5 };

struct flex fa[4] = { { .length = 2 } };
struct mid_flex ma[4] = { { .m = 5 } };
struct end_flex ea[4] = { { .m = 5 } };

These all work.


But yes, I see why you can't move Thread_Control trivially to the end. It
looks like you're depending on the implicit overlapping memory locations
between struct _Thread_Control and things that include it as the first
struct member, like struct Thread_Configured_control above:

cpukit/score/src/threaditerate.c:      the_thread = (Thread_Control *) information->local_table[ index ];

(In the Linux kernel we found this kind of open casting to be very
fragile and instead use a horrific wrapper called "container_of"[3] that
does the pointer math (possibly to an offset of 0 for a direct cast) to
find the member.)

Anyway, for avoiding the warning, you can just keep using the extension
and add -Wno-... if it ever ends up in -Wall, or you can redefine struct
_Thread_Control to avoid having the "extensions" member at all. This is
what we've done in several cases in Linux. For example if we had this
again, but made to look more like Thread_Control:

struct flex     { int foo; int bar; char data[]; };
struct mid_flex { struct flex hdr; int n; int o; };

It could be changed to:

struct flex_hdr { int foo; int bar; };
struct flex     { struct flex_hdr hdr; char data[]; };
struct mid_flex { struct flex_hdr hdr; int n; int o; };

This has some collateral changes needed to reference the struct flex_hdr
members from struct flex now (f->hdr.foo instead of f->foo). Sometimes
this can be avoided by using a union, as I did in a recent refactoring
in Linux: [4]

For more complex cases in Linux we've handled this by using our
"struct_group"[5] macro, which allows for a union and tagged struct to
be constructed:

struct flex {
	__struct_group(flex_hdr, hdr,,
		int foo;
		int bar;
	);
	char data[];
};
struct mid_flex { struct flex_hdr hdr; int n; int o; };

Then struct flex member names don't have to change, but if anything is
trying to get at struct flex::data through struct mid_flex::hdr, that'll
need casting. But it _shouldn't_ since it has "n" and "o".

-Kees

[1] https://gcc.gnu.org/pipermail/gcc-patches/2023-May/620122.html
[2] https://github.com/RTEMS/rtems
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/container_of.h#n10
[4] https://git.kernel.org/linus/896880ff30866f386ebed14ab81ce1ad3710cfc4
[5] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/stddef.h?h=v6.8#n11

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
  2024-05-07 21:52                 ` Kees Cook
@ 2024-05-08  7:25                   ` Richard Biener
  2024-05-08 19:28                   ` Qing Zhao
  1 sibling, 0 replies; 20+ messages in thread
From: Richard Biener @ 2024-05-08  7:25 UTC (permalink / raw)
  To: Kees Cook; +Cc: Sebastian Huber, Qing Zhao, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 9581 bytes --]

On Tue, 7 May 2024, Kees Cook wrote:

> On Tue, May 07, 2024 at 06:34:19PM +0000, Qing Zhao wrote:
> > On May 7, 2024, at 13:57, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote:
> > > On 07.05.24 16:26, Qing Zhao wrote:
> > > > Hi, Sebastian,
> > > > Thanks for your explanation.
> > > > Our goal is to deprecate the GCC extension on  structure
> > > > containing a flexible array member not at the end of another
> > > > structure. In order to achieve this goal, we provided the warning option
> > > > -Wflex-array-member-not-at-end for the users to locate all such
> > > > cases in their source code and update the source code to eliminate
> > > > such cases.
> > >
> > > What is the benefit of deprecating this GCC extension? If GCC
> > > extensions are removed, then it would be nice to enable the associated
> > > warnings by default.
> 
> The goal of all of the recent array bounds and flexible array work is to
> make sizing information unambiguous (e.g. via __builtin_object_size(),
> __builtin_dynamic_object_size(), and the array-bounds sanitizer). For
> the compiler to be able to deterministically report size information
> on arrays, we needed to deprecate this case even though it had been
> supported in the past. (Though we also _added_ extensions to support
> for other things, like flexible arrays in unions, and the coming
> __counted_by attribute.)
> 
> For example:
> 
> struct flex     { int length; char data[]; };
> struct mid_flex { int m; struct flex flex_data; int n; int o; };

It might be reasonable to allow tag-less "anonymous" struct that's
"completed" by means of the static initializer of a declared object, thus

struct { int m; struct { int length, char data[]; } flex_data; int n; int 
o; } object
 = { 3, { 2, { 1, 2 } }, 4, 5 };

The frontend would make the size of data[] static, determined by the
initializer.  I _think_ the C standard makes object.flex_data
inter-operate with struct flex from above but struct mid_flex would
be an invalid type declaration and thus there's no way to have
an API with a pointer to such structure(?) which makes the extension
somewhat less useful.

> 
> #define SZ(p)	__builtin_dynamic_object_size(p, 1)
> 
> void foo(struct flex *f, struct mid_flex *m)
> {
> 	printf("%zu\n", SZ(f));
> 	printf("%zu\n", SZ(m->flex_data));
> }
> 
> int main(void)
> {
>         struct mid_flex m = { .flex_data.length = 8 };
> 	foo(&m->flex_data, &m);
> 	return 0;
> }
> 
> This is printing the size of the same object. But the desired results
> are ambiguous. Does m->flex_data have an unknown size (i.e. SIZE_MAX)
> because it's a flex array, or does it contain 8 bytes, since it overlaps
> with the other structure's trailing 2 ints?
> 
> The answer from GCC 13 was neither:
> 
> 18446744073709551615
> 4
> 
> It considered flex_data to be only the size of it's non-flex-array
> members, but only when there was semantic context that it was part of
> another structure. (Yet more ambiguity.)
> 
> In GCC 14, this is "resolved" to be unknown since it is a flex array
> which has no sizing info, and context doesn't matter:
> 
> 18446744073709551615
> 18446744073709551615
> 
> But this paves the way for the coming 'counted_by' attribute which will
> allow for struct flex above to be defined as:
> 
> struct flex { int length; char data[] __attribute__((counted_by(length))); };
> 
> At which point GCC can deterministically report the object size.
> 
> Hopefully I've captured this all correctly -- Qing can correct me. :)
> 
> > >
> > > > We had a long discussion before deciding to deprecating this GCC
> > > > extension. Please see details here:
> > > >
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832
> > > >
> > > > Yes, we do plan to enable this warning by default before final
> > > > deprecation.  (Might consider to enable this warning by default in
> > > > GCC15… and then deprecate it in the next release)
> > > >
> > > > Right now, there is an ongoing work in Linux kernel to get rid of
> > > > all such cases. Kees might have more information on this.
> > > >
> > > >
> > > > The static initialization of structures with flexible array members
> > > > will still work as long as the flexible array members are at the end of
> > > > the structures.
> > >
> > > Removing the support for flexible array members in the middle of
> > > compounds will make the static initialization practically infeasible.
> >
> > If the flexible array member is moved to the end of the compounds,
> > the static initialization still work. What’s the issue here?
> >
> > > > My question: is it possible to update your source code to move
> > > > the structure with flexible array member to the end of the containing
> > > > structure?
> > > >
> > > > i.e, in your example, in the struct Thread_Configured_control,
> > > > move the field “Thread_Control Control” to the end of the structure?
> > >
> > > If we move the Thread_Control to the end, how would I add a
> > > configuration defined number of elements at the end?
> >
> > Don’t understand this, why moving the Thread_Control Control” to
> > the end of the containing structure will make this a problem?
> > Could you please explain this with a simplified example?
> 
> I found your example at [2] and tried to trim/summarize it here:
> 
> 
> struct _Thread_Control {
>     Objects_Control Object;
>     ...
>     void            *extensions[];
> };
> typedef struct _Thread_Control Thread_Control;
> 
> struct Thread_Configured_control {
>   Thread_Control Control;
> 
>   #if CONFIGURE_MAXIMUM_USER_EXTENSIONS > 0
>     void *extensions[ CONFIGURE_MAXIMUM_USER_EXTENSIONS + 1 ];
>   #endif
>   Configuration_Scheduler_node Scheduler_nodes[ _CONFIGURE_SCHEDULER_COUNT ];
>   RTEMS_API_Control API_RTEMS;
>   #ifdef RTEMS_POSIX_API
>     POSIX_API_Control API_POSIX;
>   #endif
>   #if CONFIGURE_MAXIMUM_THREAD_NAME_SIZE > 1
>     char name[ CONFIGURE_MAXIMUM_THREAD_NAME_SIZE ];
>   #endif
>   #if defined(_CONFIGURE_ENABLE_NEWLIB_REENTRANCY) && \
>     !defined(_REENT_THREAD_LOCAL)
>     struct _reent Newlib;
>   #endif
> };
> 
> #define THREAD_INFORMATION_DEFINE( name, api, cls, max ) \
> ...
> static ... \
> Thread_Configured_control \
> name##_Objects[ _Objects_Maximum_per_allocation( max ) ]; \
> ...
> 
> 
> I don't see any static initialization of struct _Thread_Control::extensions
> nor any member initialization of the name##_Objects, and even then that
> is all legal in any arrangement:
> 
> truct flex      { int length; char data[]; };
> struct mid_flex { int m; struct flex flex_data; int n; int o; };
> struct end_flex { int m; int n; struct flex flex_data; };
> 
> struct flex f = { .length = 2 };
> struct mid_flex m = { .m = 5 };
> struct end_flex e = { .m = 5 };
> 
> struct flex fa[4] = { { .length = 2 } };
> struct mid_flex ma[4] = { { .m = 5 } };
> struct end_flex ea[4] = { { .m = 5 } };
> 
> These all work.
> 
> 
> But yes, I see why you can't move Thread_Control trivially to the end. It
> looks like you're depending on the implicit overlapping memory locations
> between struct _Thread_Control and things that include it as the first
> struct member, like struct Thread_Configured_control above:
> 
> cpukit/score/src/threaditerate.c:      the_thread = (Thread_Control *) information->local_table[ index ];
> 
> (In the Linux kernel we found this kind of open casting to be very
> fragile and instead use a horrific wrapper called "container_of"[3] that
> does the pointer math (possibly to an offset of 0 for a direct cast) to
> find the member.)
> 
> Anyway, for avoiding the warning, you can just keep using the extension
> and add -Wno-... if it ever ends up in -Wall, or you can redefine struct
> _Thread_Control to avoid having the "extensions" member at all. This is
> what we've done in several cases in Linux. For example if we had this
> again, but made to look more like Thread_Control:
> 
> struct flex     { int foo; int bar; char data[]; };
> struct mid_flex { struct flex hdr; int n; int o; };
> 
> It could be changed to:
> 
> struct flex_hdr { int foo; int bar; };
> struct flex     { struct flex_hdr hdr; char data[]; };
> struct mid_flex { struct flex_hdr hdr; int n; int o; };
> 
> This has some collateral changes needed to reference the struct flex_hdr
> members from struct flex now (f->hdr.foo instead of f->foo). Sometimes
> this can be avoided by using a union, as I did in a recent refactoring
> in Linux: [4]
> 
> For more complex cases in Linux we've handled this by using our
> "struct_group"[5] macro, which allows for a union and tagged struct to
> be constructed:
> 
> struct flex {
> 	__struct_group(flex_hdr, hdr,,
> 		int foo;
> 		int bar;
> 	);
> 	char data[];
> };
> struct mid_flex { struct flex_hdr hdr; int n; int o; };
> 
> Then struct flex member names don't have to change, but if anything is
> trying to get at struct flex::data through struct mid_flex::hdr, that'll
> need casting. But it _shouldn't_ since it has "n" and "o".
> 
> -Kees
> 
> [1] https://gcc.gnu.org/pipermail/gcc-patches/2023-May/620122.html
> [2] https://github.com/RTEMS/rtems
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/container_of.h#n10
> [4] https://git.kernel.org/linus/896880ff30866f386ebed14ab81ce1ad3710cfc4
> [5] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/stddef.h?h=v6.8#n11
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members.
  2024-05-07 21:52                 ` Kees Cook
  2024-05-08  7:25                   ` Richard Biener
@ 2024-05-08 19:28                   ` Qing Zhao
  1 sibling, 0 replies; 20+ messages in thread
From: Qing Zhao @ 2024-05-08 19:28 UTC (permalink / raw)
  To: Kees Cook, Sebastian Huber; +Cc: Richard Biener, gcc-patches


> On May 7, 2024, at 17:52, Kees Cook <keescook@chromium.org> wrote:
> 
> On Tue, May 07, 2024 at 06:34:19PM +0000, Qing Zhao wrote:
>> On May 7, 2024, at 13:57, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote:
>>> On 07.05.24 16:26, Qing Zhao wrote:
>>>> Hi, Sebastian,
>>>> Thanks for your explanation.
>>>> Our goal is to deprecate the GCC extension on  structure
>>>> containing a flexible array member not at the end of another
>>>> structure. In order to achieve this goal, we provided the warning option
>>>> -Wflex-array-member-not-at-end for the users to locate all such
>>>> cases in their source code and update the source code to eliminate
>>>> such cases.
>>> 
>>> What is the benefit of deprecating this GCC extension? If GCC
>>> extensions are removed, then it would be nice to enable the associated
>>> warnings by default.
> 
> The goal of all of the recent array bounds and flexible array work is to
> make sizing information unambiguous (e.g. via __builtin_object_size(),
> __builtin_dynamic_object_size(), and the array-bounds sanitizer). For
> the compiler to be able to deterministically report size information
> on arrays, we needed to deprecate this case even though it had been
> supported in the past. (Though we also _added_ extensions to support
> for other things, like flexible arrays in unions, and the coming
> __counted_by attribute.)
> 
> For example:
> 
> struct flex     { int length; char data[]; };
> struct mid_flex { int m; struct flex flex_data; int n; int o; };
> 
> #define SZ(p) __builtin_dynamic_object_size(p, 1)
> 
> void foo(struct flex *f, struct mid_flex *m)
> {
> printf("%zu\n", SZ(f));
> printf("%zu\n", SZ(m->flex_data));
> }
> 
> int main(void)
> {
>        struct mid_flex m = { .flex_data.length = 8 };
> foo(&m->flex_data, &m);
> return 0;
> }
> 
> This is printing the size of the same object. But the desired results
> are ambiguous. Does m->flex_data have an unknown size (i.e. SIZE_MAX)
> because it's a flex array, or does it contain 8 bytes, since it overlaps
> with the other structure's trailing 2 ints?
> 
> The answer from GCC 13 was neither:
> 
> 18446744073709551615
> 4
> 
> It considered flex_data to be only the size of it's non-flex-array
> members, but only when there was semantic context that it was part of
> another structure. (Yet more ambiguity.)
> 
> In GCC 14, this is "resolved" to be unknown since it is a flex array
> which has no sizing info, and context doesn't matter:
> 
> 18446744073709551615
> 18446744073709551615
> 
> But this paves the way for the coming 'counted_by' attribute which will
> allow for struct flex above to be defined as:
> 
> struct flex { int length; char data[] __attribute__((counted_by(length))); };
> 
> At which point GCC can deterministically report the object size.
> 
> Hopefully I've captured this all correctly -- Qing can correct me. :)
> 
>>> 
>>>> We had a long discussion before deciding to deprecating this GCC
>>>> extension. Please see details here:
>>>> 
>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832
>>>> 
>>>> Yes, we do plan to enable this warning by default before final
>>>> deprecation.  (Might consider to enable this warning by default in
>>>> GCC15… and then deprecate it in the next release)
>>>> 
>>>> Right now, there is an ongoing work in Linux kernel to get rid of
>>>> all such cases. Kees might have more information on this.
>>>> 
>>>> 
>>>> The static initialization of structures with flexible array members
>>>> will still work as long as the flexible array members are at the end of
>>>> the structures.
>>> 
>>> Removing the support for flexible array members in the middle of
>>> compounds will make the static initialization practically infeasible.
>> 
>> If the flexible array member is moved to the end of the compounds,
>> the static initialization still work. What’s the issue here?
>> 
>>>> My question: is it possible to update your source code to move
>>>> the structure with flexible array member to the end of the containing
>>>> structure?
>>>> 
>>>> i.e, in your example, in the struct Thread_Configured_control,
>>>> move the field “Thread_Control Control” to the end of the structure?
>>> 
>>> If we move the Thread_Control to the end, how would I add a
>>> configuration defined number of elements at the end?
>> 
>> Don’t understand this, why moving the Thread_Control Control” to
>> the end of the containing structure will make this a problem?
>> Could you please explain this with a simplified example?
> 
> I found your example at [2] and tried to trim/summarize it here:
> 
> 
> struct _Thread_Control {
>    Objects_Control Object;
>    ...
>    void            *extensions[];
> };
> typedef struct _Thread_Control Thread_Control;
> 
> struct Thread_Configured_control {
>  Thread_Control Control;
> 
>  #if CONFIGURE_MAXIMUM_USER_EXTENSIONS > 0
>    void *extensions[ CONFIGURE_MAXIMUM_USER_EXTENSIONS + 1 ];
>  #endif
>  Configuration_Scheduler_node Scheduler_nodes[ _CONFIGURE_SCHEDULER_COUNT ];
>  RTEMS_API_Control API_RTEMS;
>  #ifdef RTEMS_POSIX_API
>    POSIX_API_Control API_POSIX;
>  #endif
>  #if CONFIGURE_MAXIMUM_THREAD_NAME_SIZE > 1
>    char name[ CONFIGURE_MAXIMUM_THREAD_NAME_SIZE ];
>  #endif
>  #if defined(_CONFIGURE_ENABLE_NEWLIB_REENTRANCY) && \
>    !defined(_REENT_THREAD_LOCAL)
>    struct _reent Newlib;
>  #endif
> };
> 
> #define THREAD_INFORMATION_DEFINE( name, api, cls, max ) \
> ...
> static ... \
> Thread_Configured_control \
> name##_Objects[ _Objects_Maximum_per_allocation( max ) ]; \
> ...
> 
> 
> I don't see any static initialization of struct _Thread_Control::extensions
> nor any member initialization of the name##_Objects, and even then that
> is all legal in any arrangement:

Thanks for the simplified code portion. From the above, I guess:

For the structure: 

struct Thread_Configured_control {
  Thread_Control Control;

  #if CONFIGURE_MAXIMUM_USER_EXTENSIONS > 0
     void *extensions[ CONFIGURE_MAXIMUM_USER_EXTENSIONS + 1 ];
  #endif
….
}

The 2nd field “void *extensions[CONFIGURE_MAXIMUM_USER_EXTENSIONS + 1 ] ” is following 
the 1st field “Thread_Control Control”, whose type is a structure with trailing flexible array member field 

struct _Thread_Control {
   Objects_Control Object;
   ...
   void            *extensions[];
};

So, the Thread_Configured_control.Control.extensions[] is overlapped with Thread_Configured_control.extensions[CONFIGURE_MAXIMUM_USER_EXTENSIONS + 1].

And the source code depends on the above overlapping relationship. (The initialization to the field Thread_Configured_control.extensions[CONFIGURE_MAXIMUM_USER_EXTENSIONS + 1] will be an initialization to the overlapped flexible array member field).

If so, moving the field “Thread_Control Control” to the last field of the containing structure will break the overlapping relationship. Therefore resulting incorrect code. 

Not sure whether the above a correct understanding or not. 

If my understanding is correct, I do feel that such coding style is really not good, not easily to be understood or to be debugged. 

Not sure whether the new GCC extension: permitting flexible array member in union, could be used to replace such code? 

Qing
> 
> truct flex      { int length; char data[]; };
> struct mid_flex { int m; struct flex flex_data; int n; int o; };
> struct end_flex { int m; int n; struct flex flex_data; };
> 
> struct flex f = { .length = 2 };
> struct mid_flex m = { .m = 5 };
> struct end_flex e = { .m = 5 };
> 
> struct flex fa[4] = { { .length = 2 } };
> struct mid_flex ma[4] = { { .m = 5 } };
> struct end_flex ea[4] = { { .m = 5 } };
> 
> These all work.
> 
> 
> But yes, I see why you can't move Thread_Control trivially to the end. It
> looks like you're depending on the implicit overlapping memory locations
> between struct _Thread_Control and things that include it as the first
> struct member, like struct Thread_Configured_control above:
> 
> cpukit/score/src/threaditerate.c:      the_thread = (Thread_Control *) information->local_table[ index ];
> 
> (In the Linux kernel we found this kind of open casting to be very
> fragile and instead use a horrific wrapper called "container_of"[3] that
> does the pointer math (possibly to an offset of 0 for a direct cast) to
> find the member.)
> 
> Anyway, for avoiding the warning, you can just keep using the extension
> and add -Wno-... if it ever ends up in -Wall, or you can redefine struct
> _Thread_Control to avoid having the "extensions" member at all. This is
> what we've done in several cases in Linux. For example if we had this
> again, but made to look more like Thread_Control:
> 
> struct flex     { int foo; int bar; char data[]; };
> struct mid_flex { struct flex hdr; int n; int o; };
> 
> It could be changed to:
> 
> struct flex_hdr { int foo; int bar; };
> struct flex     { struct flex_hdr hdr; char data[]; };
> struct mid_flex { struct flex_hdr hdr; int n; int o; };
> 
> This has some collateral changes needed to reference the struct flex_hdr
> members from struct flex now (f->hdr.foo instead of f->foo). Sometimes
> this can be avoided by using a union, as I did in a recent refactoring
> in Linux: [4]
> 
> For more complex cases in Linux we've handled this by using our
> "struct_group"[5] macro, which allows for a union and tagged struct to
> be constructed:
> 
> struct flex {
> __struct_group(flex_hdr, hdr,,
> int foo;
> int bar;
> );
> char data[];
> };
> struct mid_flex { struct flex_hdr hdr; int n; int o; };
> 
> Then struct flex member names don't have to change, but if anything is
> trying to get at struct flex::data through struct mid_flex::hdr, that'll
> need casting. But it _shouldn't_ since it has "n" and "o".
> 
> -Kees
> 
> [1] https://gcc.gnu.org/pipermail/gcc-patches/2023-May/620122.html
> [2] https://github.com/RTEMS/rtems
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/container_of.h#n10
> [4] https://git.kernel.org/linus/896880ff30866f386ebed14ab81ce1ad3710cfc4
> [5] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/stddef.h?h=v6.8#n11
> 
> -- 
> Kees Cook



^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2024-05-08 19:29 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-07 14:22 [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members Qing Zhao
2023-08-08  7:55 ` Richard Biener
2023-09-25 18:24 ` 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.) Tobias Burnus
2023-09-26  6:49   ` Richard Biener
2023-09-26  7:26     ` Improve -Wflex-array-member-not-at-end changes.html wording |Plus: and warning bug? Tobias Burnus
2023-10-02  3:58   ` 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.) Qing Zhao
2023-10-19 20:49   ` Qing Zhao
2023-10-19 23:29     ` Kees Cook
2024-05-04 11:11 ` [V2][PATCH] gcc-14/changes.html: Deprecate a GCC C extension on flexible array members Sebastian Huber
2024-05-06  7:08   ` Richard Biener
2024-05-06 13:29     ` Sebastian Huber
2024-05-06 14:20       ` Qing Zhao
2024-05-07 13:15         ` Sebastian Huber
2024-05-07 14:26           ` Qing Zhao
2024-05-07 17:57             ` Sebastian Huber
2024-05-07 18:34               ` Qing Zhao
2024-05-07 21:52                 ` Kees Cook
2024-05-08  7:25                   ` Richard Biener
2024-05-08 19:28                   ` Qing Zhao
2024-05-07 20:14               ` Qing Zhao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).