Re: [PATCH] allow more strncat calls with -Wstringop-truncation (PR 85700)
On 05/22/2018 07:40 PM, Martin Sebor wrote: > 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 > > gcc-85700.diff > > > 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. OK for the trunk. Sorry for the delay. I don't see that strong of a case for backporting -- the distro rebuilds with gcc-8 didn't trip over it (at least not in a -Werror build). It doesn't fix a codegen issue and we've only got the one report (unknown what source the report originated with). But you can certainly ask Jakub or Richi. jeff
PING 4 [PATCH] allow more strncat calls with -Wstringop-truncation (PR 85700)
Ping: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01189.html On 06/11/2018 11:34 AM, Martin Sebor wrote: Ping: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01189.html On 06/04/2018 05:45 PM, Martin Sebor wrote: Ping: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01189.html On 05/29/2018 10:19 AM, Martin Sebor wrote: Ping: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01189.html On 05/22/2018 07:40 PM, Martin Sebor wrote: 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
PING 3 [PATCH] allow more strncat calls with -Wstringop-truncation (PR 85700)
Ping: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01189.html On 06/04/2018 05:45 PM, Martin Sebor wrote: Ping: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01189.html On 05/29/2018 10:19 AM, Martin Sebor wrote: Ping: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01189.html On 05/22/2018 07:40 PM, Martin Sebor wrote: 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
PING 2 [PATCH] allow more strncat calls with -Wstringop-truncation (PR 85700)
Ping: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01189.html On 05/29/2018 10:19 AM, Martin Sebor wrote: Ping: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01189.html On 05/22/2018 07:40 PM, Martin Sebor wrote: 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
Re: [PATCH] allow more strncat calls with -Wstringop-truncation (PR 85700)
Ping: https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01189.html On 05/22/2018 07:40 PM, Martin Sebor wrote: 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
[PATCH] allow more strncat calls with -Wstringop-truncation (PR 85700)
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 000..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 (stmt);
+ /*
