On Mon, 20 Mar 2017, Jakub Jelinek wrote:
> Hi!
>
> libtsan atomics aren't throwing, so if we transform atomics which
> are throwing with -fnon-call-exceptions, we need to clean up EH stuff.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Huh, but this means with TSAN we create wrong-code with
-fnon-call-exceptions and programs will crash instead of properly
catching things like null-pointer accesses?
We should at least document this or reject sanitize=thread with
-fnon-call-exceptions.
Richard.
> 2017-03-20 Jakub Jelinek <ja...@redhat.com>
>
> PR sanitizer/80110
> * tsan.c: Include tree-eh.h.
> (instrument_builtin_call): Call maybe_clean_eh_stmt or
> maybe_clean_or_replace_eh_stmt where needed.
> (instrument_memory_accesses): Add cfg_changed argument.
> Call gimple_purge_dead_eh_edges on each block and set *cfg_changed
> if it returned true.
> (tsan_pass): Adjust caller. Return TODO_cleanup_cfg if cfg_changed.
>
> * g++.dg/tsan/pr80110.C: New test.
>
> --- gcc/tsan.c.jj 2017-03-20 17:55:25.000000000 +0100
> +++ gcc/tsan.c 2017-03-20 19:43:34.715321105 +0100
> @@ -38,6 +38,7 @@ along with GCC; see the file COPYING3.
> #include "tree-iterator.h"
> #include "tree-ssa-propagate.h"
> #include "tree-ssa-loop-ivopts.h"
> +#include "tree-eh.h"
> #include "tsan.h"
> #include "asan.h"
> #include "builtins.h"
> @@ -504,6 +505,7 @@ instrument_builtin_call (gimple_stmt_ite
> return;
> gimple_call_set_fndecl (stmt, decl);
> update_stmt (stmt);
> + maybe_clean_eh_stmt (stmt);
> if (tsan_atomic_table[i].action == fetch_op)
> {
> args[1] = gimple_call_arg (stmt, 1);
> @@ -524,6 +526,7 @@ instrument_builtin_call (gimple_stmt_ite
> ? MEMMODEL_SEQ_CST
> : MEMMODEL_ACQUIRE);
> update_gimple_call (gsi, decl, num + 1, args[0], args[1], args[2]);
> + maybe_clean_or_replace_eh_stmt (stmt, gsi_stmt (*gsi));
> stmt = gsi_stmt (*gsi);
> if (tsan_atomic_table[i].action == fetch_op_seq_cst)
> {
> @@ -572,6 +575,7 @@ instrument_builtin_call (gimple_stmt_ite
> return;
> update_gimple_call (gsi, decl, 5, args[0], args[1], args[2],
> args[4], args[5]);
> + maybe_clean_or_replace_eh_stmt (stmt, gsi_stmt (*gsi));
> return;
> case bool_cas:
> case val_cas:
> @@ -599,6 +603,7 @@ instrument_builtin_call (gimple_stmt_ite
> MEMMODEL_SEQ_CST),
> build_int_cst (NULL_TREE,
> MEMMODEL_SEQ_CST));
> + maybe_clean_or_replace_eh_stmt (stmt, gsi_stmt (*gsi));
> if (tsan_atomic_table[i].action == val_cas && lhs)
> {
> tree cond;
> @@ -623,6 +628,7 @@ instrument_builtin_call (gimple_stmt_ite
> build_int_cst (t, 0),
> build_int_cst (NULL_TREE,
> MEMMODEL_RELEASE));
> + maybe_clean_or_replace_eh_stmt (stmt, gsi_stmt (*gsi));
> return;
> case bool_clear:
> case bool_test_and_set:
> @@ -651,11 +657,13 @@ instrument_builtin_call (gimple_stmt_ite
> {
> update_gimple_call (gsi, decl, 3, gimple_call_arg (stmt, 0),
> build_int_cst (t, 0), last_arg);
> + maybe_clean_or_replace_eh_stmt (stmt, gsi_stmt (*gsi));
> return;
> }
> t = build_int_cst (t, targetm.atomic_test_and_set_trueval);
> update_gimple_call (gsi, decl, 3, gimple_call_arg (stmt, 0),
> t, last_arg);
> + maybe_clean_or_replace_eh_stmt (stmt, gsi_stmt (*gsi));
> stmt = gsi_stmt (*gsi);
> lhs = gimple_call_lhs (stmt);
> if (lhs == NULL_TREE)
> @@ -766,7 +774,7 @@ instrument_func_exit (void)
> Return true if func entry/exit should be instrumented. */
>
> static bool
> -instrument_memory_accesses (void)
> +instrument_memory_accesses (bool *cfg_changed)
> {
> basic_block bb;
> gimple_stmt_iterator gsi;
> @@ -775,20 +783,24 @@ instrument_memory_accesses (void)
> auto_vec<gimple *> tsan_func_exits;
>
> FOR_EACH_BB_FN (bb, cfun)
> - 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_TSAN_FUNC_EXIT))
> - {
> - if (fentry_exit_instrument)
> - replace_func_exit (stmt);
> - else
> - tsan_func_exits.safe_push (stmt);
> - func_exit_seen = true;
> - }
> - else
> - fentry_exit_instrument |= instrument_gimple (&gsi);
> - }
> + {
> + 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_TSAN_FUNC_EXIT))
> + {
> + if (fentry_exit_instrument)
> + replace_func_exit (stmt);
> + else
> + tsan_func_exits.safe_push (stmt);
> + func_exit_seen = true;
> + }
> + else
> + fentry_exit_instrument |= instrument_gimple (&gsi);
> + }
> + if (gimple_purge_dead_eh_edges (bb))
> + *cfg_changed = true;
> + }
> unsigned int i;
> gimple *stmt;
> FOR_EACH_VEC_ELT (tsan_func_exits, i, stmt)
> @@ -835,9 +847,10 @@ static unsigned
> tsan_pass (void)
> {
> initialize_sanitizer_builtins ();
> - if (instrument_memory_accesses ())
> + bool cfg_changed = false;
> + if (instrument_memory_accesses (&cfg_changed))
> instrument_func_entry ();
> - return 0;
> + return cfg_changed ? TODO_cleanup_cfg : 0;
> }
>
> /* Inserts __tsan_init () into the list of CTORs. */
> --- gcc/testsuite/g++.dg/tsan/pr80110.C.jj 2017-03-20 19:51:02.525497662
> +0100
> +++ gcc/testsuite/g++.dg/tsan/pr80110.C 2017-03-20 19:50:14.000000000
> +0100
> @@ -0,0 +1,16 @@
> +// PR sanitizer/80110
> +// { dg-do compile }
> +// { dg-options "-fnon-call-exceptions -fsanitize=thread" }
> +
> +struct A
> +{
> + int b ();
> + void c () { static int d = b (); }
> +};
> +
> +void
> +foo ()
> +{
> + A e;
> + e.c ();
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
