While testing the patch for PR 92226 I posted earlier today
I ran into a few cases where I expected the strlen range
optimization to take place but it didn't.
In other instances this wouldn't be surprising because
the optimization was only introduced for multi-character stores
and with the expectation that it would be slowly extended to
other functions over time. But these cases were among those
the optimization was meant to be in place for, so its absence
is an oversight. The attached near-trivial patch fills this
gap.
As with all these changes, enabling the optimization also makes
it possible to detect more instances of buffer overflow.
Tested on x86_64-linux.
Martin
PS There are quite a few remaining opportunities to make use of
the strlen ranges in the pass. Rather than enhancing the whole
pass in one go I think it will be safer to do it one small step
at a time, using little patchlets like in the attachment.
gcc/ChangeLog:
* tree-ssa-strlen.c (get_addr_stridx): Add argument and use it.
(handle_store): Pass argument to get_addr_stridx.
gcc/testsuite/ChangeLog:
* gcc.dg/strlenopt-89.c: New test.
* gcc.dg/strlenopt-90.c: New test.
* gcc.dg/Wstringop-overflow-20.c: New test.
Index: gcc/tree-ssa-strlen.c
===================================================================
--- gcc/tree-ssa-strlen.c (revision 277521)
+++ gcc/tree-ssa-strlen.c (working copy)
@@ -272,7 +280,8 @@ get_next_strinfo (strinfo *si)
*OFFSET_OUT. */
static int
-get_addr_stridx (tree exp, tree ptr, unsigned HOST_WIDE_INT *offset_out)
+get_addr_stridx (tree exp, tree ptr, unsigned HOST_WIDE_INT *offset_out,
+ const vr_values *rvals = NULL)
{
HOST_WIDE_INT off;
struct stridxlist *list, *last = NULL;
@@ -310,7 +319,7 @@ static int
unsigned HOST_WIDE_INT rel_off
= (unsigned HOST_WIDE_INT) off - last->offset;
strinfo *si = get_strinfo (last->idx);
- if (si && compare_nonzero_chars (si, rel_off) >= 0)
+ if (si && compare_nonzero_chars (si, rel_off, rvals) >= 0)
{
if (offset_out)
{
@@ -4319,7 +4328,7 @@ handle_store (gimple_stmt_iterator *gsi, bool *zer
}
else
{
- idx = get_addr_stridx (lhs, NULL_TREE, &offset);
+ idx = get_addr_stridx (lhs, NULL_TREE, &offset, rvals);
if (idx > 0)
si = get_strinfo (idx);
}
Index: gcc/testsuite/gcc.dg/strlenopt-89.c
===================================================================
--- gcc/testsuite/gcc.dg/strlenopt-89.c (nonexistent)
+++ gcc/testsuite/gcc.dg/strlenopt-89.c (working copy)
@@ -0,0 +1,89 @@
+/* PR tree-optimization/92226 - live nul char store to array eliminated
+ { dg-do compile }
+ { dg-options "-O2 -fdump-tree-strlen" } */
+
+#include "strlenopt.h"
+
+#define NOIPA __attribute__ ((noipa))
+
+/* Verify that the nul store into the destination is only eliminated
+ when overwrites the existing terminating nul added by the strcpy call.
+ Also verify that the second strlen call is eliminated in all cases. */
+#define T(SIZE, IDX) \
+ NOIPA void test_ ## SIZE ## _store_nul_ ## IDX (const char *s) \
+ { \
+ extern char a ## SIZE[SIZE]; \
+ char *d = a ## SIZE; \
+ size_t len = SIZE - 1; \
+ size_t idx = IDX; \
+ if (strlen (s) == len) \
+ { \
+ strcpy (d, s); \
+ d[idx] = 0; \
+ if (strlen (d) != idx) \
+ abort (); \
+ } \
+ } typedef void dummy_type
+
+
+T (1, 0); // expect nul store to be eliminated
+
+T (2, 0); // nul store must be retained
+T (2, 1); // expect nul store to be eliminated
+
+// Same as above but for larger arrays.
+T (3, 0);
+T (3, 1);
+T (3, 2);
+
+T (4, 0);
+T (4, 1);
+T (4, 2);
+T (4, 3);
+
+T (5, 0);
+T (5, 1);
+T (5, 2);
+T (5, 3);
+T (5, 4);
+
+T (6, 0);
+T (6, 1);
+T (6, 2);
+T (6, 3);
+T (6, 4);
+T (6, 5);
+
+T (7, 0);
+T (7, 1);
+T (7, 2);
+T (7, 3);
+T (7, 4);
+T (7, 5);
+T (7, 6);
+
+T (8, 0);
+T (8, 1);
+T (8, 2);
+T (8, 3);
+T (8, 4);
+T (8, 5);
+T (8, 6);
+T (8, 7);
+
+/* Verify that each function makes just one call to strlen to compute
+ the length of its argument (and not also to compute the length of
+ the copy):
+ { dg-final { scan-tree-dump-times "strlen \\(s_" 36 "strlen1" } }
+ { dg-final { scan-tree-dump-not "strlen \\(\\&a" "strlen1" } }
+
+ Verify that nul stores into the last array element have been eliminated
+ (they are preceded by a strcpy storing into all the elements of the array:
+ { dg-final { scan-tree-dump-not "a1\\\] = 0;" "strlen1" } }
+ { dg-final { scan-tree-dump-not "a2 \\\+ 1B\\\] = 0;" "strlen1" } }
+ { dg-final { scan-tree-dump-not "a3 \\\+ 2B\\\] = 0;" "strlen1" } }
+ { dg-final { scan-tree-dump-not "a4 \\\+ 3B\\\] = 0;" "strlen1" } }
+ { dg-final { scan-tree-dump-not "a5 \\\+ 4B\\\] = 0;" "strlen1" } }
+ { dg-final { scan-tree-dump-not "a6 \\\+ 5B\\\] = 0;" "strlen1" } }
+ { dg-final { scan-tree-dump-not "a7 \\\+ 6B\\\] = 0;" "strlen1" } }
+ { dg-final { scan-tree-dump-not "a8 \\\+ 7B\\\] = 0;" "strlen1" } } */
Index: gcc/testsuite/gcc.dg/strlenopt-90.c
===================================================================
--- gcc/testsuite/gcc.dg/strlenopt-90.c (nonexistent)
+++ gcc/testsuite/gcc.dg/strlenopt-90.c (working copy)
@@ -0,0 +1,83 @@
+/* PR tree-optimization/92226 - live nul char store to array eliminated
+ { dg-do compile }
+ { dg-options "-O2 -fdump-tree-strlen" } */
+
+#include "strlenopt.h"
+
+#define NOIPA __attribute__ ((noipa))
+
+#define T(MIN, MAX, SIZE, IDX) \
+ NOIPA void \
+ test_ ## MIN ## _ ## MAX ## _ ## SIZE ## _ ## IDX (const char *s) \
+ { \
+ extern char a ## SIZE[SIZE]; \
+ char *d = a ## SIZE; \
+ size_t len = strlen (s); \
+ size_t idx = IDX; \
+ if (MIN <= len && len <= MAX) \
+ { \
+ strcpy (d, s); \
+ d[idx] = 0; \
+ if (strlen (d) != idx) \
+ abort (); \
+ } \
+ } typedef void dummy_type
+
+
+/* The final nul store must be retained but the second strlen call should
+ be eliminated because the final length of the destination after the nul
+ store must be equal to the index of the store. */
+T (0, 2, 4, 0);
+
+/* Not handled yet (see below):
+ T (0, 2, 4, 1); */
+
+/* Not handled yet: in addition to the cases above, the second strlen
+ call can also be eliminated in those below because in both the final
+ length of the destination after the nul store must be in the same
+ range as the length of the source.
+ T (0, 2, 4, 2);
+ T (0, 2, 4, 3); */
+
+T (2, 3, 4, 0);
+T (2, 3, 4, 1);
+
+/* Not handled yet (see above):
+ T (2, 3, 4, 2);
+ T (2, 3, 4, 3); */
+
+T (3, 4, 5, 0);
+T (3, 4, 5, 1);
+T (3, 4, 5, 2);
+
+/* Not handled yet (see above):
+ T (3, 4, 5, 3);
+ T (3, 4, 5, 4); */
+
+T (3, 4, 6, 0);
+T (3, 4, 6, 1);
+T (3, 4, 6, 2);
+
+/* Not handled yet (see above):
+ T (3, 4, 6, 3);
+ T (3, 4, 6, 4);
+ T (3, 4, 6, 5); */
+
+
+/* Verify that each function makes just one call to strlen to compute
+ the length of its argument (and not also to compute the length of
+ the copy):
+ { dg-final { scan-tree-dump-times "strlen \\(s_" 9 "strlen1" } }
+ { dg-final { scan-tree-dump-not "strlen \\(\\&a" "strlen1" } }
+
+ Verify that nul stores into the destination have not been eliminated:
+ { dg-final { scan-tree-dump-times "a4\\\] = 0;" 2 "strlen1" } }
+ { dg-final { scan-tree-dump-times "a4 \\\+ 1B\\\] = 0;" 1 "strlen1" } }
+
+ { dg-final { scan-tree-dump-times "a5\\\] = 0;" 1 "strlen1" } }
+ { dg-final { scan-tree-dump-times "a5 \\\+ 1B\\\] = 0;" 1 "strlen1" } }
+ { dg-final { scan-tree-dump-times "a5 \\\+ 2B\\\] = 0;" 1 "strlen1" } }
+
+ { dg-final { scan-tree-dump-times "a6\\\] = 0;" 1 "strlen1" } }
+ { dg-final { scan-tree-dump-times "a6 \\\+ 1B\\\] = 0;" 1 "strlen1" } }
+ { dg-final { scan-tree-dump-times "a6 \\\+ 2B\\\] = 0;" 1 "strlen1" } } */
Index: gcc/testsuite/gcc.dg/Wstringop-overflow-20.c
===================================================================
--- gcc/testsuite/gcc.dg/Wstringop-overflow-20.c (nonexistent)
+++ gcc/testsuite/gcc.dg/Wstringop-overflow-20.c (working copy)
@@ -0,0 +1,40 @@
+/* PR tree-optimization/92226 - live nul char store to array eliminated
+ Test to verify warnings are issued for overflow detected thanks to
+ the enhancement committed on top of the fix for PR 92226.
+ { dg-do compile }
+ { dg-options "-O2 -Wall -Wno-array-bounds -ftrack-macro-expansion=0" } */
+
+#include "strlenopt.h"
+
+#define NOIPA __attribute__ ((noipa))
+
+#define T(MIN, MAX, SIZE, IDX) \
+ NOIPA void \
+ test_ ## MIN ## _ ## MAX ## _ ## SIZE ## _ ## IDX (const char *s) \
+ { \
+ size_t len = strlen (s); \
+ if (MIN <= len && len <= MAX) \
+ { \
+ extern char d[]; \
+ strcpy (d, s); \
+ d[IDX] = 0; \
+ extern char a ## SIZE[SIZE]; \
+ strcpy (a ## SIZE, d); \
+ } \
+ } typedef void dummy_type
+
+
+T (2, 2, 1, 0);
+T (2, 2, 1, 1); // { dg-warning "writing 2 bytes into a region of size 1" }
+T (2, 2, 1, 2); // { dg-warning "writing 3 bytes into a region of size 1" }
+
+T (2, 3, 1, 0);
+T (2, 3, 1, 1); // { dg-warning "writing 2 bytes into a region of size 1" }
+T (2, 3, 1, 2); // { dg-warning "writing 3 bytes into a region of size 1" "" { xfail *-*-*} }
+T (2, 3, 1, 3); // { dg-warning "writing 4 bytes into a region of size 1" "" { xfail *-*-* } }
+
+T (5, 7, 3, 1);
+T (5, 7, 3, 2);
+T (5, 7, 3, 3); // { dg-warning "writing 4 bytes into a region of size 1" }
+T (5, 7, 3, 4); // { dg-warning "writing 5 bytes into a region of size 1" }
+T (5, 7, 3, 5); // { dg-warning "writing 6 bytes into a region of size 1" "" { xfail *-*-* } }
