public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r13-7944] tree-ssa-strlen: optimization skips clobbering store [PR111519]
@ 2023-10-11 7:23 Jakub Jelinek
0 siblings, 0 replies; only message in thread
From: Jakub Jelinek @ 2023-10-11 7:23 UTC (permalink / raw)
To: gcc-cvs
https://gcc.gnu.org/g:16a4df27436c8f594a784028591dd3e47cabe5c0
commit r13-7944-g16a4df27436c8f594a784028591dd3e47cabe5c0
Author: Jakub Jelinek <jakub@redhat.com>
Date: Wed Oct 11 08:58:29 2023 +0200
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 <rguenther@suse.de>
Jakub Jelinek <jakub@redhat.com>
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.
(cherry picked from commit e75bf1985fdc9a5d3a307882a9251d8fd6e93def)
Diff:
---
gcc/testsuite/gcc.dg/torture/pr111519.c | 48 +++++++++++++++++++++++++++++
gcc/tree-ssa-strlen.cc | 53 +++++++++++++++++++--------------
2 files changed, 79 insertions(+), 22 deletions(-)
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 db72b98d1a1..f8e2b1dace3 100644
--- a/gcc/tree-ssa-strlen.cc
+++ b/gcc/tree-ssa-strlen.cc
@@ -280,14 +280,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,
@@ -4552,8 +4552,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).
@@ -4567,7 +4567,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,
@@ -4587,22 +4587,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
@@ -4611,11 +4612,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;
@@ -4673,7 +4674,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);
}
@@ -4786,7 +4787,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,
@@ -4796,6 +4797,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;
@@ -4855,14 +4864,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
@@ -4871,11 +4880,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))
@@ -4923,7 +4932,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);
}
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2023-10-11 7:23 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-11 7:23 [gcc r13-7944] tree-ssa-strlen: optimization skips clobbering store [PR111519] Jakub Jelinek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).