On Mon, Apr 10, 2017 at 06:55:07PM +0200, Marek Polacek wrote:
> 2017-04-10 Marek Polacek <pola...@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
