diff mbox series

avoid eliminating live nul stores into strings of bounded length (PR 92226)

Message ID 85cc163f-a0b1-9ac0-32fd-89e73dd3e02b@gmail.com
State New
Headers show
Series avoid eliminating live nul stores into strings of bounded length (PR 92226) | expand

Commit Message

Martin Sebor Oct. 28, 2019, 8:29 p.m. UTC
A recent enhancement to take advantage of non-constant strlen
results constrained to a known range interacts badly with
the nul-over-nul optimization.  The optimization eliminates
nul stores that overwrite the exiting terminating nul of
the destination string.  This interaction causes the nul
store to be eliminated in subset of cases when it shouldn't
be.

The attached patch fixes the bug by avoiding the optimization
for such destinations.  It also adjusts the comment that
describes the function with the bug to make its return value
clearer.

Tested on x86_64-linux.

Martin

Comments

Jeff Law Oct. 28, 2019, 10:11 p.m. UTC | #1
On 10/28/19 2:29 PM, Martin Sebor wrote:
> A recent enhancement to take advantage of non-constant strlen
> results constrained to a known range interacts badly with
> the nul-over-nul optimization.  The optimization eliminates
> nul stores that overwrite the exiting terminating nul of
> the destination string.  This interaction causes the nul
> store to be eliminated in subset of cases when it shouldn't
> be.
> 
> The attached patch fixes the bug by avoiding the optimization
> for such destinations.  It also adjusts the comment that
> describes the function with the bug to make its return value
> clearer.
> 
> Tested on x86_64-linux.
> 
> Martin
> 
> gcc-92226.diff
> 
> PR tree-optimization/92226 - live nul char store to array eliminated
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/92226
> 	* gcc.dg/strlenopt-88.c: New test.
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/92226
> 	* tree-ssa-strlen.c (compare_nonzero_chars): Return -1 also when
> 	the offset is in the open range outlined by SI's length.
> 

OK
jeff
diff mbox series

Patch

PR tree-optimization/92226 - live nul char store to array eliminated

gcc/testsuite/ChangeLog:

	PR tree-optimization/92226
	* gcc.dg/strlenopt-88.c: New test.

gcc/ChangeLog:

	PR tree-optimization/92226
	* tree-ssa-strlen.c (compare_nonzero_chars): Return -1 also when
	the offset is in the open range outlined by SI's length.

Index: gcc/tree-ssa-strlen.c
===================================================================
--- gcc/tree-ssa-strlen.c	(revision 277521)
+++ gcc/tree-ssa-strlen.c	(working copy)
@@ -193,10 +193,11 @@  static void handle_builtin_stxncpy (built_in_funct
 
    *  +1  if SI is known to start with more than OFF nonzero characters.
 
-   *   0  if SI is known to start with OFF nonzero characters,
-	  but is not known to start with more.
+   *   0  if SI is known to start with exactly OFF nonzero characters.
 
-   *  -1  if SI might not start with OFF nonzero characters.  */
+   *  -1  if SI either does not start with OFF nonzero characters
+          or the relationship between the number of leading nonzero
+          characters in SI and OFF is unknown.  */
 
 static inline int
 compare_nonzero_chars (strinfo *si, unsigned HOST_WIDE_INT off)
@@ -221,7 +221,7 @@  compare_nonzero_chars (strinfo *si, unsigned HOST_
   if (TREE_CODE (si->nonzero_chars) == INTEGER_CST)
     return compare_tree_int (si->nonzero_chars, off);
 
-  if (TREE_CODE (si->nonzero_chars) != SSA_NAME)
+  if (!rvals || TREE_CODE (si->nonzero_chars) != SSA_NAME)
     return -1;
 
   const value_range *vr
@@ -232,7 +232,15 @@  compare_nonzero_chars (strinfo *si, unsigned HOST_
   if (rng != VR_RANGE || !range_int_cst_p (vr))
     return -1;
 
-  return compare_tree_int (vr->min (), off);
+  /* If the offset is less than the minimum length or if the bounds
+     of the length range are equal return the result of the comparison
+     same as in the constant case.  Otherwise return a conservative
+     result.  */
+  int cmpmin = compare_tree_int (vr->min (), off);
+  if (cmpmin > 0 || tree_int_cst_equal (vr->min (), vr->max ()))
+    return cmpmin;
+
+  return -1;
 }
 
 /* Return true if SI is known to be a zero-length string.  */
Index: gcc/testsuite/gcc.dg/strlenopt-88.c
===================================================================
--- gcc/testsuite/gcc.dg/strlenopt-88.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/strlenopt-88.c	(working copy)
@@ -0,0 +1,196 @@ 
+/* PR tree-optimization/92226 - live nul char store to array eliminated
+   { dg-do run }
+   { dg-options "-O2 -Wall" } */
+
+#include "strlenopt.h"
+
+#define NOIPA __attribute__ ((noipa))
+
+unsigned nfails;
+
+char a[8];
+
+void test (int line, const char *func, size_t expect)
+{
+  size_t len = strlen (a);
+  if (len == expect)
+    return;
+
+  ++nfails;
+
+  __builtin_printf ("assertion failed in %s on line %i: "
+		    "strlen (\"%s\") == %zu, got %zu\n",
+		    func, line, a, expect, len);
+}
+
+NOIPA const char* str (size_t n)
+{
+  return "9876543210" + 10 - n;
+}
+
+#define T(name, CMPEXP, LEN, IDX, EXPECT)	\
+  NOIPA static void name (void)			\
+  {						\
+    const char *s = str (LEN);			\
+    if (strlen (s) CMPEXP)			\
+      {						\
+	strcpy (a, s);				\
+	a[IDX] = 0;				\
+	test (__LINE__, #name, EXPECT);		\
+      }						\
+  } typedef void dummy_type
+
+
+T (len_eq_1_store_nul_0, == 1, 1, 0, 0);
+T (len_eq_1_store_nul_1, == 1, 1, 1, 1);
+T (len_eq_1_store_nul_2, == 1, 1, 2, 1);
+T (len_eq_1_store_nul_3, == 1, 1, 3, 1);
+T (len_eq_1_store_nul_4, == 1, 1, 4, 1);
+
+T (len_eq_2_store_nul_0, == 2, 2, 0, 0);
+T (len_eq_2_store_nul_1, == 2, 2, 1, 1);
+T (len_eq_2_store_nul_2, == 2, 2, 2, 2);
+T (len_eq_2_store_nul_3, == 2, 2, 3, 2);
+T (len_eq_2_store_nul_4, == 2, 2, 4, 2);
+
+T (len_eq_3_store_nul_0, == 3, 3, 0, 0);
+T (len_eq_3_store_nul_1, == 3, 3, 1, 1);
+T (len_eq_3_store_nul_2, == 3, 3, 2, 2);
+T (len_eq_3_store_nul_3, == 3, 3, 3, 3);
+T (len_eq_3_store_nul_4, == 3, 3, 4, 3);
+
+
+T (len_gt_1_store_nul_0, > 2, 2, 0, 0);
+T (len_gt_1_store_nul_1, > 2, 2, 1, 1);
+T (len_gt_1_store_nul_2, > 2, 2, 2, 2);
+T (len_gt_1_store_nul_3, > 2, 2, 3, 2);
+T (len_gt_1_store_nul_4, > 2, 2, 4, 2);
+
+T (len_gt_2_store_nul_0, > 2, 3, 0, 0);
+T (len_gt_2_store_nul_1, > 2, 3, 1, 1);
+T (len_gt_2_store_nul_2, > 2, 3, 2, 2);
+T (len_gt_2_store_nul_3, > 2, 3, 3, 3);
+T (len_gt_2_store_nul_4, > 2, 3, 4, 3);
+
+T (len_gt_3_store_nul_0, > 2, 4, 0, 0);
+T (len_gt_3_store_nul_1, > 2, 4, 1, 1);
+T (len_gt_3_store_nul_2, > 2, 4, 2, 2);
+T (len_gt_3_store_nul_3, > 2, 4, 3, 3);
+T (len_gt_3_store_nul_4, > 2, 4, 4, 4);
+
+
+T (len_1_lt_4_store_nul_0, < 4, 1, 0, 0);
+T (len_1_lt_4_store_nul_1, < 4, 1, 1, 1);
+T (len_1_lt_4_store_nul_2, < 4, 1, 2, 1);
+T (len_1_lt_4_store_nul_3, < 4, 1, 3, 1);
+T (len_1_lt_4_store_nul_4, < 4, 1, 4, 1);
+T (len_1_lt_4_store_nul_5, < 4, 1, 5, 1);
+T (len_1_lt_4_store_nul_6, < 4, 1, 6, 1);
+T (len_1_lt_4_store_nul_7, < 4, 1, 7, 1);
+
+T (len_2_lt_4_store_nul_0, < 4, 2, 0, 0);
+T (len_2_lt_4_store_nul_1, < 4, 2, 1, 1);
+T (len_2_lt_4_store_nul_2, < 4, 2, 2, 2);
+T (len_2_lt_4_store_nul_3, < 4, 2, 3, 2);
+T (len_2_lt_4_store_nul_4, < 4, 2, 4, 2);
+T (len_2_lt_4_store_nul_5, < 4, 2, 5, 2);
+T (len_2_lt_4_store_nul_6, < 4, 2, 6, 2);
+T (len_2_lt_4_store_nul_7, < 4, 2, 7, 2);
+
+T (len_3_lt_4_store_nul_0, < 4, 3, 0, 0);
+T (len_3_lt_4_store_nul_1, < 4, 3, 1, 1);
+T (len_3_lt_4_store_nul_2, < 4, 3, 2, 2);
+T (len_3_lt_4_store_nul_3, < 4, 3, 3, 3);
+T (len_3_lt_4_store_nul_4, < 4, 3, 4, 3);
+T (len_3_lt_4_store_nul_5, < 4, 3, 5, 3);
+T (len_3_lt_4_store_nul_6, < 4, 3, 6, 3);
+T (len_3_lt_4_store_nul_7, < 4, 3, 7, 3);
+
+T (len_7_lt_8_store_nul_0, < 8, 7, 0, 0);
+T (len_7_lt_8_store_nul_1, < 8, 7, 1, 1);
+T (len_7_lt_8_store_nul_2, < 8, 7, 2, 2);
+T (len_7_lt_8_store_nul_3, < 8, 7, 3, 3);
+T (len_7_lt_8_store_nul_4, < 8, 7, 4, 4);
+T (len_7_lt_8_store_nul_5, < 8, 7, 5, 5);
+T (len_7_lt_8_store_nul_6, < 8, 7, 6, 6);
+T (len_7_lt_8_store_nul_7, < 8, 7, 7, 7);
+
+
+int main (void)
+{
+  len_eq_1_store_nul_0 ();
+  len_eq_1_store_nul_1 ();
+  len_eq_1_store_nul_2 ();
+  len_eq_1_store_nul_3 ();
+  len_eq_1_store_nul_4 ();
+
+  len_eq_2_store_nul_0 ();
+  len_eq_2_store_nul_1 ();
+  len_eq_2_store_nul_2 ();
+  len_eq_2_store_nul_3 ();
+  len_eq_2_store_nul_4 ();
+
+  len_eq_3_store_nul_0 ();
+  len_eq_3_store_nul_1 ();
+  len_eq_3_store_nul_2 ();
+  len_eq_3_store_nul_3 ();
+  len_eq_3_store_nul_4 ();
+
+
+  len_gt_1_store_nul_0 ();
+  len_gt_1_store_nul_1 ();
+  len_gt_1_store_nul_2 ();
+  len_gt_1_store_nul_3 ();
+  len_gt_1_store_nul_4 ();
+
+  len_gt_2_store_nul_0 ();
+  len_gt_2_store_nul_1 ();
+  len_gt_2_store_nul_2 ();
+  len_gt_2_store_nul_3 ();
+  len_gt_2_store_nul_4 ();
+
+  len_gt_3_store_nul_0 ();
+  len_gt_3_store_nul_1 ();
+  len_gt_3_store_nul_2 ();
+  len_gt_3_store_nul_3 ();
+  len_gt_3_store_nul_4 ();
+
+  len_1_lt_4_store_nul_0 ();
+  len_1_lt_4_store_nul_1 ();
+  len_1_lt_4_store_nul_2 ();
+  len_1_lt_4_store_nul_3 ();
+  len_1_lt_4_store_nul_4 ();
+  len_1_lt_4_store_nul_5 ();
+  len_1_lt_4_store_nul_6 ();
+  len_1_lt_4_store_nul_7 ();
+
+  len_2_lt_4_store_nul_0 ();
+  len_2_lt_4_store_nul_1 ();
+  len_2_lt_4_store_nul_2 ();
+  len_2_lt_4_store_nul_3 ();
+  len_2_lt_4_store_nul_4 ();
+  len_2_lt_4_store_nul_5 ();
+  len_2_lt_4_store_nul_6 ();
+  len_2_lt_4_store_nul_7 ();
+
+  len_3_lt_4_store_nul_0 ();
+  len_3_lt_4_store_nul_1 ();
+  len_3_lt_4_store_nul_2 ();
+  len_3_lt_4_store_nul_3 ();
+  len_3_lt_4_store_nul_4 ();
+  len_3_lt_4_store_nul_5 ();
+  len_3_lt_4_store_nul_6 ();
+  len_3_lt_4_store_nul_7 ();
+
+  len_7_lt_8_store_nul_0 ();
+  len_7_lt_8_store_nul_1 ();
+  len_7_lt_8_store_nul_2 ();
+  len_7_lt_8_store_nul_3 ();
+  len_7_lt_8_store_nul_4 ();
+  len_7_lt_8_store_nul_5 ();
+  len_7_lt_8_store_nul_6 ();
+  len_7_lt_8_store_nul_7 ();
+
+  if (nfails)
+    abort ();
+}