[Bug tree-optimization/97505] [11 Regression] ICE in extract_range_basic, at vr-values.c:1439 since r11-4130-g16e4f1ad44e3c00b8b73c9e4ade3d236ea7044a8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97505 --- Comment #7 from CVS Commits --- The master branch has been updated by Aldy Hernandez : https://gcc.gnu.org/g:f3a3327fe3d4e20a8fe863c2a3ad949864191f5d commit r11-4604-gf3a3327fe3d4e20a8fe863c2a3ad949864191f5d Author: Aldy Hernandez Date: Mon Nov 2 11:34:47 2020 +0100 Add test for PR97505. gcc/testsuite/ChangeLog: PR tree-optimization/97505 * gcc.dg/pr97505.c: New test.
[Bug tree-optimization/97505] [11 Regression] ICE in extract_range_basic, at vr-values.c:1439 since r11-4130-g16e4f1ad44e3c00b8b73c9e4ade3d236ea7044a8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97505 Aldy Hernandez changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #6 from Aldy Hernandez --- fixed
[Bug tree-optimization/97505] [11 Regression] ICE in extract_range_basic, at vr-values.c:1439 since r11-4130-g16e4f1ad44e3c00b8b73c9e4ade3d236ea7044a8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97505
--- Comment #5 from CVS Commits ---
The master branch has been updated by Aldy Hernandez :
https://gcc.gnu.org/g:054d7b9f6f6816a83dcadfdfe2532795cae04ff3
commit r11-4532-g054d7b9f6f6816a83dcadfdfe2532795cae04ff3
Author: Aldy Hernandez
Date: Thu Oct 22 08:39:04 2020 +0200
Selectively trap if ranger and vr-values disagree on range builtins.
The UBSAN builtins degrade into PLUS/MINUS/MULT and call
extract_range_from_binary_expr, which as the PR shows, can special
case some symbolics which the ranger doesn't currently handle.
Looking at vr_values::extract_range_builtin(), I see that every single
place where we ask for a range, we bail on non-integers (symbolics,
etc). That is, with the exception of the UBSAN builtins.
Since this seems to be particular to UBSAN, we could still go with the
original plan of removing the duplicity in ranger vs vr-values, but
leave in the UBSAN builtin handling. This isn't ideal, as we'd like
to remove all the common code, but I'd be willing to put up with UBSAN
duplication for the time being.
This patch disables the assert on the UBSAN builtins, while still
trapping if any other differences are found between the vr_values and
the ranger versions of builtin range handling.
As a follow-up, once Fedora can test this approach, I'll remove all
the builtin code from extract_range_builtin, with the exception of the
UBSAN stuff (renaming it to extract_range_ubsan_builtin).
Since the builtin code has proven fickle across architectures, I've
tested this with {-m32,-m64,-fsanitize=signed-integer-overflow} on
x86, ppc64le, and aarch64. I think this should be enough. If it
isn't, we can revert the patch, and leave the duplicate code until
the next release cycle when hopefully vr_values, evrp, and friends
will all be overhauled.
gcc/ChangeLog:
PR tree-optimization/97505
* vr-values.c (vr_values::extract_range_basic): Enable
trap again for everything except UBSAN builtins.
[Bug tree-optimization/97505] [11 Regression] ICE in extract_range_basic, at vr-values.c:1439 since r11-4130-g16e4f1ad44e3c00b8b73c9e4ade3d236ea7044a8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97505
--- Comment #4 from Aldy Hernandez ---
Looking at vr_values::extract_range_builtin(), I see that every single place
where we use ask for a range, we bail on non-integers (symbolics, etc). That
is, with the exception of the UBSAN builtins.
The UBSAN builtins degrade into PLUS/MINUS/MULT and call
extract_range_from_binary_expr, which as I've shown, can special case some
symbolics which the ranger doesn't currently handle.
Since this is a UBSAN issue, we could still go with the original plan of
removing the duplicity in ranger vs vr-values, but we'd still have to leave in
the UBSAN builtin handling, perhaps as vr_values::extract_ubsan_builtin().
This isn't ideal, as we'd like to remove all the common code, but I'd be
willing to put up with UBSAN duplication for the time being.
A possible solution for this PR would be to disable the assert on the UBSAN
builtins:
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 67c88006f13..3021f619319 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -1432,14 +1432,17 @@ vr_values::extract_range_basic (value_range_equiv *vr,
gimple *stmt)
if (is_gimple_call (stmt) && extract_range_builtin (vr, stmt))
{
+ combined_fn cfn = gimple_call_combined_fn (stmt);
+ if (cfn == CFN_UBSAN_CHECK_ADD
+ || cfn == CFN_UBSAN_CHECK_SUB
+ || cfn == CFN_UBSAN_CHECK_MUL)
+ return;
+
value_range_equiv tmp;
And then, as a follow-up, remove all the builtin code from
extract_range_builtin, with the exception of the UBSAN stuff (renaming it to
extract_ubsan_builtin).
Now... the question is, whether this is worth it, since the plan for next
stage1 is to overhaul evrp and vrp, causing vr_values and friends to be
entirely removed. If we leave the duplicate code in, we'll have to remember to
update the vr_values and ranger versions of this code in sync for the next
year. I'm guessing this code doesn't change much??
If we do decide to remove it starting with the proposed patch above, we'd have
to do more extensive testing to make sure shit doesn't break. Perhaps testing
on {-m32,-m64,-fsanitize=signed-integer-overflow} on x86, ppc64, and possibly
other slowpokes such as aarch64.
I'm torn. It may be a lot of testing and possible breakage in return for
removing 90% of code that will hopefully be entirely removed during the next
cycle.
Thoughts?
[Bug tree-optimization/97505] [11 Regression] ICE in extract_range_basic, at vr-values.c:1439 since r11-4130-g16e4f1ad44e3c00b8b73c9e4ade3d236ea7044a8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97505 --- Comment #3 from CVS Commits --- The master branch has been updated by Andrew Macleod : https://gcc.gnu.org/g:292c92715b282f7c6617c94351d3e38ec027d637 commit r11-4141-g292c92715b282f7c6617c94351d3e38ec027d637 Author: Andrew MacLeod Date: Tue Oct 20 16:55:14 2020 -0400 Temporarily disable trap in in extract_range_builtin check. Until we figure out how to adjust ubsan for symbolics, disable the trap. gcc/ChangeLog: PR tree-optimization/97505 * vr-values.c (vr_values::extract_range_basic): Trap if vr_values version disagrees with range_of_builtin_call.
[Bug tree-optimization/97505] [11 Regression] ICE in extract_range_basic, at vr-values.c:1439 since r11-4130-g16e4f1ad44e3c00b8b73c9e4ade3d236ea7044a8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97505 --- Comment #2 from Aldy Hernandez --- Created attachment 49411 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49411=edit proposed patch We should disable the assert while this PR is fixed, so it doesn't hold anyone else up. Patch needs a testcase.
[Bug tree-optimization/97505] [11 Regression] ICE in extract_range_basic, at vr-values.c:1439 since r11-4130-g16e4f1ad44e3c00b8b73c9e4ade3d236ea7044a8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97505
--- Comment #1 from Aldy Hernandez ---
We are calculating ranges for the following:
(gdb) dd stmt
_18 = .UBSAN_CHECK_SUB (_58, _57);
which gets turned into a MINUS_EXPR. Then we call
extract_range_from_binary_expr on the MINUS_EXPR:
/* Pretend the arithmetics is wrapping. If there is
any overflow, we'll complain, but will actually do
wrapping operation. */
flag_wrapv = 1;
extract_range_from_binary_expr (vr, subcode, type,
gimple_call_arg (stmt, 0),
gimple_call_arg (stmt, 1));
flag_wrapv = saved_flag_wrapv;
In extract_range_from_binary_expr, we calculate the range for _58 and _57
respectively as:
(gdb) dd vr0
integer(kind=8) [-INF, _57 - 1]
(gdb) dd vr1
integer(kind=8) [_58 + 1, +INF]
Which extract_range_from_binary_expr can then use to reduce the MINUS_EXPR to
~[0,0]:
/* If we didn't derive a range for MINUS_EXPR, and
op1's range is ~[op0,op0] or vice-versa, then we
can derive a non-null range. This happens often for
pointer subtraction. */
if (vr->varying_p ()
&& (code == MINUS_EXPR || code == POINTER_DIFF_EXPR)
&& TREE_CODE (op0) == SSA_NAME
&& ((vr0.kind () == VR_ANTI_RANGE
&& vr0.min () == op1
&& vr0.min () == vr0.max ())
|| (vr1.kind () == VR_ANTI_RANGE
&& vr1.min () == op0
&& vr1.min () == vr1.max (
{
vr->set_nonzero (expr_type);
vr->equiv_clear ();
}
The ranger version is not handling these symbolics.
[Bug tree-optimization/97505] [11 Regression] ICE in extract_range_basic, at vr-values.c:1439 since r11-4130-g16e4f1ad44e3c00b8b73c9e4ade3d236ea7044a8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97505 Martin Liška changed: What|Removed |Added Known to work||10.2.0 Known to fail||11.0 Ever confirmed|0 |1 Priority|P3 |P1 Status|UNCONFIRMED |NEW Target Milestone|--- |11.0 Last reconfirmed||2020-10-20
