This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PING] [PATCH] refine -Wstringop-truncation and -Wsizeof-pointer-memaccess for strncat of nonstrings (PR 85602)
To make review and testing easier (thank you, Franz), attached
is an updated patch rebased on top of today's trunk.
(Note that the patch intentionally doesn't suppress the warning
for the submitted test case without adding the nonstring attribute.)
On 05/25/2018 02:59 PM, Martin Sebor wrote:
Ping:
https://gcc.gnu.org/ml/gcc-patches/2018-05/msg00869.html
On 05/17/2018 08:01 PM, Martin Sebor wrote:
The -Wstringop-truncation and -Wsizeof-pointer-memaccess warnings
I added and enhanced, respectively, in GCC 8 are arguably overly
strict for source arguments declared with the nonstring attribute.
For example, -Wsizeof-pointer-memaccess triggers for the strncat
call below:
__attribute__ ((nonstring)) char nonstr[8];
extern char *d;
strncat (d, nonstr, sizeof nonstr);
even though it's still a fairly common (if unsafe) idiom from
the early UNIX days (V7 from 1979 to be exact) where strncat
was introduced. (This use case, modulo the attribute, was
reduced from coreutils.)
Simialrly, -Wstringop-truncation warns for some strcat calls that
are actually safe, such as in:
strcpy (nonstr, "123");
strncat (d, nonstr, 32);
To help with the adoption of the warnings and the attribute and
avoid unnecessary churn the attached patch relaxes both warnings
to accept code like this without diagnostics.
The patch doesn't add any new warnings so I'd like it considered
for GCC 8 in addition to trunk.
Thanks
Martin
PR middle-end/85602 - -Wsizeof-pointer-memaccess for strncat with size of source
gcc/c-family/ChangeLog:
PR middle-end/85602
* c-warn.c (sizeof_pointer_memaccess_warning): Check for attribute
nonstring.
gcc/ChangeLog:
PR middle-end/85602
* calls.c (maybe_warn_nonstring_arg): Handle strncat.
* tree-ssa-strlen.c (is_strlen_related_p): Make extern.
Handle integer subtraction.
(maybe_diag_stxncpy_trunc): Handle nonstring source arguments.
* tree-ssa-strlen.h (is_strlen_related_p): Declare.
gcc/testsuite/ChangeLog:
PR middle-end/85602
* c-c++-common/attr-nonstring-8.c: New test.
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index e7bcbb1..53cd827 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see
#include "gcc-rich-location.h"
#include "gimplify.h"
#include "c-family/c-indentation.h"
+#include "calls.h"
/* Print a warning if a constant expression had overflow in folding.
Invoke this function on every expression that the language
@@ -798,7 +799,12 @@ sizeof_pointer_memaccess_warning (location_t *sizeof_arg_loc, tree callee,
tem = tree_strip_nop_conversions (src);
if (TREE_CODE (tem) == ADDR_EXPR)
tem = TREE_OPERAND (tem, 0);
- if (operand_equal_p (tem, sizeof_arg[idx], OEP_ADDRESS_OF))
+
+ /* Avoid diagnosing sizeof SRC when SRC is declared with
+ attribute nonstring. */
+ tree dummy;
+ if (operand_equal_p (tem, sizeof_arg[idx], OEP_ADDRESS_OF)
+ && !get_attr_nonstring_decl (tem, &dummy))
warning_at (sizeof_arg_loc[idx], OPT_Wsizeof_pointer_memaccess,
"argument to %<sizeof%> in %qD call is the same "
"expression as the source; did you mean to use "
diff --git a/gcc/calls.c b/gcc/calls.c
index 1f2cde6..8b86c8a 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -50,6 +50,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-chkp.h"
#include "tree-vrp.h"
#include "tree-ssanames.h"
+#include "tree-ssa-strlen.h"
#include "rtl-chkp.h"
#include "intl.h"
#include "stringpool.h"
@@ -1623,8 +1624,10 @@ maybe_warn_nonstring_arg (tree fndecl, tree exp)
/* It's safe to call "bounded" string functions with a non-string
argument since the functions provide an explicit bound for this
- purpose. */
- switch (DECL_FUNCTION_CODE (fndecl))
+ purpose. The exception is strncat where the bound may refer to
+ either the destination or the source. */
+ int fncode = DECL_FUNCTION_CODE (fndecl);
+ switch (fncode)
{
case BUILT_IN_STRCMP:
case BUILT_IN_STRNCMP:
@@ -1645,6 +1648,7 @@ maybe_warn_nonstring_arg (tree fndecl, tree exp)
}
/* Fall through. */
+ case BUILT_IN_STRNCAT:
case BUILT_IN_STPNCPY:
case BUILT_IN_STPNCPY_CHK:
case BUILT_IN_STRNCPY:
@@ -1743,15 +1747,36 @@ maybe_warn_nonstring_arg (tree fndecl, tree exp)
if (!decl)
continue;
- tree type = TREE_TYPE (decl);
-
/* The maximum number of array elements accessed. */
offset_int wibnd = 0;
- if (bndrng[0])
+
+ if (argno && fncode == BUILT_IN_STRNCAT)
+ {
+ /* See if the bound in strncat is derived from the length
+ of the strlen of the destination (as it's expected to be).
+ If so, reset BOUND and FNCODE to trigger a warning. */
+ tree dstarg = CALL_EXPR_ARG (exp, 0);
+ if (is_strlen_related_p (dstarg, bound))
+ {
+ /* The bound applies to the destination, not to the source,
+ so reset these to trigger a warning without mentioning
+ the bound. */
+ bound = NULL;
+ fncode = 0;
+ }
+ else if (bndrng[1])
+ /* Use the upper bound of the range for strncat. */
+ wibnd = wi::to_offset (bndrng[1]);
+ }
+ else if (bndrng[0])
+ /* Use the lower bound of the range for functions other than
+ strncat. */
wibnd = wi::to_offset (bndrng[0]);
- /* Size of the array. */
+ /* Determine the size of the argument array if it is one. */
offset_int asize = wibnd;
+ bool known_size = false;
+ tree type = TREE_TYPE (decl);
/* Determine the array size. For arrays of unknown bound and
pointers reset BOUND to trigger the appropriate warning. */
@@ -1760,7 +1785,10 @@ maybe_warn_nonstring_arg (tree fndecl, tree exp)
if (tree arrbnd = TYPE_DOMAIN (type))
{
if ((arrbnd = TYPE_MAX_VALUE (arrbnd)))
- asize = wi::to_offset (arrbnd) + 1;
+ {
+ asize = wi::to_offset (arrbnd) + 1;
+ known_size = true;
+ }
}
else if (bound == void_type_node)
bound = NULL_TREE;
@@ -1768,15 +1796,49 @@ maybe_warn_nonstring_arg (tree fndecl, tree exp)
else if (bound == void_type_node)
bound = NULL_TREE;
+ /* In a call to strncat with a bound in a range whose lower but
+ not upper bound is less than the array size, reset ASIZE to
+ be the same as the bound and the other variable to trigger
+ the apprpriate warning below. */
+ if (fncode == BUILT_IN_STRNCAT
+ && bndrng[0] != bndrng[1]
+ && wi::ltu_p (wi::to_offset (bndrng[0]), asize)
+ && (!known_size
+ || wi::ltu_p (asize, wibnd)))
+ {
+ asize = wibnd;
+ bound = NULL_TREE;
+ fncode = 0;
+ }
+
location_t loc = EXPR_LOCATION (exp);
bool warned = false;
if (wi::ltu_p (asize, wibnd))
- warned = warning_at (loc, OPT_Wstringop_overflow_,
- "%qD argument %i declared attribute %<nonstring%> "
- "is smaller than the specified bound %E",
- fndecl, argno + 1, bndrng[0]);
+ {
+ if (bndrng[0] == bndrng[1])
+ warned = warning_at (loc, OPT_Wstringop_overflow_,
+ "%qD argument %i declared attribute "
+ "%<nonstring%> is smaller than the specified "
+ "bound %wu",
+ fndecl, argno + 1, wibnd.to_uhwi ());
+ else if (wi::ltu_p (asize, wi::to_offset (bndrng[0])))
+ warned = warning_at (loc, OPT_Wstringop_overflow_,
+ "%qD argument %i declared attribute "
+ "%<nonstring%> is smaller than "
+ "the specified bound [%E, %E]",
+ fndecl, argno + 1, bndrng[0], bndrng[1]);
+ else
+ warned = warning_at (loc, OPT_Wstringop_overflow_,
+ "%qD argument %i declared attribute "
+ "%<nonstring%> may be smaller than "
+ "the specified bound [%E, %E]",
+ fndecl, argno + 1, bndrng[0], bndrng[1]);
+ }
+ else if (fncode == BUILT_IN_STRNCAT)
+ ; /* Avoid warning for calls to strncat() when the bound
+ is equal to the size of the non-string argument. */
else if (!bound)
warned = warning_at (loc, OPT_Wstringop_overflow_,
"%qD argument %i declared attribute %<nonstring%>",
diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-8.c b/gcc/testsuite/c-c++-common/attr-nonstring-8.c
new file mode 100644
index 0000000..15b68ed
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-nonstring-8.c
@@ -0,0 +1,147 @@
+/* PR middle-end/85602 - -Wsizeof-pointer-memaccess for strncat with size
+ of source
+ { dg-do compile }
+ { dg-options "-O2 -Wno-array-bounds -Wsizeof-pointer-memaccess -Wstringop-truncation -ftrack-macro-expansion=0" } */
+
+#include "../gcc.dg/range.h"
+
+typedef __SIZE_TYPE__ size_t;
+
+#if __cplusplus
+extern "C" {
+#endif
+
+char* strcpy (char*, const char*);
+size_t strlen (const char*);
+char* strncat (char*, const char*, __SIZE_TYPE__);
+char* strncpy (char*, const char*, __SIZE_TYPE__);
+
+#if __cplusplus
+}
+#endif
+
+#define NONSTR __attribute__ ((nonstring))
+
+NONSTR char nd3[3];
+NONSTR char nd4[4];
+NONSTR char nd5[5];
+
+NONSTR char ns3[3];
+NONSTR char ns4[4];
+NONSTR char ns5[5];
+
+NONSTR char* pns;
+
+void sink (void*, ...);
+
+#define T(call) sink (call)
+
+/* Verify that for a nonstring source array of an unknown length
+ a warning is issued only when the bound exceeds the array size. */
+
+void test_strncat_nonstring_cst (char *d)
+{
+ T (strncat (d, ns3, 1));
+ T (strncat (d, ns3, 2));
+ T (strncat (d, ns3, 3));
+ T (strncat (d, ns3, sizeof ns3));
+ T (strncat (d, ns3, 4)); /* { dg-warning "argument 2 declared attribute .nonstring. is smaller than the specified bound 4" } */
+
+ T (strncat (d, ns4, 1));
+ T (strncat (d, ns4, 2));
+ T (strncat (d, ns4, 3));
+ T (strncat (d, ns4, 4));
+ T (strncat (d, ns4, sizeof ns4));
+ T (strncat (d, ns4, 5)); /* { dg-warning "argument 2 declared attribute .nonstring. is smaller than the specified bound 5" } */
+
+ T (strncat (nd3, ns3, 1));
+ T (strncat (nd3, ns3, 2));
+ T (strncat (nd3, ns3, 3)); /* { dg-warning "specified bound 3 equals destination size" } */
+ T (strncat (nd3, ns3, 4)); /* { dg-warning "argument 2 declared attribute .nonstring. is smaller than the specified bound 4" } */
+ /* { dg-warning "specified bound 4 exceeds destination size 3" "" { target *-*-* } .-1 } */
+
+ T (strncat (d, pns, sizeof pns)); /* { dg-warning "argument to .sizeof. in .strncat. call is the same expression as the source" } */
+}
+
+
+void test_strncat_nonstring_var (char *d, size_t n)
+{
+ /* In the following the bound coulld apply to either the destination
+ or the source. The expected use of strncat() is to pass it as
+ the bound DSIZE - strlen(D) - 1 so the call below is diagnosed. */
+ T (strncat (d, ns3, n)); /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+ T (strncat (d, ns3, UR (0, 1)));
+ T (strncat (d, ns3, UR (1, 2)));
+ T (strncat (d, ns3, UR (2, 3)));
+ T (strncat (d, ns3, UR (3, 4))); /* { dg-warning "argument 2 declared attribute 'nonstring' may be smaller than the specified bound \\\[3, 4]" } */
+ T (strncat (d, ns3, UR (4, 5))); /* { dg-warning "argument 2 declared attribute 'nonstring' is smaller than the specified bound \\\[4, 5]" } */
+
+ /* Verify that the call below (the intended use of strncat()) is
+ also diagnosed. */
+ T (strncat (d, ns3, 256 - strlen (d) - 1)); /* { dg-warning "argument 2 declared attribute .nonstring." } */
+
+ T (strncat (nd3, ns5, UR (0, 1)));
+ T (strncat (nd3, ns5, UR (1, 2)));
+ T (strncat (nd3, ns5, UR (2, 3)));
+ T (strncat (nd3, ns5, UR (3, 4)));
+ T (strncat (nd3, ns5, UR (4, 5))); /* { dg-warning "specified bound between 4 and 5 exceeds destination size 3" } */
+
+ T (strncat (nd5, ns3, UR (0, 1)));
+ T (strncat (nd5, ns3, UR (1, 2)));
+ T (strncat (nd5, ns3, UR (2, 3)));
+ T (strncat (nd5, ns3, UR (3, 4))); /* { dg-warning "argument 2 declared attribute 'nonstring' may be smaller than the specified bound \\\[3, 4]" } */
+}
+
+/* Verify that for a nonstring source array of a known length (i.e.,
+ a nonstring array containing a nul-terminated string) a warning
+ is issued only for certain truncation.
+
+ The test cases are split up to work around bug 81343 (or one like
+ it). */
+
+void test_strncat_string_1_1 (char *d)
+{
+ strcpy (ns3, "1");
+ T (strncat (d, ns3, 1)); /* { dg-warning "output truncated before terminating nul copying 1 byte from a string of the same length" } */
+}
+
+void test_strncat_string_1_2 (char *d)
+{
+ strcpy (ns3, "1");
+ T (strncat (d, ns3, 2));
+}
+
+void test_strncat_string_1_3 (char *d)
+{
+ strcpy (ns3, "1");
+ T (strncat (d, ns3, 3));
+}
+
+void test_strncat_string_2_1 (char *d)
+{
+ strcpy (ns3, "12");
+ T (strncat (d, ns3, 1)); /* { dg-warning "output truncated copying 1 byte from a string of length 2" } */
+}
+
+void test_strncat_string_2_2 (char *d)
+{
+ strcpy (ns3, "12");
+ T (strncat (d, ns3, 2)); /* { dg-warning "output truncated before terminating nul copying 2 bytes from a string of the same length" } */
+}
+
+void test_strncat_string_2_3 (char *d)
+{
+ strcpy (ns3, "12");
+ T (strncat (d, ns3, 3));
+}
+
+
+void test_strcncpy_nonstring_cst (char *d)
+{
+ T (strncpy (d, ns3, 1));
+ T (strncpy (d, ns3, 2));
+ T (strncpy (d, ns3, 3));
+ T (strncpy (d, ns3, sizeof ns3));
+ T (strncpy (d, ns3, 4)); /* { dg-warning "argument 2 declared attribute .nonstring. is smaller than the specified bound 4" } */
+}
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 556c5bc..34e4a0b 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -1735,7 +1735,7 @@ handle_builtin_strncat (built_in_function bcode, gimple_stmt_iterator *gsi)
assumed to be the argument in some call to strlen() whose relationship
to SRC is being ascertained. */
-static bool
+bool
is_strlen_related_p (tree src, tree len)
{
if (TREE_CODE (TREE_TYPE (len)) == POINTER_TYPE
@@ -1772,12 +1772,20 @@ is_strlen_related_p (tree src, tree len)
&& (code == BIT_AND_EXPR
|| code == NOP_EXPR)))
{
- /* Pointer plus (an integer) and integer cast or truncation are
- considered among the (potentially) related expressions to strlen.
- Others are not. */
+ /* Pointer plus (an integer), and truncation are considered among
+ the (potentially) related expressions to strlen. */
return is_strlen_related_p (src, rhs1);
}
+ if (tree rhs2 = gimple_assign_rhs2 (def_stmt))
+ {
+ /* Integer subtraction is considered strlen-related when both
+ arguments are integers and second one is strlen-related. */
+ rhstype = TREE_TYPE (rhs2);
+ if (INTEGRAL_TYPE_P (rhstype) && code == MINUS_EXPR)
+ return is_strlen_related_p (src, rhs2);
+ }
+
return false;
}
@@ -1848,9 +1856,23 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
if (TREE_CODE (dstdecl) == ADDR_EXPR)
dstdecl = TREE_OPERAND (dstdecl, 0);
- /* If the destination refers to a an array/pointer declared nonstring
- return early. */
tree ref = NULL_TREE;
+
+ if (!sidx)
+ {
+ /* If the source is a non-string return early to avoid warning
+ for possible truncation (if the truncation is certain SIDX
+ is non-zero). */
+ bool with_bounds = gimple_call_with_bounds_p (stmt);
+ tree srcdecl = gimple_call_arg (stmt, with_bounds ? 2 : 1);
+ if (TREE_CODE (srcdecl) == ADDR_EXPR)
+ srcdecl = TREE_OPERAND (srcdecl, 0);
+ if (get_attr_nonstring_decl (srcdecl, &ref))
+ return false;
+ }
+
+ /* Likewise, if the destination refers to a an array/pointer declared
+ nonstring return early. */
if (get_attr_nonstring_decl (dstdecl, &ref))
return false;
diff --git a/gcc/tree-ssa-strlen.h b/gcc/tree-ssa-strlen.h
index 1399a78..f427fb7 100644
--- a/gcc/tree-ssa-strlen.h
+++ b/gcc/tree-ssa-strlen.h
@@ -21,6 +21,7 @@
#ifndef GCC_TREE_SSA_STRLEN_H
#define GCC_TREE_SSA_STRLEN_H
+extern bool is_strlen_related_p (tree, tree);
extern bool maybe_diag_stxncpy_trunc (gimple_stmt_iterator, tree, tree);
#endif // GCC_TREE_SSA_STRLEN_H