On 3/17/22 12:02, Siddhesh Poyarekar wrote:
On 17/03/2022 23:21, Martin Sebor wrote:
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.
That's odd, shouldn't it show up as -Warray-parameter then and not
-Wstringop-overread?
I should have said: the array parameter feature is controlled by
-Warray-parameter. (The warning above is obviously
-Wstringop-overread).
But since -Warray-parameter controls the array parameter feature,
it might be appropriate to also make the -Wstringop-overread and
-Wstringop-overflow instances for calls to such functions conditional
on the former option being enabled. (Otherwise, -Warray-parameter
has a separate function of its own.)
I do see a minor issue with this warning in GCC 12: it's issued three
times for the same call, rather than once. That's probably because
the new warning pass that issues it runs multiple times and doesn't
suppress the warning after issuing it the first time.
Martin
Siddhesh
