On 02/08/2018 07:39 AM, Richard Biener wrote:
On Thu, Feb 8, 2018 at 6:35 AM, Jeff Law <l...@redhat.com> wrote:
On 02/06/2018 05:57 AM, Jakub Jelinek wrote:
On Tue, Feb 06, 2018 at 01:46:21PM +0100, Marek Polacek wrote:
--- gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
+++ gcc/testsuite/c-c++-common/Wstringop-truncation-3.c
@@ -0,0 +1,20 @@
+/* PR tree-optimization/84228 */
+/* { dg-do compile } */
+/* { dg-options "-Wstringop-truncation -O2 -g" } */
+
+char *strncpy (char *, const char *, __SIZE_TYPE__);
+struct S
+{
+ char arr[64];
+};
+
+int
+foo (struct S *p1, const char *a)
+{
+ if (a)
+ goto err;
+ strncpy (p1->arr, a, sizeof p1->arr); /* { dg-bogus "specified bound" } */
Just curious, what debug stmt is in between those?
Wouldn't it be better to force them a couple of debug stmts?
Say
int b = 5, c = 6, d = 7;
at the start of the function and
b = 8; c = 9; d = 10;
in between strncpy and the '\0' store?
+ p1->arr[3] = '\0';
+err:
+ return 0;
+}
diff --git gcc/tree-ssa-strlen.c gcc/tree-ssa-strlen.c
index c3cf432a921..f0f6535017b 100644
--- gcc/tree-ssa-strlen.c
+++ gcc/tree-ssa-strlen.c
@@ -1849,7 +1849,7 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree
src, tree cnt)
/* Look for dst[i] = '\0'; after the stxncpy() call and if found
avoid the truncation warning. */
- gsi_next (&gsi);
+ gsi_next_nondebug (&gsi);
gimple *next_stmt = gsi_stmt (gsi);
if (!gsi_end_p (gsi) && is_gimple_assign (next_stmt))
Ok for trunk, though generally looking at just next stmt is very fragile, might
be
better to look at the strncpy's vuse immediate uses if they are within the
same basic block and either don't alias with it, or are the store it is
looking for, or something similar.
Martin and I wandered down this approach a bit and ultimately decided
against it. While yes it could avoid a false positive by looking at the
immediate uses, but I'm not sure avoiding the false positive in those
cases is actually good!
THe larger the separation between the strcpy and the truncation the more
likely it is that the code is wrong or at least poorly written and
deserves a developer looksie.
But it's the next _GIMPLE_ stmt it looks at. Make it
char d[3];
void f (const char *s, int x)
{
char d[x];
__builtin_strncpy (d, s, sizeof d);
d[x-1] = 0;
}
and I bet it will again warn since x-1 is a separate GIMPLE stmt.
It doesn't but only because it doesn't know how to handle VLAs.
The patch is of course ok but still... simply looking at all
immediate uses of the VDEF of the strncpy call, stopping
at the single stmt with the "next" VDEF should be better
(we don't have a next_vdef () helper).
IIRC, one of the problems I ran into was how to handle code
like this:
void f (const char *s)
{
char d[8];
char *p = strncpy (d, s, sizeof d);
foo (p);
d[7] = 0;
}
I.e., having to track all pointers to d between the call to
strncpy and the assignment of the nul and make sure none of
them ends up used in a string function. It didn't seem
the additional complexity would have been worth the effort
(or the likely false negatives).
Martin
