Hi!
As the PRs show, -fsanitize=thread hasn't been instrumenting __atomic_clear
and __atomic_test_and_set builtins (i.e. those that operate on bool always),
so we reported a data race on the testcase even when there wasn't one.
And, as the other testcase shows, there were various ICEs with
-fnon-call-exceptions -fsanitize=thread.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2016-09-06 Jakub Jelinek <ja...@redhat.com>
PR sanitizer/68260
* tsan.c: Include target.h and tree-eh.h.
(enum tsan_atomic_action): Add bool_clear and bool_test_and_set.
(BOOL_CLEAR, BOOL_TEST_AND_SET): Define.
(tsan_atomic_table): Add BUILT_IN_ATOMIC_CLEAR and
BUILT_IN_ATOMIC_TEST_AND_SET entries.
(instrument_builtin_call): Remove EH edges of replaced call stmts.
Handle bool_clear and bool_test_and_set.
(instrument_memory_accesses): Add RET argument, purge dead eh edges
at the end of bbs and set *RET in that case to TODO_cleanup_cfg.
(tsan_pass): Adjust instrument_memory_accesses caller, return ret.
* c-c++-common/tsan/pr68260.c: New test.
* g++.dg/tsan/pr68260.C: New test.
--- gcc/tsan.c.jj 2016-07-11 22:18:08.000000000 +0200
+++ gcc/tsan.c 2016-09-06 14:06:21.193642590 +0200
@@ -40,6 +40,8 @@ along with GCC; see the file COPYING3.
#include "tsan.h"
#include "asan.h"
#include "builtins.h"
+#include "target.h"
+#include "tree-eh.h"
/* Number of instrumented memory accesses in the current function. */
@@ -240,7 +242,8 @@ instrument_expr (gimple_stmt_iterator gs
enum tsan_atomic_action
{
check_last, add_seq_cst, add_acquire, weak_cas, strong_cas,
- bool_cas, val_cas, lock_release, fetch_op, fetch_op_seq_cst
+ bool_cas, val_cas, lock_release, fetch_op, fetch_op_seq_cst,
+ bool_clear, bool_test_and_set
};
/* Table how to map sync/atomic builtins to their corresponding
@@ -274,6 +277,10 @@ static const struct tsan_map_atomic
TRANSFORM (fcode, tsan_fcode, fetch_op, code)
#define FETCH_OPS(fcode, tsan_fcode, code) \
TRANSFORM (fcode, tsan_fcode, fetch_op_seq_cst, code)
+#define BOOL_CLEAR(fcode, tsan_fcode) \
+ TRANSFORM (fcode, tsan_fcode, bool_clear, ERROR_MARK)
+#define BOOL_TEST_AND_SET(fcode, tsan_fcode) \
+ TRANSFORM (fcode, tsan_fcode, bool_test_and_set, ERROR_MARK)
CHECK_LAST (ATOMIC_LOAD_1, TSAN_ATOMIC8_LOAD),
CHECK_LAST (ATOMIC_LOAD_2, TSAN_ATOMIC16_LOAD),
@@ -463,7 +470,11 @@ static const struct tsan_map_atomic
LOCK_RELEASE (SYNC_LOCK_RELEASE_2, TSAN_ATOMIC16_STORE),
LOCK_RELEASE (SYNC_LOCK_RELEASE_4, TSAN_ATOMIC32_STORE),
LOCK_RELEASE (SYNC_LOCK_RELEASE_8, TSAN_ATOMIC64_STORE),
- LOCK_RELEASE (SYNC_LOCK_RELEASE_16, TSAN_ATOMIC128_STORE)
+ LOCK_RELEASE (SYNC_LOCK_RELEASE_16, TSAN_ATOMIC128_STORE),
+
+ BOOL_CLEAR (ATOMIC_CLEAR, TSAN_ATOMIC8_STORE),
+
+ BOOL_TEST_AND_SET (ATOMIC_TEST_AND_SET, TSAN_ATOMIC8_EXCHANGE)
};
/* Instrument an atomic builtin. */
@@ -493,6 +504,8 @@ instrument_builtin_call (gimple_stmt_ite
if (!tree_fits_uhwi_p (last_arg)
|| memmodel_base (tree_to_uhwi (last_arg)) >= MEMMODEL_LAST)
return;
+ if (lookup_stmt_eh_lp (stmt))
+ remove_stmt_from_eh_lp (stmt);
gimple_call_set_fndecl (stmt, decl);
update_stmt (stmt);
if (tsan_atomic_table[i].action == fetch_op)
@@ -514,6 +527,8 @@ instrument_builtin_call (gimple_stmt_ite
!= add_acquire
? MEMMODEL_SEQ_CST
: MEMMODEL_ACQUIRE);
+ if (lookup_stmt_eh_lp (stmt))
+ remove_stmt_from_eh_lp (stmt);
update_gimple_call (gsi, decl, num + 1, args[0], args[1], args[2]);
stmt = gsi_stmt (*gsi);
if (tsan_atomic_table[i].action == fetch_op_seq_cst)
@@ -561,6 +576,8 @@ instrument_builtin_call (gimple_stmt_ite
if (!tree_fits_uhwi_p (args[5])
|| memmodel_base (tree_to_uhwi (args[5])) >= MEMMODEL_LAST)
return;
+ if (lookup_stmt_eh_lp (stmt))
+ remove_stmt_from_eh_lp (stmt);
update_gimple_call (gsi, decl, 5, args[0], args[1], args[2],
args[4], args[5]);
return;
@@ -584,6 +601,8 @@ instrument_builtin_call (gimple_stmt_ite
g = gimple_build_assign (t, args[1]);
gsi_insert_before (gsi, g, GSI_SAME_STMT);
lhs = gimple_call_lhs (stmt);
+ if (lookup_stmt_eh_lp (stmt))
+ remove_stmt_from_eh_lp (stmt);
update_gimple_call (gsi, decl, 5, args[0],
build_fold_addr_expr (t), args[2],
build_int_cst (NULL_TREE,
@@ -610,11 +629,66 @@ instrument_builtin_call (gimple_stmt_ite
gcc_assert (num == 1);
t = TYPE_ARG_TYPES (TREE_TYPE (decl));
t = TREE_VALUE (TREE_CHAIN (t));
+ if (lookup_stmt_eh_lp (stmt))
+ remove_stmt_from_eh_lp (stmt);
update_gimple_call (gsi, decl, 3, gimple_call_arg (stmt, 0),
build_int_cst (t, 0),
build_int_cst (NULL_TREE,
MEMMODEL_RELEASE));
return;
+ case bool_clear:
+ case bool_test_and_set:
+ if (BOOL_TYPE_SIZE != 8)
+ {
+ decl = NULL_TREE;
+ for (j = 1; j < 5; j++)
+ if (BOOL_TYPE_SIZE == (8 << j))
+ {
+ enum built_in_function tsan_fcode
+ = (enum built_in_function)
+ (tsan_atomic_table[i].tsan_fcode + j);
+ decl = builtin_decl_implicit (tsan_fcode);
+ break;
+ }
+ if (decl == NULL_TREE)
+ return;
+ }
+ last_arg = gimple_call_arg (stmt, num - 1);
+ if (!tree_fits_uhwi_p (last_arg)
+ || memmodel_base (tree_to_uhwi (last_arg)) >= MEMMODEL_LAST)
+ return;
+ t = TYPE_ARG_TYPES (TREE_TYPE (decl));
+ t = TREE_VALUE (TREE_CHAIN (t));
+ if (lookup_stmt_eh_lp (stmt))
+ remove_stmt_from_eh_lp (stmt);
+ if (tsan_atomic_table[i].action == bool_clear)
+ {
+ update_gimple_call (gsi, decl, 3, gimple_call_arg (stmt, 0),
+ build_int_cst (t, 0), last_arg);
+ 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);
+ stmt = gsi_stmt (*gsi);
+ lhs = gimple_call_lhs (stmt);
+ if (lhs == NULL_TREE)
+ return;
+ if (targetm.atomic_test_and_set_trueval != 1
+ || !useless_type_conversion_p (TREE_TYPE (lhs),
+ TREE_TYPE (t)))
+ {
+ tree new_lhs = make_ssa_name (TREE_TYPE (t));
+ gimple_call_set_lhs (stmt, new_lhs);
+ if (targetm.atomic_test_and_set_trueval != 1)
+ g = gimple_build_assign (lhs, NE_EXPR, new_lhs,
+ build_int_cst (TREE_TYPE (t), 0));
+ else
+ g = gimple_build_assign (lhs, NOP_EXPR, new_lhs);
+ gsi_insert_after (gsi, g, GSI_NEW_STMT);
+ update_stmt (stmt);
+ }
+ return;
default:
continue;
}
@@ -706,7 +780,7 @@ instrument_func_exit (void)
Return true if func entry/exit should be instrumented. */
static bool
-instrument_memory_accesses (void)
+instrument_memory_accesses (unsigned int *ret)
{
basic_block bb;
gimple_stmt_iterator gsi;
@@ -715,22 +789,26 @@ 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 (is_gimple_call (stmt)
- && gimple_call_internal_p (stmt)
- && gimple_call_internal_fn (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 (is_gimple_call (stmt)
+ && gimple_call_internal_p (stmt)
+ && gimple_call_internal_fn (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))
+ *ret = TODO_cleanup_cfg;
+ }
unsigned int i;
gimple *stmt;
FOR_EACH_VEC_ELT (tsan_func_exits, i, stmt)
@@ -776,10 +854,11 @@ instrument_func_entry (void)
static unsigned
tsan_pass (void)
{
+ unsigned int ret = 0;
initialize_sanitizer_builtins ();
- if (instrument_memory_accesses ())
+ if (instrument_memory_accesses (&ret))
instrument_func_entry ();
- return 0;
+ return ret;
}
/* Inserts __tsan_init () into the list of CTORs. */
--- gcc/testsuite/c-c++-common/tsan/pr68260.c.jj 2016-09-06
15:56:27.772209548 +0200
+++ gcc/testsuite/c-c++-common/tsan/pr68260.c 2016-09-06 16:08:23.424203133
+0200
@@ -0,0 +1,28 @@
+/* PR sanitizer/68260 */
+
+#include <pthread.h>
+#include <stdbool.h>
+
+bool lock;
+int counter;
+
+void *
+tf (void *arg)
+{
+ (void) arg;
+ while (__atomic_test_and_set (&lock, __ATOMIC_ACQUIRE))
+ ;
+ ++counter;
+ __atomic_clear (&lock, __ATOMIC_RELEASE);
+ return (void *) 0;
+}
+
+int
+main ()
+{
+ pthread_t thr;
+ pthread_create (&thr, 0, tf, 0);
+ tf ((void *) 0);
+ pthread_join (thr, 0);
+ return 0;
+}
--- gcc/testsuite/g++.dg/tsan/pr68260.C.jj 2016-09-06 15:58:41.118530940
+0200
+++ gcc/testsuite/g++.dg/tsan/pr68260.C 2016-09-06 16:03:11.988121144 +0200
@@ -0,0 +1,19 @@
+/* PR sanitizer/68260 */
+/* { dg-do compile } */
+/* { dg-additional-options "-fnon-call-exceptions" } */
+
+int
+foo (bool *p, int *q)
+{
+ try
+ {
+ while (__atomic_test_and_set (p, __ATOMIC_ACQUIRE))
+ ;
+ __atomic_clear (p, __ATOMIC_RELEASE);
+ return __sync_fetch_and_add_4 (q, 4);
+ }
+ catch (...)
+ {
+ return -1;
+ }
+}
Jakub
