From e75bf1985fdc9a5d3a307882a9251d8fd6e93def Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Wed, 11 Oct 2023 08:58:29 +0200 Subject: [PATCH] tree-ssa-strlen: optimization skips clobbering store [PR111519] The following testcase is miscompiled, because count_nonzero_bytes incorrectly uses get_strinfo information on a pointer from which an earlier instruction loads SSA_NAME stored at the current instruction. get_strinfo shows a state right before the current store though, so if there are some stores in between the current store and the load, the string length information might have changed. The patch passes around gimple_vuse from the store and punts instead of using strinfo on loads from MEM_REF which have different gimple_vuse from that. 2023-10-11 Richard Biener Jakub Jelinek PR tree-optimization/111519 * tree-ssa-strlen.cc (strlen_pass::count_nonzero_bytes): Add vuse argument and pass it through to recursive calls and count_nonzero_bytes_addr calls. Don't shadow the stmt argument, but change stmt for gimple_assign_single_p statements for which we don't immediately punt. (strlen_pass::count_nonzero_bytes_addr): Add vuse argument and pass it through to recursive calls and count_nonzero_bytes calls. Don't use get_strinfo if gimple_vuse (stmt) is different from vuse. Don't shadow the stmt argument. * gcc.dg/torture/pr111519.c: New testcase. --- gcc/testsuite/gcc.dg/torture/pr111519.c | 48 ++++++++++++++++++++++ gcc/tree-ssa-strlen.cc | 53 +++++++++++++++---------- 2 files changed, 79 insertions(+), 22 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr111519.c diff --git a/gcc/testsuite/gcc.dg/torture/pr111519.c b/gcc/testsuite/gcc.dg/torture/pr111519.c new file mode 100644 index 00000000000..ef34c269df8 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr111519.c @@ -0,0 +1,48 @@ +/* PR tree-optimization/111519 */ +/* { dg-do run } */ + +int a, o; +char b, f, i; +long c; +static signed char d; +static char g; +unsigned *h; +signed char *e = &f; +static signed char **j = &e; +static long k[2]; +unsigned **l = &h; +short m; +volatile int z; + +__attribute__((noipa)) void +foo (char *p) +{ + (void) p; +} + +int +main () +{ + int p = z; + signed char *n = &d; + *n = 0; + while (c) + for (; i; i--) + ; + for (g = 0; g <= 1; g++) + { + *n = **j; + k[g] = 0 != &m; + *e = l && k[0]; + } + if (p) + foo (&b); + for (; o < 4; o++) + { + a = d; + if (p) + foo (&b); + } + if (a != 1) + __builtin_abort (); +} diff --git a/gcc/tree-ssa-strlen.cc b/gcc/tree-ssa-strlen.cc index 8b7ef919826..083a10dc22e 100644 --- a/gcc/tree-ssa-strlen.cc +++ b/gcc/tree-ssa-strlen.cc @@ -281,14 +281,14 @@ public: gimple *stmt, unsigned lenrange[3], bool *nulterm, bool *allnul, bool *allnonnul); - bool count_nonzero_bytes (tree exp, + bool count_nonzero_bytes (tree exp, tree vuse, gimple *stmt, unsigned HOST_WIDE_INT offset, unsigned HOST_WIDE_INT nbytes, unsigned lenrange[3], bool *nulterm, bool *allnul, bool *allnonnul, ssa_name_limit_t &snlim); - bool count_nonzero_bytes_addr (tree exp, + bool count_nonzero_bytes_addr (tree exp, tree vuse, gimple *stmt, unsigned HOST_WIDE_INT offset, unsigned HOST_WIDE_INT nbytes, @@ -4531,8 +4531,8 @@ nonzero_bytes_for_type (tree type, unsigned lenrange[3], } /* Recursively determine the minimum and maximum number of leading nonzero - bytes in the representation of EXP and set LENRANGE[0] and LENRANGE[1] - to each. + bytes in the representation of EXP at memory state VUSE and set + LENRANGE[0] and LENRANGE[1] to each. Sets LENRANGE[2] to the total size of the access (which may be less than LENRANGE[1] when what's being referenced by EXP is a pointer rather than an array). @@ -4546,7 +4546,7 @@ nonzero_bytes_for_type (tree type, unsigned lenrange[3], Returns true on success and false otherwise. */ bool -strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt, +strlen_pass::count_nonzero_bytes (tree exp, tree vuse, gimple *stmt, unsigned HOST_WIDE_INT offset, unsigned HOST_WIDE_INT nbytes, unsigned lenrange[3], bool *nulterm, @@ -4566,22 +4566,23 @@ strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt, exact value is not known) recurse once to set the range for an arbitrary constant. */ exp = build_int_cst (type, 1); - return count_nonzero_bytes (exp, stmt, + return count_nonzero_bytes (exp, vuse, stmt, offset, 1, lenrange, nulterm, allnul, allnonnul, snlim); } - gimple *stmt = SSA_NAME_DEF_STMT (exp); - if (gimple_assign_single_p (stmt)) + gimple *g = SSA_NAME_DEF_STMT (exp); + if (gimple_assign_single_p (g)) { - exp = gimple_assign_rhs1 (stmt); + exp = gimple_assign_rhs1 (g); if (!DECL_P (exp) && TREE_CODE (exp) != CONSTRUCTOR && TREE_CODE (exp) != MEM_REF) return false; /* Handle DECLs, CONSTRUCTOR and MEM_REF below. */ + stmt = g; } - else if (gimple_code (stmt) == GIMPLE_PHI) + else if (gimple_code (g) == GIMPLE_PHI) { /* Avoid processing an SSA_NAME that has already been visited or if an SSA_NAME limit has been reached. Indicate success @@ -4590,11 +4591,11 @@ strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt, return res > 0; /* Determine the minimum and maximum from the PHI arguments. */ - unsigned int n = gimple_phi_num_args (stmt); + unsigned int n = gimple_phi_num_args (g); for (unsigned i = 0; i != n; i++) { - tree def = gimple_phi_arg_def (stmt, i); - if (!count_nonzero_bytes (def, stmt, + tree def = gimple_phi_arg_def (g, i); + if (!count_nonzero_bytes (def, vuse, g, offset, nbytes, lenrange, nulterm, allnul, allnonnul, snlim)) return false; @@ -4652,7 +4653,7 @@ strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt, return false; /* Handle MEM_REF = SSA_NAME types of assignments. */ - return count_nonzero_bytes_addr (arg, stmt, + return count_nonzero_bytes_addr (arg, vuse, stmt, offset, nbytes, lenrange, nulterm, allnul, allnonnul, snlim); } @@ -4765,7 +4766,7 @@ strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt, bytes that are pointed to by EXP, which should be a pointer. */ bool -strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt, +strlen_pass::count_nonzero_bytes_addr (tree exp, tree vuse, gimple *stmt, unsigned HOST_WIDE_INT offset, unsigned HOST_WIDE_INT nbytes, unsigned lenrange[3], bool *nulterm, @@ -4775,6 +4776,14 @@ strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt, int idx = get_stridx (exp, stmt); if (idx > 0) { + /* get_strinfo reflects string lengths before the current statement, + where the current statement is the outermost count_nonzero_bytes + stmt. If there are any stores in between stmt and that + current statement, the string length information might describe + something significantly different. */ + if (gimple_vuse (stmt) != vuse) + return false; + strinfo *si = get_strinfo (idx); if (!si) return false; @@ -4835,14 +4844,14 @@ strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt, } if (TREE_CODE (exp) == ADDR_EXPR) - return count_nonzero_bytes (TREE_OPERAND (exp, 0), stmt, + return count_nonzero_bytes (TREE_OPERAND (exp, 0), vuse, stmt, offset, nbytes, lenrange, nulterm, allnul, allnonnul, snlim); if (TREE_CODE (exp) == SSA_NAME) { - gimple *stmt = SSA_NAME_DEF_STMT (exp); - if (gimple_code (stmt) == GIMPLE_PHI) + gimple *g = SSA_NAME_DEF_STMT (exp); + if (gimple_code (g) == GIMPLE_PHI) { /* Avoid processing an SSA_NAME that has already been visited or if an SSA_NAME limit has been reached. Indicate success @@ -4851,11 +4860,11 @@ strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt, return res > 0; /* Determine the minimum and maximum from the PHI arguments. */ - unsigned int n = gimple_phi_num_args (stmt); + unsigned int n = gimple_phi_num_args (g); for (unsigned i = 0; i != n; i++) { - tree def = gimple_phi_arg_def (stmt, i); - if (!count_nonzero_bytes_addr (def, stmt, + tree def = gimple_phi_arg_def (g, i); + if (!count_nonzero_bytes_addr (def, vuse, g, offset, nbytes, lenrange, nulterm, allnul, allnonnul, snlim)) @@ -4903,7 +4912,7 @@ strlen_pass::count_nonzero_bytes (tree expr_or_type, gimple *stmt, ssa_name_limit_t snlim; tree expr = expr_or_type; - return count_nonzero_bytes (expr, stmt, + return count_nonzero_bytes (expr, gimple_vuse (stmt), stmt, 0, 0, lenrange, nulterm, allnul, allnonnul, snlim); }