Here's another small refinement to -Wstringop-truncation to
avoid diagnosing more arguably "safe" cases of strncat() that
match the expected pattern of
strncat (d, s, sizeof d - strlen (d) - 1);
such as
extern char a[4];
strncat (a, "12", sizeof d - strlen (a) - 1);
Since the bound is derived from the length of the destination
as GCC documents is the expected use, the call should probably
not be diagnosed even though truncation is possible.
The trouble with strncat is that it specifies a single count
that can be (and has been) used to specify either the remaining
space in the destination or the maximum number of characters
to append, but not both. It's nearly impossible to tell for
certain which the author meant, and if it's safe, hence all
this fine-tuning. I suspect this isn't the last tweak, either.
In any event, I'd like to commit the patch to both trunk and
gcc-8-branch. The bug isn't marked regression but I suppose
it could (should) well be considered one.
Martin
PR tree-optimization/85700 - Spurious -Wstringop-truncation warning with strncat
gcc/ChangeLog:
PR tree-optimization/85700
* gimple-fold.c (gimple_fold_builtin_strncat): Adjust comment.
* tree-ssa-strlen.c (is_strlen_related_p): Handle integer subtraction.
(maybe_diag_stxncpy_trunc): Distinguish strncat from strncpy.
gcc/testsuite/ChangeLog:
PR tree-optimization/85700
* gcc.dg/Wstringop-truncation-3.c: New test.
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index b45798c..c37abe1 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -2062,10 +2062,12 @@ gimple_fold_builtin_strncat (gimple_stmt_iterator *gsi)
if (!nowarn && cmpsrc == 0)
{
tree fndecl = gimple_call_fndecl (stmt);
-
- /* To avoid certain truncation the specified bound should also
- not be equal to (or less than) the length of the source. */
location_t loc = gimple_location (stmt);
+
+ /* To avoid possible overflow the specified bound should also
+ not be equal to the length of the source, even when the size
+ of the destination is unknown (it's not an uncommon mistake
+ to specify as the bound to strncpy the length of the source). */
if (warning_at (loc, OPT_Wstringop_overflow_,
"%G%qD specified bound %E equals source length",
stmt, fndecl, len))
diff --git a/gcc/testsuite/gcc.dg/Wstringop-truncation-3.c b/gcc/testsuite/gcc.dg/Wstringop-truncation-3.c
new file mode 100644
index 0000000..f394863
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-truncation-3.c
@@ -0,0 +1,63 @@
+/* PR tree-optimization/85700 - Spurious -Wstringop-truncation warning
+ with strncat
+ { dg-do compile }
+ { dg-options "-O2 -Wno-stringop-overflow -Wstringop-truncation -ftrack-macro-expansion=0" } */
+
+#define NOIPA __attribute__ ((noipa))
+#define strncat __builtin_strncat
+#define strlen __builtin_strlen
+
+extern char a4[4], b4[4], ax[];
+
+NOIPA void cat_a4_s1_1 (void)
+{
+ /* There is no truncation here but since the bound of 1 equals
+ the length of the source string it's likely a mistake that
+ could cause overflow so it's diagnosed by -Wstringop-overflow */
+ strncat (a4, "1", 1);
+}
+
+NOIPA void cat_a4_s1_2 (void)
+{
+ strncat (a4, "1", 2);
+}
+
+NOIPA void cat_a4_s1_3 (void)
+{
+ strncat (a4, "1", 3);
+}
+
+NOIPA void cat_a4_s1_4 (void)
+{
+ /* There is no truncation here but since the bound of 1 equals
+ the length of the source string it's likely a mistake that
+ could cause overflow so it's diagnosed by -Wstringop-overflow */
+ strncat (a4, "1", 4);
+}
+
+NOIPA void cat_a4_s1_5 (void)
+{
+ /* A bound in excess of the destination size is diagnosed by
+ -Wstringop-overflow. */
+ strncat (a4, "1", 5);
+}
+
+NOIPA void cat_a4_s1_dlen (void)
+{
+ strncat (a4, "1", sizeof a4 - strlen (a4) - 1);
+}
+
+NOIPA void cat_a4_s2_dlen (void)
+{
+ strncat (a4, "12", sizeof a4 - strlen (a4) - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */
+}
+
+NOIPA void cat_a4_b4_dlen (void)
+{
+ strncat (a4, b4, sizeof a4 - strlen (a4) - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */
+}
+
+NOIPA void cat_ax_b4_dlen (void)
+{
+ strncat (ax, b4, 32 - strlen (ax) - 1); /* { dg-bogus "\\\[-Wstringop-truncation]" } */
+}
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 556c5bc..22a17d6 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -1778,6 +1778,15 @@ is_strlen_related_p (tree src, tree len)
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;
}
@@ -1969,6 +1978,12 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
gcall *call = as_a <gcall *> (stmt);
+ /* Set to true for strncat whose bound is derived from the length
+ of the destination (the expected usage pattern). */
+ bool cat_dstlen_bounded = false;
+ if (DECL_FUNCTION_CODE (func) == BUILT_IN_STRNCAT)
+ cat_dstlen_bounded = is_strlen_related_p (dst, cnt);
+
if (lenrange[0] == cntrange[1] && cntrange[0] == cntrange[1])
return warning_n (callloc, OPT_Wstringop_truncation,
cntrange[0].to_uhwi (),
@@ -1998,7 +2013,8 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
call, func, cntrange[0].to_uhwi (),
cntrange[1].to_uhwi (), lenrange[0].to_uhwi ());
}
- else if (wi::geu_p (lenrange[1], cntrange[1]))
+ else if (!cat_dstlen_bounded
+ && wi::geu_p (lenrange[1], cntrange[1]))
{
/* The longest string is longer than the upper bound of
the count so the truncation is possible. */
@@ -2012,13 +2028,14 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
call, func, cnt, lenrange[1].to_uhwi ());
return warning_at (callloc, OPT_Wstringop_truncation,
- "%G%qD output may be truncated copying between %wu "
- "and %wu bytes from a string of length %wu",
+ "%G%qD output may be truncated copying between "
+ "%wu and %wu bytes from a string of length %wu",
call, func, cntrange[0].to_uhwi (),
cntrange[1].to_uhwi (), lenrange[1].to_uhwi ());
}
- if (cntrange[0] != cntrange[1]
+ if (!cat_dstlen_bounded
+ && cntrange[0] != cntrange[1]
&& wi::leu_p (cntrange[0], lenrange[0])
&& wi::leu_p (cntrange[1], lenrange[0] + 1))
{
