On 10/07/2022 08:59, Jeff Law via Gcc-patches wrote:
On 3/9/2022 5:39 PM, Siddhesh Poyarekar wrote:
The size argument larger than size of SRC for strnlen and strndup is
problematic only if SRC is not NULL terminated, which invokes undefined
behaviour. In all other cases, as long as SRC is large enough to have a
NULL char (i.e. size 1 or more), a larger N should not invoke a warning
during compilation.
Such a warning may be a suitable check for the static analyzer instead
with slightly different wording suggesting that choice of size argument
makes the function call equivalent to strlen/strdup.
This change results in the following code going through without a
warning:
------------------
char *buf;
char *
foo (void)
{
buf = __builtin_malloc (4);
__builtin_memset (buf, 'A', 4);
return __builtin_strndup (buf, 5);
}
int
main ()
{
__builtin_printf ("%s\n", foo ());
}
------------------
but the problem above is a missing NULL, not N being larger than the
size of SRC and the overread warning in this context is confusing at
best and misleading (and hinting at the wrong solution) in the worst
case.
gcc/ChangeLog:
middle-end/104854
* gimple-ssa-warn-access.cc (check_access<GimpleOrTree>):
New parameter. Skip warning if in read-only mode, source string
is NULL terminated and has non-zero object size.
(check_access<gimple *>): New parameter.
(check_access<tree>): Adjust.
(check_read_access): New parameter. Adjust for check_access
change.
(pass_waccess::check_builtin): Adjust check_read_access call for
memcmp, memchr.
(pass_waccess::maybe_check_access_sizes): Likewise.
gcc/testsuite/ChangeLog:
middle-end/104854
* gcc.dg/Wstringop-overread.c
(test_strnlen_array, test_strndup_array): Don't expect warning
for non-zero source sizes.
* gcc.dg/attr-nonstring-4.c (strnlen_range): Likewise.
* gcc.dg/pr78902.c: Likewise.
* gcc.dg/warn-strnlen-no-nul.c: Likewise.
I know this is old and the BZ has been set as CLOSED/INVALID. But it
was in my TODO list, and I've got thoughts here so I might as well
chime in ;-)
The potential overread warning for that code seems quite reasonable to
me. Yes it is the case that the length argument is sometimes
unrelated to the source string. But even then where's the line for when
we should and should not warn?
The argument I was trying to make in the context of strnlen and strndup
was that it is more likely in practice for the length argument to be a
function of some other property, e.g. a destination buffer or an
external limit that it is to be related to the source string. However I
don't have any concrete evidence (or the cycles to find it at the
moment) to either back up my claim or refute it. strndup for example
seems popular for a substring alloc+copy and also for a general string
copy with an application-specific upper bound, e.g. PATH_MAX.
Thanks,
Sid
