[Bug sanitizer/81065] UBSAN: false positive as a result of distribution involving different types

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
  Known to work||7.2.1
 Resolution|--- |FIXED
   Target Milestone|--- |7.3
  Known to fail||7.2.0

--- Comment #7 from Richard Biener  ---
Fixed for 7.3+.

[Bug sanitizer/81065] UBSAN: false positive as a result of distribution involving different types

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener  ---
Author: rguenth
Date: Mon Aug 28 12:49:55 2017
New Revision: 251381

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

Backport from mainline
2017-06-14  Richard Biener  

PR middle-end/81088
* fold-const.c (split_tree): Drop TREE_OVERFLOW flag from
literal constants.
(fold_binary_loc): When associating do not treat pre-existing
TREE_OVERFLOW on literal constants as a reason to allow
TREE_OVERFLOW on associated literal constants.

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

2017-06-13  Richard Biener  

PR middle-end/81065
* fold-const.c (extract_muldiv_1): Remove bogus distribution
case of C * (x * C2 + C3).
(fold_addr_of_array_ref_difference): Properly fold index difference.

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

2017-06-08  Marek Polacek  

PR sanitize/80932
* c-c++-common/ubsan/pr80932.c: Test with ints, not with long ints.

2017-06-07  Marek Polacek  

PR sanitizer/80932
* fold-const.c (extract_muldiv_1) : Add
TYPE_OVERFLOW_WRAPS check. 

* c-c++-common/ubsan/pr80932.c: New test.

Added:
branches/gcc-7-branch/gcc/testsuite/c-c++-common/ubsan/pr80932.c
branches/gcc-7-branch/gcc/testsuite/c-c++-common/ubsan/pr81065.c
branches/gcc-7-branch/gcc/testsuite/c-c++-common/ubsan/pr81088.c
Modified:
branches/gcc-7-branch/gcc/ChangeLog
branches/gcc-7-branch/gcc/fold-const.c
branches/gcc-7-branch/gcc/testsuite/ChangeLog

[Bug sanitizer/81065] UBSAN: false positive as a result of distribution involving different types

[babokin at gmail dot com]

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

--- Comment #5 from Dmitry Babokin  ---
Thanks for blazingly fast fixes. This enables filing more bugs, as it's
difficult to distinguish between unrelated fails before one of them is actually
fixed.

[Bug sanitizer/81065] UBSAN: false positive as a result of distribution involving different types

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

  Known to work||8.0

--- Comment #4 from Richard Biener  ---
Fixed on trunk sofar.

[Bug sanitizer/81065] UBSAN: false positive as a result of distribution involving different types

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Biener  ---
Author: rguenth
Date: Tue Jun 13 07:07:08 2017
New Revision: 249144

URL: https://gcc.gnu.org/viewcvs?rev=249144=gcc=rev
Log:
2017-06-13  Richard Biener  

PR middle-end/81065
* fold-const.c (extract_muldiv_1): Remove bogus distribution
case of C * (x * C2 + C3).
(fold_addr_of_array_ref_difference): Properly fold index difference.

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

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

[Bug sanitizer/81065] UBSAN: false positive as a result of distribution involving different types

[rguenth at gcc dot gnu.org]

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

Richard Biener  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  ---
Oh, and I think it's fine to always distribute

 CST * (x * CST1 + CST2)

in case x * CST1 and CST2 have the same sign.

So the previous fix was incomplete and we now hit

  /* If we were able to eliminate our operation from the first side,
 apply our operation to the second side and reform the PLUS.  */
  if (t1 != 0 && (TREE_CODE (t1) != code || code == MULT_EXPR))
return fold_build2 (tcode, ctype, fold_convert (ctype, t1), op1);

where the code immediately following it is correct:

  /* The last case is if we are a multiply.  In that case, we can
 apply the distributive law to commute the multiply and addition
 if the multiplication of the constants doesn't overflow
 and overflow is defined.  With undefined overflow
 op0 * c might overflow, while (op0 + orig_op1) * c doesn't.  */
  if (code == MULT_EXPR && TYPE_OVERFLOW_WRAPS (ctype))
return fold_build2 (tcode, ctype,
fold_build2 (code, ctype,
 fold_convert (ctype, op0),
 fold_convert (ctype, c)),
op1);


Mine.  Testing

Index: gcc/fold-const.c
===
--- gcc/fold-const.c(revision 249112)
+++ gcc/fold-const.c(working copy)
@@ -6243,7 +6243,7 @@ extract_muldiv_1 (tree t, tree c, enum t

   /* If we were able to eliminate our operation from the first side,
 apply our operation to the second side and reform the PLUS.  */
-  if (t1 != 0 && (TREE_CODE (t1) != code || code == MULT_EXPR))
+  if (t1 != 0 && TREE_CODE (t1) != code)
return fold_build2 (tcode, ctype, fold_convert (ctype, t1), op1);

   /* The last case is if we are a multiply.  In that case, we can

[Bug sanitizer/81065] UBSAN: false positive as a result of distribution involving different types

[rguenth at gcc dot gnu.org]

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

Richard Biener  changed:

   What|Removed |Added

   Keywords||wrong-code
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2017-06-12
 Ever confirmed|0   |1

--- Comment #1 from Richard Biener  ---
Thanks for reporting these bugs, they are all latent wrong-code even w/o UBSAN.

.original w/o ubsan:

;; Function foo (null)
;; enabled by -tree-original


{
  return (int) x * 14165900 + -77663775;
}


and as usual, it's extract_muldiv ...

(gdb) p debug_generic_expr (op0)
(int) x * 1652 + -9057
$1 = void
(gdb) p debug_generic_expr (op1)
8575

turning that into

(int) x * 14165900 + -77663775


it really means that this kind of distribution is never safe unless we rewrite
the inner multiplication into unsigned arithmetic (given the cast of x we
do have an idea about the value range of the other operand so we could
handle some cases -- but I'd rather not do that in extract_muldiv but
in a match.pd pattern).

I'd love to say bye-bye to extract_muldiv in it's current state...