On Thu, Dec 04, 2014 at 05:34:35PM +0100, Richard Biener wrote:
> 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.
Sure.
> Rather than disabling the transform I'd rather make it defined using unsigned
> types.
Such as the following then? That is, instead of -(long int) ul do (long int)
-ul.
Bootstrapped/regtested on ppc64-linux.
2014-12-04 Marek Polacek <pola...@redhat.com>
PR middle-end/56917
* fold-const.c (fold_unary_loc): Perform the negation in A's type
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..4ab443d 100644
--- gcc/fold-const.c
+++ gcc/fold-const.c
@@ -8141,9 +8141,14 @@ fold_unary_loc (location_t loc, enum tree_code code,
tree type, tree op0)
&& integer_onep (TREE_OPERAND (arg0, 1)))
|| (TREE_CODE (arg0) == PLUS_EXPR
&& integer_all_onesp (TREE_OPERAND (arg0, 1)))))
- return fold_build1_loc (loc, NEGATE_EXPR, type,
- fold_convert_loc (loc, type,
- TREE_OPERAND (arg0, 0)));
+ {
+ /* Perform the negation in ARG0's type and only then convert
+ to TYPE as to avoid introducing undefined behavior. */
+ tree t = fold_build1_loc (loc, NEGATE_EXPR,
+ TREE_TYPE (TREE_OPERAND (arg0, 0)),
+ TREE_OPERAND (arg0, 0));
+ return fold_convert_loc (loc, type, t);
+ }
/* Convert ~(X ^ Y) to ~X ^ Y or X ^ ~Y if ~X or ~Y simplify. */
else if (TREE_CODE (arg0) == BIT_XOR_EXPR
&& (tem = fold_unary_loc (loc, BIT_NOT_EXPR, type,
diff --git gcc/testsuite/c-c++-common/ubsan/pr56917.c
gcc/testsuite/c-c++-common/ubsan/pr56917.c
index e69de29..cfbae97 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
