Similar to #80932, but note that x is unsigned char, it's essential. > cat f.cpp unsigned char x = 154; int foo() { // 8575 * (254408 - 9057) = 8575 * 245351 = 2103884825 = 0x7d66bc19 return 8575 * (1652 * x - 9057); } int main() { foo(); return 0; } > g++ -fsanitize=undefined -O0 f.cpp -o out > ./out f.cpp:4:33: runtime error: signed integer overflow: 154 * 14165900 cannot be represented in type 'int' f.cpp:4:33: runtime error: signed integer overflow: -2113418696 + -77663775 cannot be represented in type 'int'
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...
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
Author: rguenth Date: Tue Jun 13 07:07:08 2017 New Revision: 249144 URL: https://gcc.gnu.org/viewcvs?rev=249144&root=gcc&view=rev Log: 2017-06-13 Richard Biener <rguenther@suse.de> 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
Fixed on trunk sofar.
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.
Author: rguenth Date: Mon Aug 28 12:49:55 2017 New Revision: 251381 URL: https://gcc.gnu.org/viewcvs?rev=251381&root=gcc&view=rev Log: 2017-08-28 Richard Biener <rguenther@suse.de> Backport from mainline 2017-06-14 Richard Biener <rguenther@suse.de> 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 <rguenther@suse.de> 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 <polacek@redhat.com> PR sanitize/80932 * c-c++-common/ubsan/pr80932.c: Test with ints, not with long ints. 2017-06-07 Marek Polacek <polacek@redhat.com> PR sanitizer/80932 * fold-const.c (extract_muldiv_1) <case MINUS_EXPR>: 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
Fixed for 7.3+.