Message ID | 20141215185006.GT1667@tucnak.redhat.com |
---|---|
State | New |
Headers | show |
I am not qualified to review the actual code changes, but from the description it looks good to me. It adds a EH frame to every function, right? In 64-bit mode there is no runtime penalty, right? Do you have any idea about binary size increase? Does gcc build in C++ mode nowadays? It can be a good test. I am somewhat worried about potential corner cases that can lead to compiler crashes or missed/excessive func_entry/exit calls. But I guess there is no way to make it working w/o getting some code in first. Rigorous testing would require running a large C++ app that actually throws and catches exception and precisely verifying stacks in reports. On Mon, Dec 15, 2014 at 9:50 PM, Jakub Jelinek <jakub@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 <jakub@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
Cross referenced this patch from https://code.google.com/p/thread-sanitizer/issues/detail?id=78 On Tue, Dec 16, 2014 at 12:25 PM, Dmitry Vyukov <dvyukov@google.com> wrote: > I am not qualified to review the actual code changes, but from the > description it looks good to me. > > It adds a EH frame to every function, right? In 64-bit mode there is > no runtime penalty, right? Do you have any idea about binary size > increase? Does gcc build in C++ mode nowadays? It can be a good test. > > I am somewhat worried about potential corner cases that can lead to > compiler crashes or missed/excessive func_entry/exit calls. But I > guess there is no way to make it working w/o getting some code in > first. Rigorous testing would require running a large C++ app that > actually throws and catches exception and precisely verifying stacks > in reports. > > > On Mon, Dec 15, 2014 at 9:50 PM, Jakub Jelinek <jakub@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 <jakub@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
On Mon, Dec 15, 2014 at 7:50 PM, Jakub Jelinek <jakub@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? So the issue is externally throwing EH, right? I wonder if we can teach the unwinder to do __tsan_func_exit instead, or have hooks in it that can be used by tsan? At least the issue seems generic enough for code instrumentation to consider a more general solution? How does -finstrument-functions work with externally throwing EH? Thanks, Richard. > 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 <jakub@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
On Tue, Dec 16, 2014 at 10:47:06AM +0100, Richard Biener wrote: > On Mon, Dec 15, 2014 at 7:50 PM, Jakub Jelinek <jakub@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? > > So the issue is externally throwing EH, right? I wonder if we can teach > the unwinder to do __tsan_func_exit instead, or have hooks in it that can > be used by tsan? At least the issue seems generic enough for > code instrumentation to consider a more general solution? How does > -finstrument-functions work with externally throwing EH? -finstrument-functions works the way I wrote the patch, in fact the gimplify_function_tree bits I've added were right after the -finstrument-functions handling of the same. Anyway, the alternative would be to wrap the various personality functions like __gcc_personality_v0 __gxx_personality_v0 __gcj_personality_v0 __gccgo_personality_v0 __gnu_objc_personality_v0 call the dlsym (, RTLD_NEXT) version from there and if it returns _URC_INSTALL_CONTEXT , try to figure out what frame it will be in. We can query the IP (_Unwind_GetIP), or the CFA (_Unwind_GetCFA), but then map it through supposedly target dependent code to the actual frame pointers __tsan_func_* store (do they?). The problem with wrapping those personality functions is that I'm not sure how could it work with the various -static* options. Say if you -static-libstdc++ (and link libtsan dynamically), then the __gxx_personality_v0 in the binary will not be wrapped by what is in libtsan.so. If you -static-libstdc++ -static-libtsan, then __gxx_personality_v0 linked in will be very likely from libstdc++ and thus again not overloaded, or if -ltsan would come first (right now it doesn't), then you still couldn't use dlsym to get at the overloaded symbol. So, while the __tsan_func_exit in cleanup is more expensive at runtime, it will work with all the wierdo options people are using. Jakub
On Tue, Dec 16, 2014 at 01:25:54PM +0400, Dmitry Vyukov wrote: > I am not qualified to review the actual code changes, but from the > description it looks good to me. > > It adds a EH frame to every function, right? In 64-bit mode there is > no runtime penalty, right? Do you have any idea about binary size No, not to every function. Only to function that throws or could throw something externally, and only if exceptions are on. > increase? Does gcc build in C++ mode nowadays? It can be a good test. GCC builds with C++, but with -fno-exceptions, so it is not a good example, but e.g. libstdc++, as it generally supports exceptions, could be an example. Jakub
On Tue, Dec 16, 2014 at 11:23 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Tue, Dec 16, 2014 at 10:47:06AM +0100, Richard Biener wrote: >> On Mon, Dec 15, 2014 at 7:50 PM, Jakub Jelinek <jakub@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? >> >> So the issue is externally throwing EH, right? I wonder if we can teach >> the unwinder to do __tsan_func_exit instead, or have hooks in it that can >> be used by tsan? At least the issue seems generic enough for >> code instrumentation to consider a more general solution? How does >> -finstrument-functions work with externally throwing EH? > > -finstrument-functions works the way I wrote the patch, in fact the > gimplify_function_tree bits I've added were right after the > -finstrument-functions handling of the same. > > Anyway, the alternative would be to wrap the various personality functions > like > __gcc_personality_v0 > __gxx_personality_v0 > __gcj_personality_v0 > __gccgo_personality_v0 > __gnu_objc_personality_v0 > call the dlsym (, RTLD_NEXT) version from there and if it returns > _URC_INSTALL_CONTEXT , try to figure out what frame it will be in. > We can query the IP (_Unwind_GetIP), or the CFA (_Unwind_GetCFA), but then > map it through supposedly target dependent code to the actual frame pointers > __tsan_func_* store (do they?). I suppose we could annotate the CFA with appropriate information? > The problem with wrapping those personality functions is that I'm not sure > how could it work with the various -static* options. Say if you > -static-libstdc++ (and link libtsan dynamically), then the > __gxx_personality_v0 in the binary will not be wrapped by what is in > libtsan.so. > If you > -static-libstdc++ -static-libtsan, then __gxx_personality_v0 linked in > will be very likely from libstdc++ and thus again not overloaded, or if > -ltsan would come first (right now it doesn't), then you still couldn't > use dlsym to get at the overloaded symbol. Yeah, which is why I suggested that one might want to have a generic (list of) callbacks that can be registered (like malloc hooks). It would be all extensions but GCC controls the unwinding ABI, right? > So, while the __tsan_func_exit in cleanup is more expensive at runtime, > it will work with all the wierdo options people are using. True. As it follows existing practice with -finstrument-functions the patch is probably the way to go for GCC 5. I was just wondering of a less heavy-weight solution piggy-backing on the unwinder. I suppose that idea wouldn't work for SJLJ exceptions so you'd need both approaches anyway. Richard. > Jakub
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 <jakub@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 <jakub@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
On Sat, Jun 27, 2015 at 04:53:22PM +0200, Dmitry Vyukov wrote: > 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. It is already in GCC 5.1 and on the trunk too. Jakub
On Sat, Jun 27, 2015 at 7:12 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Sat, Jun 27, 2015 at 04:53:22PM +0200, Dmitry Vyukov wrote: >> 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. > > It is already in GCC 5.1 and on the trunk too. Great! Thanks I've grepped "excep.*tsan" in ChangeLog, but found nothing. Nevermind
================== 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 <jakub@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