On Fri, Dec 09, 2016 at 12:39:24PM +0100, Martin Liška wrote:
> + if (is_gimple_call (stmt) && gimple_call_internal_p (stmt))
> + {
> + enum internal_fn ifn = gimple_call_internal_fn (stmt);
> + switch (ifn)
> + {
> + case IFN_ASAN_MARK:
> + {
> + HOST_WIDE_INT flags = tree_to_shwi (gimple_call_arg (stmt,
> 0));
> + bool is_clobber = (flags & ASAN_MARK_CLOBBER) != 0;
> + if (is_clobber)
> + {
> + bitmap_set_bit (with_poison, bb->index);
> + finish = true;
> + }
> + break;
> + }
> + default:
> + break;
> + }
This looks weird. Wouldn't it be nicer to just use
if (gimple_call_internal_fn (stmt) == IFN_ASAN_MARK)
{
HOST_WIDE_INT flags = tree_to_shwi (gimple_call_arg (stmt,
0));
bool is_clobber = (flags & ASAN_MARK_CLOBBER) != 0;
if (is_clobber)
{
bitmap_set_bit (with_poison, bb->index);
finish = true;
}
}
(or don't use finish and just break; there)?
> + enum internal_fn ifn = gimple_call_internal_fn (stmt);
> + switch (ifn)
> + {
> + case IFN_ASAN_MARK:
> + {
Likewise.
No function comment:
> +static bool
> +maybe_contains_asan_check (gimple *stmt)
> +{
> + if (is_gimple_call (stmt))
> + {
> + if (gimple_call_internal_p (stmt))
> + {
> + enum internal_fn ifn = gimple_call_internal_fn (stmt);
> + switch (ifn)
> + {
> + case IFN_ASAN_CHECK:
> + return true;
> + default:
> + return false;
Are all internal functions really known to be ASAN_CHECK free?
> + }
> + }
> + else
> + return true;
What about builtins? Many will not be fine, but many should be known
to be ASAN_CHECK free. Consider e.g. most math builtins (except sincos).
> @@ -698,6 +928,9 @@ pass_sanopt::execute (function *fun)
> bool use_calls = ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
> && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
>
> + sanitize_asan_mark_unpoison ();
> + sanitize_asan_mark_poison ();
> +
What I don't really like here is that you force several IL walks for all
sanitizers, just in case. Look e.g. how asan_num_accesses is computed,
couldn't you compute similarly has_asan_marks and guard those two functions
on that?
Jakub
