Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
Sorry, Please ignore this email.
Qing
> On Dec 6, 2022, at 11:14 AM, Qing Zhao wrote:
>
> '-Wstrict-flex-arrays'
> Warn about inproper usages of flexible array members according to
> the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to
> the trailing array field of a structure if it's available,
> otherwise according to the LEVEL of the option
> '-fstrict-flex-arrays=LEVEL'.
>
> This option is effective only when LEVEL is bigger than 0.
> Otherwise, it will be ignored with a warning.
>
> when LEVEL=1, warnings will be issued for a trailing array
> reference of a structure that have 2 or more elements if the
> trailing array is referenced as a flexible array member.
>
> when LEVEL=2, in addition to LEVEL=1, additional warnings will be
> issued for a trailing one-element array reference of a structure if
> the array is referenced as a flexible array member.
>
> when LEVEL=3, in addition to LEVEL=2, additional warnings will be
> issued for a trailing zero-length array reference of a structure if
> the array is referenced as a flexible array member.
>
> At the same time, -Warray-bounds is updated:
>
> A. add the following to clarify the relationship with the LEVEL of
> -fstrict-flex-array:
>
> By default, the trailing array of a structure will be treated as a
> flexible array member by '-Warray-bounds' or '-Warray-bounds=N' if
> it is declared as either a flexible array member per C99 standard
> onwards ('[]'), a GCC zero-length array extension ('[0]'), or an
> one-element array ('[1]'). As a result, out of bounds subscripts
> or offsets into zero-length arrays or one-element arrays are not
> warned by default.
>
> You can add the option '-fstrict-flex-arrays' or
> '-fstrict-flex-arrays=LEVEL' to control how this option treat
> trailing array of a structure as a flexible array member.
>
> when LEVEL<=1, no change to the default behavior.
>
> when LEVEL=2, additional warnings will be issued for out of bounds
> subscripts or offsets into one-element arrays;
>
> when LEVEL=3, in addition to LEVEL=2, additional warnings will be
> issued for out of bounds subscripts or offsets into zero-length
> arrays.
>
> B. change the -Warray-bounds=2 to exclude the control on how to treat
> trailing arrays as flexible array members:
>
> '-Warray-bounds=2'
> This warning level also warns about the intermediate results
> of pointer arithmetic that may yield out of bounds values.
> This warning level may give a larger number of false positives
> and is deactivated by default.
>
> gcc/ChangeLog:
>
> * attribs.cc (strict_flex_array_level_of): New function.
> * attribs.h (strict_flex_array_level_of): Prototype for new function.
> * doc/invoke.texi: Document -Wstrict-flex-arrays option. Update
> -Warray-bounds by specifying the impact from -fstrict-flex-arrays.
> Also update -Warray-bounds=2 by eliminating its impact on treating
> trailing arrays as flexible array members.
> * gimple-array-bounds.cc (array_bounds_checker::check_array_ref):
> Issue warnings for -Wstrict-flex-arrays.
> (get_up_bounds_for_array_ref): New function.
> (check_out_of_bounds_and_warn): New function.
> * opts.cc (finish_options): Issue warning for unsupported combination
> of -Wstrict_flex_arrays and -fstrict-flex-array.
> * tree-vrp.cc (execute_ranger_vrp): Enable the pass when
> warn_strict_flex_array is true.
> * tree.cc (array_ref_flexible_size_p): Add one new argument.
> (component_ref_sam_type): New function.
> (component_ref_size): Control with level of strict-flex-array.
> * tree.h (array_ref_flexible_size_p): Update prototype.
> (enum struct special_array_member): Add two new enum values.
> (component_ref_sam_type): New prototype.
>
> gcc/c-family/ChangeLog:
>
> * c.opt (Wstrict-flex-arrays): New option.
>
> gcc/c/ChangeLog:
>
> * c-decl.cc (is_flexible_array_member_p): Call new function
> strict_flex_array_level_of.
>
> gcc/testsuite/ChangeLog:
>
> * c-c++-common/Wstrict-flex-arrays.c: New test.
> * c-c++-common/Wstrict-flex-arrays_2.c: New test.
> * gcc.dg/Warray-bounds-11.c: Update warnings for -Warray-bounds=2.
> * gcc.dg/Wstrict-flex-arrays-2.c: New test.
> * gcc.dg/Wstrict-flex-arrays-3.c: New test.
> * gcc.dg/Wstrict-flex-arrays-4.c: New test.
> * gcc.dg/Wstrict-flex-arrays-5.c: New test.
> * gcc.dg/Wstrict-flex-arrays-6.c: New test.
> * gcc.dg/Wstrict-flex-arrays-7.c: New test.
> * gcc.dg/Wstrict-flex-arrays-8.c: New test.
> * gcc.dg/Wstrict-flex-arrays-9.c: New test.
> * gcc.dg/Wstrict-flex-arrays.c: New test.
> ---
> gcc/attribs.cc| 30 +++
> gcc/attribs.h
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
> On Dec 5, 2022, at 10:16 AM, Richard Biener wrote: > > On Fri, 2 Dec 2022, Qing Zhao wrote: > >> >> >>> On Dec 2, 2022, at 2:20 AM, Richard Biener wrote: >>> >>> On Fri, 2 Dec 2022, Richard Biener wrote: >>> On Thu, 1 Dec 2022, Siddhesh Poyarekar wrote: > On 2022-12-01 11:42, Kees Cook wrote: >> On Wed, Nov 30, 2022 at 02:25:56PM +, Qing Zhao wrote: >>> '-Wstrict-flex-arrays' >>> Warn about inproper usages of flexible array members according to >>> the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to >>> the trailing array field of a structure if it's available, >>> otherwise according to the LEVEL of the option >>> '-fstrict-flex-arrays=LEVEL'. >>> >>> This option is effective only when LEVEL is bigger than 0. >>> Otherwise, it will be ignored with a warning. >>> >>> when LEVEL=1, warnings will be issued for a trailing array >>> reference of a structure that have 2 or more elements if the >>> trailing array is referenced as a flexible array member. >>> >>> when LEVEL=2, in addition to LEVEL=1, additional warnings will be >>> issued for a trailing one-element array reference of a structure if >>> the array is referenced as a flexible array member. >>> >>> when LEVEL=3, in addition to LEVEL=2, additional warnings will be >>> issued for a trailing zero-length array reference of a structure if >>> the array is referenced as a flexible array member. >>> >>> At the same time, -Warray-bounds is updated: >> >> Why is there both -Wstrict-flex-arrays and -Warray-bounds? I thought >> only the latter was going to exist? Sorry for appearantly not being clear - I was requesting -Wstrict-flex-arrays to be dropped and instead adjusting -Warray-bounds to adhere to -fstrict-flex-arrays in both =1 and =2 where then =2 would only add the intermediate pointer results verification. I think that's reasonable if documented since the default behavior with -Wall will not change then unless the -fstrict-flex-arrays default is altered. >>> >>> Btw, your patch seems to implement the above plus adds >>> -Wstrict-flex-arrays. It seems it could be split into two, doing >>> the -Warray-bounds adjustment as first and the -Wstrict-flex-arrays >>> addition as second. >> >> Yes, implementation should be very easy to be adjusted to drop the new >> -Wstrict-flex-arrays option. >> But I still feel the new -Wstrict-flex-arrays option is good to add. > > Can you split the patch and re-post? I'll quickly approve the first > part and will think harder on the second. Okay, I will do that. thanks. Qing > > Thanks, > Richard. > >> Qing >>> We do all seem to agree on the first so it's easy >>> to go forward with that? >>> >>> Thanks, >>> Richard. >> >> > > -- > Richard Biener > SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, > Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; > HRB 36809 (AG Nuernberg)
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
On Fri, 2 Dec 2022, Qing Zhao wrote: > > > > On Dec 2, 2022, at 2:20 AM, Richard Biener wrote: > > > > On Fri, 2 Dec 2022, Richard Biener wrote: > > > >> On Thu, 1 Dec 2022, Siddhesh Poyarekar wrote: > >> > >>> On 2022-12-01 11:42, Kees Cook wrote: > On Wed, Nov 30, 2022 at 02:25:56PM +, Qing Zhao wrote: > > '-Wstrict-flex-arrays' > > Warn about inproper usages of flexible array members according to > > the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to > > the trailing array field of a structure if it's available, > > otherwise according to the LEVEL of the option > > '-fstrict-flex-arrays=LEVEL'. > > > > This option is effective only when LEVEL is bigger than 0. > > Otherwise, it will be ignored with a warning. > > > > when LEVEL=1, warnings will be issued for a trailing array > > reference of a structure that have 2 or more elements if the > > trailing array is referenced as a flexible array member. > > > > when LEVEL=2, in addition to LEVEL=1, additional warnings will be > > issued for a trailing one-element array reference of a structure if > > the array is referenced as a flexible array member. > > > > when LEVEL=3, in addition to LEVEL=2, additional warnings will be > > issued for a trailing zero-length array reference of a structure if > > the array is referenced as a flexible array member. > > > > At the same time, -Warray-bounds is updated: > > Why is there both -Wstrict-flex-arrays and -Warray-bounds? I thought > only the latter was going to exist? > >> > >> Sorry for appearantly not being clear - I was requesting > >> -Wstrict-flex-arrays to be dropped and instead adjusting -Warray-bounds > >> to adhere to -fstrict-flex-arrays in both =1 and =2 where then =2 > >> would only add the intermediate pointer results verification. > >> > >> I think that's reasonable if documented since the default behavior > >> with -Wall will not change then unless the -fstrict-flex-arrays > >> default is altered. > > > > Btw, your patch seems to implement the above plus adds > > -Wstrict-flex-arrays. It seems it could be split into two, doing > > the -Warray-bounds adjustment as first and the -Wstrict-flex-arrays > > addition as second. > > Yes, implementation should be very easy to be adjusted to drop the new > -Wstrict-flex-arrays option. > But I still feel the new -Wstrict-flex-arrays option is good to add. Can you split the patch and re-post? I'll quickly approve the first part and will think harder on the second. Thanks, Richard. > Qing > > We do all seem to agree on the first so it's easy > > to go forward with that? > > > > Thanks, > > Richard. > > -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
> On Dec 2, 2022, at 2:20 AM, Richard Biener wrote: > > On Fri, 2 Dec 2022, Richard Biener wrote: > >> On Thu, 1 Dec 2022, Siddhesh Poyarekar wrote: >> >>> On 2022-12-01 11:42, Kees Cook wrote: On Wed, Nov 30, 2022 at 02:25:56PM +, Qing Zhao wrote: > '-Wstrict-flex-arrays' > Warn about inproper usages of flexible array members according to > the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to > the trailing array field of a structure if it's available, > otherwise according to the LEVEL of the option > '-fstrict-flex-arrays=LEVEL'. > > This option is effective only when LEVEL is bigger than 0. > Otherwise, it will be ignored with a warning. > > when LEVEL=1, warnings will be issued for a trailing array > reference of a structure that have 2 or more elements if the > trailing array is referenced as a flexible array member. > > when LEVEL=2, in addition to LEVEL=1, additional warnings will be > issued for a trailing one-element array reference of a structure if > the array is referenced as a flexible array member. > > when LEVEL=3, in addition to LEVEL=2, additional warnings will be > issued for a trailing zero-length array reference of a structure if > the array is referenced as a flexible array member. > > At the same time, -Warray-bounds is updated: Why is there both -Wstrict-flex-arrays and -Warray-bounds? I thought only the latter was going to exist? >> >> Sorry for appearantly not being clear - I was requesting >> -Wstrict-flex-arrays to be dropped and instead adjusting -Warray-bounds >> to adhere to -fstrict-flex-arrays in both =1 and =2 where then =2 >> would only add the intermediate pointer results verification. >> >> I think that's reasonable if documented since the default behavior >> with -Wall will not change then unless the -fstrict-flex-arrays >> default is altered. > > Btw, your patch seems to implement the above plus adds > -Wstrict-flex-arrays. It seems it could be split into two, doing > the -Warray-bounds adjustment as first and the -Wstrict-flex-arrays > addition as second. Yes, implementation should be very easy to be adjusted to drop the new -Wstrict-flex-arrays option. But I still feel the new -Wstrict-flex-arrays option is good to add. Qing > We do all seem to agree on the first so it's easy > to go forward with that? > > Thanks, > Richard.
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
> On Dec 2, 2022, at 2:16 AM, Richard Biener wrote: > > On Thu, 1 Dec 2022, Siddhesh Poyarekar wrote: > >> On 2022-12-01 11:42, Kees Cook wrote: >>> On Wed, Nov 30, 2022 at 02:25:56PM +, Qing Zhao wrote: '-Wstrict-flex-arrays' Warn about inproper usages of flexible array members according to the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to the trailing array field of a structure if it's available, otherwise according to the LEVEL of the option '-fstrict-flex-arrays=LEVEL'. This option is effective only when LEVEL is bigger than 0. Otherwise, it will be ignored with a warning. when LEVEL=1, warnings will be issued for a trailing array reference of a structure that have 2 or more elements if the trailing array is referenced as a flexible array member. when LEVEL=2, in addition to LEVEL=1, additional warnings will be issued for a trailing one-element array reference of a structure if the array is referenced as a flexible array member. when LEVEL=3, in addition to LEVEL=2, additional warnings will be issued for a trailing zero-length array reference of a structure if the array is referenced as a flexible array member. At the same time, -Warray-bounds is updated: >>> >>> Why is there both -Wstrict-flex-arrays and -Warray-bounds? I thought >>> only the latter was going to exist? > > Sorry for appearantly not being clear - I was requesting > -Wstrict-flex-arrays to be dropped and instead adjusting -Warray-bounds > to adhere to -fstrict-flex-arrays in both =1 and =2 where then =2 > would only add the intermediate pointer results verification. So, you suggested to drop the new option -Wstrict-flex-arrays? How about the new warnings on the misuse of flex arrays? Shall we drop them too? Or we issue such new warnings with -Warray-bounds + -fstrict-flex-arrays=N? I still think that the new -Wstrict-flex-arrays to only issue the misuse of flex arrays is necessary to add. Otherwise, such warning messages will be buried among a lot of out-of-bounds warnings. > > I think that's reasonable if documented since the default behavior > with -Wall will not change then unless the -fstrict-flex-arrays > default is altered. Yes, the default behavior for -Wall, or -Warray-bounds are not changed. Qing > >> Oh my understanding of the consensus was to move flex array related diagnosis >> from -Warray-bounds to -Wstring-flex-arrays as Qing has done. If only the >> former exists then instead of removing the flex array related statement in >> the >> documentation as Richard suggested, we need to enhance it to say that >> behaviour of -Warray-bounds will depend on -fstrict-flex-arrays. >> >> -Warray-bounds does diagnosis beyond just flexible arrays, in case that's the >> confusion. > > Richard. > > -- > Richard Biener > SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, > Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; > HRB 36809 (AG Nuernberg)
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
On Fri, 2 Dec 2022, Richard Biener wrote: > On Thu, 1 Dec 2022, Siddhesh Poyarekar wrote: > > > On 2022-12-01 11:42, Kees Cook wrote: > > > On Wed, Nov 30, 2022 at 02:25:56PM +, Qing Zhao wrote: > > >> '-Wstrict-flex-arrays' > > >> Warn about inproper usages of flexible array members according to > > >> the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to > > >> the trailing array field of a structure if it's available, > > >> otherwise according to the LEVEL of the option > > >> '-fstrict-flex-arrays=LEVEL'. > > >> > > >> This option is effective only when LEVEL is bigger than 0. > > >> Otherwise, it will be ignored with a warning. > > >> > > >> when LEVEL=1, warnings will be issued for a trailing array > > >> reference of a structure that have 2 or more elements if the > > >> trailing array is referenced as a flexible array member. > > >> > > >> when LEVEL=2, in addition to LEVEL=1, additional warnings will be > > >> issued for a trailing one-element array reference of a structure if > > >> the array is referenced as a flexible array member. > > >> > > >> when LEVEL=3, in addition to LEVEL=2, additional warnings will be > > >> issued for a trailing zero-length array reference of a structure if > > >> the array is referenced as a flexible array member. > > >> > > >> At the same time, -Warray-bounds is updated: > > > > > > Why is there both -Wstrict-flex-arrays and -Warray-bounds? I thought > > > only the latter was going to exist? > > Sorry for appearantly not being clear - I was requesting > -Wstrict-flex-arrays to be dropped and instead adjusting -Warray-bounds > to adhere to -fstrict-flex-arrays in both =1 and =2 where then =2 > would only add the intermediate pointer results verification. > > I think that's reasonable if documented since the default behavior > with -Wall will not change then unless the -fstrict-flex-arrays > default is altered. Btw, your patch seems to implement the above plus adds -Wstrict-flex-arrays. It seems it could be split into two, doing the -Warray-bounds adjustment as first and the -Wstrict-flex-arrays addition as second. We do all seem to agree on the first so it's easy to go forward with that? Thanks, Richard.
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
On Thu, 1 Dec 2022, Siddhesh Poyarekar wrote: > On 2022-12-01 11:42, Kees Cook wrote: > > On Wed, Nov 30, 2022 at 02:25:56PM +, Qing Zhao wrote: > >> '-Wstrict-flex-arrays' > >> Warn about inproper usages of flexible array members according to > >> the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to > >> the trailing array field of a structure if it's available, > >> otherwise according to the LEVEL of the option > >> '-fstrict-flex-arrays=LEVEL'. > >> > >> This option is effective only when LEVEL is bigger than 0. > >> Otherwise, it will be ignored with a warning. > >> > >> when LEVEL=1, warnings will be issued for a trailing array > >> reference of a structure that have 2 or more elements if the > >> trailing array is referenced as a flexible array member. > >> > >> when LEVEL=2, in addition to LEVEL=1, additional warnings will be > >> issued for a trailing one-element array reference of a structure if > >> the array is referenced as a flexible array member. > >> > >> when LEVEL=3, in addition to LEVEL=2, additional warnings will be > >> issued for a trailing zero-length array reference of a structure if > >> the array is referenced as a flexible array member. > >> > >> At the same time, -Warray-bounds is updated: > > > > Why is there both -Wstrict-flex-arrays and -Warray-bounds? I thought > > only the latter was going to exist? Sorry for appearantly not being clear - I was requesting -Wstrict-flex-arrays to be dropped and instead adjusting -Warray-bounds to adhere to -fstrict-flex-arrays in both =1 and =2 where then =2 would only add the intermediate pointer results verification. I think that's reasonable if documented since the default behavior with -Wall will not change then unless the -fstrict-flex-arrays default is altered. > Oh my understanding of the consensus was to move flex array related diagnosis > from -Warray-bounds to -Wstring-flex-arrays as Qing has done. If only the > former exists then instead of removing the flex array related statement in the > documentation as Richard suggested, we need to enhance it to say that > behaviour of -Warray-bounds will depend on -fstrict-flex-arrays. > > -Warray-bounds does diagnosis beyond just flexible arrays, in case that's the > confusion. Richard. -- Richard Biener SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; HRB 36809 (AG Nuernberg)
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
On 2022-12-01 18:19, Kees Cook wrote: On Thu, Dec 01, 2022 at 10:27:41PM +, Qing Zhao wrote: Hi, Sid, Thanks a lot for the input. After more thinking based on your and Kees’ comments, I have the following thought: 1. -fstrict-flex-arrays=level should control both GCC code gen and warnings consistently; 2. We need warnings specifically for -fstrict-flex-arrays=level to report any misuse of flexible array corresponding to the “level” to gradually encourage language standard. So, based on the above two, I think what I did in this current patch is correct: 1. We eliminate the control from -Warray-bounds=level on treating flex arrays, now only "-fstrict-flex-arrasy=level" controls how the warning treating the flex arrays. 2. We add a separate new warning -Wstrict-flex-arrays to report any misuse corresponding to the different level of -fstrict-flex-arrays. Although we can certainly merge these new warnings into -Warray-bounds, however, as Sid mentioned, -Warray-bounds does issue a lot more warnings than just flexible arrays misuse. I think it’s necessary To provide a seperate warning to only issue flexible array misuse. Let me know if you have any more comments on this. That's how I understood Richard's comment. Okay, that seems good. Given that -Warray-bounds is part of -Wall, what should happen for -Wstrict-flex-arrays=N? I suppose it would be independent of -Wall, dependent completely on -fstrict-flex-arrays. Thanks, Sid
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
On Thu, Dec 01, 2022 at 10:27:41PM +, Qing Zhao wrote: > Hi, Sid, > > Thanks a lot for the input. > > After more thinking based on your and Kees’ comments, I have the following > thought: > > 1. -fstrict-flex-arrays=level should control both GCC code gen and warnings > consistently; > 2. We need warnings specifically for -fstrict-flex-arrays=level to report any > misuse of flexible > array corresponding to the “level” to gradually encourage language > standard. > > So, based on the above two, I think what I did in this current patch is > correct: > > 1. We eliminate the control from -Warray-bounds=level on treating flex > arrays, > now only "-fstrict-flex-arrasy=level" controls how the warning treating > the flex arrays. > 2. We add a separate new warning -Wstrict-flex-arrays to report any misuse > corresponding to > the different level of -fstrict-flex-arrays. > > Although we can certainly merge these new warnings into -Warray-bounds, > however, as Sid mentioned, > -Warray-bounds does issue a lot more warnings than just flexible arrays > misuse. I think it’s necessary > To provide a seperate warning to only issue flexible array misuse. > > Let me know if you have any more comments on this. Okay, that seems good. Given that -Warray-bounds is part of -Wall, what should happen for -Wstrict-flex-arrays=N? -- Kees Cook
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
Hi, Sid, Thanks a lot for the input. After more thinking based on your and Kees’ comments, I have the following thought: 1. -fstrict-flex-arrays=level should control both GCC code gen and warnings consistently; 2. We need warnings specifically for -fstrict-flex-arrays=level to report any misuse of flexible array corresponding to the “level” to gradually encourage language standard. So, based on the above two, I think what I did in this current patch is correct: 1. We eliminate the control from -Warray-bounds=level on treating flex arrays, now only "-fstrict-flex-arrasy=level" controls how the warning treating the flex arrays. 2. We add a separate new warning -Wstrict-flex-arrays to report any misuse corresponding to the different level of -fstrict-flex-arrays. Although we can certainly merge these new warnings into -Warray-bounds, however, as Sid mentioned, -Warray-bounds does issue a lot more warnings than just flexible arrays misuse. I think it’s necessary To provide a seperate warning to only issue flexible array misuse. Let me know if you have any more comments on this. thanks. Qing > On Dec 1, 2022, at 2:45 PM, Siddhesh Poyarekar wrote: > > On 2022-12-01 11:42, Kees Cook wrote: >> On Wed, Nov 30, 2022 at 02:25:56PM +, Qing Zhao wrote: >>> '-Wstrict-flex-arrays' >>> Warn about inproper usages of flexible array members according to >>> the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to >>> the trailing array field of a structure if it's available, >>> otherwise according to the LEVEL of the option >>> '-fstrict-flex-arrays=LEVEL'. >>> >>> This option is effective only when LEVEL is bigger than 0. >>> Otherwise, it will be ignored with a warning. >>> >>> when LEVEL=1, warnings will be issued for a trailing array >>> reference of a structure that have 2 or more elements if the >>> trailing array is referenced as a flexible array member. >>> >>> when LEVEL=2, in addition to LEVEL=1, additional warnings will be >>> issued for a trailing one-element array reference of a structure if >>> the array is referenced as a flexible array member. >>> >>> when LEVEL=3, in addition to LEVEL=2, additional warnings will be >>> issued for a trailing zero-length array reference of a structure if >>> the array is referenced as a flexible array member. >>> >>> At the same time, -Warray-bounds is updated: >> Why is there both -Wstrict-flex-arrays and -Warray-bounds? I thought >> only the latter was going to exist? > > Oh my understanding of the consensus was to move flex array related diagnosis > from -Warray-bounds to -Wstring-flex-arrays as Qing has done. If only the > former exists then instead of removing the flex array related statement in > the documentation as Richard suggested, we need to enhance it to say that > behaviour of -Warray-bounds will depend on -fstrict-flex-arrays. > > -Warray-bounds does diagnosis beyond just flexible arrays, in case that's the > confusion. > > Sid
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
On 2022-12-01 11:42, Kees Cook wrote: On Wed, Nov 30, 2022 at 02:25:56PM +, Qing Zhao wrote: '-Wstrict-flex-arrays' Warn about inproper usages of flexible array members according to the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to the trailing array field of a structure if it's available, otherwise according to the LEVEL of the option '-fstrict-flex-arrays=LEVEL'. This option is effective only when LEVEL is bigger than 0. Otherwise, it will be ignored with a warning. when LEVEL=1, warnings will be issued for a trailing array reference of a structure that have 2 or more elements if the trailing array is referenced as a flexible array member. when LEVEL=2, in addition to LEVEL=1, additional warnings will be issued for a trailing one-element array reference of a structure if the array is referenced as a flexible array member. when LEVEL=3, in addition to LEVEL=2, additional warnings will be issued for a trailing zero-length array reference of a structure if the array is referenced as a flexible array member. At the same time, -Warray-bounds is updated: Why is there both -Wstrict-flex-arrays and -Warray-bounds? I thought only the latter was going to exist? Oh my understanding of the consensus was to move flex array related diagnosis from -Warray-bounds to -Wstring-flex-arrays as Qing has done. If only the former exists then instead of removing the flex array related statement in the documentation as Richard suggested, we need to enhance it to say that behaviour of -Warray-bounds will depend on -fstrict-flex-arrays. -Warray-bounds does diagnosis beyond just flexible arrays, in case that's the confusion. Sid
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
Richard, What’s your opinion on this? Do we need one separate warning option to report the misuse of flexible array member only? Or we just combine such warnings into -Warray-bounds when it combines with -fstrict-flex-arrays? Thanks. Qing > On Dec 1, 2022, at 12:18 PM, Kees Cook wrote: > > On Thu, Dec 01, 2022 at 05:04:02PM +, Qing Zhao wrote: >> >> >>> On Dec 1, 2022, at 11:42 AM, Kees Cook wrote: >>> >>> On Wed, Nov 30, 2022 at 02:25:56PM +, Qing Zhao wrote: '-Wstrict-flex-arrays' Warn about inproper usages of flexible array members according to the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to the trailing array field of a structure if it's available, otherwise according to the LEVEL of the option '-fstrict-flex-arrays=LEVEL'. This option is effective only when LEVEL is bigger than 0. Otherwise, it will be ignored with a warning. when LEVEL=1, warnings will be issued for a trailing array reference of a structure that have 2 or more elements if the trailing array is referenced as a flexible array member. when LEVEL=2, in addition to LEVEL=1, additional warnings will be issued for a trailing one-element array reference of a structure if the array is referenced as a flexible array member. when LEVEL=3, in addition to LEVEL=2, additional warnings will be issued for a trailing zero-length array reference of a structure if the array is referenced as a flexible array member. At the same time, -Warray-bounds is updated: >>> >>> Why is there both -Wstrict-flex-arrays and -Warray-bounds? I thought >>> only the latter was going to exist? >> >> Yes, It’s very easy to merge these two warnings into one: >> >> -Warray-bounds, when combined with -fstrict-flex-arrays, in addition to >> report all the out-of-bounds warnings, it also report >> the misuse of flexible array members according to the LEVEL of >> -fstrict-flex-arrays >> >> The major question is, do we need one separate warning option to report the >> misuse of flexible array member only? >> If so, then we need to add a new one. > > I guess it is up to you, but I think it just makes things needlessly > complex. I think having 1 option for behavior (-ftrict-flex-arrays), > and 1 option for warnings (-Warray-bounds) is sufficient. I think adding > -Wstrict-flex-arrays is confusing. > > -- > Kees Cook
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
On Thu, Dec 01, 2022 at 05:04:02PM +, Qing Zhao wrote: > > > > On Dec 1, 2022, at 11:42 AM, Kees Cook wrote: > > > > On Wed, Nov 30, 2022 at 02:25:56PM +, Qing Zhao wrote: > >> '-Wstrict-flex-arrays' > >> Warn about inproper usages of flexible array members according to > >> the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to > >> the trailing array field of a structure if it's available, > >> otherwise according to the LEVEL of the option > >> '-fstrict-flex-arrays=LEVEL'. > >> > >> This option is effective only when LEVEL is bigger than 0. > >> Otherwise, it will be ignored with a warning. > >> > >> when LEVEL=1, warnings will be issued for a trailing array > >> reference of a structure that have 2 or more elements if the > >> trailing array is referenced as a flexible array member. > >> > >> when LEVEL=2, in addition to LEVEL=1, additional warnings will be > >> issued for a trailing one-element array reference of a structure if > >> the array is referenced as a flexible array member. > >> > >> when LEVEL=3, in addition to LEVEL=2, additional warnings will be > >> issued for a trailing zero-length array reference of a structure if > >> the array is referenced as a flexible array member. > >> > >> At the same time, -Warray-bounds is updated: > > > > Why is there both -Wstrict-flex-arrays and -Warray-bounds? I thought > > only the latter was going to exist? > > Yes, It’s very easy to merge these two warnings into one: > > -Warray-bounds, when combined with -fstrict-flex-arrays, in addition to > report all the out-of-bounds warnings, it also report > the misuse of flexible array members according to the LEVEL of > -fstrict-flex-arrays > > The major question is, do we need one separate warning option to report the > misuse of flexible array member only? > If so, then we need to add a new one. I guess it is up to you, but I think it just makes things needlessly complex. I think having 1 option for behavior (-ftrict-flex-arrays), and 1 option for warnings (-Warray-bounds) is sufficient. I think adding -Wstrict-flex-arrays is confusing. -- Kees Cook
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
> On Dec 1, 2022, at 11:42 AM, Kees Cook wrote: > > On Wed, Nov 30, 2022 at 02:25:56PM +, Qing Zhao wrote: >> '-Wstrict-flex-arrays' >> Warn about inproper usages of flexible array members according to >> the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to >> the trailing array field of a structure if it's available, >> otherwise according to the LEVEL of the option >> '-fstrict-flex-arrays=LEVEL'. >> >> This option is effective only when LEVEL is bigger than 0. >> Otherwise, it will be ignored with a warning. >> >> when LEVEL=1, warnings will be issued for a trailing array >> reference of a structure that have 2 or more elements if the >> trailing array is referenced as a flexible array member. >> >> when LEVEL=2, in addition to LEVEL=1, additional warnings will be >> issued for a trailing one-element array reference of a structure if >> the array is referenced as a flexible array member. >> >> when LEVEL=3, in addition to LEVEL=2, additional warnings will be >> issued for a trailing zero-length array reference of a structure if >> the array is referenced as a flexible array member. >> >> At the same time, -Warray-bounds is updated: > > Why is there both -Wstrict-flex-arrays and -Warray-bounds? I thought > only the latter was going to exist? Yes, It’s very easy to merge these two warnings into one: -Warray-bounds, when combined with -fstrict-flex-arrays, in addition to report all the out-of-bounds warnings, it also report the misuse of flexible array members according to the LEVEL of -fstrict-flex-arrays The major question is, do we need one separate warning option to report the misuse of flexible array member only? If so, then we need to add a new one. > > Are you trying to split code gen (-fstrict-flex-arrays) from warnings? No. After this patch, the -fstrict-flex-arrays will consistently control code gens and warnings in GCC except the default behavior without -fstrict-flex-arrays: For code gen, the default behavior is treating all trailing arrays as FAM; For warnings, the default behavior is treating [], [0],[1] trailing arrays as FAM; [n] is not treated as FAM. Qing > Is that needed? > > -- > Kees Cook
Re: [V2][PATCH 1/1] Add a new warning option -Wstrict-flex-arrays.
On Wed, Nov 30, 2022 at 02:25:56PM +, Qing Zhao wrote: > '-Wstrict-flex-arrays' > Warn about inproper usages of flexible array members according to > the LEVEL of the 'strict_flex_array (LEVEL)' attribute attached to > the trailing array field of a structure if it's available, > otherwise according to the LEVEL of the option > '-fstrict-flex-arrays=LEVEL'. > > This option is effective only when LEVEL is bigger than 0. > Otherwise, it will be ignored with a warning. > > when LEVEL=1, warnings will be issued for a trailing array > reference of a structure that have 2 or more elements if the > trailing array is referenced as a flexible array member. > > when LEVEL=2, in addition to LEVEL=1, additional warnings will be > issued for a trailing one-element array reference of a structure if > the array is referenced as a flexible array member. > > when LEVEL=3, in addition to LEVEL=2, additional warnings will be > issued for a trailing zero-length array reference of a structure if > the array is referenced as a flexible array member. > > At the same time, -Warray-bounds is updated: Why is there both -Wstrict-flex-arrays and -Warray-bounds? I thought only the latter was going to exist? Are you trying to split code gen (-fstrict-flex-arrays) from warnings? Is that needed? -- Kees Cook
