On 06/19/2017 04:13 PM, Jakub Jelinek wrote:
> On Mon, Jun 19, 2017 at 03:50:42PM +0200, Martin Liška wrote:
>> @@ -858,6 +862,117 @@ sanitize_asan_mark_poison (void)
>> }
>> }
>>
>
> Missing function comment.
>
>> +static tree
>> +rewrite_usage_of_param (tree *op, int *walk_subtrees, void *data)
>> +{
>> + struct walk_stmt_info *wi = (struct walk_stmt_info *) data;
>> + std::pair<tree, tree> *replacement = (std::pair<tree, tree> *)wi->info;
>
> Missing space after )
>
>> +
>> + if (*op == replacement->first)
>> + {
>> + *op = replacement->second;
>> + *walk_subtrees = 0;
>> + }
>> +
>> + return NULL;
>> +}
>
>> +static void
>> +sanitize_rewrite_addressable_params (function *fun)
>> +{
>> + basic_block entry_bb = NULL;
>> +
>> + for (tree arg = DECL_ARGUMENTS (current_function_decl);
>> + arg; arg = DECL_CHAIN (arg))
>> + {
>> + if (TREE_ADDRESSABLE (arg) && !TREE_ADDRESSABLE (TREE_TYPE (arg)))
>> + {
>> + /* The parameter is no longer addressable. */
>> + tree type = TREE_TYPE (arg);
>> + TREE_ADDRESSABLE (arg) = 0;
>> +
>> + /* Create a new automatic variable. */
>> + tree var = build_decl (DECL_SOURCE_LOCATION (arg),
>> + VAR_DECL, DECL_NAME (arg), type);
>> + TREE_ADDRESSABLE (var) = 1;
>> + DECL_ARTIFICIAL (var) = 1;
>> + DECL_SEEN_IN_BIND_EXPR_P (var) = 0;
>
> I think it is highly inefficient to walk the whole IL for every addressable
> argument. Can't you first find out what PARM_DECLs you need to change,
> stick the corresponding VAR_DECL somewhere (dunno, e.g. a vector with pairs
> perhaps sorted by DECL_UID, or stick it into DECL_VALUE_EXPR or whatever),
> then if you create at least one, walk whole IL and replace all the
> PARM_DECLs you want to replace, then finally clear the TREE_ADDRESSABLE
> flag for all of them and emit the initialization sequence?
Yes, this is doable. I've done that.
> Then something needs to be done for debugging too. If it is without VTA,
> then probably just having DECL_VALUE_EXPR is good enough, otherwise
> (VTA) you probably don't want that (or can reset it at that point), but
> instead emit after the initialization stmt a debug stmt that the variable
> value now lives in a different var. Though ideally we want the debugger
> to be able to also change the value of the var, that might be harder.
> With DECL_VALUE_EXPR on the other side the debug info will be incorrect in
> the prologue until it is assigned to the slot.
Here I'm not sure about how to distinguish whether to build or not to build
the debug statement. According to flag_var_tracking?
You mean something like:
g = gimple_build_debug_bind (arg, var, g);
?
>
>> +
>> + gimple_add_tmp_var (var);
>> +
>> + if (dump_file)
>> + fprintf (dump_file,
>> + "Rewritting parameter whos address is taken: %s\n",
>> + IDENTIFIER_POINTER (DECL_NAME (arg)));
>
> s/tting/ting/, s/whos/whose/
>> +
>> + gimple_seq stmts = NULL;
>> +
>> + /* Assign value of parameter to newly created variable. */
>> + if ((TREE_CODE (type) == COMPLEX_TYPE
>> + || TREE_CODE (type) == VECTOR_TYPE))
>> + {
>> + /* We need to create a SSA name that will be used for the
>> + assignment. */
>> + tree tmp = make_ssa_name (type);
>> + gimple *g = gimple_build_assign (tmp, arg);
>> + gimple_set_location (g, DECL_SOURCE_LOCATION (arg));
>> + gimple_seq_add_stmt (&stmts, g);
>> + g = gimple_build_assign (var, tmp);
>> + gimple_set_location (g, DECL_SOURCE_LOCATION (arg));
>> + gimple_seq_add_stmt (&stmts, g);
>> + }
>> + else
>> + {
>> + gimple *g = gimple_build_assign (var, arg);
>> + gimple_set_location (g, DECL_SOURCE_LOCATION (arg));
>> + gimple_seq_add_stmt (&stmts, g);
>> + }
>
> I don't understand the distinction. If you turn the original parm
> for complex/vector DECL_GIMPLE_REG_P, you should need the exact same code
> (but I think it would be better to use the default SSA_NAME of the PARM_DECL
> if it is a gimple reg type, rather than use the PARM_DECL itself
> and wait for update_ssa).
Yes, the test-case /gcc/testsuite/g++.dg/asan/function-argument-3.C fails for me
as one needs to have a temporary SSA name, otherwise:
/home/marxin/Programming/gcc/gcc/testsuite/g++.dg/asan/function-argument-3.C:13:1:
error: invalid rhs for gimple memory store
foo (v4si arg)
^~~
arg
arg
# .MEM_4 = VDEF <.MEM_1(D)>
arg = arg;
during GIMPLE pass: sanopt
If I see correctly the function in my test-case does not have default def SSA
name for the parameter.
Thus I guess I need to create a SSA name?
Thanks,
Martin
>
> Jakub
>
