On February 9, 2018 5:08:24 PM GMT+01:00, Paolo Bonzini <bonz...@gnu.org> wrote:
>Hi all,
>
>in this PR, a dead reference to a function pointer cannot be optimized
>out by the compiler because some ASAN_MARK UNPOISON calls, which are
>placed before the store, cause the containing struct to escape.
>(Without -fsanitize=address, the dead code is eliminated by the first
>DSE pass).
>
>The fix, which works at least for this testcase, is to copy part of the
>sanopt dead code elimination pass early, so that the compiler can see
>fewer UNPOISON calls. I am not sure this is general enough, due to
>the very limited data flow analysis done by
>sanitize_asan_mark_unpoison.
>Another possibility which I considered but did not implement is to mark
>the UNPOISON calls so that they do not cause the parameter to escape.
I'd do this, thus assign proper fnspec attributes to the asan functions.
Richard.
>Any suggestions on how to proceed here (or approval is welcome too :))?
>
>Paolo
>
>2018-02-09 Paolo Bonzini <bonz...@gnu.org>
>
> * passes.def: add pass_sandce before first DSE pass.
> * sanopt.c (pass_data_sandce, pass_sandce, make_pass_sandce): New.
> * tree-pass.h (make_pass_sandce): Declare it.
>
>testsuite:
>2018-02-09 Paolo Bonzini <bonz...@gnu.org>
>
> PR sanitizer/84307
> * gcc.dg/asan/pr84307.c: New test.
>
>diff --git a/gcc/passes.def b/gcc/passes.def
>index 9802f08..180df50 100644
>--- a/gcc/passes.def
>+++ b/gcc/passes.def
>@@ -244,6 +244,7 @@ along with GCC; see the file COPYING3. If not see
> only examines PHIs to discover const/copy propagation
> opportunities. */
> NEXT_PASS (pass_phi_only_cprop);
>+ NEXT_PASS (pass_sandce);
> NEXT_PASS (pass_dse);
> NEXT_PASS (pass_reassoc, true /* insert_powi_p */);
> NEXT_PASS (pass_dce);
>diff --git a/gcc/sanopt.c b/gcc/sanopt.c
>index cd94638..23b8e6e 100644
>--- a/gcc/sanopt.c
>+++ b/gcc/sanopt.c
>@@ -906,6 +906,32 @@ sanopt_optimize (function *fun, bool
>*contains_asan_mark)
>
> namespace {
>
>+const pass_data pass_data_sandce =
>+{
>+ GIMPLE_PASS, /* type */
>+ "sandce", /* name */
>+ OPTGROUP_NONE, /* optinfo_flags */
>+ TV_NONE, /* tv_id */
>+ ( PROP_ssa | PROP_cfg | PROP_gimple_leh ), /* properties_required */
>+ 0, /* properties_provided */
>+ 0, /* properties_destroyed */
>+ 0, /* todo_flags_start */
>+ TODO_update_ssa, /* todo_flags_finish */
>+};
>+
>+class pass_sandce : public gimple_opt_pass
>+{
>+public:
>+ pass_sandce (gcc::context *ctxt)
>+ : gimple_opt_pass (pass_data_sandce, ctxt)
>+ {}
>+
>+ /* opt_pass methods: */
>+ virtual bool gate (function *) { return flag_sanitize &
>SANITIZE_ADDRESS; }
>+ virtual unsigned int execute (function *);
>+
>+}; // class pass_sanopt
>+
> const pass_data pass_data_sanopt =
> {
> GIMPLE_PASS, /* type */
>@@ -1244,6 +1270,31 @@ sanitize_rewrite_addressable_params (function
>*fun)
> }
>
> unsigned int
>+pass_sandce::execute (function *fun)
>+{
>+ basic_block bb;
>+ bool contains_asan_mark = false;
>+
>+ /* Try to remove redundant unpoisoning. */
>+ gimple_stmt_iterator gsi;
>+ FOR_EACH_BB_FN (bb, fun)
>+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>+ {
>+ gimple *stmt = gsi_stmt (gsi);
>+ if (gimple_call_internal_p (stmt, IFN_ASAN_MARK))
>+ {
>+ contains_asan_mark = true;
>+ break;
>+ }
>+ }
>+
>+ if (contains_asan_mark)
>+ sanitize_asan_mark_unpoison ();
>+
>+ return 0;
>+}
>+
>+unsigned int
> pass_sanopt::execute (function *fun)
> {
> basic_block bb;
>@@ -1367,6 +1418,12 @@ pass_sanopt::execute (function *fun)
> } // anon namespace
>
> gimple_opt_pass *
>+make_pass_sandce (gcc::context *ctxt)
>+{
>+ return new pass_sandce (ctxt);
>+}
>+
>+gimple_opt_pass *
> make_pass_sanopt (gcc::context *ctxt)
> {
> return new pass_sanopt (ctxt);
>diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
>index 93a6a99..d5eb055 100644
>--- a/gcc/tree-pass.h
>+++ b/gcc/tree-pass.h
>@@ -350,6 +350,7 @@ extern gimple_opt_pass *make_pass_tsan
>(gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_tsan_O0 (gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_sancov (gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_sancov_O0 (gcc::context *ctxt);
>+extern gimple_opt_pass *make_pass_sandce (gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_lower_cf (gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_refactor_eh (gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_lower_eh (gcc::context *ctxt);
>diff --git a/gcc/testsuite/gcc.dg/asan/pr84307.c
>b/gcc/testsuite/gcc.dg/asan/pr84307.c
>new file mode 100644
>index 0000000..6e1a197
>--- /dev/null
>+++ b/gcc/testsuite/gcc.dg/asan/pr84307.c
>@@ -0,0 +1,21 @@
>+/* PR middle-end/83185 */
>+/* { dg-do link } */
>+/* { dg-options "-O1" } */
>+
>+struct f {
>+ void (*func)(void);
>+};
>+
>+extern void link_error(void);
>+extern int printf(const char *f, ...);
>+
>+static inline struct f *gimme_null(struct f *result)
>+{
>+ return 0;
>+}
>+
>+int main(int argc, char **argv)
>+{
>+ struct f *x = gimme_null(&(struct f) { .func = link_error });
>+ printf("%p", x);
>+}
