On Thu, Nov 06, 2014 at 10:56:17AM +0100, Marek Polacek wrote:
> --- gcc/sanopt.c
> +++ gcc/sanopt.c
> @@ -130,7 +130,30 @@ sanopt_optimize_walker (basic_block bb, struct
> sanopt_ctx *ctx)
> /* At this point we shouldn't have any statements
> that aren't dominating the current BB. */
> tree align = gimple_call_arg (g, 2);
> - remove = tree_int_cst_le (cur_align, align);
> + int kind = tree_to_shwi (gimple_call_arg (g, 1));
> + /* If this is a NULL pointer check where we had segv
> + anyway, we can remove it. */
> + if (integer_zerop (align)
> + && (kind == UBSAN_LOAD_OF
> + || kind == UBSAN_STORE_OF
> + || kind == UBSAN_MEMBER_ACCESS))
> + remove = true;
> + /* Otherwise remove the check in non-recovering
> + mode, or if the stmts have same location. */
> + else if (integer_zerop (align))
> + remove = !(flag_sanitize_recover & SANITIZE_NULL)
> + || flag_sanitize_undefined_trap_on_error
> + || gimple_location (g)
> + == gimple_location (stmt);
> + else if (tree_int_cst_le (cur_align, align))
> + remove = !(flag_sanitize_recover
> + & SANITIZE_ALIGNMENT)
> + || flag_sanitize_undefined_trap_on_error
> + || gimple_location (g)
> + == gimple_location (stmt);
> + if (!remove && gimple_bb (g) == gimple_bb (stmt)
> + && tree_int_cst_compare (cur_align, align) == 0)
Sorry for being slow on this, but in the current algorithm you are using
the latest vector element from each bb only anyway, so the
&& tree_int_cst_compare (cur_align, align) == 0
is unnecessary, even if the condition isn't true, and thus in your patch we
would not pop the old one, we'd still never use the not popped element,
as the stmt pushed after it would be used instead, or both stmts popped up
if the domwalk left the bb already.
If you want, commit your patch as is and in test the one liner together with
some other change.
Jakub
