On December 4, 2014 4:45:25 PM CET, Marek Polacek <pola...@redhat.com> wrote:
>The PR shows a case in which fold introduces undefined behavior in a
>valid program, because what it does here is
>-(long int) (ul + ULONG_MAX) - 1 ->
>~(long int) (ul + ULONG_MAX) ->
>-(long int) ul
>But the latter transformation is wrong if ul is unsigned long and
>equals
>LONG_MAX + 1UL, because that introduces -LONG_MIN, and ubsan/-ftrapv
>errors on that, even though the program is valid.
>(Converting LONG_MAX + 1UL to long is implementation-defined.)
Implementation defined behavior is OK.
Rather than disabling the transform I'd rather make it defined using unsigned
types.
Richard.
>I tried to limit the folding a bit so that it doesn't creep in UB,
>but I would really appreciate second pair of eyes here...
>
>Bootstrapped/regtested on ppc64-linux, ok for trunk?
>
>2014-12-04 Marek Polacek <pola...@redhat.com>
>
> PR middle-end/56917
> * fold-const.c (fold_unary_loc): Don't introduce undefined behavior
> when transforming ~ (A - 1) or ~ (A + -1) to -A.
>
> * c-c++-common/ubsan/pr56917.c: New test.
>
>diff --git gcc/fold-const.c gcc/fold-const.c
>index 17eb5bb..1e56b28 100644
>--- gcc/fold-const.c
>+++ gcc/fold-const.c
>@@ -8140,7 +8140,16 @@ fold_unary_loc (location_t loc, enum tree_code
>code, tree type, tree op0)
> && ((TREE_CODE (arg0) == MINUS_EXPR
> && integer_onep (TREE_OPERAND (arg0, 1)))
> || (TREE_CODE (arg0) == PLUS_EXPR
>- && integer_all_onesp (TREE_OPERAND (arg0, 1)))))
>+ && integer_all_onesp (TREE_OPERAND (arg0, 1))))
>+ /* Be careful not to introduce new overflows when
>+ transforming to -A. That might happen if type of A
>+ is unsigned type for TYPE and the value of A is equal
>+ TYPE_MAX + 1. */
>+ && (TYPE_PRECISION (type)
>+ != TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (arg0, 1)))
>+ || TYPE_OVERFLOW_WRAPS (type)
>+ || TYPE_UNSIGNED (type)
>+ == TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (arg0, 1)))))
> return fold_build1_loc (loc, NEGATE_EXPR, type,
> fold_convert_loc (loc, type,
> TREE_OPERAND (arg0, 0)));
>diff --git gcc/testsuite/c-c++-common/ubsan/pr56917.c
>gcc/testsuite/c-c++-common/ubsan/pr56917.c
>index e69de29..e676962 100644
>--- gcc/testsuite/c-c++-common/ubsan/pr56917.c
>+++ gcc/testsuite/c-c++-common/ubsan/pr56917.c
>@@ -0,0 +1,34 @@
>+/* PR middle-end/56917 */
>+/* { dg-do run } */
>+/* { dg-options "-fsanitize=undefined -fno-sanitize-recover=undefined"
>} */
>+
>+#define INT_MIN (-__INT_MAX__ - 1)
>+#define LONG_MIN (-__LONG_MAX__ - 1L)
>+#define LLONG_MIN (-__LONG_LONG_MAX__ - 1LL)
>+
>+int __attribute__ ((noinline,noclone))
>+fn1 (unsigned int u)
>+{
>+ return (-(int) (u - 1U)) - 1;
>+}
>+
>+long __attribute__ ((noinline,noclone))
>+fn2 (unsigned long int ul)
>+{
>+ return (-(long) (ul - 1UL)) - 1L;
>+}
>+
>+long long __attribute__ ((noinline,noclone))
>+fn3 (unsigned long long int ull)
>+{
>+ return (-(long long) (ull - 1ULL)) - 1LL;
>+}
>+
>+int
>+main (void)
>+{
>+ if (fn1 (__INT_MAX__ + 1U) != INT_MIN
>+ || fn2 (__LONG_MAX__ + 1UL) != LONG_MIN
>+ || fn3 (__LONG_LONG_MAX__ + 1ULL) != LLONG_MIN)
>+ __builtin_abort ();
>+}
>
> Marek
