Re: [wwwdocs] changes.html - document new warning options

[Martin Sebor <msebor at gmail dot com>]

On 01/31/2017 02:16 PM, Gerald Pfeifer wrote:
> Wow, that's quite a patch.  And quite some contributions behind that! :-)
>
> Let me offer some comments, and then I suggest you commit what you
> have (taking these comments into account), and we can still improve
> things then if there is further feedback.

Sounds good.

> Index: gcc-7/changes.html
> ===================================================================
> +  <li>GCC 7 can determine the return value or range of return values of
> +    some calls to the <code>sprintf</code> family of functions and make
> +    it available to other optimization passes.  Some calls to the <code>
> +      snprintf</code> function with a zero size argument can be folded
>
> Formatting here appears a little odd?  I wouldn't have that line break
> afer <code>.

Sure.

> +    into constants.  The optimization is included in <code>-O1</code>
> +    and can be selectively controlled by the
>
> "This optimization"

Okay.

> +<li>GCC 7 contains a number of enhancements that help detect buffer overflow
> +  and other forms of invalid memory accesses.
>
> "buffer overflows" (plural) ?

Both are common in literature.  Google returns about 5,220 hits for
the string "detect buffer overflow and" and about 2,980 for the
singular, so I kept it as is.

> +	errors.  Such bugs have been known to be the source of
> +	vulnerabilities and a target of exploits.
>
> Perhaps say "security vulnerabilities"?  Not sure about that.

Sure.

> +      <code>-Walloc-size-larger-than=<i>PTRDIFF_MAX</i></code> is included
> +      in <code>-Wall</code>.</p>
>
> PTRDIFF_MAX without <i>...</i> I would say, since it's not a variable.

Okay.

> +      <p>For example, the following call to <b>malloc</b> incorrectly tries
>
> <code>malloc</code>

Done.

> +void* f (int n)
> +{
> +  return malloc (n > 0 ? 0 : n);
> +}
>
> Great example! :-)
>
> +    <li><p>The <code>-Walloc-zero</code> option detects calls to standard
> +	and user-defined memory allocation functions decorated with attribute
> +	<code>alloc_size</code> with a zero argument.  <code>-Walloc-zero</code>
> +	is not included in either <code>-Wall</code> or <code>-Wextra</code>
> +	and must be explicitly enabled.</p></li>
>
> Why are you adding <p> within <li>?  This should not be necessary.

Only for consistency with the other list items where it separates
paragraphs (it made it easier for me to spot missing tags and such).
I removed from where it was redundant.

> +      <b><code>sprintf</code></b> is diagnosed because even though its
>        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> Why <b>...</b> here?

It was a copy and paste typo.  I removed the tags.

> +      output has been constrained using the modulo operation it could
> +      result in as many as three bytes if <code>mday</code> were negative.
> +      The solution is to either allocate a larger buffer or make sure
> +      the argument is not negative, for example by changing <code>mday</code>'s
> +      type to <code>unsigned</code> or by making the type of the second operand
> +      of the modulo expression to <code>unsigned</code>: <code>100U</code>.
>
> "changing the type"

Right, thanks.

> +	<code>-Wformat-overflow=<i>1</i></code> is included in
>
> No <i>...</i> around 1, since it's a constant (not a variable).

Done.

> +	<code>nonnull</code>). By taking advantage of optimization the option
> +	can detect many more cases of the problem than in prior GCC
> +	versions.</p></li>
>
> "optimizations"  (Or "compiler optimizations", to avoid ambiguity
> whether the option was optimized or is now leveraging compiler
> optimizations?)
>
> +    <li><p>The <code>-Wstringop-overflow=<i>type</i></code> option detects
> +	buffer overflow in calls to string manipulation functions like
>
> "overflows"
>
> +	<code>memcpy</code> and <code>strcpy</code>. The option relies
>
> Is memcpy really a string manipulation function?

The C standard calls all the functions in <string.h> (including memcpy)
string handling functions.  I'm not sure where I got "manipulation"
from.  I changed it to string handling though I'm not sure it's a big
improvement given that memcpy arguments need not be strings.

> +	on Object Size Checking and has an effect similar to defining
> +	the <code>_FORTIFY_SOURCE</code> macro.
>
> Naive question: What is "Object Size Checking", and where is it
> introduced or desribed?

It's described in the GCC manual.  I added a link to it.

> +	<code>-Wstringop-overflow=<i>1</i></code> is enabled by default.</p>
>
> No <i>s around constants.
>
> +      <p>For example, in the following snippet, because the call to
> +	<code>strncat</code> specifies a maximum that allows the function to
> +	write past the end of the destination, it is diagnosed.  To correct
> +	the problem and avoid the overflow the function should be called
> +	with a size of at most <code>sizeof d - strlen(d)</code>.
>
> I'm getting tired this evening, but is this taking care of the \0
> being added?  Or would that require sizeof d - strlen(d) - 1?

It sure would!

I've committed the patch with the changes above (plus another example
to make Aldy extra happy).  Please let me know if you spot something
else that needs adjusting.

Thanks for the careful review (and debugging)!

Martin