PR middle-end/97631 - bogus "writing one too many bytes" warning for memcpy with strlen argument

gcc/ChangeLog:

	PR middle-end/97631
	* tree-ssa-strlen.c (maybe_warn_overflow): Test rawmem.
	(handle_builtin_stxncpy_strncat): Rename locals.  Determine
	destination size from allocation calls.  Issue a more appropriate
	kind of warning.
	(handle_builtin_memcpy): Pass true as rawmem to maybe_warn_overflow.
	(handle_builtin_memset): Same.

gcc/testsuite/ChangeLog:

	PR middle-end/97631
	* c-c++-common/Wstringop-overflow.c: Remove unexpected warnings.
	Add an xfail.
	* c-c++-common/Wstringop-truncation.c: Add expected warnings.
	* gcc.dg/Wstringop-overflow-10.c: Also enable -Wstringop-truncation.
	* gcc.dg/Wstringop-overflow-66.c: New test.
	* gcc.dg/tree-ssa/strncpy-2.c: Adjust expected warning.
This commit is contained in:
Martin Sebor 2021-03-08 13:28:52 -07:00
parent b64551af51
commit 7f5ff78ff3
6 changed files with 242 additions and 47 deletions

View File

@ -115,28 +115,31 @@ void test_strncpy (char **d, const char* s, int i)
T (d, "123", sizeof "123");
T (d, ar, sizeof ar);
T (d, s, strlen (s)); /* { dg-warning "\\\[-Wstringop-overflow=]" } */
/* There is no overflow in the following calls but they are diagnosed
by -Wstringop-truncation. Verify that they aren'y also diagnosed
by -Wstringop-overflow. */
T (d, s, strlen (s));
{
int n = strlen (s); /* { dg-message "length computed here" } */
T (d, s, n); /* { dg-warning "\\\[-Wstringop-overflow=]" } */
int n = strlen (s);
T (d, s, n);
}
{
unsigned n = strlen (s); /* { dg-message "length computed here" } */
T (d, s, n); /* { dg-warning "\\\[-Wstringop-overflow=]" } */
unsigned n = strlen (s);
T (d, s, n);
}
{
size_t n;
n = strlen (s); /* { dg-message "length computed here" } */
T (d, s, n); /* { dg-warning "\\\[-Wstringop-overflow=]" } */
n = strlen (s);
T (d, s, n);
}
{
size_t n;
n = strlen (s) - 1; /* { dg-message "length computed here" } */
T (d, s, n); /* { dg-warning "\\\[-Wstringop-overflow=]" } */
n = strlen (s) - 1;
T (d, s, n);
}
{
@ -148,11 +151,8 @@ void test_strncpy (char **d, const char* s, int i)
{
/* This use of strncpy is certainly dubious and it could well be
diagnosed by -Wstringop-truncation but it isn't. That it is
diagnosed with -Wstringop-overflow is more by accident than
by design. -Wstringop-overflow considers any dependency of
the bound on strlen(s) a potential bug. */
size_t n = i < strlen (s) ? i : strlen (s); /* { dg-message "length computed here" } */
T (d, s, n); /* { dg-message ".strncpy\[^\n\r]* specified bound depends on the length of the source argument" } */
diagnosed by -Wstringop-truncation but it isn't. */
size_t n = i < strlen (s) ? i : strlen (s); /* { dg-message "length computed here" "note" { xfail *-*-* } } */
T (d, s, n); /* { dg-message ".strncpy\[^\n\r]* specified bound depends on the length of the source argument" "pr?????" { xfail *-*-* } } */
}
}

View File

@ -226,19 +226,18 @@ void test_strncpy_ptr (char *d, const char* s, const char *t, int i)
}
{
/* The following is likely buggy but there's no apparent truncation
so it's not diagnosed by -Wstringop-truncation. Instead, it is
diagnosed by -Wstringop-overflow (tested elsewhere). */
/* The following truncates the terminating nul. The warning should
say that but doesn't. */
int n;
n = strlen (s) - 1;
CPY (d, s, n);
CPY (d, s, n); /* { dg-warning "\\\[-Wstringop-truncation" } */
}
{
/* Same as above. */
size_t n;
n = strlen (s) - 1;
CPY (d, s, n);
CPY (d, s, n); /* { dg-warning "\\\[-Wstringop-truncation" } */
}
{

View File

@ -1,5 +1,7 @@
/* { dg-do compile } */
/* { dg-options "-O2 -Wstringop-overflow" } */
/* PR tree-optimization/89500 - ICE: tree check: expected integer_cst,
have ssa_name in get_len
{ dg-do compile }
{ dg-options "-O2 -Wstringop-overflow -Wstringop-truncation" } */
void
foo (char *a)

View File

@ -0,0 +1,180 @@
/* PR middle-end/97631 - bogus "writing one too many bytes" warning for
memcpy with strlen argument
{ dg-do compile }
{ dg-options "-O2 -Wall" } */
#define NOIPA __attribute__ ((noipa))
typedef __SIZE_TYPE__ size_t;
extern void* malloc (size_t);
extern void* memcpy (void*, const void*, size_t);
extern void* memmove (void*, const void*, size_t);
extern void* memset (void*, int, size_t);
extern char* strcpy (char*, const char*);
extern char* strncpy (char*, const char*, size_t);
extern size_t strlen (const char*);
NOIPA char* nowarn_strcpy (char *s)
{
size_t n = strlen (s);
char *d = malloc (n + 1);
strcpy (d, s);
return d;
}
NOIPA char* warn_strcpy (char *s)
{
size_t n = strlen (s);
char *d = malloc (n);
strcpy (d, s); // { dg-warning "\\\[-Wstringop-overflow" }
return d;
}
NOIPA char* warn_strcpy_nz (char *s)
{
size_t n = strlen (s);
if (n == 0)
return 0;
char *d = malloc (n);
strcpy (d, s); // { dg-warning "\\\[-Wstringop-overflow" }
return d;
}
NOIPA char* warn_strcpy_nn (char *s)
{
size_t n = strlen (s);
char *d = malloc (n);
if (!d)
return 0;
strcpy (d, s); // { dg-warning "\\\[-Wstringop-overflow" }
return d;
}
NOIPA char* warn_strcpy_nz_nn (char *s)
{
size_t n = strlen (s);
if (n == 0)
return 0;
char *d = malloc (n);
if (!d)
return 0;
strcpy (d, s); // { dg-warning "\\\[-Wstringop-overflow" }
return d;
}
NOIPA char* nowarn_strncpy_1 (char *s)
{
/* There's no overflow or truncation below so verify there is no
warning either. */
size_t n = strlen (s) + 1;
char *d = malloc (n);
strncpy (d, s, n);
return d;
}
NOIPA char* warn_strncpy (char *s)
{
size_t n = strlen (s);
char *d = malloc (n);
strncpy (d, s, n); // { dg-warning "\\\[-Wstringop-truncation" }
return d;
}
NOIPA char* warn_strncpy_p1 (char *s)
{
size_t n = strlen (s);
char *d = malloc (n + 1);
strncpy (d, s, n); // { dg-warning "\\\[-Wstringop-truncation" }
return d;
}
NOIPA char* warn_strncpy_nz (char *s)
{
size_t n = strlen (s);
if (n == 0)
return 0;
char *d = malloc (n);
strncpy (d, s, n); // { dg-warning "\\\[-Wstringop-truncation" }
return d;
}
NOIPA char* nowarn_memcpy (char *s)
{
size_t n = strlen (s);
char *d = malloc (n);
memcpy (d, s, n); // { dg-bogus "\\\[-Wstringop-overflow" }
return d;
}
NOIPA char* nowarn_memcpy_nz (char *s)
{
size_t n = strlen (s);
if (n == 0)
return 0;
char *d = malloc (n);
memcpy (d, s, n); // { dg-bogus "\\\[-Wstringop-overflow" }
return d;
}
NOIPA char* nowarn_memcpy_nn (char *s)
{
size_t n = strlen (s);
char *d = malloc (n);
if (!d)
return 0;
memcpy (d, s, n); // { dg-bogus "\\\[-Wstringop-overflow" }
return d;
}
NOIPA char* nowarn_memcpy_nn_nz (char *s)
{
size_t n = strlen (s);
if (n == 0)
return 0;
char *d = malloc (n);
if (!d)
return 0;
memcpy (d, s, n); // { dg-bogus "\\\[-Wstringop-overflow" }
return d;
}
NOIPA char* nowarn_memmove (char *s)
{
size_t n = strlen (s);
if (n == 0)
return 0;
char *d = malloc (n);
memmove (d, s, n); // { dg-bogus "\\\[-Wstringop-overflow" }
return d;
}
NOIPA char* nowarn_memset (char *s, int c)
{
size_t n = strlen (s);
if (n == 0)
return 0;
char *d = malloc (n);
memset (d, c, n); // { dg-bogus "\\\[-Wstringop-overflow" }
return d;
}

View File

@ -1,6 +1,6 @@
/* PR tree-optimization/83075 - Invalid strncpy optimization */
/* { dg-do run } */
/* { dg-options "-O2 -Wstringop-overflow" } */
/* { dg-options "-O2 -Wstringop-truncation" } */
typedef __SIZE_TYPE__ size_t;
@ -8,7 +8,7 @@ __attribute__((noipa)) size_t
foo (char *p, char *q, size_t *r)
{
size_t n0 = __builtin_strlen (p);
__builtin_strncpy (q, p, n0); /* { dg-warning "specified bound depends on the length" } */
__builtin_strncpy (q, p, n0); /* { dg-warning "\\\[-Wstringop-truncation" } */
size_t n1 = __builtin_strlen (p);
*r = n0;
return n1;

View File

@ -1992,17 +1992,12 @@ maybe_warn_overflow (gimple *stmt, tree len, pointer_query &ptr_qry,
&& wi::leu_p (lenrng[1], spcrng[1]))
return;
if (lenrng[0] == spcrng[1]
&& (len != destsize
|| !si || !is_strlen_related_p (si->ptr, len)))
return;
location_t loc = gimple_or_expr_nonartificial_location (stmt, dest);
bool warned = false;
if (wi::leu_p (lenrng[0], spcrng[1]))
{
if (len != destsize
&& (!si || !is_strlen_related_p (si->ptr, len)))
&& (!si || rawmem || !is_strlen_related_p (si->ptr, len)))
return;
warned = (writefn
@ -3083,7 +3078,12 @@ handle_builtin_stxncpy_strncat (bool append_p, gimple_stmt_iterator *gsi)
tree dst = gimple_call_arg (stmt, 0);
tree src = gimple_call_arg (stmt, 1);
tree len = gimple_call_arg (stmt, 2);
tree dstsize = NULL_TREE, srcsize = NULL_TREE;
/* An upper bound of the size of the destination. */
tree dstsize = NULL_TREE;
/* The length of the destination and source strings (plus 1 for those
whose FULL_STRING_P is set, i.e., whose length is exact rather than
a lower bound). */
tree dstlenp1 = NULL_TREE, srclenp1 = NULL_TREE;;
int didx = get_stridx (dst);
if (strinfo *sidst = didx > 0 ? get_strinfo (didx) : NULL)
@ -3096,11 +3096,16 @@ handle_builtin_stxncpy_strncat (bool append_p, gimple_stmt_iterator *gsi)
{
/* String is known to be nul-terminated. */
tree type = TREE_TYPE (sidst->nonzero_chars);
dstsize = fold_build2 (PLUS_EXPR, type, sidst->nonzero_chars,
dstlenp1 = fold_build2 (PLUS_EXPR, type, sidst->nonzero_chars,
build_int_cst (type, 1));
}
else
dstsize = sidst->nonzero_chars;
dstlenp1 = sidst->nonzero_chars;
}
else if (TREE_CODE (sidst->ptr) == SSA_NAME)
{
gimple *def_stmt = SSA_NAME_DEF_STMT (sidst->ptr);
dstsize = gimple_call_alloc_size (def_stmt);
}
dst = sidst->ptr;
@ -3121,19 +3126,19 @@ handle_builtin_stxncpy_strncat (bool append_p, gimple_stmt_iterator *gsi)
if (sisrc->full_string_p)
{
tree type = TREE_TYPE (sisrc->nonzero_chars);
srcsize = fold_build2 (PLUS_EXPR, type, sisrc->nonzero_chars,
srclenp1 = fold_build2 (PLUS_EXPR, type, sisrc->nonzero_chars,
build_int_cst (type, 1));
}
else
srcsize = sisrc->nonzero_chars;
srclenp1 = sisrc->nonzero_chars;
}
src = sisrc->ptr;
}
else
srcsize = NULL_TREE;
srclenp1 = NULL_TREE;
if (check_bounds_or_overlap (stmt, dst, src, dstsize, srcsize))
if (check_bounds_or_overlap (stmt, dst, src, dstlenp1, srclenp1))
{
gimple_set_no_warning (stmt, true);
return;
@ -3165,9 +3170,10 @@ handle_builtin_stxncpy_strncat (bool append_p, gimple_stmt_iterator *gsi)
/* When -Wstringop-truncation is set, try to determine truncation
before diagnosing possible overflow. Truncation is implied by
the LEN argument being equal to strlen(SRC), regardless of
whether its value is known. Otherwise, issue the more generic
-Wstringop-overflow which triggers for LEN arguments that in
any meaningful way depend on strlen(SRC). */
whether its value is known. Otherwise, when appending, or
when copying into a destination of known size, issue the more
generic -Wstringop-overflow which triggers for LEN arguments
that in any meaningful way depend on strlen(SRC). */
if (!append_p
&& sisrc == silen
&& is_strlen_related_p (src, len)
@ -3176,11 +3182,19 @@ handle_builtin_stxncpy_strncat (bool append_p, gimple_stmt_iterator *gsi)
"copying as many bytes from a string as its length",
stmt, func))
warned = true;
else if (silen && is_strlen_related_p (src, silen->ptr))
warned = warning_at (callloc, OPT_Wstringop_overflow_,
"%G%qD specified bound depends on the length "
"of the source argument",
stmt, func);
else if ((append_p || !dstsize || len == dstlenp1)
&& silen && is_strlen_related_p (src, silen->ptr))
{
/* Issue -Wstringop-overflow when appending or when writing into
a destination of a known size. Otherwise, when copying into
a destination of an unknown size, it's truncation. */
int opt = (append_p || dstsize
? OPT_Wstringop_overflow_ : OPT_Wstringop_truncation);
warned = warning_at (callloc, opt,
"%G%qD specified bound depends on the length "
"of the source argument",
stmt, func);
}
if (warned)
{
location_t strlenloc = pss->second;
@ -3216,7 +3230,7 @@ handle_builtin_memcpy (enum built_in_function bcode, gimple_stmt_iterator *gsi,
if (olddsi != NULL
&& !integer_zerop (len))
{
maybe_warn_overflow (stmt, len, ptr_qry, olddsi, false, false);
maybe_warn_overflow (stmt, len, ptr_qry, olddsi, false, true);
adjust_last_stmt (olddsi, stmt, false, ptr_qry);
}
@ -3684,7 +3698,7 @@ handle_builtin_memset (gimple_stmt_iterator *gsi, bool *zero_write,
tree memset_size = gimple_call_arg (memset_stmt, 2);
/* Check for overflow. */
maybe_warn_overflow (memset_stmt, memset_size, ptr_qry, NULL, false, false);
maybe_warn_overflow (memset_stmt, memset_size, ptr_qry, NULL, false, true);
/* Bail when there is no statement associated with the destination
(the statement may be null even when SI1->ALLOC is not). */