[Bug tree-optimization/113551] [13 Regression] Miscompilation with -O1 -fno-strict-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113551 --- Comment #18 from Yuxuan Shui --- (In reply to Andrew Pinski from comment #17) > (In reply to Yuxuan Shui from comment #16) > > ... > > So -fno-strict-overflow does -fno-wrapv-pointer so we can assume pointer > arithmetic wraps now and then `a+1` could in theory wrap to nullptr. hmm, but why does that make the null check that previously was not removable, removable? also another observation, if I change `struct obj *dso` to `int *dso`, and `>i` to `[1]`, then the null check will be preserved. despite this code still being undefined?
[Bug tree-optimization/113551] [13 Regression] Miscompilation with -O1 -fno-strict-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113551 --- Comment #17 from Andrew Pinski --- (In reply to Yuxuan Shui from comment #16) > I see. but if it's undefined, why was the `if (dso)` only removed when > -fno-strict-overflow is enabled? and it still happens with > `-fno-delete-null-pointer-checks` So -fno-strict-overflow does -fno-wrapv-pointer so we can assume pointer arithmetic wraps now and then `a+1` could in theory wrap to nullptr. So many different hooks/options. It is better to use -fwrapv if you only want signed integer overflow being defined (as wrapping) rather than pointer arithmetic overflow being defined too.
[Bug tree-optimization/113551] [13 Regression] Miscompilation with -O1 -fno-strict-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113551 --- Comment #16 from Yuxuan Shui --- (In reply to Andrew Pinski from comment #13) > (In reply to Yuxuan Shui from comment #12) >> ... > > Except that is undefined ... > Manually unswitching introduces the undefined behavior in the code. > So even though the code was unswitched before GCC 13 incorrectly, GCC didn't > take into that account before hand. > > I am 99% sure what is happening is GCC is see `if (a+1)` and then assuming a > can't be a nullptr. Which is due to undefinedness there of adding 1 to an > null ptr ... (for C that is). > > Basically the unswitch is the issue ... Or we maybe we should turn `if > (a+1)` into just `if (a)` ... Likewise for `if (>i)` into `if (a)` I see. but if it's undefined, why was the `if (dso)` only removed when -fno-strict-overflow is enabled? and it still happens with `-fno-delete-null-pointer-checks`
[Bug tree-optimization/113551] [13 Regression] Miscompilation with -O1 -fno-strict-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113551
--- Comment #15 from Yuxuan Shui ---
(In reply to Marek Polacek from comment #14)
> I don't see a complete testcase that I could bisect.
Please use the code sample in the original comment. since there are questions
that the manually unswitched version is undefined.
link that code with this:
#include
struct obj {
int __pad;
int i;
};
/* aborts when called with NULL */
int assert_not_null(void *n) {
if (!n)
abort();
}
void bug(struct obj **root, struct obj *dso);
int main() {
struct obj x = {};
struct obj *y =
bug(y, NULL);
}
[Bug tree-optimization/113551] [13 Regression] Miscompilation with -O1 -fno-strict-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113551 --- Comment #14 from Marek Polacek --- I don't see a complete testcase that I could bisect.
[Bug tree-optimization/113551] [13 Regression] Miscompilation with -O1 -fno-strict-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113551
--- Comment #13 from Andrew Pinski ---
(In reply to Yuxuan Shui from comment #12)
> I think this is the MRE:
>
>
> void bug(struct obj *dso) {
> if (>i) {
> if (dso == (void *)0)
> return;
>
> assert_not_null(dso);
> }
> }
Except that is undefined ...
Manually unswitching introduces the undefined behavior in the code.
So even though the code was unswitched before GCC 13 incorrectly, GCC didn't
take into that account before hand.
I am 99% sure what is happening is GCC is see `if (a+1)` and then assuming a
can't be a nullptr. Which is due to undefinedness there of adding 1 to an null
ptr ... (for C that is).
Basically the unswitch is the issue ... Or we maybe we should turn `if (a+1)`
into just `if (a)` ... Likewise for `if (>i)` into `if (a)`
[Bug tree-optimization/113551] [13 Regression] Miscompilation with -O1 -fno-strict-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113551
--- Comment #12 from Yuxuan Shui ---
I think this is the MRE:
void bug(struct obj *dso) {
if (>i) {
if (dso == (void *)0)
return;
assert_not_null(dso);
}
}
[Bug tree-optimization/113551] [13 Regression] Miscompilation with -O1 -fno-strict-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113551
--- Comment #11 from Yuxuan Shui ---
reduced it a bit:
void bug(struct obj **root, struct obj *dso) {
if (>i) {
while (1) {
struct obj *this = *root;
if (dso == (void *)0)
// should return here
return;
if (dso == this)
return;
// shouldn't reach here
assert_not_null(dso);
}
}
}
[Bug tree-optimization/113551] [13 Regression] Miscompilation with -O1 -fno-strict-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113551 --- Comment #10 from Yuxuan Shui --- the manually unswitched version can probably be reduced further.
[Bug tree-optimization/113551] [13 Regression] Miscompilation with -O1 -fno-strict-overflow
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113551
Yuxuan Shui changed:
What|Removed |Added
Summary|[13 Regression] |[13 Regression]
|Miscompilation with -O1 |Miscompilation with -O1
|-funswitch-loops|-fno-strict-overflow
|-fno-strict-overflow|
--- Comment #9 from Yuxuan Shui ---
So, to add some details to people said above, and to make sure I understood
this correctly.
What gcc did here is:
1. move the `if (!>i)` branch out of the loop, duplicate the loop for the
then/else branches.
2. `>i` cannot be 0, so the else branch is eliminated.
3. in the then branch, this condition confused the compiler. it thought since
`>i` is not 0, `dso` is not 0 either, causing the bug.
I diffed `-fdump` outputs and it does match what I expected to see.
(minus is the correct one, plus the incorrect)
@@ -2686,7 +2686,11 @@
;; 4 succs { 5 6 }
;; 6 succs { 3 }
;; 5 succs { 1 }
-Removing basic block 6
+Folding predicate _1 == 0B to 0
+Exported global range table:
+
+_1 : [irange] int * [1, +INF]
+Merging blocks 4 and 6
* * *
this made me think whether loop unswitching is actually relevant here. since
12.3 also unswitches this loop, but doesn't miscompile. so I manually
unswitched the loop:
void bug(struct obj **root, struct obj *dso) {
if (!>i) {
while (1) {
struct obj *this = *root;
if (dso == (void *)0)
// should return here
return;
if (dso == this)
return;
// shouldn't reach here
assert_not_null(dso);
break;
}
} else {
while (1) {
struct obj *this = *root;
if (dso == (void *)0)
// should return here
return;
if (dso == this)
return;
// shouldn't reach here
assert_not_null(dso);
}
}
}
and now the miscompile happens without -funswitch-loops. I wonder if this
happens on trunk as well.
looks this this is a -fno-strict-overflow related ranger bug.
