diff mbox series

Update -Warray-bounds documentation [PR104355]

Message ID 99b28146-551c-f1b5-02ab-db6325d74a19@gmail.com
State New
Headers show
Series Update -Warray-bounds documentation [PR104355] | expand

Commit Message

Martin Sebor Feb. 11, 2022, midnight UTC
The -Warray-bounds description in the manual is out of date in
a couple of ways.  First it claims that the option is only active
with optimization, which isn't entirely correct since at least one
instance is issued even without it.  Second, the description of
its level 2 suggests it controls the warning for all trailing
array members, when it only controls it for trailing one-element
arrays (this was made tighter in GCC 10 but we neglected to update
the manual).

In addition, the word "always" in the description of the option
is also interpreted by some as implying that every instance of
the warning is necessarily a true positive.  I've reworded
the description to hopefully avoid this misreading(*).

Finally, the generic text that talks about the interaction with
optimizations says that -Wmaybe-uninitialized is not issued unless
optimization is enabled.  That's also not accurate anymore since
at least one instance of the warning is independent of optimization
(passing uninitialized objects by reference to const arguments).

The attached changes correct these oversights.

Martin

[*] It should probably be made clearer in the generic text that
no instance of any warning, not just -Warray-bounds, should be
taken to be a definitive indication of a bug in the code.  I've
left that for later.

Comments

Richard Sandiford Feb. 11, 2022, 6:48 a.m. UTC | #1
Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> The -Warray-bounds description in the manual is out of date in
> a couple of ways.  First it claims that the option is only active
> with optimization, which isn't entirely correct since at least one
> instance is issued even without it.  Second, the description of
> its level 2 suggests it controls the warning for all trailing
> array members, when it only controls it for trailing one-element
> arrays (this was made tighter in GCC 10 but we neglected to update
> the manual).
>
> In addition, the word "always" in the description of the option
> is also interpreted by some as implying that every instance of
> the warning is necessarily a true positive.  I've reworded
> the description to hopefully avoid this misreading(*).
>
> Finally, the generic text that talks about the interaction with
> optimizations says that -Wmaybe-uninitialized is not issued unless
> optimization is enabled.  That's also not accurate anymore since
> at least one instance of the warning is independent of optimization
> (passing uninitialized objects by reference to const arguments).
>
> The attached changes correct these oversights.
>
> Martin
>
> [*] It should probably be made clearer in the generic text that
> no instance of any warning, not just -Warray-bounds, should be
> taken to be a definitive indication of a bug in the code.  I've
> left that for later.

Yeah, maybe, but I guess it's unlikely to be useful in practice.
The chances of users happening to read a given bit of generic text
seem pretty low.

Doesn't mean we shouldn't do it of course (provided that we don't then
castigate users for having failed to notice it).

> Update -Warray-bounds documentation [PR104355].
>
> Resolves:
> PR middle-end/104355 - Misleading and outdated -Warray-bounds documentation
>
> gcc/ChangeLog:
> 	* doc/invoke.texi (-Warray-bounds): Update documentation.
>
>
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index b49ba22df89..b7b1f47a5ce 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -5641,8 +5641,10 @@ warns that an unrecognized option is present.
>  
>  The effectiveness of some warnings depends on optimizations also being
>  enabled. For example @option{-Wsuggest-final-types} is more effective
> -with link-time optimization and @option{-Wmaybe-uninitialized} does not
> -warn at all unless optimization is enabled.
> +with link-time optimization and some instances of other warnings may
> +not be issued at all unless optimization is enabled.  While optimization
> +in general improves the efficacy of control and data flow sensitive
> +warnings, in some cases it may also cause false positives.
>  
>  @table @gcctabopt
>  @item -Wpedantic
> @@ -7691,20 +7693,22 @@ void f (char c, int i)
>  @itemx -Warray-bounds=@var{n}
>  @opindex Wno-array-bounds
>  @opindex Warray-bounds
> -This option is only active when @option{-ftree-vrp} is active
> -(default for @option{-O2} and above). It warns about subscripts to arrays
> -that are always out of bounds. This warning is enabled by @option{-Wall}.
> +Warn about out of bounds subscripts or offsets into arrays. This warning
> +level is enabled by @option{-Wall}.  It is the most effective when

It's not clear to me which level is “this level” here.  How about
something like “This warning is enabled at level 1…”?

“It is more effective when” seems more natural to me than “It is the most
effective when”, but maybe that's just me.

Looks good to me otherwise FWIW.  OK with those changes if you agree and
if no-one has further comments by Monday.

Thanks,
Richard

> +@option{-ftree-vrp} is active (the default for @option{-O2} and above)
> +but a subset of instances are issued even without optimization.
>  
>  @table @gcctabopt
>  @item -Warray-bounds=1
> -This is the warning level of @option{-Warray-bounds} and is enabled
> +This is the default warning level of @option{-Warray-bounds} and is enabled
>  by @option{-Wall}; higher levels are not, and must be explicitly requested.
>  
>  @item -Warray-bounds=2
> -This warning level also warns about out of bounds access for
> -arrays at the end of a struct and for arrays accessed through
> -pointers. This warning level may give a larger number of
> -false positives and is deactivated by default.
> +This warning level also warns out of bounds accesses to trailing struct
> +members of one-element array types (@pxref{Zero Length}) and 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.
>  @end table
>  
>  @item -Warray-compare
Martin Sebor Feb. 14, 2022, 10:56 p.m. UTC | #2
On 2/10/22 23:48, Richard Sandiford wrote:
> Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> The -Warray-bounds description in the manual is out of date in
>> a couple of ways.  First it claims that the option is only active
>> with optimization, which isn't entirely correct since at least one
>> instance is issued even without it.  Second, the description of
>> its level 2 suggests it controls the warning for all trailing
>> array members, when it only controls it for trailing one-element
>> arrays (this was made tighter in GCC 10 but we neglected to update
>> the manual).
>>
>> In addition, the word "always" in the description of the option
>> is also interpreted by some as implying that every instance of
>> the warning is necessarily a true positive.  I've reworded
>> the description to hopefully avoid this misreading(*).
>>
>> Finally, the generic text that talks about the interaction with
>> optimizations says that -Wmaybe-uninitialized is not issued unless
>> optimization is enabled.  That's also not accurate anymore since
>> at least one instance of the warning is independent of optimization
>> (passing uninitialized objects by reference to const arguments).
>>
>> The attached changes correct these oversights.
>>
>> Martin
>>
>> [*] It should probably be made clearer in the generic text that
>> no instance of any warning, not just -Warray-bounds, should be
>> taken to be a definitive indication of a bug in the code.  I've
>> left that for later.
> 
> Yeah, maybe, but I guess it's unlikely to be useful in practice.
> The chances of users happening to read a given bit of generic text
> seem pretty low.
> 
> Doesn't mean we shouldn't do it of course (provided that we don't then
> castigate users for having failed to notice it).

I'm sure you're right that few users go to the trouble of studying
the manual when interpreting a warning (and some not even before
submitting a problem report).

> 
>> Update -Warray-bounds documentation [PR104355].
>>
>> Resolves:
>> PR middle-end/104355 - Misleading and outdated -Warray-bounds documentation
>>
>> gcc/ChangeLog:
>> 	* doc/invoke.texi (-Warray-bounds): Update documentation.
>>
>>
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index b49ba22df89..b7b1f47a5ce 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -5641,8 +5641,10 @@ warns that an unrecognized option is present.
>>   
>>   The effectiveness of some warnings depends on optimizations also being
>>   enabled. For example @option{-Wsuggest-final-types} is more effective
>> -with link-time optimization and @option{-Wmaybe-uninitialized} does not
>> -warn at all unless optimization is enabled.
>> +with link-time optimization and some instances of other warnings may
>> +not be issued at all unless optimization is enabled.  While optimization
>> +in general improves the efficacy of control and data flow sensitive
>> +warnings, in some cases it may also cause false positives.
>>   
>>   @table @gcctabopt
>>   @item -Wpedantic
>> @@ -7691,20 +7693,22 @@ void f (char c, int i)
>>   @itemx -Warray-bounds=@var{n}
>>   @opindex Wno-array-bounds
>>   @opindex Warray-bounds
>> -This option is only active when @option{-ftree-vrp} is active
>> -(default for @option{-O2} and above). It warns about subscripts to arrays
>> -that are always out of bounds. This warning is enabled by @option{-Wall}.
>> +Warn about out of bounds subscripts or offsets into arrays. This warning
>> +level is enabled by @option{-Wall}.  It is the most effective when
> 
> It's not clear to me which level is “this level” here.  How about
> something like “This warning is enabled at level 1…”?

Yes, good catch, that was a mistake on my part.

> 
> “It is more effective when” seems more natural to me than “It is the most
> effective when”, but maybe that's just me.

-Warray-bounds doesn't do much without -ftree-vrp (it detects out-of-
bounds offsets in calls to built-ins), and does almost nothing below
-O2 (all it diagnoses at -O0 is strlen() calls with past-the-end
offsets to string constants).  But since the rest of the sentence
mention that it's not completely silent even at -O0 your words sound
good to me too.

> 
> Looks good to me otherwise FWIW.  OK with those changes if you agree and
> if no-one has further comments by Monday.

Pushed now in r12-7234.

Martin

> 
> Thanks,
> Richard
> 
>> +@option{-ftree-vrp} is active (the default for @option{-O2} and above)
>> +but a subset of instances are issued even without optimization.
>>   
>>   @table @gcctabopt
>>   @item -Warray-bounds=1
>> -This is the warning level of @option{-Warray-bounds} and is enabled
>> +This is the default warning level of @option{-Warray-bounds} and is enabled
>>   by @option{-Wall}; higher levels are not, and must be explicitly requested.
>>   
>>   @item -Warray-bounds=2
>> -This warning level also warns about out of bounds access for
>> -arrays at the end of a struct and for arrays accessed through
>> -pointers. This warning level may give a larger number of
>> -false positives and is deactivated by default.
>> +This warning level also warns out of bounds accesses to trailing struct
>> +members of one-element array types (@pxref{Zero Length}) and 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.
>>   @end table
>>   
>>   @item -Warray-compare
diff mbox series

Patch

Update -Warray-bounds documentation [PR104355].

Resolves:
PR middle-end/104355 - Misleading and outdated -Warray-bounds documentation

gcc/ChangeLog:
	* doc/invoke.texi (-Warray-bounds): Update documentation.


diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index b49ba22df89..b7b1f47a5ce 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -5641,8 +5641,10 @@  warns that an unrecognized option is present.
 
 The effectiveness of some warnings depends on optimizations also being
 enabled. For example @option{-Wsuggest-final-types} is more effective
-with link-time optimization and @option{-Wmaybe-uninitialized} does not
-warn at all unless optimization is enabled.
+with link-time optimization and some instances of other warnings may
+not be issued at all unless optimization is enabled.  While optimization
+in general improves the efficacy of control and data flow sensitive
+warnings, in some cases it may also cause false positives.
 
 @table @gcctabopt
 @item -Wpedantic
@@ -7691,20 +7693,22 @@  void f (char c, int i)
 @itemx -Warray-bounds=@var{n}
 @opindex Wno-array-bounds
 @opindex Warray-bounds
-This option is only active when @option{-ftree-vrp} is active
-(default for @option{-O2} and above). It warns about subscripts to arrays
-that are always out of bounds. This warning is enabled by @option{-Wall}.
+Warn about out of bounds subscripts or offsets into arrays. This warning
+level is enabled by @option{-Wall}.  It is the most effective when
+@option{-ftree-vrp} is active (the default for @option{-O2} and above)
+but a subset of instances are issued even without optimization.
 
 @table @gcctabopt
 @item -Warray-bounds=1
-This is the warning level of @option{-Warray-bounds} and is enabled
+This is the default warning level of @option{-Warray-bounds} and is enabled
 by @option{-Wall}; higher levels are not, and must be explicitly requested.
 
 @item -Warray-bounds=2
-This warning level also warns about out of bounds access for
-arrays at the end of a struct and for arrays accessed through
-pointers. This warning level may give a larger number of
-false positives and is deactivated by default.
+This warning level also warns out of bounds accesses to trailing struct
+members of one-element array types (@pxref{Zero Length}) and 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.
 @end table
 
 @item -Warray-compare