This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix ubsan -fsanitize=signed-integer-overflow expansion (PR sanitizer/63520)
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org, ebotcazou at adacore dot com
- Date: Wed, 19 Nov 2014 10:02:18 +0100 (CET)
- Subject: Re: [PATCH] Fix ubsan -fsanitize=signed-integer-overflow expansion (PR sanitizer/63520)
- Authentication-results: sourceware.org; auth=none
- References: <20141118220447 dot GT1745 at tucnak dot redhat dot com>
On Tue, 18 Nov 2014, Jakub Jelinek wrote:
> Hi!
>
> Apparently, expand_expr with EXPR_WRITE can return
> a SUBREG with SUBREG_PROMOTED_VAR_P set on it. For
Huh, that looks bogus to me. But of course I know nothing
(read: not enough) to really tell. Eric?
Richard.
> UBSAN_CHECK_{ADD,SUB,MUL} expansion, I've been doing just
> emit_move_insn into it, which apparently is wrong in that case,
> store_expr instead uses convert_move for it. The
> {ADD,SUB,MUL}_OVERFLOW (i.e. __builtin_*_overflow) expansion
> shouldn't need it, as the result is complex and complex integers
> aren't promoted that way. As store_expr* uses a tree expression
> to store, while I have rtx, I just wrote a short helper function
> for this.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2014-11-18 Jakub Jelinek <jakub@redhat.com>
>
> PR sanitizer/63520
> * internal-fn.c (expand_ubsan_result_store): New function.
> (expand_addsub_overflow, expand_neg_overflow, expand_mul_overflow):
> Use it instead of just emit_move_insn.
>
> * c-c++-common/ubsan/pr63520.c: New test.
>
> --- gcc/internal-fn.c.jj 2014-11-12 13:28:47.000000000 +0100
> +++ gcc/internal-fn.c 2014-11-18 15:35:46.395916823 +0100
> @@ -395,6 +395,21 @@ expand_arith_overflow_result_store (tree
> write_complex_part (target, lres, false);
> }
>
> +/* Helper for expand_*_overflow. Store RES into TARGET. */
> +
> +static void
> +expand_ubsan_result_store (rtx target, rtx res)
> +{
> + if (GET_CODE (target) == SUBREG && SUBREG_PROMOTED_VAR_P (target))
> + /* If this is a scalar in a register that is stored in a wider mode
> + than the declared mode, compute the result into its declared mode
> + and then convert to the wider mode. Our value is the computed
> + expression. */
> + convert_move (SUBREG_REG (target), res, SUBREG_PROMOTED_SIGN (target));
> + else
> + emit_move_insn (target, res);
> +}
> +
> /* Add sub/add overflow checking to the statement STMT.
> CODE says whether the operation is +, or -. */
>
> @@ -809,7 +824,7 @@ expand_addsub_overflow (location_t loc,
> if (lhs)
> {
> if (is_ubsan)
> - emit_move_insn (target, res);
> + expand_ubsan_result_store (target, res);
> else
> {
> if (do_xor)
> @@ -904,7 +919,7 @@ expand_neg_overflow (location_t loc, tre
> if (lhs)
> {
> if (is_ubsan)
> - emit_move_insn (target, res);
> + expand_ubsan_result_store (target, res);
> else
> expand_arith_overflow_result_store (lhs, target, mode, res);
> }
> @@ -1590,7 +1605,7 @@ expand_mul_overflow (location_t loc, tre
> if (lhs)
> {
> if (is_ubsan)
> - emit_move_insn (target, res);
> + expand_ubsan_result_store (target, res);
> else
> expand_arith_overflow_result_store (lhs, target, mode, res);
> }
> --- gcc/testsuite/c-c++-common/ubsan/pr63520.c.jj 2014-11-18 15:40:07.271273710 +0100
> +++ gcc/testsuite/c-c++-common/ubsan/pr63520.c 2014-11-18 15:40:40.971673904 +0100
> @@ -0,0 +1,16 @@
> +/* PR sanitizer/63520 */
> +/* { dg-do compile } */
> +/* { dg-options "-fsanitize=undefined" } */
> +
> +int a;
> +
> +void
> +foo (void)
> +{
> + while (1)
> + {
> + if (a == 1)
> + break;
> + a -= 1;
> + }
> +}
>
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284
(AG Nuernberg)
Maxfeldstrasse 5, 90409 Nuernberg, Germany