This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: C++ PATCH for sanitizer/80348, another ICE with -fsanitize=integer-divide-by-zero
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Marek Polacek <polacek at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 10 Apr 2017 19:02:04 +0200
- Subject: Re: C++ PATCH for sanitizer/80348, another ICE with -fsanitize=integer-divide-by-zero
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jakub at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 441036264B
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 441036264B
- References: <20170410165507.GF17196@redhat.com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Mon, Apr 10, 2017 at 06:55:07PM +0200, Marek Polacek wrote:
> 2017-04-10 Marek Polacek <polacek@redhat.com>
>
> PR sanitizer/80348
> * typeck.c (cp_build_binary_op): Use NULL_TREE instead of NULL. Set
> ORIG_TYPE earlier and not only when shortening.
>
> * g++.dg/ubsan/div-by-zero-3.C: New test.
>
> diff --git gcc/cp/typeck.c gcc/cp/typeck.c
> index 65a3435..e756f9a 100644
> --- gcc/cp/typeck.c
> +++ gcc/cp/typeck.c
> @@ -4105,7 +4105,7 @@ cp_build_binary_op (location_t location,
>
> /* Data type in which the computation is to be performed.
> In the simplest cases this is the common type of the arguments. */
> - tree result_type = NULL;
> + tree result_type = NULL_TREE;
>
> /* Nonzero means operands have already been type-converted
> in whatever way is necessary.
> @@ -4121,7 +4121,9 @@ cp_build_binary_op (location_t location,
> tree final_type = 0;
>
> tree result, result_ovl;
> - tree orig_type = NULL;
> +
> + /* The type to be used for diagnostic. */
> + tree orig_type;
Can't you define orig_type instead:
> /* Nonzero if this is an operation like MIN or MAX which can
> safely be computed in short if both args are promoted shorts.
> @@ -4153,7 +4155,7 @@ cp_build_binary_op (location_t location,
> bool doing_shift = false;
>
> /* Tree holding instrumentation expression. */
> - tree instrument_expr = NULL;
> + tree instrument_expr = NULL_TREE;
>
> if (code == TRUTH_AND_EXPR || code == TRUTH_ANDIF_EXPR
> || code == TRUTH_OR_EXPR || code == TRUTH_ORIF_EXPR
> @@ -5042,6 +5044,10 @@ cp_build_binary_op (location_t location,
> return tmp;
> }
>
> + /* Remember the original type; RESULT_TYPE might be changed later on
> + by shorten_binary_op. */
> + orig_type = result_type;
here? tree orig_type = result_type;
Ok with that change.
> +
> if (arithmetic_types_p)
> {
> bool first_complex = (code0 == COMPLEX_TYPE);
> @@ -5138,7 +5144,6 @@ cp_build_binary_op (location_t location,
>
> if (shorten && none_complex)
> {
> - orig_type = result_type;
> final_type = result_type;
> result_type = shorten_binary_op (result_type, op0, op1,
> shorten == -1);
> @@ -5218,13 +5223,10 @@ cp_build_binary_op (location_t location,
> original result_type. */
> tree cop0 = op0;
> tree cop1 = op1;
> - if (orig_type != NULL_TREE)
> - {
> - if (TREE_TYPE (cop0) != orig_type)
> - cop0 = cp_convert (orig_type, op0, complain);
> - if (TREE_TYPE (cop1) != orig_type)
> - cop1 = cp_convert (orig_type, op1, complain);
> - }
> + if (TREE_TYPE (cop0) != orig_type)
> + cop0 = cp_convert (orig_type, op0, complain);
> + if (TREE_TYPE (cop1) != orig_type)
> + cop1 = cp_convert (orig_type, op1, complain);
Other option would be to use instead of orig_type here
orig_type ? orig_type : result_type, but I agree your patch looks
better than that and orig_type is used solely for the ubsan
instrumentation.
Jakub