Hi!
As mentioned in the PR, on the following testcase we cp_save_expr in order
to avoid evaluating -1 * __builtin_expect (({ ... }), ...) twice and use
the SAVE_EXPR in the original shift as well as in a comparison, but then
we attempt to optimize the comparison by grabbing parts of the SAVE_EXPR,
create comparison out of it and building another SAVE_EXPR around it.
As unshare_expr/mostly_copy_tree_r doesn't unshare STATEMENT_LISTs, we end
up with the STATEMENT_LIST containing DEBUG_BEGIN_STMT and x != 0 multiple
times in the IL and as gimplification is destructive, we destroy the
STATEMENT_LIST the first time and second time ICE on it.
I don't see other way how to resolve this than to avoid this weirdo
optimization, I believe the optimization is from pre-GIMPLE era where we
didn't have hundreds of passes that can optimize it before expansion.
Bootstrapped/regtested on x86_64-linux and i686-linux, haven't seen any
regressions, ok for trunk?
2018-04-05 Jakub Jelinek <ja...@redhat.com>
PR sanitizer/85213
* fold-const.c (twoval_comparison_p): Remove SAVE_P argument and don't
look through SAVE_EXPRs with non-side-effects argument. Adjust
recursive calls.
(fold_comparison): Adjust twoval_comparison_p caller, don't handle
save_p here.
* c-c++-common/ubsan/pr85213.c: New test.
--- gcc/fold-const.c.jj 2018-03-29 12:37:23.887476367 +0200
+++ gcc/fold-const.c 2018-04-05 11:30:28.996962481 +0200
@@ -115,7 +115,7 @@ static tree negate_expr (tree);
static tree associate_trees (location_t, tree, tree, enum tree_code, tree);
static enum comparison_code comparison_to_compcode (enum tree_code);
static enum tree_code compcode_to_comparison (enum comparison_code);
-static int twoval_comparison_p (tree, tree *, tree *, int *);
+static int twoval_comparison_p (tree, tree *, tree *);
static tree eval_subst (location_t, tree, tree, tree, tree, tree);
static tree optimize_bit_field_compare (location_t, enum tree_code,
tree, tree, tree);
@@ -3549,13 +3549,12 @@ operand_equal_for_comparison_p (tree arg
two different values, which will be stored in *CVAL1 and *CVAL2; if
they are nonzero it means that some operands have already been found.
No variables may be used anywhere else in the expression except in the
- comparisons. If SAVE_P is true it means we removed a SAVE_EXPR around
- the expression and save_expr needs to be called with CVAL1 and CVAL2.
+ comparisons.
If this is true, return 1. Otherwise, return zero. */
static int
-twoval_comparison_p (tree arg, tree *cval1, tree *cval2, int *save_p)
+twoval_comparison_p (tree arg, tree *cval1, tree *cval2)
{
enum tree_code code = TREE_CODE (arg);
enum tree_code_class tclass = TREE_CODE_CLASS (code);
@@ -3568,39 +3567,23 @@ twoval_comparison_p (tree arg, tree *cva
|| code == COMPOUND_EXPR))
tclass = tcc_binary;
- else if (tclass == tcc_expression && code == SAVE_EXPR
- && ! TREE_SIDE_EFFECTS (TREE_OPERAND (arg, 0)))
- {
- /* If we've already found a CVAL1 or CVAL2, this expression is
- two complex to handle. */
- if (*cval1 || *cval2)
- return 0;
-
- tclass = tcc_unary;
- *save_p = 1;
- }
-
switch (tclass)
{
case tcc_unary:
- return twoval_comparison_p (TREE_OPERAND (arg, 0), cval1, cval2, save_p);
+ return twoval_comparison_p (TREE_OPERAND (arg, 0), cval1, cval2);
case tcc_binary:
- return (twoval_comparison_p (TREE_OPERAND (arg, 0), cval1, cval2, save_p)
- && twoval_comparison_p (TREE_OPERAND (arg, 1),
- cval1, cval2, save_p));
+ return (twoval_comparison_p (TREE_OPERAND (arg, 0), cval1, cval2)
+ && twoval_comparison_p (TREE_OPERAND (arg, 1), cval1, cval2));
case tcc_constant:
return 1;
case tcc_expression:
if (code == COND_EXPR)
- return (twoval_comparison_p (TREE_OPERAND (arg, 0),
- cval1, cval2, save_p)
- && twoval_comparison_p (TREE_OPERAND (arg, 1),
- cval1, cval2, save_p)
- && twoval_comparison_p (TREE_OPERAND (arg, 2),
- cval1, cval2, save_p));
+ return (twoval_comparison_p (TREE_OPERAND (arg, 0), cval1, cval2)
+ && twoval_comparison_p (TREE_OPERAND (arg, 1), cval1, cval2)
+ && twoval_comparison_p (TREE_OPERAND (arg, 2), cval1, cval2));
return 0;
case tcc_comparison:
@@ -8781,9 +8764,8 @@ fold_comparison (location_t loc, enum tr
if (TREE_CODE (arg1) == INTEGER_CST && TREE_CODE (arg0) != INTEGER_CST)
{
tree cval1 = 0, cval2 = 0;
- int save_p = 0;
- if (twoval_comparison_p (arg0, &cval1, &cval2, &save_p)
+ if (twoval_comparison_p (arg0, &cval1, &cval2)
/* Don't handle degenerate cases here; they should already
have been handled anyway. */
&& cval1 != 0 && cval2 != 0
@@ -8856,12 +8838,6 @@ fold_comparison (location_t loc, enum tr
return omit_one_operand_loc (loc, type, integer_one_node,
arg0);
}
- if (save_p)
- {
- tem = save_expr (build2 (code, type, cval1, cval2));
- protected_set_expr_location (tem, loc);
- return tem;
- }
return fold_build2_loc (loc, code, type, cval1, cval2);
}
}
--- gcc/testsuite/c-c++-common/ubsan/pr85213.c.jj 2018-04-05
11:43:53.157045995 +0200
+++ gcc/testsuite/c-c++-common/ubsan/pr85213.c 2018-04-05 11:43:33.954043995
+0200
@@ -0,0 +1,9 @@
+/* PR sanitizer/85213 */
+/* { dg-do compile } */
+/* { dg-options "-O1 -fsanitize=undefined -fcompare-debug" } */
+
+int
+foo (int x)
+{
+ return (__builtin_expect (({ x != 0; }) ? 0 : 1, 3) == 0) * -1 << 0;
+}
Jakub
