[Bug sanitizer/81148] UBSAN: two more false positives

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

Richard Biener  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #18 from Richard Biener  ---
Fixed meanwhile.

[Bug sanitizer/81148] UBSAN: two more false positives

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #17 from Aldy Hernandez  ---
Author: aldyh
Date: Wed Sep 13 16:25:51 2017
New Revision: 252277

URL: https://gcc.gnu.org/viewcvs?rev=252277=gcc=rev
Log:
2017-08-03 Richard Biener  

PR middle-end/81148
* fold-const.c (split_tree): Add minus_var and minus_con
arguments, remove unused loc arg.  Never generate NEGATE_EXPRs
here but always use minus_*.
(associate_trees): Assert we never associate with MINUS_EXPR
and NULL first operand.  Do not recurse for PLUS_EXPR operands
when associating as MINUS_EXPR either.
(fold_binary_loc): Track minus_var and minus_con.

* c-c++-common/ubsan/pr81148.c: New testcase.

Added:
branches/range-gen2/gcc/testsuite/c-c++-common/ubsan/pr81148.c
Modified:
branches/range-gen2/gcc/ChangeLog
branches/range-gen2/gcc/fold-const.c
branches/range-gen2/gcc/testsuite/ChangeLog

[Bug sanitizer/81148] UBSAN: two more false positives

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #16 from Richard Biener  ---
Author: rguenth
Date: Thu Aug  3 11:52:00 2017
New Revision: 250853

URL: https://gcc.gnu.org/viewcvs?rev=250853=gcc=rev
Log:
2017-08-03 Richard Biener  

PR middle-end/81148
* fold-const.c (split_tree): Add minus_var and minus_con
arguments, remove unused loc arg.  Never generate NEGATE_EXPRs
here but always use minus_*.
(associate_trees): Assert we never associate with MINUS_EXPR
and NULL first operand.  Do not recurse for PLUS_EXPR operands
when associating as MINUS_EXPR either.
(fold_binary_loc): Track minus_var and minus_con.

* c-c++-common/ubsan/pr81148.c: New testcase.

Added:
trunk/gcc/testsuite/c-c++-common/ubsan/pr81148.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/fold-const.c
trunk/gcc/testsuite/ChangeLog

[Bug sanitizer/81148] UBSAN: two more false positives

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #15 from Marek Polacek  ---
My other idea was to pass bool ok to split_tree and if it sees that it cannot
negate_expr something, set it to false, so that we don't change the expression
after split_tree has been called.  But if it worked I think I would've posted
the patch already.

[Bug sanitizer/81148] UBSAN: two more false positives

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #14 from Richard Biener  ---
Folding

-123 - (((long int) ~(x != 0) ^ 9223372036854775806) + 1)

results in split_tree of (((long int) ~(x != 0) ^ 9223372036854775806) + 1)
returning -((long int) ~(x != 0) ^ 9223372036854775806) and minus_lit1 == 1.

The bug is obviously negating of 'var'.

I don't see any way around handling var and con like lit, thus adding
minus_var and minus_con and appropriately using MINUS_EXPR when
associating.

Bah.

[Bug sanitizer/81148] UBSAN: two more false positives

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #13 from Richard Biener  ---
C/C++ testcase:

int x = -106;
int main()
{
  // -123 - (0x8000 - -1)
  return (-123 - ((9223372036854775806 ^ ~(x && 1)) - -1)) == 0;
}

[Bug sanitizer/81148] UBSAN: two more false positives

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

Richard Biener  changed:

   What|Removed |Added

   Assignee|mpolacek at gcc dot gnu.org|rguenth at gcc dot 
gnu.org

--- Comment #12 from Richard Biener  ---
Mine.

[Bug sanitizer/81148] UBSAN: two more false positives

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #11 from Marek Polacek  ---
That causes miscompiled cc1plus.  Richi, any ideas?

[Bug sanitizer/81148] UBSAN: two more false positives

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #10 from Marek Polacek  ---
It should've been

--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -874,9 +874,24 @@ split_tree (location_t loc, tree in, tree type, enum
tree_code code,
}
   if (var)
{
- /* Convert to TYPE before negating.  */
- var = fold_convert_loc (loc, type, var);
- var = negate_expr (var);
+ if ((*litp == NULL_TREE
+  && *conp == NULL_TREE
+  && *minus_litp == NULL_TREE)
+ || !INTEGRAL_TYPE_P (type)
+ || TYPE_OVERFLOW_WRAPS (type))
+   {
+ /* Convert to TYPE before negating.  */
+ var = fold_convert_loc (loc, type, var);
+ var = negate_expr (var);
+   }
+ else
+   {
+ /* Go back to blank slate.  */
+ var = in;
+ *conp = NULL_TREE;
+ *litp = NULL_TREE;
+ *minus_litp = NULL_TREE;
+   }
}
 }

[Bug sanitizer/81148] UBSAN: two more false positives

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #9 from Marek Polacek  ---
Except that isn't really correct yet...

[Bug sanitizer/81148] UBSAN: two more false positives

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #8 from Marek Polacek  ---
Seems like we need something similar to

--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -874,9 +874,23 @@ split_tree (location_t loc, tree in, tree type, enum
tree_code code,
}
   if (var)
{
- /* Convert to TYPE before negating.  */
- var = fold_convert_loc (loc, type, var);
- var = negate_expr (var);
+ if ((*litp == NULL_TREE
+  && *conp == NULL_TREE
+  && *minus_litp == NULL_TREE)
+ || negate_expr_p (var))
+   {
+ /* Convert to TYPE before negating.  */
+ var = fold_convert_loc (loc, type, var);
+ var = negate_expr (var);
+   }
+ else
+   {
+ /* Go back to blank slate.  */
+ var = in;
+ *conp = NULL_TREE;
+ *litp = NULL_TREE;
+ *minus_litp = NULL_TREE;
+   }
}
 }

[Bug sanitizer/81148] UBSAN: two more false positives

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #7 from Marek Polacek  ---
So were in "associate:", where
lit0 = -123
lit1 = -1
which is associated to
lit0 = -124
and
var0 = null
var1 = -((long int) ~(x != 0) ^ 9223372036854775806)
which is associated to
var0 = -((long int) ~(x != 0) ^ 9223372036854775806)

Then
 9765   con0 = associate_trees (loc, con0, lit0, code, atype);
 9766   return
 9767 fold_convert_loc (loc, type, associate_trees (loc, var0,
con0,
 9768   code,
atype));

creates the bogus expression with overflow.

[Bug sanitizer/81148] UBSAN: two more false positives

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #6 from Marek Polacek  ---
We basically have
-123 - (LONG_MIN + 1)
but it's being folded to
-LONG_MIN + -124
which is of course not correct.

[Bug sanitizer/81148] UBSAN: two more false positives

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #5 from Marek Polacek  ---
It's the
(flag_sanitize & SANITIZE_SI_OVERFLOW) == 0
check in fold_negate_expr_1 that makes the difference w/ and w/o ubsan.

[Bug sanitizer/81148] UBSAN: two more false positives

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #4 from Marek Polacek  ---
Started with

commit 69693ea7b7ed45a12cbd505b2a66257fd4e81669
Author: rguenth 
Date:   Fri Jun 26 10:59:27 2015 +

2015-06-26  Richard Biener  

* fold-const.c (fold_binary_loc): Remove -A CMP -B -> A CMP B
and -A CMP CST -> A CMP -CST which is redundant with a pattern
in match.pd.
Move (A | C) == D where C & ~D != 0 -> 0, (X ^ Y) ==/!= 0 -> X
==/!= Y,
(X ^ Y) ==/!= {Y,X} -> {X,Y} ==/!= 0 and
(X ^ C1) op C2 -> X op (C1 ^ C2) to ...
* match.pd: ... patterns here.

* gcc.dg/tree-ssa/forwprop-25.c: Adjust.


git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@225007
138bc75d-0d04-0410-961f-82ee72b054a4

it seems.

[Bug sanitizer/81148] UBSAN: two more false positives

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #3 from Richard Biener  ---
Tomorrow, unless Marek beats me.

[Bug sanitizer/81148] UBSAN: two more false positives

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

--- Comment #2 from Richard Biener  ---
w/o ubsan we fold this all the way to one.  With ubsan we fold it as

  bool a = -((long int) ~(x != 0) ^ 9223372036854775806) + -124 != 0;

so there's some stupid TYPE_OVERFLOW_SANITIZED check in the way somewhere.

It looks association related again, I will have a look.

[Bug sanitizer/81148] UBSAN: two more false positives

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81148

Marek Polacek  changed:

   What|Removed |Added

 Status|UNCONFIRMED |ASSIGNED
   Last reconfirmed||2017-06-21
 CC||mpolacek at gcc dot gnu.org
   Assignee|unassigned at gcc dot gnu.org  |mpolacek at gcc dot 
gnu.org
 Ever confirmed|0   |1

--- Comment #1 from Marek Polacek  ---
Confirmed, mine.