public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/101031] New: wrong code at -O2 on x86_64-linux-gnu
@ 2021-06-11 13:28 qrzhang at gatech dot edu
2021-06-11 13:29 ` [Bug tree-optimization/101031] " qrzhang at gatech dot edu
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: qrzhang at gatech dot edu @ 2021-06-11 13:28 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101031
Bug ID: 101031
Summary: wrong code at -O2 on x86_64-linux-gnu
Product: gcc
Version: unknown
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: tree-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: qrzhang at gatech dot edu
Target Milestone: ---
It appears to be a recent regression.
$ gcc-trunk -v
Supported LTO compression algorithms: zlib
gcc version 12.0.0 20210611 (experimental) [master revision
336c41dbcb2:00da4bcb67d:36943c6bdd3d3b535b24872bbd802d91ef0c6299] (GCC)
$ gcc-trunk abc.c ; ./a.out
0
$ gcc-trunk -O2 abc.c ; ./a.out
1
$ cat abc.c
int a;
char b, e;
static char *c = &b;
static long d;
void f(void);
void h() {
int g = 0;
for (; g < 2; ++g) {
d = *c;
*c = 1;
b = 0;
}
f();
}
void f() {
if (d++)
c = &e;
for (; a;)
;
}
int main() {
h();
printf("%d\n", b);
}
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug tree-optimization/101031] wrong code at -O2 on x86_64-linux-gnu
2021-06-11 13:28 [Bug tree-optimization/101031] New: wrong code at -O2 on x86_64-linux-gnu qrzhang at gatech dot edu
@ 2021-06-11 13:29 ` qrzhang at gatech dot edu
2021-06-11 13:31 ` [Bug tree-optimization/101031] [12 Regression] " jakub at gcc dot gnu.org
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: qrzhang at gatech dot edu @ 2021-06-11 13:29 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101031
Qirun Zhang <qrzhang at gatech dot edu> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |rguenther at suse dot de
--- Comment #1 from Qirun Zhang <qrzhang at gatech dot edu> ---
My bisection points to g:d1d01a66012a93cc8cb7dafbe1b5ec453e
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug tree-optimization/101031] [12 Regression] wrong code at -O2 on x86_64-linux-gnu
2021-06-11 13:28 [Bug tree-optimization/101031] New: wrong code at -O2 on x86_64-linux-gnu qrzhang at gatech dot edu
2021-06-11 13:29 ` [Bug tree-optimization/101031] " qrzhang at gatech dot edu
@ 2021-06-11 13:31 ` jakub at gcc dot gnu.org
2021-06-11 13:48 ` rguenth at gcc dot gnu.org
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: jakub at gcc dot gnu.org @ 2021-06-11 13:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101031
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Ever confirmed|0 |1
Summary|wrong code at -O2 on |[12 Regression] wrong code
|x86_64-linux-gnu |at -O2 on x86_64-linux-gnu
Version|unknown |12.0
Priority|P3 |P1
Last reconfirmed| |2021-06-11
CC| |jakub at gcc dot gnu.org
Target Milestone|--- |12.0
Status|UNCONFIRMED |NEW
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug tree-optimization/101031] [12 Regression] wrong code at -O2 on x86_64-linux-gnu
2021-06-11 13:28 [Bug tree-optimization/101031] New: wrong code at -O2 on x86_64-linux-gnu qrzhang at gatech dot edu
2021-06-11 13:29 ` [Bug tree-optimization/101031] " qrzhang at gatech dot edu
2021-06-11 13:31 ` [Bug tree-optimization/101031] [12 Regression] " jakub at gcc dot gnu.org
@ 2021-06-11 13:48 ` rguenth at gcc dot gnu.org
2021-06-11 13:52 ` rguenth at gcc dot gnu.org
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-06-11 13:48 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101031
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot gnu.org
--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
Confirmed, mine. Not a dup of PR101009.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug tree-optimization/101031] [12 Regression] wrong code at -O2 on x86_64-linux-gnu
2021-06-11 13:28 [Bug tree-optimization/101031] New: wrong code at -O2 on x86_64-linux-gnu qrzhang at gatech dot edu
` (2 preceding siblings ...)
2021-06-11 13:48 ` rguenth at gcc dot gnu.org
@ 2021-06-11 13:52 ` rguenth at gcc dot gnu.org
2021-06-14 7:35 ` rguenth at gcc dot gnu.org
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-06-11 13:52 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101031
--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Simpler IL:
int a;
char b, e;
static char *c = &b;
static long d;
void f(void);
void __attribute__((noipa)) h() {
int g = 0;
for (; g < 2; ++g) {
d = *c;
*c = 1;
b = 0;
}
f();
}
void __attribute__((noipa)) f() {
if (d++)
c = &e;
for (; a;)
;
}
int main() {
h();
if (b != 0)
__builtin_abort ();
return 0;
}
we end up with
void h ()
{
char * c.0_14;
char _24;
long int _25;
<bb 2> [local count: 357878152]:
c.0_14 = c;
*c.0_14 = 1;
b = 0;
_24 = *c.0_14;
_25 = (long int) _24;
d = _25;
*c.0_14 = 1;
f (); [tail call]
return;
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug tree-optimization/101031] [12 Regression] wrong code at -O2 on x86_64-linux-gnu
2021-06-11 13:28 [Bug tree-optimization/101031] New: wrong code at -O2 on x86_64-linux-gnu qrzhang at gatech dot edu
` (3 preceding siblings ...)
2021-06-11 13:52 ` rguenth at gcc dot gnu.org
@ 2021-06-14 7:35 ` rguenth at gcc dot gnu.org
2021-06-14 9:14 ` cvs-commit at gcc dot gnu.org
2021-06-14 10:21 ` rguenth at gcc dot gnu.org
6 siblings, 0 replies; 8+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-06-14 7:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101031
--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Interestingly the bogus store removal happens in the strlen pass.
> diff -u b/a-t3.c.190t.dom3 b/a-t3.c.191t.strlen1
--- b/a-t3.c.190t.dom3 2021-06-14 08:59:33.483709029 +0200
+++ b/a-t3.c.191t.strlen1 2021-06-14 08:59:33.487709082 +0200
@@ -78,7 +80,6 @@
_25 = (long int) _24;
d = _25;
*c.0_14 = 1;
- b = 0;
f ();
return;
in
#0 gsi_remove (i=0x7fffffffd830, remove_permanently=true)
at /home/rguenther/src/gcc2/gcc/gimple-iterator.c:558
#1 0x0000000001619e63 in handle_store (gsi=0x7fffffffd830,
zero_write=0x7fffffffd7df, ptr_qry=...)
at /home/rguenther/src/gcc2/gcc/tree-ssa-strlen.c:4852
#2 0x000000000161bb9a in check_and_optimize_stmt (gsi=0x7fffffffd830,
cleanup_eh=0x7fffffffd89f, ptr_qry=...)
at /home/rguenther/src/gcc2/gcc/tree-ssa-strlen.c:5439
#3 0x000000000161c30c in strlen_dom_walker::before_dom_children (
this=0x7fffffffd970, bb=<basic_block 0x7ffff6576270 (2)>)
at /home/rguenther/src/gcc2/gcc/tree-ssa-strlen.c:5635
#4 0x000000000228e9b6 in dom_walker::walk (this=0x7fffffffd970,
bb=<basic_block 0x7ffff6576270 (2)>)
at /home/rguenther/src/gcc2/gcc/domwalk.c:309
the *c stores have string info index 1 and the 'b' store have index 2.
Interestingly the first *c store is handled by handle_store but the
second is not and we return false from check_and_optimize_stmt. In
particular we run into
if (store_before_nul[1] > 0
&& storing_nonzero_p
&& lenrange[0] == lenrange[1]
&& lenrange[0] == lenrange[2]
&& TREE_CODE (TREE_TYPE (rhs)) == INTEGER_TYPE)
{
/* Handle a store of one or more non-nul characters that ends
before the terminating nul of the destination and so does
not affect its length
If si->nonzero_chars > OFFSET, we aren't overwriting '\0',
and if we aren't storing '\0', we know that the length of
the string and any other zero terminated string in memory
remains the same. In that case we move to the next gimple
statement and return to signal the caller that it shouldn't
invalidate anything.
...
gsi_next (gsi);
return false;
}
and skip the invalidation of 'b'.
Now, we're not invalidating the first *c store at the point we stroe to b
either because maybe_invalidate has
ao_ref r;
tree size = NULL_TREE;
if (si->nonzero_chars)
{
/* Include the terminating nul in the size of the string
to consider when determining possible clobber. */
tree type = TREE_TYPE (si->nonzero_chars);
size = fold_build2 (PLUS_EXPR, type, si->nonzero_chars,
build_int_cst (type, 1));
}
ao_ref_init_from_ptr_and_size (&r, si->ptr, size);
if (stmt_may_clobber_ref_p_1 (stmt, &r))
and thus we ask the oracle whether b = 0; may clobber a store/load of
*c of size 2 which it can't (an access of size two cannot refer to 'b').
Looking at compare_nonzero_chars the si->nonzero_chars check should likely
be si->nonzero_chars && !integer_zerop (si->nonzero_chars) as well.
Now, adding 1 is done for correctness AFAIU but then extending the access
beyond the size of the object is invalid. So IMHO we have to adjust
max_size to be 1 bigger than size instead.
The following fixes the testcase:
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 423075b2bd1..6add8c99032 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -1284,16 +1284,19 @@ maybe_invalidate (gimple *stmt, bool zero_write =
false)
continue;
ao_ref r;
- tree size = NULL_TREE;
- if (si->nonzero_chars)
+ tree size = si->nonzero_chars;
+ ao_ref_init_from_ptr_and_size (&r, si->ptr, size);
+ /* Include the terminating nul in the size of the string
+ to consider when determining possible clobber. But do not
+ add it to 'size' since we don't know whether it would
+ actually fit the allocated area. */
+ if (known_size_p (r.size))
{
- /* Include the terminating nul in the size of the string
- to consider when determining possible clobber. */
- tree type = TREE_TYPE (si->nonzero_chars);
- size = fold_build2 (PLUS_EXPR, type, si->nonzero_chars,
- build_int_cst (type, 1));
+ if (known_le (r.size, HOST_WIDE_INT_MAX - BITS_PER_UNIT))
+ r.max_size += BITS_PER_UNIT;
+ else
+ r.max_size = -1;
}
- ao_ref_init_from_ptr_and_size (&r, si->ptr, size);
if (stmt_may_clobber_ref_p_1 (stmt, &r))
{
if (dump_file && (dump_flags & TDF_DETAILS))
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug tree-optimization/101031] [12 Regression] wrong code at -O2 on x86_64-linux-gnu
2021-06-11 13:28 [Bug tree-optimization/101031] New: wrong code at -O2 on x86_64-linux-gnu qrzhang at gatech dot edu
` (4 preceding siblings ...)
2021-06-14 7:35 ` rguenth at gcc dot gnu.org
@ 2021-06-14 9:14 ` cvs-commit at gcc dot gnu.org
2021-06-14 10:21 ` rguenth at gcc dot gnu.org
6 siblings, 0 replies; 8+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-06-14 9:14 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101031
--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:
https://gcc.gnu.org/g:08ce1f4c5091b80b680d15c53a17237544a3cca8
commit r12-1419-g08ce1f4c5091b80b680d15c53a17237544a3cca8
Author: Richard Biener <rguenther@suse.de>
Date: Mon Jun 14 09:37:24 2021 +0200
tree-optimization/101031 - fix strlen opt invalidation logic
strlen opt uses ao_ref_init_from_ptr_and_size to prepare alias
queries to invalidate its knowledge about strings. It constrains
the size using the number of known-nonzero chars and adds one
for a terminating nul - without knowing whether such nul exists
or even fits the object. The latter is now a problem since the
oracle disambiguates an access of size two (as built so) against
a store to a plain char variable (where a terminating nul does not
fit). The fix is to instead increment max_size but leave size to
the number of chars we know are accessed.
2021-06-14 Richard Biener <rguenther@suse.de>
PR tree-optimization/101031
* tree-ssa-strlen.c (maybe_invalidate): Increment max_size
instead of size when accounting for a possibly string
terminating nul.
* gcc.dg/torture/pr101031.c: New testcase.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug tree-optimization/101031] [12 Regression] wrong code at -O2 on x86_64-linux-gnu
2021-06-11 13:28 [Bug tree-optimization/101031] New: wrong code at -O2 on x86_64-linux-gnu qrzhang at gatech dot edu
` (5 preceding siblings ...)
2021-06-14 9:14 ` cvs-commit at gcc dot gnu.org
@ 2021-06-14 10:21 ` rguenth at gcc dot gnu.org
6 siblings, 0 replies; 8+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-06-14 10:21 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101031
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-06-14 10:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-11 13:28 [Bug tree-optimization/101031] New: wrong code at -O2 on x86_64-linux-gnu qrzhang at gatech dot edu
2021-06-11 13:29 ` [Bug tree-optimization/101031] " qrzhang at gatech dot edu
2021-06-11 13:31 ` [Bug tree-optimization/101031] [12 Regression] " jakub at gcc dot gnu.org
2021-06-11 13:48 ` rguenth at gcc dot gnu.org
2021-06-11 13:52 ` rguenth at gcc dot gnu.org
2021-06-14 7:35 ` rguenth at gcc dot gnu.org
2021-06-14 9:14 ` cvs-commit at gcc dot gnu.org
2021-06-14 10:21 ` rguenth at gcc dot gnu.org
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).