Re: [PATCH] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

[Jeff Law <law at redhat dot com>]

On 08/06/2017 02:07 PM, Martin Sebor wrote:
>>>
>>> You're right that there is no truncation and the effect is
>>> the same but only in the unlikely case when the destination
>>> is empty.  Otherwise the result is truncated.
>> Maybe this is where I'm confused.  How does the destination play into
>> truncation issues?  I've always been under the impression that the
>> destination has to be large enough to hold the result, but that it
>> doesn't affect truncation of the source.
> 
> No, you're right.  It's me who's been confused, either thinking
> of strncpy or -Wstringop-overflow.  The difference between the
> two warnings is just one byte in some cases and I got them mixed
> up.  Sorry about that and thanks for spotting this silly  mistake!
> I've updated the code to issue -Wstringop-overflow here and added
> a better example to the manual.
Thanks.  I kept looking thinking I must have missed something somewhere...


> 
> 1) In the following, the strncpy call would normally trigger
> -Wstringop-truncation because of the possible missing terminating
> NUL, but because the immediately following statement inserts the
> NUL the call is safe.
> 
>    strncpy (d, s, sizeof d);   // possible missing NUL
>    d[sizeof d - 1] = '\0';     // okay, NUL add here
At first I wondered if this was an optimization opportunity.  BUt
thinking more about it, I don't think it is, unless you happen to know
that sizeof d == sizeof s, which I doubt happens often enough to matter.


> 
> 2) Building Glibc made me realize that in my effort to detect
> the (common) misuses of strncpy I neglected the original (and
> only intended but now rare) use case of filling a buffer
> without necessarily NUL-terminating it (as in struct dirent::
> d_name).  To allow it the patch adds a new attribute that can
> be used to annotate char arrays and pointers that are intended
> not to be NUL-terminated.  This suppresses the truncation
> warning.  When the patch is approved I'll propose the (trivial)
> Glibc changes.  In the future, the attribute will also let GCC
> warn when passing such objects to functions that expect a NUL-
> terminated string argument (e.g., strlen or strcpy).
Seems reasonable.

> 
> 3) Finally, to add inlining context to diagnostics issued by
> the middle end, I've added a new %G directive to complement
> %K by accepting a gcall* argument.
Also seems reasonable.  I think we've wanted something like this for a
while.


> 
> To make the patch easier to review I've broken it up into
> four parts:
> 
>   1. Add %G.
>   2. Add attribute nostring.
>   3. Implement -Wstringop-truncation and enhance -Wstringop-
>      overflow (the meat of the patch).
>   4. Fix up GCC to compile with the new and enhanced warnings.
I'll try to get to them today.

Thanks,
jeff