gcc rev249427, x86_64. > cat f.cpp int x = -106; int main() { // -123 - (0x8000000000000000 - -1) bool a = -123 - ((9223372036854775806 ^ ~(x && true)) - -1); return 0; } > gcc -fsanitize=undefined f.cpp -o out > ./out f.cpp:4:41: runtime error: negation of -9223372036854775808 cannot be represented in type 'long int'; cast to an unsigned type to negate this value to itself f.cpp:4:17: runtime error: signed integer overflow: -9223372036854775808 + -124 cannot be represented in type 'long int'
Confirmed, mine.
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.
Tomorrow, unless Marek beats me.
Started with commit 69693ea7b7ed45a12cbd505b2a66257fd4e81669 Author: rguenth <rguenth@138bc75d-0d04-0410-961f-82ee72b054a4> Date: Fri Jun 26 10:59:27 2015 +0000 2015-06-26 Richard Biener <rguenther@suse.de> * 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.
It's the (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0 check in fold_negate_expr_1 that makes the difference w/ and w/o ubsan.
We basically have -123 - (LONG_MIN + 1) but it's being folded to -LONG_MIN + -124 which is of course not correct.
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.
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; + } } }
Except that isn't really correct yet...
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; + } } }
That causes miscompiled cc1plus. Richi, any ideas?
Mine.
C/C++ testcase: int x = -106; int main() { // -123 - (0x8000000000000000 - -1) return (-123 - ((9223372036854775806 ^ ~(x && 1)) - -1)) == 0; }
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.
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.
Author: rguenth Date: Thu Aug 3 11:52:00 2017 New Revision: 250853 URL: https://gcc.gnu.org/viewcvs?rev=250853&root=gcc&view=rev Log: 2017-08-03 Richard Biener <rguenther@suse.de> 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
Author: aldyh Date: Wed Sep 13 16:25:51 2017 New Revision: 252277 URL: https://gcc.gnu.org/viewcvs?rev=252277&root=gcc&view=rev Log: 2017-08-03 Richard Biener <rguenther@suse.de> 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
Fixed meanwhile.