On Wed, Nov 16, 2016 at 01:25:04PM +0100, Martin Liška wrote:
>
> +
> +/* Expand the ASAN_{LOAD,STORE} builtins. */
Stale comment.
> +
> +bool
> +asan_expand_poison_ifn (gimple_stmt_iterator *iter,
> + bool *need_commit_edge_insert)
> +{
...
> + use_operand_p use_p;
> + imm_use_iterator imm_iter;
> + FOR_EACH_IMM_USE_FAST (use_p, imm_iter, poisoned_var)
> + {
> + gimple *use = USE_STMT (use_p);
> +
You want to ignore debug stmts uses here (or reset them).
> + built_in_function b = (recover_p
> + ? BUILT_IN_ASAN_REPORT_USE_AFTER_SCOPE_NOABORT
> + : BUILT_IN_ASAN_REPORT_USE_AFTER_SCOPE);
> + tree fun = builtin_decl_implicit (b);
> + pretty_printer pp;
> + pp_tree_identifier (&pp, DECL_NAME (var_decl));
> +
> + gcall *call = gimple_build_call (fun, 2, asan_pp_string (&pp),
> + DECL_SIZE_UNIT (var_decl));
> + gimple_set_location (call, gimple_location (g));
Is that the location you want? I mean shouldn't it use gimple_location (use)
instead? The bug is on the use, not on the spot where it went out of scope.
Though the question is what to use if gimple_location (use) is
UNKNOWN_LOCATION.
> +
> + /* If ASAN_POISON is used in a PHI node, let's insert the call on
> + the leading to the PHI node BB. */
The comment doesn't make sense gramatically to me.
> + if (is_a <gphi *> (use))
> + {
> + gphi * phi = dyn_cast<gphi *> (use);
> + for (unsigned i = 0; i < gimple_phi_num_args (phi); ++i)
> + if (gimple_phi_arg_def (phi, i) == poisoned_var)
> + {
> + edge e = gimple_phi_arg_edge (phi, i);
> + gsi_insert_seq_on_edge (e, call);
> + *need_commit_edge_insert = true;
What if there are multiple PHI args with that use?
Shouldn't you use just FOR_EACH_USE_ON_STMT or what macros we have?
> --- a/libsanitizer/asan/asan_errors.cc
> +++ b/libsanitizer/asan/asan_errors.cc
> @@ -279,6 +279,27 @@ void ErrorInvalidPointerPair::Print() {
> ReportErrorSummary(bug_type, &stack);
> }
As I wrote on IRC, we have to submit this to compiler-rt and only
if it is accepted, cherry-pick it together with the gcc changes.
> --- a/libsanitizer/asan/asan_errors.h
> +++ b/libsanitizer/asan/asan_errors.h
> @@ -294,6 +294,24 @@ struct ErrorInvalidPointerPair : ErrorBase {
> void Print();
> };
>
> +struct ErrorUseAfterScope : ErrorBase {
> + uptr pc, bp, sp;
> + const char *variable_name;
> + u32 variable_size;
Shouldn't this be uptr?
> + ErrorUseAfterScope(u32 tid, uptr pc_, uptr bp_, uptr sp_,
> + const char *variable_name_, u32 variable_size_)
And here.
> +// ----------------------- ReportUseAfterScope ----------- {{{1
> +void ReportUseAfterScope(const char *variable_name, u32 variable_size,
And here?
> +void ReportUseAfterScope(const char *variable_name, u32 variable_size,
> + bool fatal);
And here?
Jakub
