https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104854
Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|-Wstringop-overread should |-Wstringop-overread should
|not warn for strnlen and |not warn for strnlen,
|strndup |strndup and strncmp
Assignee|unassigned at gcc dot gnu.org |siddhesh at gcc dot
gnu.org
--- Comment #4 from Siddhesh Poyarekar <siddhesh at gcc dot gnu.org> ---
(In reply to Martin Sebor from comment #3)
> The same warning is also issued for some calls to memchr(), strncmp() and
> probably other built-in functions as well. For example:
>
> const char a[] = "123";
>
> char* f (int i)
> {
> return __builtin_memchr (a + i, '3', 7);
> }
>
> warning: ‘__builtin_memchr’ specified bound 7 exceeds source size 4
> [-Wstringop-overread]
> 5 | return __builtin_memchr (a + i, '3', 7);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> y.c:1:12: note: source object allocated here
> 1 | const char a[] = "123";
> | ^
The warning is arguably legitimate (not this one of course, since it's evident
at compile time that the operation is safe but for non-const `a` it may not be)
for memchr because it operates on an object, not string and will traverse all
of the object for a matching char to the extent of the object size. The
*string* functions are not the same in that context.
> In all these cases the warnings are intentional (i.e., it's a feature, and
> so not a regression). Their purpose is to point out what could be a coding
> mistake. With strndup(), since the use case for it rather than strdup() is
> to copy an initial substring, specifying a bound that's larger than the
> length of the string is pointless.
>
> In general, the GCC manual documents warnings as
>
> diagnostic messages that report constructions that are not inherently
> erroneous but that are risky or suggest there may have been an error.
I don't think these strnlen or strndup warnings point to *risky* or potentially
erroneous code; at best they point to instances where a specific protection is
absent, i.e. the behaviour reduces to strlen and strdup respectively, which is
much more benign than what it currently suggests. That kind of warning seems
more suited to a static analyzer and not a compiler IMO.
Besides, the core cause of a potential overread here is not because the size
argument is larger but because the string may not be NULL terminated. It may
make more sense to invest in that aspect of risky behaviour than the length for
these functions.
> So not all instances of any warning should be expected to indicate errors.
> In fact, many are known to be benign by design (for example, all instances
> of -Wchar-subscripts are benign when -funsigned-char is used; many instance
> of -Waddress are also benign, as are many -Wparentheses, etc.). And
They're not clubbed in with potentially important warnings though, which is a
key distinction. For example, one could mark -Wstirngop-overread as important
warnings but not -Wparentheses, but the high noise could make it difficult to
actually do so.
> although most middle end warnings tend to be designed to trigger only for
> invalid statements (i.e., statements that have undefined behavior if reached
> during execution), some do also trigger for code that's strictly valid but
> suspect. Besides the -Wstringop-overread examples here, other such warnings
> include dynamic allocation calls with sizes in excess of PTRDIFF_MAX
> (-Walloc-size-larger-than), returning the address of a local variable
> (-Wreturn-local-addr), or in GCC 12, storing the address of a local variable
> in an extern pointer (-Wdangling-pointer).
>
> Deciding what code is suspect enough to warn and under what option (-Wall or
> -Wextra) is a judgment call; different people have very different ideas.
I'm testing a patch to resolve this. I noticed strncmp late but it seems to
match this category as well, where a too-long length only reduces the max
protection provided by the n-versions of the string functions.
