On Wed, Jun 12, 2013 at 05:17:45PM +0200, Marek Polacek wrote:
> @@ -3867,6 +3868,7 @@ cp_build_binary_op (location_t location,
> tree final_type = 0;
>
> tree result;
> + tree orig_type = NULL;
>
> /* Nonzero if this is an operation like MIN or MAX which can
> safely be computed in short if both args are promoted shorts.
> @@ -3891,6 +3893,15 @@ cp_build_binary_op (location_t location,
> op0 = orig_op0;
> op1 = orig_op1;
>
> + /* Remember whether we're doing / or %. */
> + bool doing_div_or_mod = false;
> +
> + /* Remember whether we're doing << or >>. */
> + bool doing_shift = false;
> +
> + /* Tree holding instrumentation expression. */
> + tree instrument_expr = NULL;
> +
> if (code == TRUTH_AND_EXPR || code == TRUTH_ANDIF_EXPR
> || code == TRUTH_OR_EXPR || code == TRUTH_ORIF_EXPR
> || code == TRUTH_XOR_EXPR)
> @@ -4070,8 +4081,12 @@ cp_build_binary_op (location_t location,
> {
> enum tree_code tcode0 = code0, tcode1 = code1;
> tree cop1 = fold_non_dependent_expr_sfinae (op1, tf_none);
> + cop1 = maybe_constant_value (cop1);
>
> - warn_for_div_by_zero (location, maybe_constant_value (cop1));
> + if (!processing_template_decl && tcode0 == INTEGER_TYPE)
> + doing_div_or_mod = true;
Either the !processing_template_decl here is unneeded, or
if you'd check it (and perhaps flag_ubsan too) in this part of code,
then you wouldn't need to check it later.
> @@ -4832,6 +4853,31 @@ cp_build_binary_op (location_t location,
> if (build_type == NULL_TREE)
> build_type = result_type;
>
> + if (flag_ubsan && !processing_template_decl)
But, I'd certainly avoid doing the cp_save_expr/maybe_constant_value
etc. for all the binary operations you don't want to instrument
(thus check (doing_div_or_mod || doing_shift) also).
-
> + {
> + /* OP0 and/or OP1 might have side-effects. */
> + op0 = cp_save_expr (op0);
> + op1 = cp_save_expr (op1);
> + op0 = maybe_constant_value (fold_non_dependent_expr_sfinae (op0,
> + tf_none));
> + op1 = maybe_constant_value (fold_non_dependent_expr_sfinae (op1,
> + tf_none));
> + if (doing_div_or_mod)
> + {
> + /* For diagnostics we want to use the promoted types without
> + shorten_binary_op. So convert the arguments to the
> + original result_type. */
> + if (orig_type != NULL && result_type != orig_type)
> + {
> + op0 = cp_convert (orig_type, op0, complain);
> + op1 = cp_convert (orig_type, op1, complain);
And you don't want to change op0/op1, have your own tree vars, assign
op{0,1} to them and change here if result_type is not orig_type,
then pass those vars to ubsan_instrument_division.
Jakub
