On 11/01/2016 04:12 PM, Jakub Jelinek wrote: > On Tue, Nov 01, 2016 at 03:53:46PM +0100, Martin Liška wrote: >> @@ -1504,7 +1505,7 @@ non_rewritable_lvalue_p (tree lhs) >> >> static void >> maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs, >> - bitmap suitable_for_renaming) >> + bitmap suitable_for_renaming, bitmap marked_nonaddressable) >> { >> /* Global Variables, result decls cannot be changed. */ >> if (is_global_var (var) >> @@ -1522,6 +1523,7 @@ maybe_optimize_var (tree var, bitmap addresses_taken, >> bitmap not_reg_needs, >> || !bitmap_bit_p (not_reg_needs, DECL_UID (var)))) >> { >> TREE_ADDRESSABLE (var) = 0; >> + bitmap_set_bit (marked_nonaddressable, DECL_UID (var)); > > Why do you need the marked_nonaddressable bitmap?
Because the later loop (which visits every gimple statement) iterates only if there's an entry in suitable_for_renaming. > >> if (is_gimple_reg (var)) >> bitmap_set_bit (suitable_for_renaming, DECL_UID (var)); >> if (dump_file) >> @@ -1550,20 +1552,43 @@ maybe_optimize_var (tree var, bitmap >> addresses_taken, bitmap not_reg_needs, >> } >> } >> >> -/* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables. */ >> +/* Return true when STMT is ASAN mark where second argument is an address >> + of a local variable. */ >> >> -void >> -execute_update_addresses_taken (void) >> +static bool >> +is_asan_mark_p (gimple *stmt) >> +{ >> + if (!gimple_call_internal_p (stmt, IFN_ASAN_MARK)) >> + return false; >> + >> + tree addr = get_base_address (gimple_call_arg (stmt, 1)); >> + if (TREE_CODE (addr) == ADDR_EXPR >> + && TREE_CODE (TREE_OPERAND (addr, 0)) == VAR_DECL) > > Just check here if dropping TREE_ADDRESSABLE from the VAR (use VAR_P btw) > would turn it into is_gimple_reg), and don't return true if not. Well, the predicate is called once before maybe_optimize_var, thus I need to have it conservative and not consider TREE_ADDRESSABLE flag. Having argument would work for that? > >> + return true; >> + >> + return false; >> +} >> + >> +/* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables. >> + If SANITIZE_ASAN_MARK is set to true, sanitize also ASAN_MARK built-ins. >> */ >> + >> + >> +static void >> +execute_update_addresses_taken (bool sanitize_asan_mark = false) > > I wonder if the sanitize_asan_mark wouldn't better be some PROP_* property > set during the asan pass and kept on until end of compilation of that > function. That means even if a var only addressable because of ASAN_MARK is > discovered after this pass we'd still be able to rewrite it into SSA. It's doable (please see attached patch) and also nicer. However, one would need to extend curr_properties to long type as we already use 16 existing values. Martin > > Jakub >
>From ad5f68a010674118fac7ca8b6953f7b99fd3c2a8 Mon Sep 17 00:00:00 2001 From: marxin <mli...@suse.cz> Date: Tue, 1 Nov 2016 11:21:20 +0100 Subject: [PATCH] Use-after-scope: do not mark variables that are no longer addressable gcc/ChangeLog: 2016-11-02 Martin Liska <mli...@suse.cz> * asan.c: Update properties_provided and todo_flags_finish. * function.h (struct GTY): Change int to long as there's not enough space for a new value. * tree-pass.h: Define PROP_asan_check_done. * tree-ssa.c (maybe_optimize_var): Add new argument. (is_asan_mark_p): New function. (execute_update_addresses_taken): Handle ASAN_MARK internal fns. --- gcc/asan.c | 5 +++-- gcc/function.h | 2 +- gcc/tree-pass.h | 1 + gcc/tree-ssa.c | 69 +++++++++++++++++++++++++++++++++++++++++++++------------ 4 files changed, 60 insertions(+), 17 deletions(-) diff --git a/gcc/asan.c b/gcc/asan.c index 95495d2..94ee877 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -59,6 +59,7 @@ along with GCC; see the file COPYING3. If not see #include "params.h" #include "builtins.h" #include "fnmatch.h" +#include "tree-ssa.h" /* AddressSanitizer finds out-of-bounds and use-after-free bugs with <2x slowdown on average. @@ -2993,10 +2994,10 @@ const pass_data pass_data_asan = OPTGROUP_NONE, /* optinfo_flags */ TV_NONE, /* tv_id */ ( PROP_ssa | PROP_cfg | PROP_gimple_leh ), /* properties_required */ - 0, /* properties_provided */ + PROP_asan_check_done, /* properties_provided */ 0, /* properties_destroyed */ 0, /* todo_flags_start */ - TODO_update_ssa, /* todo_flags_finish */ + TODO_update_ssa | TODO_update_address_taken, /* todo_flags_finish */ }; class pass_asan : public gimple_opt_pass diff --git a/gcc/function.h b/gcc/function.h index e854c7f..5600488 100644 --- a/gcc/function.h +++ b/gcc/function.h @@ -289,7 +289,7 @@ struct GTY(()) function { location_t function_end_locus; /* Properties used by the pass manager. */ - unsigned int curr_properties; + unsigned long curr_properties; unsigned int last_verified; /* Non-null if the function does something that would prevent it from diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h index da9ba13..ea43593 100644 --- a/gcc/tree-pass.h +++ b/gcc/tree-pass.h @@ -222,6 +222,7 @@ protected: of math functions; the current choices have been optimized. */ +#define PROP_asan_check_done (1 << 16) /* ASAN check is emitted */ #define PROP_trees \ (PROP_gimple_any | PROP_gimple_lcf | PROP_gimple_leh | PROP_gimple_lomp) diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c index 135952b..1492f47 100644 --- a/gcc/tree-ssa.c +++ b/gcc/tree-ssa.c @@ -41,6 +41,7 @@ along with GCC; see the file COPYING3. If not see #include "cfgexpand.h" #include "tree-cfg.h" #include "tree-dfa.h" +#include "asan.h" /* Pointer map of variable mappings, keyed by edge. */ static hash_map<edge, auto_vec<edge_var_map> > *edge_var_maps; @@ -1504,7 +1505,7 @@ non_rewritable_lvalue_p (tree lhs) static void maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs, - bitmap suitable_for_renaming) + bitmap suitable_for_renaming, bitmap marked_nonaddressable) { /* Global Variables, result decls cannot be changed. */ if (is_global_var (var) @@ -1522,6 +1523,7 @@ maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs, || !bitmap_bit_p (not_reg_needs, DECL_UID (var)))) { TREE_ADDRESSABLE (var) = 0; + bitmap_set_bit (marked_nonaddressable, DECL_UID (var)); if (is_gimple_reg (var)) bitmap_set_bit (suitable_for_renaming, DECL_UID (var)); if (dump_file) @@ -1550,6 +1552,22 @@ maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs, } } +/* Return true when STMT is ASAN mark where second argument is an address + of a local variable. */ + +static bool +is_asan_mark_p (gimple *stmt) +{ + if (!gimple_call_internal_p (stmt, IFN_ASAN_MARK)) + return false; + + tree addr = get_base_address (gimple_call_arg (stmt, 1)); + if (TREE_CODE (addr) == ADDR_EXPR && VAR_P (TREE_OPERAND (addr, 0))) + return true; + + return false; +} + /* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables. */ void @@ -1559,9 +1577,12 @@ execute_update_addresses_taken (void) bitmap addresses_taken = BITMAP_ALLOC (NULL); bitmap not_reg_needs = BITMAP_ALLOC (NULL); bitmap suitable_for_renaming = BITMAP_ALLOC (NULL); + bitmap marked_nonaddressable = BITMAP_ALLOC (NULL); tree var; unsigned i; + bool sanitize_asan_mark = cfun->curr_properties & PROP_asan_check_done; + timevar_push (TV_ADDRESS_TAKEN); /* Collect into ADDRESSES_TAKEN all variables whose address is taken within @@ -1575,17 +1596,23 @@ execute_update_addresses_taken (void) enum gimple_code code = gimple_code (stmt); tree decl; - if (code == GIMPLE_CALL - && optimize_atomic_compare_exchange_p (stmt)) + if (code == GIMPLE_CALL) { - /* For __atomic_compare_exchange_N if the second argument - is &var, don't mark var addressable; - if it becomes non-addressable, we'll rewrite it into - ATOMIC_COMPARE_EXCHANGE call. */ - tree arg = gimple_call_arg (stmt, 1); - gimple_call_set_arg (stmt, 1, null_pointer_node); - gimple_ior_addresses_taken (addresses_taken, stmt); - gimple_call_set_arg (stmt, 1, arg); + if (optimize_atomic_compare_exchange_p (stmt)) + { + /* For __atomic_compare_exchange_N if the second argument + is &var, don't mark var addressable; + if it becomes non-addressable, we'll rewrite it into + ATOMIC_COMPARE_EXCHANGE call. */ + tree arg = gimple_call_arg (stmt, 1); + gimple_call_set_arg (stmt, 1, null_pointer_node); + gimple_ior_addresses_taken (addresses_taken, stmt); + gimple_call_set_arg (stmt, 1, arg); + } + else if (sanitize_asan_mark && is_asan_mark_p (stmt)) + ; + else + gimple_ior_addresses_taken (addresses_taken, stmt); } else /* Note all addresses taken by the stmt. */ @@ -1675,15 +1702,17 @@ execute_update_addresses_taken (void) for -g vs. -g0. */ for (var = DECL_ARGUMENTS (cfun->decl); var; var = DECL_CHAIN (var)) maybe_optimize_var (var, addresses_taken, not_reg_needs, - suitable_for_renaming); + suitable_for_renaming, marked_nonaddressable); FOR_EACH_VEC_SAFE_ELT (cfun->local_decls, i, var) maybe_optimize_var (var, addresses_taken, not_reg_needs, - suitable_for_renaming); + suitable_for_renaming, marked_nonaddressable); /* Operand caches need to be recomputed for operands referencing the updated variables and operands need to be rewritten to expose bare symbols. */ - if (!bitmap_empty_p (suitable_for_renaming)) + if (!bitmap_empty_p (suitable_for_renaming) + || (asan_sanitize_use_after_scope () + && !bitmap_empty_p (marked_nonaddressable))) { FOR_EACH_BB_FN (bb, cfun) for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);) @@ -1841,6 +1870,17 @@ execute_update_addresses_taken (void) continue; } } + else if (sanitize_asan_mark && is_asan_mark_p (stmt)) + { + tree var = TREE_OPERAND (gimple_call_arg (stmt, 1), 0); + if (bitmap_bit_p (suitable_for_renaming, DECL_UID (var)) + || bitmap_bit_p (marked_nonaddressable, DECL_UID (var))) + { + unlink_stmt_vdef (stmt); + gsi_remove (&gsi, true); + continue; + } + } for (i = 0; i < gimple_call_num_args (stmt); ++i) { tree *argp = gimple_call_arg_ptr (stmt, i); @@ -1896,6 +1936,7 @@ execute_update_addresses_taken (void) BITMAP_FREE (not_reg_needs); BITMAP_FREE (addresses_taken); BITMAP_FREE (suitable_for_renaming); + BITMAP_FREE (marked_nonaddressable); timevar_pop (TV_ADDRESS_TAKEN); } -- 2.10.1