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.
Signed-off-by: Siddhesh Poyarekar <siddh...@gotplt.org>
---
Tested with an x86_64 bootstrap. strncmp has a similar issue, I'll post a
separate patch for it.
gcc/gimple-ssa-warn-access.cc | 35 ++++++++++++++--------
gcc/testsuite/gcc.dg/Wstringop-overread.c | 26 ++++++++--------
gcc/testsuite/gcc.dg/attr-nonstring-4.c | 2 +-
gcc/testsuite/gcc.dg/pr78902.c | 1 -
gcc/testsuite/gcc.dg/warn-strnlen-no-nul.c | 16 +++++-----
5 files changed, 45 insertions(+), 35 deletions(-)
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index c36cd5d45d4..972e80e4b62 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -1256,7 +1256,7 @@ static bool
check_access (GimpleOrTree exp, tree dstwrite,
tree maxread, tree srcstr, tree dstsize,
access_mode mode, const access_data *pad,
- range_query *rvals)
+ range_query *rvals, bool null_terminated)
{
/* The size of the largest object is half the address space, or
PTRDIFF_MAX. (This is way too permissive.) */
@@ -1431,6 +1431,15 @@ check_access (GimpleOrTree exp, tree dstwrite,
}
}
+ /* For functions that take string inputs and stop reading on encountering a
+ NULL, if remaining size in the source is non-zero, it is legitimate for
+ such functions to pass a larger size (that perhaps is the maximum object
+ size of all possible inputs), making the MAXREAD comparison noisy. */
+ if (null_terminated
+ && pad && pad->mode == access_read_only
+ && pad->src.size_remaining () != 0)
+ return true;
+
/* Check the maximum length of the source sequence against the size
of the destination object if known, or against the maximum size
of an object. */
@@ -1522,10 +1531,10 @@ static bool
check_access (gimple *stmt, tree dstwrite,
tree maxread, tree srcstr, tree dstsize,
access_mode mode, const access_data *pad,
- range_query *rvals)
+ range_query *rvals, bool null_terminated = true)
{
return check_access<gimple *> (stmt, dstwrite, maxread, srcstr, dstsize,
- mode, pad, rvals);
+ mode, pad, rvals, null_terminated);
}
bool
@@ -1534,7 +1543,7 @@ check_access (tree expr, tree dstwrite,
access_mode mode, const access_data *pad /* = NULL */)
{
return check_access<tree> (expr, dstwrite, maxread, srcstr, dstsize,
- mode, pad, nullptr);
+ mode, pad, nullptr, true);
}
/* Return true if STMT is a call to an allocation function. Unless
@@ -2109,7 +2118,8 @@ private:
void check_stxncpy (gcall *);
void check_strncmp (gcall *);
void check_memop_access (gimple *, tree, tree, tree);
- void check_read_access (gimple *, tree, tree = NULL_TREE, int = 1);
+ void check_read_access (gimple *, tree, tree = NULL_TREE, int = 1,
+ bool = true);
void maybe_check_dealloc_call (gcall *);
void maybe_check_access_sizes (rdwr_map *, tree, tree, gimple *);
@@ -2745,7 +2755,7 @@ pass_waccess::check_memop_access (gimple *stmt, tree
dest, tree src, tree size)
tree dstsize = compute_objsize (dest, stmt, 0, &data.dst, &m_ptr_qry);
check_access (stmt, size, /*maxread=*/NULL_TREE, srcsize, dstsize,
- data.mode, &data, m_ptr_qry.rvals);
+ data.mode, &data, m_ptr_qry.rvals, false);
}
/* A convenience wrapper for check_access to check access by a read-only
@@ -2754,7 +2764,8 @@ pass_waccess::check_memop_access (gimple *stmt, tree
dest, tree src, tree size)
void
pass_waccess::check_read_access (gimple *stmt, tree src,
tree bound /* = NULL_TREE */,
- int ost /* = 1 */)
+ int ost /* = 1 */,
+ bool null_terminated /* = true */)
{
if (m_early_checks_p || !warn_stringop_overread)
return;
@@ -2770,7 +2781,7 @@ pass_waccess::check_read_access (gimple *stmt, tree src,
compute_objsize (src, stmt, ost, &data.src, &m_ptr_qry);
check_access (stmt, /*dstwrite=*/ NULL_TREE, /*maxread=*/ bound,
/*srcstr=*/ src, /*dstsize=*/ NULL_TREE, data.mode,
- &data, m_ptr_qry.rvals);
+ &data, m_ptr_qry.rvals, null_terminated);
}
/* Return true if memory model ORD is constant in the context of STMT and
@@ -3230,8 +3241,8 @@ pass_waccess::check_builtin (gcall *stmt)
tree a1 = call_arg (stmt, 0);
tree a2 = call_arg (stmt, 1);
tree len = call_arg (stmt, 2);
- check_read_access (stmt, a1, len, 0);
- check_read_access (stmt, a2, len, 0);
+ check_read_access (stmt, a1, len, 0, false);
+ check_read_access (stmt, a2, len, 0, false);
return true;
}
@@ -3250,7 +3261,7 @@ pass_waccess::check_builtin (gcall *stmt)
{
tree src = call_arg (stmt, 0);
tree len = call_arg (stmt, 2);
- check_read_access (stmt, src, len, 0);
+ check_read_access (stmt, src, len, 0, false);
return true;
}
@@ -3546,7 +3557,7 @@ pass_waccess::maybe_check_access_sizes (rdwr_map *rwm,
tree fndecl, tree fntype,
if (mode == access_deferred)
mode = TYPE_READONLY (argtype) ? access_read_only : access_read_write;
check_access (stmt, access_size, /*maxread=*/ NULL_TREE, srcsize,
- dstsize, mode, &data, m_ptr_qry.rvals);
+ dstsize, mode, &data, m_ptr_qry.rvals, false);
if (warning_suppressed_p (stmt, OPT_Wstringop_overflow_))
opt_warned = OPT_Wstringop_overflow_;
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overread.c
b/gcc/testsuite/gcc.dg/Wstringop-overread.c
index 7db74029819..709e7fff63e 100644
--- a/gcc/testsuite/gcc.dg/Wstringop-overread.c
+++ b/gcc/testsuite/gcc.dg/Wstringop-overread.c
@@ -313,7 +313,7 @@ void test_strnlen_array (int i, int i0, unsigned n)
T (strnlen (a1, 0));
T (strnlen (a1, 1));
- T (strnlen (a1, 2)); // { dg-warning "'strnlen' specified bound 2
exceeds source size 1" "pr87492" { xfail *-*-* } }
+ T (strnlen (a1, 2));
T (strnlen (a1, n));
T (strnlen (a1 + 1, 0));
@@ -323,16 +323,16 @@ void test_strnlen_array (int i, int i0, unsigned n)
T (strnlen (a1 + 1, n));
T (strnlen (a1 + i, 0));
T (strnlen (a1 + i, 1));
- T (strnlen (a1 + i, 2)); // { dg-warning "'strnlen' specified bound 2
exceeds source size 1" }
+ T (strnlen (a1 + i, 2));
T (strnlen (a1 + i, n));
T (strnlen (a1 + i + 1, 0));
T (strnlen (a1 + i + 1, 1));
- T (strnlen (a1 + i + 1, 2)); // { dg-warning "'strnlen' specified bound 2
exceeds source size 1" }
+ T (strnlen (a1 + i + 1, 2));
T (strnlen (a1 + i + 1, n));
T (strnlen (a1 + i0, 0));
T (strnlen (a1 + i0, 1));
- T (strnlen (a1 + i0, 2)); // { dg-warning "'strnlen' specified bound 2
exceeds source size 1" }
+ T (strnlen (a1 + i0, 2));
T (strnlen (a1 + i0, n));
T (strnlen (a1 + i0 + 1, 0));
T (strnlen (a1 + i0 + 1, 1)); // { dg-warning "'strnlen' specified bound 1
exceeds source size 0" }
@@ -344,7 +344,7 @@ void test_strnlen_array (int i, int i0, unsigned n)
T (strnlen (a2, n));
T (strnlen (a2 + 1, 0));
T (strnlen (a2 + 1, 1));
- T (strnlen (a2 + 1, 2)); // { dg-warning "'strnlen' specified bound 2
exceeds source size 1" "pr87492" }
+ T (strnlen (a2 + 1, 2));
T (strnlen (a2 + 1, n));
T (strnlen (a2 + 2, 0));
T (strnlen (a2 + 2, 1)); // { dg-warning "'strnlen' specified bound 1
exceeds source size 0" }
@@ -360,12 +360,12 @@ void test_strnlen_array (int i, int i0, unsigned n)
T (strnlen (a2 + i0, 0));
T (strnlen (a2 + i0, 1));
T (strnlen (a2 + i0, 2));
- T (strnlen (a2 + i0, 3)); // { dg-warning "'strnlen' specified bound 3
exceeds source size 2" }
+ T (strnlen (a2 + i0, 3));
T (strnlen (a2 + i0, n));
T (strnlen (a2 + i0 + 1, 0));
T (strnlen (a2 + i0 + 1, 1));
- T (strnlen (a2 + i0 + 1, 2)); // { dg-warning "'strnlen' specified bound 2
exceeds source size 1" }
+ T (strnlen (a2 + i0 + 1, 2));
T (strnlen (a2 + i0 + 1, n));
T (strnlen (a2 + i0 + 2, 0));
@@ -509,7 +509,7 @@ void test_strndup_array (int i, int i0, unsigned n)
T (strndup (a1, 0));
T (strndup (a1, 1));
- T (strndup (a1, 2)); // { dg-warning "'strndup' specified bound 2
exceeds source size 1" }
+ T (strndup (a1, 2));
T (strndup (a1, n));
T (strndup (a1 + 1, 0));
T (strndup (a1 + 1, 1)); // { dg-warning "'strndup' specified bound 1
exceeds source size 0" }
@@ -518,11 +518,11 @@ void test_strndup_array (int i, int i0, unsigned n)
T (strndup (a1 + 1, n));
T (strndup (a1 + i, 0));
T (strndup (a1 + i, 1));
- T (strndup (a1 + i, 2)); // { dg-warning "'strndup' specified bound 2
exceeds source size 1" }
+ T (strndup (a1 + i, 2));
T (strndup (a1 + i, n));
T (strndup (a1 + i + 1, 0));
T (strndup (a1 + i + 1, 1));
- T (strndup (a1 + i + 1, 2)); // { dg-warning "'strndup' specified bound 2
exceeds source size 1" }
+ T (strndup (a1 + i + 1, 2));
T (strndup (a1 + i + 1, n));
T (strndup (a1 + i0, 0));
@@ -538,7 +538,7 @@ void test_strndup_array (int i, int i0, unsigned n)
T (strndup (a2, n));
T (strndup (a2 + 1, 0));
T (strndup (a2 + 1, 1));
- T (strndup (a2 + 1, 2)); // { dg-warning "'strndup' specified bound 2
exceeds source size 1" }
+ T (strndup (a2 + 1, 2));
T (strndup (a2 + 1, n));
T (strndup (a2 + 2, 0));
T (strndup (a2 + 2, 1)); // { dg-warning "'strndup' specified bound 1
exceeds source size 0" }
@@ -554,12 +554,12 @@ void test_strndup_array (int i, int i0, unsigned n)
T (strndup (a2 + i0, 0));
T (strndup (a2 + i0, 1));
T (strndup (a2 + i0, 2));
- T (strndup (a2 + i0, 3)); // { dg-warning "'strndup' specified bound 3
exceeds source size 2" }
+ T (strndup (a2 + i0, 3));
T (strndup (a2 + i0, n));
T (strndup (a2 + i0 + 1, 0));
T (strndup (a2 + i0 + 1, 1));
- T (strndup (a2 + i0 + 1, 2)); // { dg-warning "'strndup' specified bound 2
exceeds source size 1" }
+ T (strndup (a2 + i0 + 1, 2));
T (strndup (a2 + i0 + 1, n));
T (strndup (a2 + i0 + 2, 0));
diff --git a/gcc/testsuite/gcc.dg/attr-nonstring-4.c
b/gcc/testsuite/gcc.dg/attr-nonstring-4.c
index 6f03a562026..0ce062e513b 100644
--- a/gcc/testsuite/gcc.dg/attr-nonstring-4.c
+++ b/gcc/testsuite/gcc.dg/attr-nonstring-4.c
@@ -53,7 +53,7 @@ void strnlen_range (void)
T (STR, /* [] */, n);
T (STR, /* [] */, n + 1); /* { dg-warning "specified bound \\\[\[0-9\]+,
\[0-9\]+] exceeds maximum object size \[0-9\]+" } */
- T (STR, 1, n); /* { dg-warning "specified bound \\\[\[0-9\]+,
\[0-9\]+] exceeds source size 1" } */
+ T (STR, 1, n);
T (STR, 2, n + 1); /* { dg-warning "specified bound \\\[\[0-9\]+,
\[0-9\]+] exceeds maximum object size \[0-9\]+" } */
T (NS, /* [] */, n);
diff --git a/gcc/testsuite/gcc.dg/pr78902.c b/gcc/testsuite/gcc.dg/pr78902.c
index a9a617cde23..51b42548071 100644
--- a/gcc/testsuite/gcc.dg/pr78902.c
+++ b/gcc/testsuite/gcc.dg/pr78902.c
@@ -12,5 +12,4 @@ void foo(void)
__builtin_aligned_alloc (10, 16); /* { dg-warning "ignoring return value of
'__builtin_aligned_alloc' declared with attribute 'warn_unused_result'" } */
__builtin_strdup ("pes"); /* { dg-warning "ignoring return value of
'__builtin_strdup' declared with attribute 'warn_unused_result'" } */
__builtin_strndup ("pes", 10); /* { dg-warning "ignoring return value of
'__builtin_strndup' declared with attribute 'warn_unused_result'" } */
- /* { dg-warning "\\\[-Wstringop-overread" "strndup excessive bound" { target
*-*-* } .-1 } */
}
diff --git a/gcc/testsuite/gcc.dg/warn-strnlen-no-nul.c
b/gcc/testsuite/gcc.dg/warn-strnlen-no-nul.c
index 70f6a432b97..b9ee69c0a2a 100644
--- a/gcc/testsuite/gcc.dg/warn-strnlen-no-nul.c
+++ b/gcc/testsuite/gcc.dg/warn-strnlen-no-nul.c
@@ -147,11 +147,11 @@ T (v0 ? b[3] : "", bsz);
the strnlen calls are safe because the reads are bounded by
the length of the constant arguments. Most of the calls are
not diagnosed anymore as a result of the fix for PR 103215. */
-T (v0 ? "" : b[0], bsz + 1); /* { dg-warning "bound 6 exceeds source size
5" } */
+T (v0 ? "" : b[0], bsz + 1);
T (v0 ? "" : b[1], bsz + 1);
T (v0 ? "" : b[2], bsz + 1);
T (v0 ? "" : b[3], bsz + 1);
-T (v0 ? b[0] : "", bsz + 1); /* { dg-warning "bound 6 exceeds source size
5" } */
+T (v0 ? b[0] : "", bsz + 1);
T (v0 ? b[1] : "", bsz + 1);
T (v0 ? b[2] : "", bsz + 1);
T (v0 ? b[3] : "", bsz + 1);
@@ -218,10 +218,10 @@ T (&s.a[i1] + v0, asz);
T (s.a, asz + 1);
T (&s.a[0], asz + 1);
T (&s.a[0] + 1, asz + 1);
-T (&s.a[0] + v0, asz + 1); /* { dg-warning "specified bound 6 exceeds
source size 5 " } */
+T (&s.a[0] + v0, asz + 1);
T (&s.a[1], asz + 1);
T (&s.a[1] + 1, asz + 1);
-T (&s.a[1] + v0, asz + 1); /* { dg-bogus "specified bound 6 exceeds
source size 5" "pr95794" { xfail *-*-* } } */
+T (&s.a[1] + v0, asz + 1);
T (&s.a[i0], asz + 1);
T (&s.a[i0] + i1, asz + 1);
@@ -272,10 +272,10 @@ const struct B ba[] = {
T (ba[0].a[0].a, asz + 1);
T (&ba[0].a[0].a[0], asz + 1);
T (&ba[0].a[0].a[0] + 1, asz + 1);
-T (&ba[0].a[0].a[0] + v0, asz + 1); /* { dg-bogus "specified bound 6 exceeds
source size 5" pr95794" { xfail *-*-* } } */
+T (&ba[0].a[0].a[0] + v0, asz + 1);
T (&ba[0].a[0].a[1], asz + 1);
T (&ba[0].a[0].a[1] + 1, asz + 1);
-T (&ba[0].a[0].a[1] + v0, asz + 1); /* { dg-bogus "specified bound 6 exceeds
source size 5" pr95794" { xfail *-*-* } } */
+T (&ba[0].a[0].a[1] + v0, asz + 1);
T (ba[0].a[0].b, bsz);
T (&ba[0].a[0].b[0], bsz);
@@ -308,10 +308,10 @@ T (&ba[0].a[1].a[1] + v0, asz + 1); /* { dg-warning
"unterminated" } */
T (ba[0].a[1].b, bsz + 1);
T (&ba[0].a[1].b[0], bsz + 1);
T (&ba[0].a[1].b[0] + 1, bsz + 1);
-T (&ba[0].a[1].b[0] + v0, bsz + 1); /* { dg-bogus "specified bound 6 exceeds
source size 5" pr95794" { xfail *-*-* } } */
+T (&ba[0].a[1].b[0] + v0, bsz + 1);
T (&ba[0].a[1].b[1], bsz + 1);
T (&ba[0].a[1].b[1] + 1, bsz + 1);
-T (&ba[0].a[1].b[1] + v0, bsz + 1); /* { dg-bogus "specified bound 6 exceeds
source size 5" pr95794" { xfail *-*-* } } */
+T (&ba[0].a[1].b[1] + v0, bsz + 1);
T (ba[1].a[0].a, asz);
T (&ba[1].a[0].a[0], asz);
--
2.35.1
