Add support for exceptions to tsan (PR sanitizer/64265)

Message ID	20141215185006.GT1667@tucnak.redhat.com
State	New
Headers	show Return-Path: <gcc-patches-return-387443-incoming=patchwork.ozlabs.org@gcc.gnu.org> DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:reply-to:mime-version :content-type; q=dns; s=default; b=EqfKKk6FLFyw1r4U4Vx7CuXNLzpFf ZlvOCUYCpVYOui7UC1kNAqD2XA7J8/5dBmnzuXLeAQllNrGbV7nq3TnKSu5NRxJC kQSkJgkUYD3/DzDyT4rFuzeU+QZW6DljoR0BnFWsbhvGAfqyLf1hx1f8ulKf8JTA wS9E/kWngdnxIw= Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk Sender: gcc-patches-owner@gcc.gnu.org Date: Mon, 15 Dec 2014 19:50:06 +0100 From: Jakub Jelinek <jakub@redhat.com> To: Dodji Seketeli <dseketel@redhat.com>, Dmitry Vyukov <dvyukov@google.com> Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] Add support for exceptions to tsan (PR sanitizer/64265) Message-ID: <20141215185006.GT1667@tucnak.redhat.com> Reply-To: Jakub Jelinek <jakub@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12)

Jakub Jelinek Dec. 15, 2014, 6:50 p.m. UTC

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:

	Jakub

Dmitry Vyukov Dec. 16, 2014, 9:25 a.m. UTC | #1

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 (&copy_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 (&copy_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 (&copy_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

Dmitry Vyukov Dec. 16, 2014, 9:27 a.m. UTC | #2

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 (&copy_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 (&copy_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 (&copy_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

Richard Biener Dec. 16, 2014, 9:47 a.m. UTC | #3

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 (&copy_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 (&copy_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 (&copy_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

Jakub Jelinek Dec. 16, 2014, 10:23 a.m. UTC | #4

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

Jakub Jelinek Dec. 16, 2014, 11:16 a.m. UTC | #5

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

Richard Biener Dec. 16, 2014, 3:02 p.m. UTC | #6

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

Dmitry Vyukov June 27, 2015, 2:53 p.m. UTC | #7

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 (&copy_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 (&copy_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 (&copy_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

Jakub Jelinek June 27, 2015, 5:12 p.m. UTC | #8

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

Dmitry Vyukov June 27, 2015, 6:33 p.m. UTC | #9

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

Add support for exceptions to tsan (PR sanitizer/64265)

Commit Message

Comments

Patch