Hi Jakub, Do you plan to move forward with this patch? Is it waiting to be reviewed? People keep asking about exception support for tsan (I guess it is critical for a significant potion of C++ out there). I am trying to sketch support in llvm. And I am leaning towards an approach similar to yours, that is, add cleanup blocks in middle-end. I think it is more portable and reliable then interception of __personality/_Unwind routines.
On Mon, Dec 15, 2014 at 7:50 PM, Jakub Jelinek <ja...@redhat.com> wrote: > 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