Hi! As discussed in the PR, to support exceptions in -fsanitize=thread code, it is desirable to call __tsan_func_exit also when leaving functions by means of exceptions.
Adding EH too late sounds too hard to me, so this patch instead adds an internal call during gimplification, makes sure the inliner removes the internal calls from the inline functions (we don't care about inlines, only about functions we emit), and for functions that didn't go through gimplify_function_tree (e.g. omp/tm etc. functions) just keeps using the old __tsan_func_exit additions. Bootstrapped/regtested on x86_64-linux and i686-linux. Ok for trunk? On: #include <pthread.h> int v; int foo (int x) { if (x < 99) throw x; v++; return x; } void * tf (void *) { for (int i = 0; i < 100; i++) try { foo (i); } catch (int) {} return NULL; } int main () { pthread_t th; if (pthread_create (&th, NULL, tf, NULL)) return 0; v++; pthread_join (th, NULL); return 0; } I used to get without this patch: ================== WARNING: ThreadSanitizer: data race (pid=20449) Read of size 4 at 0x0000006020e0 by thread T1: #0 foo(int) /usr/src/gcc/obj/gcc/ts.C:10 (ts+0x000000400cb9) #1 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #2 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #3 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #4 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #5 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #6 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #7 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #8 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #9 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #10 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #11 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #12 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #13 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #14 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #15 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #16 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #17 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #18 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #19 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #20 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #21 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #22 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #23 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #24 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #25 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #26 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #27 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #28 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #29 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #30 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #31 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #32 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #33 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #34 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #35 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #36 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #37 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #38 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #39 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #40 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #41 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #42 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #43 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #44 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #45 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #46 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #47 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #48 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #49 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #50 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #51 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #52 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #53 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #54 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #55 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #56 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #57 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #58 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #59 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #60 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #61 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #62 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #63 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #64 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #65 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #66 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #67 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #68 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #69 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #70 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #71 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #72 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #73 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #74 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #75 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #76 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #77 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #78 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #79 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #80 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #81 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #82 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #83 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #84 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #85 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #86 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #87 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #88 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #89 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #90 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #91 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #92 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #93 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #94 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #95 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #96 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #97 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #98 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #99 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) #100 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d13) Previous write of size 4 at 0x0000006020e0 by main thread: #0 main /usr/src/gcc/obj/gcc/ts.C:28 (ts+0x000000400db9) Location is global '<null>' of size 0 at 0x000000000000 (ts+0x0000006020e0) Thread T1 (tid=20451, running) created by main thread at: #0 pthread_create ../../../../libsanitizer/tsan/tsan_interceptors.cc:895 #(libtsan.so.0+0x0000000278d4) #1 main /usr/src/gcc/obj/gcc/ts.C:26 (ts+0x000000400d8c) SUMMARY: ThreadSanitizer: data race /usr/src/gcc/obj/gcc/ts.C:10 foo(int) ================== ThreadSanitizer: reported 1 warnings and with this patch I get: ================== WARNING: ThreadSanitizer: data race (pid=20788) Read of size 4 at 0x0000006020e0 by thread T1: #0 foo(int) /usr/src/gcc/obj/gcc/ts.C:10 (ts+0x000000400cb9) #1 tf(void*) /usr/src/gcc/obj/gcc/ts.C:18 (ts+0x000000400d28) Previous write of size 4 at 0x0000006020e0 by main thread: #0 main /usr/src/gcc/obj/gcc/ts.C:28 (ts+0x000000400dd9) Location is global '<null>' of size 0 at 0x000000000000 (ts+0x0000006020e0) Thread T1 (tid=20790, running) created by main thread at: #0 pthread_create ../../../../libsanitizer/tsan/tsan_interceptors.cc:895 #(libtsan.so.0+0x0000000278d4) #1 main /usr/src/gcc/obj/gcc/ts.C:26 (ts+0x000000400dac) SUMMARY: ThreadSanitizer: data race /usr/src/gcc/obj/gcc/ts.C:10 foo(int) ================== ThreadSanitizer: reported 1 warnings 2014-12-15 Jakub Jelinek <ja...@redhat.com> PR sanitizer/64265 * gimplify.c (gimplify_function_tree): Add TSAN_FUNC_EXIT internal call as cleanup of the whole body. * internal-fn.def (TSAN_FUNC_EXIT): New internal call. * tsan.c (replace_func_exit): New function. (instrument_func_exit): Moved earlier. (instrument_memory_accesses): Adjust TSAN_FUNC_EXIT internal calls. Call instrument_func_exit if no TSAN_FUNC_EXIT internal calls have been found. (tsan_pass): Don't call instrument_func_exit. * internal-fn.c (expand_TSAN_FUNC_EXIT): New function. * tree-inline.c (copy_bb): Drop TSAN_FUNC_EXIT internal calls during inlining. --- gcc/internal-fn.def.jj 2014-12-13 12:09:38.545301645 +0100 +++ gcc/internal-fn.def 2014-12-15 13:26:41.315704348 +0100 @@ -60,3 +60,4 @@ DEF_INTERNAL_FN (ASAN_CHECK, ECF_TM_PURE DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (SUB_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) DEF_INTERNAL_FN (MUL_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL) +DEF_INTERNAL_FN (TSAN_FUNC_EXIT, ECF_NOVOPS | ECF_LEAF | ECF_NOTHROW, NULL) --- gcc/tree-inline.c.jj 2014-11-29 12:35:01.000000000 +0100 +++ gcc/tree-inline.c 2014-12-15 13:40:49.477018309 +0100 @@ -1902,7 +1902,7 @@ copy_bb (copy_body_data *id, basic_block gsi_replace (©_gsi, new_call, false); stmt = new_call; } - else if (is_gimple_call (stmt) + else if (call_stmt && id->call_stmt && (decl = gimple_call_fndecl (stmt)) && DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL @@ -1929,6 +1929,15 @@ copy_bb (copy_body_data *id, basic_block gsi_replace (©_gsi, new_stmt, false); stmt = new_stmt; } + else if (call_stmt + && id->call_stmt + && gimple_call_internal_p (stmt) + && gimple_call_internal_fn (stmt) == IFN_TSAN_FUNC_EXIT) + { + /* Drop TSAN_FUNC_EXIT () internal calls during inlining. */ + gsi_remove (©_gsi, false); + continue; + } /* Statements produced by inlining can be unfolded, especially when we constant propagated some operands. We can't fold --- gcc/gimplify.c.jj 2014-12-13 12:09:38.923295132 +0100 +++ gcc/gimplify.c 2014-12-15 13:26:41.315704348 +0100 @@ -9049,6 +9049,22 @@ gimplify_function_tree (tree fndecl) seq = NULL; gimple_seq_add_stmt (&seq, new_bind); gimple_set_body (fndecl, seq); + bind = new_bind; + } + + if (flag_sanitize & SANITIZE_THREAD) + { + gcall *call = gimple_build_call_internal (IFN_TSAN_FUNC_EXIT, 0); + gimple tf = gimple_build_try (seq, call, GIMPLE_TRY_FINALLY); + gbind *new_bind = gimple_build_bind (NULL, tf, gimple_bind_block (bind)); + /* Clear the block for BIND, since it is no longer directly inside + the function, but within a try block. */ + gimple_bind_set_block (bind, NULL); + /* Replace the current function body with the body + wrapped in the try/finally TF. */ + seq = NULL; + gimple_seq_add_stmt (&seq, new_bind); + gimple_set_body (fndecl, seq); } DECL_SAVED_TREE (fndecl) = NULL_TREE; --- gcc/tsan.c.jj 2014-12-15 10:37:21.853609776 +0100 +++ gcc/tsan.c 2014-12-15 13:29:51.768406631 +0100 @@ -631,6 +631,47 @@ instrument_gimple (gimple_stmt_iterator return instrumented; } +/* Replace TSAN_FUNC_EXIT internal call with function exit tsan builtin. */ + +static void +replace_func_exit (gimple stmt) +{ + tree builtin_decl = builtin_decl_implicit (BUILT_IN_TSAN_FUNC_EXIT); + gimple g = gimple_build_call (builtin_decl, 0); + gimple_set_location (g, cfun->function_end_locus); + gimple_stmt_iterator gsi = gsi_for_stmt (stmt); + gsi_replace (&gsi, g, true); +} + +/* Instrument function exit. Used when TSAN_FUNC_EXIT does not exist. */ + +static void +instrument_func_exit (void) +{ + location_t loc; + basic_block exit_bb; + gimple_stmt_iterator gsi; + gimple stmt, g; + tree builtin_decl; + edge e; + edge_iterator ei; + + /* Find all function exits. */ + exit_bb = EXIT_BLOCK_PTR_FOR_FN (cfun); + FOR_EACH_EDGE (e, ei, exit_bb->preds) + { + gsi = gsi_last_bb (e->src); + stmt = gsi_stmt (gsi); + gcc_assert (gimple_code (stmt) == GIMPLE_RETURN + || gimple_call_builtin_p (stmt, BUILT_IN_RETURN)); + loc = gimple_location (stmt); + builtin_decl = builtin_decl_implicit (BUILT_IN_TSAN_FUNC_EXIT); + g = gimple_build_call (builtin_decl, 0); + gimple_set_location (g, loc); + gsi_insert_before (&gsi, g, GSI_SAME_STMT); + } +} + /* Instruments all interesting memory accesses in the current function. Return true if func entry/exit should be instrumented. */ @@ -640,10 +681,38 @@ instrument_memory_accesses (void) basic_block bb; gimple_stmt_iterator gsi; bool fentry_exit_instrument = false; + bool func_exit_seen = false; + 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)) - fentry_exit_instrument |= instrument_gimple (&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); + } + unsigned int i; + gimple stmt; + FOR_EACH_VEC_ELT (tsan_func_exits, i, stmt) + if (fentry_exit_instrument) + replace_func_exit (stmt); + else + { + gsi = gsi_for_stmt (stmt); + gsi_remove (&gsi, true); + } + if (fentry_exit_instrument && !func_exit_seen) + instrument_func_exit (); return fentry_exit_instrument; } @@ -672,35 +741,6 @@ instrument_func_entry (void) gsi_insert_seq_on_edge_immediate (e, seq); } -/* Instruments function exits. */ - -static void -instrument_func_exit (void) -{ - location_t loc; - basic_block exit_bb; - gimple_stmt_iterator gsi; - gimple stmt, g; - tree builtin_decl; - edge e; - edge_iterator ei; - - /* Find all function exits. */ - exit_bb = EXIT_BLOCK_PTR_FOR_FN (cfun); - FOR_EACH_EDGE (e, ei, exit_bb->preds) - { - gsi = gsi_last_bb (e->src); - stmt = gsi_stmt (gsi); - gcc_assert (gimple_code (stmt) == GIMPLE_RETURN - || gimple_call_builtin_p (stmt, BUILT_IN_RETURN)); - loc = gimple_location (stmt); - builtin_decl = builtin_decl_implicit (BUILT_IN_TSAN_FUNC_EXIT); - g = gimple_build_call (builtin_decl, 0); - gimple_set_location (g, loc); - gsi_insert_before (&gsi, g, GSI_SAME_STMT); - } -} - /* ThreadSanitizer instrumentation pass. */ static unsigned @@ -708,10 +748,7 @@ tsan_pass (void) { initialize_sanitizer_builtins (); if (instrument_memory_accesses ()) - { - instrument_func_entry (); - instrument_func_exit (); - } + instrument_func_entry (); return 0; } --- gcc/internal-fn.c.jj 2014-12-13 12:09:38.822296872 +0100 +++ gcc/internal-fn.c 2014-12-15 13:26:41.317704313 +0100 @@ -208,6 +208,14 @@ expand_ASAN_CHECK (gcall *stmt ATTRIBUTE gcc_unreachable (); } +/* This should get expanded in the tsan pass. */ + +static void +expand_TSAN_FUNC_EXIT (gcall *) +{ + gcc_unreachable (); +} + /* Helper function for expand_addsub_overflow. Return 1 if ARG interpreted as signed in its precision is known to be always positive or 2 if ARG is known to be always negative, or 3 if ARG may Jakub