On Wed, 24 Jun 2020, Jakub Jelinek wrote:
> Hi!
>
> We folded A <= 0 ? A : -A into -ABS (A), which is for signed integral types
> incorrect - can invoke on INT_MIN UB twice, once on ABS and once on its
> negation.
>
> The following patch fixes it by instead folding it to (type)-ABSU (A).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2020-06-24 Jakub Jelinek
>
> PR middle-end/95810
> * fold-const.c (fold_cond_expr_with_comparison): Optimize
> A <= 0 ? A : -A into (type)-absu(A) rather than -abs(A).
>
> * gcc.dg/ubsan/pr95810.c: New test.
>
> --- gcc/fold-const.c.jj 2020-05-28 16:25:00.240712958 +0200
> +++ gcc/fold-const.c 2020-06-22 11:45:20.940170934 +0200
> @@ -5770,8 +5770,22 @@ fold_cond_expr_with_comparison (location
>case LT_EXPR:
> if (TYPE_UNSIGNED (TREE_TYPE (arg1)))
> break;
> - tem = fold_build1_loc (loc, ABS_EXPR, TREE_TYPE (arg1), arg1);
> - return negate_expr (fold_convert_loc (loc, type, tem));
> + if (ANY_INTEGRAL_TYPE_P (TREE_TYPE (arg1))
> + && !TYPE_OVERFLOW_WRAPS (TREE_TYPE (arg1)))
> + {
> + /* A <= 0 ? A : -A for A INT_MIN is valid, but -abs(INT_MIN)
> +is not, invokes UB both in abs and in the negation of it.
> +So, use ABSU_EXPR instead. */
> + tree utype = unsigned_type_for (TREE_TYPE (arg1));
> + tem = fold_build1_loc (loc, ABSU_EXPR, utype, arg1);
> + tem = negate_expr (tem);
> + return fold_convert_loc (loc, type, tem);
> + }
> + else
> + {
> + tem = fold_build1_loc (loc, ABS_EXPR, TREE_TYPE (arg1), arg1);
> + return negate_expr (fold_convert_loc (loc, type, tem));
> + }
>default:
> gcc_assert (TREE_CODE_CLASS (comp_code) == tcc_comparison);
> break;
> --- gcc/testsuite/gcc.dg/ubsan/pr95810.c.jj 2020-06-22 11:49:03.666910264
> +0200
> +++ gcc/testsuite/gcc.dg/ubsan/pr95810.c 2020-06-22 11:48:55.057036313
> +0200
> @@ -0,0 +1,13 @@
> +/* PR middle-end/95810 */
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=undefined -fno-sanitize-recover=undefined" } */
> +
> +int
> +main ()
> +{
> + int x = -__INT_MAX__ - 1;
> + x = (x <= 0 ? x : -x);
> + if (x != -__INT_MAX__ - 1)
> +__builtin_abort ();
> + return 0;
> +}
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
