[PATCH] Speed-up use-after-scope (re-writing to SSA) (version 2)
Martin Liška
mliska@suse.cz
Tue Jan 17 16:22:00 GMT 2017
On 01/16/2017 03:20 PM, Jakub Jelinek wrote:
> On Mon, Jan 09, 2017 at 03:58:04PM +0100, Martin Liška wrote:
>>>> Well, having following sample:
>>>>
>>>> int
>>>> main (int argc, char **argv)
>>>> {
>>>> int *ptr = 0;
>>>>
>>>> {
>>>> int a;
>>>> ptr = &a;
>>>> *ptr = 12345;
>>>> }
>>>>
>>>> *ptr = 12345;
>>>> return *ptr;
>>>> }
>>>>
>
>> I'm still not sure how to do that. Problem is that transformation from:
>>
>> ASAN_MARK (UNPOISON, &a, 4);
>> a = 5;
>> ASAN_MARK (POISON, &a, 4);
>>
>> to
>>
>> a_8 = 5;
>> a_9 = ASAN_POISON ();
>>
>> happens in tree-ssa.c, after SSA is created, in situation where we prove the 'a'
>> does not need to live in memory. Thus said, question is how to identify that we
>> need to transform into SSA in a different way:
>>
>> a_10 = ASAN_POISON ();
>> ASAN_POISON (a_10);
>
> I meant something like this (completely untested, and without the testcase
> added to the testsuite).
> The incremental patch as is relies on the ASAN_POISON_USE call having the
> argument the result of ASAN_POISON, it would ICE if that is not the case
> (especially if -fsanitize-recover=address). Dunno if some optimization
> might decide to create a PHI in between, say merge two unrelated vars for
> if (something)
> {
> x_1 = ASAN_POISON ();
> ...
> ASAN_POISON_USE (x_1);
> }
> else
> {
> y_2 = ASAN_POISON ();
> ...
> ASAN_POISON_USE (y_2);
> }
> to turn that into:
> if (something)
> x_1 = ASAN_POISON ();
> else
> y_2 = ASAN_POISON ();
> _3 = PHI <x_1, y_2>;
> ...
> ASAN_POISON_USE (_3);
>
> If it did, we would ICE because ASAN_POISON_USE would survive this way until
> expansion. A quick fix for the ICE (if it can ever happen) would be easy,
> in sanopt remove ASAN_POISON_USE calls which have argument that is not lhs
> of ASAN_POISON (all other ASAN_POISON_USE calls will be handled by my
> incremental patch). Of course that would also mean in that case we'd report
> a read rather than write. But if it can't happen or is very unlikely to
> happen, then it is a non-issue.
Thank you Jakub for working on that.
The patch is fine, I added DCE support and a test-case. Please see attached patch.
asan.exp regression tests look fine and I've been building linux kernel with KASAN
enabled. I'll also do asan-boostrap.
I would like to commit the patch soon, should I squash both patches together, or would it
be preferred to separate basic optimization and support for stores?
Thanks,
Martin
> Something missing from the patch is some change in DCE to remove ASAN_POISON
> calls without lhs earlier. I think we can't make ASAN_POISON ECF_CONST, we
> don't want it to be merged for different variables.
>
> --- gcc/internal-fn.def.jj 2017-01-16 13:19:49.000000000 +0100
> +++ gcc/internal-fn.def 2017-01-16 14:25:37.427962196 +0100
> @@ -167,6 +167,7 @@ DEF_INTERNAL_FN (ABNORMAL_DISPATCHER, EC
> DEF_INTERNAL_FN (ASAN_CHECK, ECF_TM_PURE | ECF_LEAF | ECF_NOTHROW, ".R...")
> DEF_INTERNAL_FN (ASAN_MARK, ECF_LEAF | ECF_NOTHROW, ".R..")
> DEF_INTERNAL_FN (ASAN_POISON, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL)
> +DEF_INTERNAL_FN (ASAN_POISON_USE, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL)
> DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
> DEF_INTERNAL_FN (SUB_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
> DEF_INTERNAL_FN (MUL_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
> --- gcc/asan.c.jj 2017-01-16 13:19:49.000000000 +0100
> +++ gcc/asan.c 2017-01-16 14:52:34.022044223 +0100
> @@ -3094,6 +3094,8 @@ create_asan_shadow_var (tree var_decl,
> return *slot;
> }
>
> +/* Expand ASAN_POISON ifn. */
> +
> bool
> asan_expand_poison_ifn (gimple_stmt_iterator *iter,
> bool *need_commit_edge_insert,
> @@ -3107,8 +3109,8 @@ asan_expand_poison_ifn (gimple_stmt_iter
> return true;
> }
>
> - tree shadow_var = create_asan_shadow_var (SSA_NAME_VAR (poisoned_var),
> - shadow_vars_mapping);
> + tree shadow_var = create_asan_shadow_var (SSA_NAME_VAR (poisoned_var),
> + shadow_vars_mapping);
>
> bool recover_p;
> if (flag_sanitize & SANITIZE_USER_ADDRESS)
> @@ -3122,16 +3124,16 @@ asan_expand_poison_ifn (gimple_stmt_iter
> ASAN_MARK_POISON),
> build_fold_addr_expr (shadow_var), size);
>
> - use_operand_p use_p;
> + gimple *use;
> imm_use_iterator imm_iter;
> - FOR_EACH_IMM_USE_FAST (use_p, imm_iter, poisoned_var)
> + FOR_EACH_IMM_USE_STMT (use, imm_iter, poisoned_var)
> {
> - gimple *use = USE_STMT (use_p);
> if (is_gimple_debug (use))
> continue;
>
> int nargs;
> - tree fun = report_error_func (false, recover_p, tree_to_uhwi (size),
> + bool store_p = gimple_call_internal_p (use, IFN_ASAN_POISON_USE);
> + tree fun = report_error_func (store_p, recover_p, tree_to_uhwi (size),
> &nargs);
>
> gcall *call = gimple_build_call (fun, 1,
> @@ -3160,7 +3162,10 @@ asan_expand_poison_ifn (gimple_stmt_iter
> else
> {
> gimple_stmt_iterator gsi = gsi_for_stmt (use);
> - gsi_insert_before (&gsi, call, GSI_NEW_STMT);
> + if (store_p)
> + gsi_replace (&gsi, call, true);
> + else
> + gsi_insert_before (&gsi, call, GSI_NEW_STMT);
> }
> }
>
> --- gcc/tree-into-ssa.c.jj 2017-01-01 12:45:35.000000000 +0100
> +++ gcc/tree-into-ssa.c 2017-01-16 14:32:14.853808726 +0100
> @@ -38,6 +38,7 @@ along with GCC; see the file COPYING3.
> #include "tree-ssa.h"
> #include "domwalk.h"
> #include "statistics.h"
> +#include "asan.h"
>
> #define PERCENT(x,y) ((float)(x) * 100.0 / (float)(y))
>
> @@ -1807,6 +1808,26 @@ maybe_replace_use_in_debug_stmt (use_ope
> }
>
>
> +/* If DEF has x_5 = ASAN_POISON () as its current def, add
> + ASAN_POISON_USE (x_5) stmt before GSI to denote the stmt writes into
> + a poisoned (out of scope) variable. */
> +
> +static void
> +maybe_add_asan_poison_write (tree def, gimple_stmt_iterator *gsi)
> +{
> + tree cdef = get_current_def (def);
> + if (cdef != NULL
> + && TREE_CODE (cdef) == SSA_NAME
> + && gimple_call_internal_p (SSA_NAME_DEF_STMT (cdef), IFN_ASAN_POISON))
> + {
> + gcall *call
> + = gimple_build_call_internal (IFN_ASAN_POISON_USE, 1, cdef);
> + gimple_set_location (call, gimple_location (gsi_stmt (*gsi)));
> + gsi_insert_before (gsi, call, GSI_SAME_STMT);
> + }
> +}
> +
> +
> /* If the operand pointed to by DEF_P is an SSA name in NEW_SSA_NAMES
> or OLD_SSA_NAMES, or if it is a symbol marked for renaming,
> register it as the current definition for the names replaced by
> @@ -1837,7 +1858,11 @@ maybe_register_def (def_operand_p def_p,
> def = get_or_create_ssa_default_def (cfun, sym);
> }
> else
> - def = make_ssa_name (def, stmt);
> + {
> + if (asan_sanitize_use_after_scope ())
> + maybe_add_asan_poison_write (def, &gsi);
> + def = make_ssa_name (def, stmt);
> + }
> SET_DEF (def_p, def);
>
> tree tracked_var = target_for_debug_bind (sym);
> --- gcc/internal-fn.c.jj 2017-01-16 13:19:49.000000000 +0100
> +++ gcc/internal-fn.c 2017-01-16 14:26:10.828529039 +0100
> @@ -388,6 +388,14 @@ expand_ASAN_POISON (internal_fn, gcall *
> gcc_unreachable ();
> }
>
> +/* This should get expanded in the sanopt pass. */
> +
> +static void
> +expand_ASAN_POISON_USE (internal_fn, gcall *)
> +{
> + gcc_unreachable ();
> +}
> +
> /* This should get expanded in the tsan pass. */
>
> static void
>
>
> Jakub
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-use-after-scope-handle-writes-to-a-poisoned-variable.patch
Type: text/x-patch
Size: 7197 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20170117/f2a40c65/attachment.bin>
More information about the Gcc-patches
mailing list