Here, do_narrow and convert_to_integer_1 is demoting signed types to unsigned, e.g. for i = i - lmin where i is int and lmin is long int, so what we should produce is i = (int) ((long int) i - lmin) but instead it produces i = (int) ((unsigned int) i - (unsigned int) lmin); which hides the overflow. Similarly for NEGATE_EXPR. This patch prevents such demoting when the sanitizer is on.
There still might be a similar issue with division or shifting, but I couldn't trigger that. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2017-09-01 Marek Polacek <pola...@redhat.com> PR sanitizer/82072 * convert.c (do_narrow): When sanitizing signed integer overflows, bail out for signed types. (convert_to_integer_1) <case NEGATE_EXPR>: Likewise. * c-c++-common/ubsan/pr82072.c: New test. diff --git gcc/convert.c gcc/convert.c index 22152cae79b..139d790fd98 100644 --- gcc/convert.c +++ gcc/convert.c @@ -434,6 +434,13 @@ do_narrow (location_t loc, typex = lang_hooks.types.type_for_size (TYPE_PRECISION (typex), TYPE_UNSIGNED (typex)); + /* The type demotion below might cause doing unsigned arithmetic + instead of signed, and thus hide overflow bugs. */ + if ((ex_form == PLUS_EXPR || ex_form == MINUS_EXPR) + && !TYPE_UNSIGNED (typex) + && sanitize_flags_p (SANITIZE_SI_OVERFLOW)) + return NULL_TREE; + /* But now perhaps TYPEX is as wide as INPREC. In that case, do nothing special here. (Otherwise would recurse infinitely in convert. */ @@ -895,7 +902,12 @@ convert_to_integer_1 (tree type, tree expr, bool dofold) TYPE_UNSIGNED (typex)); if (!TYPE_UNSIGNED (typex)) - typex = unsigned_type_for (typex); + { + /* Using unsigned arithmetic may hide overflow bugs. */ + if (sanitize_flags_p (SANITIZE_SI_OVERFLOW)) + break; + typex = unsigned_type_for (typex); + } return convert (type, fold_build1 (ex_form, typex, convert (typex, diff --git gcc/testsuite/c-c++-common/ubsan/pr82072.c gcc/testsuite/c-c++-common/ubsan/pr82072.c index e69de29bb2d..d5683406b14 100644 --- gcc/testsuite/c-c++-common/ubsan/pr82072.c +++ gcc/testsuite/c-c++-common/ubsan/pr82072.c @@ -0,0 +1,19 @@ +/* PR sanitizer/82072 */ +/* { dg-do run } */ +/* { dg-options "-fsanitize=signed-integer-overflow" } */ + +int +main () +{ + long long l = -__LONG_LONG_MAX__ - 1; + int i = 0; + asm volatile ("" : "+r" (i)); + i -= l; + asm volatile ("" : "+r" (i)); + i = -l; + asm volatile ("" : "+r" (i)); + return 0; +} + +/* { dg-output "signed integer overflow: 0 - -9223372036854775808 cannot be represented in type 'long long int'\[^\n\r]*(\n|\r\n|\r)" } */ +/* { dg-output "\[^\n\r]*negation of -9223372036854775808 cannot be represented in type 'long long int'\[^\n\r]*; cast to an unsigned type to negate this value to itself" } */ Marek