On 3/17/22 11:22, Siddhesh Poyarekar wrote:
On 17/03/2022 22:16, Jeff Law wrote:
#include <string.h>
char a[] = "abc";
char b[] = "abcd";
int f (void)
{
return strncmp (a, b, 8);
}
where I get
t.c:7:10: warning: ‘strncmp’ specified bound 8 exceeds source size 5
[-Wstringop-overread]
7 | return strncmp (a, b, 8); // -Wstringop-overread
| ^~~~~~~~~~~~~~~~~
even without -Wall. strncmp sees that a[3] is '\0' so it stops
comparing
and there's no UB.
This one is a clear case where warning is bad. Both arguments are
constant and we can determine they are NUL terminated and an overread
will never occur. No deep analysis really needed here.
THe far more interesting case in my mind is when one or both arguments
have an unknown NUL termination state. I could argue either side of
that case. I lean towards warning but I understand that opinions
differ and my priorities have moved away from distro-level issues, so
identifying code that needs a careful review for correctness,
particularly old or security sensitive code, has become a much lower
priority for me. Combine that with the fact that we're really just
dealing with over-reads here, I can support whatever the broadest
consensus is.
Actually in the above reproducer a and b are not const, so this is in
fact the case where the NUL termination state of the strings is in
theory unknown. From the distro level (and in general for applications)
the question is how common this is and I gathered from a Red Hat
internal conversation that it's not uncommon. However David pointed out
that I need to share more specific examples to quantify this, so I need
to work on that. I'll share an update once I have it.
One case I am aware of is the pmix package in Fedora/RHEL, which has the
following warning:
pmix-3.2.3/examples/alloc.c: scope_hint: In function 'main'
pmix-3.2.3/examples/alloc.c:179:31: warning[-Wstringop-overread]:
'PMIx_Get' reading 512 bytes from a region of size 15
179 | if (PMIX_SUCCESS != (rc = PMIx_Get(&proc, PMIX_UNIV_SIZE,
NULL, 0, &val))) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pmix-3.2.3/examples/alloc.c:179:31: note: referencing argument 2 of type
'const char *'
pmix-3.2.3/examples/alloc.c:33: included_from: Included from here.
pmix-3.2.3/include/pmix.h:203:27: note: in a call to function 'PMIx_Get'
203 | PMIX_EXPORT pmix_status_t PMIx_Get(const pmix_proc_t *proc,
const pmix_key_t key,
| ^~~~~~~~
177| PMIX_PROC_CONSTRUCT(&proc);
178| PMIX_LOAD_PROCID(&proc, myproc.nspace, PMIX_RANK_WILDCARD);
179|-> if (PMIX_SUCCESS != (rc = PMIx_Get(&proc, PMIX_UNIV_SIZE,
NULL, 0, &val))) {
180| fprintf(stderr, "Client ns %s rank %d: PMIx_Get
universe size failed: %d\n", myproc.nspace, myproc.rank, rc);
181| goto done;
which is due to PMIx_Get calling strncmp a few levels within with
non-const strings and a max size of 512 (the maximum size that a key
could be; AFAICT it's the size of the buffer into which the key gets
written out), where the strings are always NULL terminated.
This warning has nothing to do with strncmp.
It's issued for the call to PMIx_Get(), where the caller passes as
the second argument PMIX_UNIV_SIZE, a macro that expands to
the string "pmix.univ.size".
The function is declared like so:
PMIX_EXPORT pmix_status_t
PMIx_Get(const pmix_proc_t *proc, const pmix_key_t key,
const pmix_info_t info[], size_t ninfo,
pmix_value_t **val);
The type of the second function argument, pmix_key_t, defined as
typedef char pmix_key_t[PMIX_MAX_KEYLEN+1];
an array of 512 elements (PMIX_MAX_KEYLEN is defined to 511), but
PMIX_UNIV_SIZE is much smaller (just 15 bytes).
The warning detects passing smaller arrays to parameters of larger
types declared using the array syntax. It's controlled by
-Warray-parameter.
Martin