From bfd714822b2c97dfcc970c722f430ca995eebafc Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Thu, 19 Mar 2015 08:53:38 +0100 Subject: [PATCH] re PR sanitizer/65400 (tsan mis-compiles inlineable C functions) 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. From-SVN: r221508 --- gcc/ChangeLog | 14 +++++ gcc/ipa-split.c | 60 +++++++++++++++++--- gcc/testsuite/ChangeLog | 12 +++- gcc/testsuite/c-c++-common/tsan/pr65400-1.c | 85 +++++++++++++++++++++++++++++ gcc/testsuite/c-c++-common/tsan/pr65400-2.c | 10 ++++ 5 files changed, 169 insertions(+), 12 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/tsan/pr65400-1.c create mode 100644 gcc/testsuite/c-c++-common/tsan/pr65400-2.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index d8585e4..251fc3a 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,17 @@ +2015-03-19 Jakub Jelinek + + 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. + 2015-03-18 DJ Delorie * config/rl78/rl78-virt.md (andqi3_virt): Allow far operands. diff --git a/gcc/ipa-split.c b/gcc/ipa-split.c index 5640238..5d5db0e 100644 --- a/gcc/ipa-split.c +++ b/gcc/ipa-split.c @@ -769,7 +769,8 @@ consider_split (struct split_point *current, bitmap non_ssa_vars, of the form: = tmp_var; return - 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 = 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, tree retval, gimple_stmt_iterator *gsi) 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 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 *split_point) || 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 *split_point) } 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 *split_point) /* 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; diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 77d24a1..405eaeb 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2015-03-19 Jakub Jelinek + + PR sanitizer/65400 + * c-c++-common/tsan/pr65400-1.c: New test. + * c-c++-common/tsan/pr65400-2.c: New test. + 2015-03-18 Paolo Carlini PR c++/59816 @@ -6,7 +12,7 @@ 2015-03-18 Paul Thomas PR fortran/59198 - * gfortran.dg/proc_ptr_comp_45.f90 : Make tests fuzzy. + * gfortran.dg/proc_ptr_comp_45.f90: Make tests fuzzy. 2015-03-18 Martin Liska @@ -55,8 +61,8 @@ 2015-03-17 Paul Thomas PR fortran/59198 - * gfortran.dg/proc_ptr_comp_44.f90 : New test - * gfortran.dg/proc_ptr_comp_45.f90 : New test + * gfortran.dg/proc_ptr_comp_44.f90: New test. + * gfortran.dg/proc_ptr_comp_45.f90: New test. 2015-03-16 Jerry DeLisle diff --git a/gcc/testsuite/c-c++-common/tsan/pr65400-1.c b/gcc/testsuite/c-c++-common/tsan/pr65400-1.c new file mode 100644 index 0000000..96fbbfd --- /dev/null +++ b/gcc/testsuite/c-c++-common/tsan/pr65400-1.c @@ -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 +#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" } */ diff --git a/gcc/testsuite/c-c++-common/tsan/pr65400-2.c b/gcc/testsuite/c-c++-common/tsan/pr65400-2.c new file mode 100644 index 0000000..de9131a --- /dev/null +++ b/gcc/testsuite/c-c++-common/tsan/pr65400-2.c @@ -0,0 +1,10 @@ +/* PR sanitizer/65400 */ +/* { dg-do compile } */ + +extern void foo (int *); + +void +baz4 (int *p) +{ + foo (p); +} -- 2.7.4