Hi! TSAN_FUNC_EXIT internal function is special, because we drop it during inlining, so for fnsplit we need to be careful about it, otherwise we can end up with unbalanced pairs of tsan entry/exit marker functions.
This patch gives up unless it is one of the two most common cases with -fsanitize=thread - either TSAN_FUNC_EXIT in the return bb, which is handled as accepting it as return bb despite the TSAN_FUNC_EXIT call - duplicating the TSAN_FUNC_EXIT is exactly the right thing to do in that case, we want it in the *.part.* function and also in the caller, if the former is later inlined into the latter again, the TSAN_FUNC_EXIT of the former is dropped. And another case that we handle is if there is TSAN_FUNC_EXIT on the predecessors of return_bb - we can handle that case easily too by duplicating TSAN_FUNC_EXIT after the *.part.* call. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2015-03-18 Jakub Jelinek <ja...@redhat.com> PR sanitizer/65400 * ipa-split.c (find_return_bb): Allow TSAN_FUNC_EXIT internal call in the return bb. (find_split_points): Add RETURN_BB argument, don't call find_return_bb. (split_function): Likewise. Add ADD_TSAN_FUNC_EXIT argument, if true append TSAN_FUNC_EXIT internal call after the call to the split off function. (execute_split_functions): Call find_return_bb here. Don't optimize if TSAN_FUNC_EXIT is found in unexpected places. Adjust find_split_points and split_function calls. * c-c++-common/tsan/pr65400-1.c: New test. * c-c++-common/tsan/pr65400-2.c: New test. --- gcc/ipa-split.c.jj 2015-03-17 18:10:11.000000000 +0100 +++ gcc/ipa-split.c 2015-03-18 17:12:45.017773858 +0100 @@ -769,7 +769,8 @@ consider_split (struct split_point *curr of the form: <retval> = tmp_var; return <retval> - but return_bb can not be more complex than this. + but return_bb can not be more complex than this (except for + -fsanitize=thread we allow TSAN_FUNC_EXIT () internal call in there). If nothing is found, return the exit block. When there are multiple RETURN statement, chose one with return value, @@ -814,6 +815,13 @@ find_return_bb (void) found_return = true; retval = gimple_return_retval (return_stmt); } + /* For -fsanitize=thread, allow also TSAN_FUNC_EXIT () in the return + bb. */ + else if ((flag_sanitize & SANITIZE_THREAD) + && is_gimple_call (stmt) + && gimple_call_internal_p (stmt) + && gimple_call_internal_fn (stmt) == IFN_TSAN_FUNC_EXIT) + ; else break; } @@ -1074,12 +1082,11 @@ typedef struct the component used by consider_split. */ static void -find_split_points (int overall_time, int overall_size) +find_split_points (basic_block return_bb, int overall_time, int overall_size) { stack_entry first; vec<stack_entry> stack = vNULL; basic_block bb; - basic_block return_bb = find_return_bb (); struct split_point current; current.header_time = overall_time; @@ -1236,19 +1243,20 @@ insert_bndret_call_after (tree retbnd, t gimple_call_set_lhs (bndret, retbnd); gsi_insert_after (gsi, bndret, GSI_CONTINUE_LINKING); } + /* Split function at SPLIT_POINT. */ static void -split_function (struct split_point *split_point) +split_function (basic_block return_bb, struct split_point *split_point, + bool add_tsan_func_exit) { vec<tree> args_to_pass = vNULL; bitmap args_to_skip; tree parm; int num = 0; cgraph_node *node, *cur_node = cgraph_node::get (current_function_decl); - basic_block return_bb = find_return_bb (); basic_block call_bb; - gcall *call; + gcall *call, *tsan_func_exit_call = NULL; edge e; edge_iterator ei; tree retval = NULL, real_retval = NULL, retbnd = NULL; @@ -1534,11 +1542,18 @@ split_function (struct split_point *spli || DECL_BY_REFERENCE (DECL_RESULT (current_function_decl)))) gimple_call_set_return_slot_opt (call, true); + if (add_tsan_func_exit) + tsan_func_exit_call = gimple_build_call_internal (IFN_TSAN_FUNC_EXIT, 0); + /* Update return value. This is bit tricky. When we do not return, do nothing. When we return we might need to update return_bb or produce a new return statement. */ if (!split_part_return_p) - gsi_insert_after (&gsi, call, GSI_NEW_STMT); + { + gsi_insert_after (&gsi, call, GSI_NEW_STMT); + if (tsan_func_exit_call) + gsi_insert_after (&gsi, tsan_func_exit_call, GSI_NEW_STMT); + } else { e = make_edge (call_bb, return_bb, @@ -1642,6 +1657,8 @@ split_function (struct split_point *spli } else gsi_insert_after (&gsi, call, GSI_NEW_STMT); + if (tsan_func_exit_call) + gsi_insert_after (&gsi, tsan_func_exit_call, GSI_NEW_STMT); } /* We don't use return block (there is either no return in function or multiple of them). So create new basic block with return statement. @@ -1684,6 +1701,8 @@ split_function (struct split_point *spli /* Build bndret call to obtain returned bounds. */ if (retbnd) insert_bndret_call_after (retbnd, retval, &gsi); + if (tsan_func_exit_call) + gsi_insert_after (&gsi, tsan_func_exit_call, GSI_NEW_STMT); ret = gimple_build_return (retval); gsi_insert_after (&gsi, ret, GSI_NEW_STMT); } @@ -1792,6 +1811,8 @@ execute_split_functions (void) /* Compute local info about basic blocks and determine function size/time. */ bb_info_vec.safe_grow_cleared (last_basic_block_for_fn (cfun) + 1); memset (&best_split_point, 0, sizeof (best_split_point)); + basic_block return_bb = find_return_bb (); + int tsan_exit_found = -1; FOR_EACH_BB_FN (bb, cfun) { int time = 0; @@ -1818,16 +1839,37 @@ execute_split_functions (void) freq, this_size, this_time); print_gimple_stmt (dump_file, stmt, 0, 0); } + + if ((flag_sanitize & SANITIZE_THREAD) + && is_gimple_call (stmt) + && gimple_call_internal_p (stmt) + && gimple_call_internal_fn (stmt) == IFN_TSAN_FUNC_EXIT) + { + /* We handle TSAN_FUNC_EXIT for splitting either in the + return_bb, or in its immediate predecessors. */ + if ((bb != return_bb && !find_edge (bb, return_bb)) + || (tsan_exit_found != -1 + && tsan_exit_found != (bb != return_bb))) + { + if (dump_file) + fprintf (dump_file, "Not splitting: TSAN_FUNC_EXIT" + " in unexpected basic block.\n"); + BITMAP_FREE (forbidden_dominators); + bb_info_vec.release (); + return 0; + } + tsan_exit_found = bb != return_bb; + } } overall_time += time; overall_size += size; bb_info_vec[bb->index].time = time; bb_info_vec[bb->index].size = size; } - find_split_points (overall_time, overall_size); + find_split_points (return_bb, overall_time, overall_size); if (best_split_point.split_bbs) { - split_function (&best_split_point); + split_function (return_bb, &best_split_point, tsan_exit_found == 1); BITMAP_FREE (best_split_point.ssa_names_to_pass); BITMAP_FREE (best_split_point.split_bbs); todo = TODO_update_ssa | TODO_cleanup_cfg; --- gcc/testsuite/c-c++-common/tsan/pr65400-1.c.jj 2015-03-18 18:33:27.478940014 +0100 +++ gcc/testsuite/c-c++-common/tsan/pr65400-1.c 2015-03-18 18:43:49.235836436 +0100 @@ -0,0 +1,85 @@ +/* PR sanitizer/65400 */ +/* { dg-shouldfail "tsan" } */ +/* { dg-additional-options "-fno-omit-frame-pointer -ldl" } */ +/* { dg-additional-sources pr65400-2.c } */ + +#include <pthread.h> +#include "tsan_barrier.h" + +static pthread_barrier_t barrier; +int v; +int q; +int o; +extern void baz4 (int *); + +__attribute__((noinline, noclone)) int +bar (int x) +{ + q += x; + return x; +} + +void +foo (int *x) +{ + if (__builtin_expect (x == 0, 1)) + return; + bar (bar (bar (bar (*x)))); +} + +__attribute__((noinline, noclone)) void +baz1 (int *x) +{ + foo (x); +} + +__attribute__((noinline, noclone)) void +baz2 (int **x) +{ + foo (*x); +} + +__attribute__((noinline, noclone)) void +baz3 (void) +{ + barrier_wait (&barrier); + v++; +} + +__attribute__((noinline, noclone)) void +baz5 (void) +{ + int i; + o = 1; + baz1 (&o); + int *p = &o; + baz2 (&p); + for (i = 0; i < 128; i++) + baz4 (&o); + if (q != 130 * 4) + __builtin_abort (); + baz3 (); +} + +__attribute__((noinline, noclone)) void * +tf (void *arg) +{ + (void) arg; + baz5 (); + return NULL; +} + +int +main () +{ + pthread_t th; + barrier_init (&barrier, 2); + if (pthread_create (&th, NULL, tf, NULL)) + return 0; + v++; + barrier_wait (&barrier); + pthread_join (th, NULL); + return 0; +} + +/* { dg-output "WARNING: ThreadSanitizer: data race.*#2 _?tf" } */ --- gcc/testsuite/c-c++-common/tsan/pr65400-2.c.jj 2015-03-18 18:40:07.855433878 +0100 +++ gcc/testsuite/c-c++-common/tsan/pr65400-2.c 2015-03-18 18:40:03.862498763 +0100 @@ -0,0 +1,10 @@ +/* PR sanitizer/65400 */ +/* { dg-do compile } */ + +extern void foo (int *); + +void +baz4 (int *p) +{ + foo (p); +} Jakub