diff mbox

Another -fsanitize=thread fix (PR sanitizer/65400)

Message ID 20150319094651.GS1746@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek March 19, 2015, 9:46 a.m. UTC
Hi!

This patch fixes the other part of the PR.  On this testcase, ICF creates
a thunk and sets gimple_call_set_tail true on the call in there.  No
TSAN_FUNC_EXIT internal call is added, which isn't a big deal, tsan pass
has code to add it if there are none.  But, nothing was clearing the tail
call flag when this was done, and with -fsanitize=thread in sanitized
functions all functions that call other functions must start with the
__tsan_func_entry call and call __tsan_func_exit at the end (unless they are
noreturn).  Thus, we can only tail call __tsan_func_exit in instrumented
functions.  The patch clears the tail call flag on all calls but
TSAN_FUNC_EXIT (for which instrument_gimple isn't called at all).

Regtested on x86_64-linux, ok for trunk?

2015-03-19  Bernd Edlinger  <bernd.edlinger@hotmail.de>
	    Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/65400
	* tsan.c (instrument_gimple): Clear tail call flag on
	calls.

	* c-c++-common/tsan/pr65400-3.c: New test.


	Jakub

Comments

Richard Biener March 19, 2015, 9:55 a.m. UTC | #1
On Thu, 19 Mar 2015, Jakub Jelinek wrote:

> Hi!
> 
> This patch fixes the other part of the PR.  On this testcase, ICF creates
> a thunk and sets gimple_call_set_tail true on the call in there.  No
> TSAN_FUNC_EXIT internal call is added, which isn't a big deal, tsan pass
> has code to add it if there are none.  But, nothing was clearing the tail
> call flag when this was done, and with -fsanitize=thread in sanitized
> functions all functions that call other functions must start with the
> __tsan_func_entry call and call __tsan_func_exit at the end (unless they are
> noreturn).  Thus, we can only tail call __tsan_func_exit in instrumented
> functions.  The patch clears the tail call flag on all calls but
> TSAN_FUNC_EXIT (for which instrument_gimple isn't called at all).
> 
> Regtested on x86_64-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2015-03-19  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 	    Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR sanitizer/65400
> 	* tsan.c (instrument_gimple): Clear tail call flag on
> 	calls.
> 
> 	* c-c++-common/tsan/pr65400-3.c: New test.
> 
> --- gcc/tsan.c.jj	2015-01-19 09:31:24.000000000 +0100
> +++ gcc/tsan.c	2015-03-19 09:51:39.086804965 +0100
> @@ -680,6 +680,10 @@ instrument_gimple (gimple_stmt_iterator
>        && (gimple_call_fndecl (stmt)
>  	  != builtin_decl_implicit (BUILT_IN_TSAN_INIT)))
>      {
> +      /* All functions with function call will have exit instrumented,
> +	 therefore no function calls other than __tsan_func_exit
> +	 shall appear in the functions.  */
> +      gimple_call_set_tail (as_a <gcall *> (stmt), false);
>        if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
>  	instrument_builtin_call (gsi);
>        return true;
> --- gcc/testsuite/c-c++-common/tsan/pr65400-3.c.jj	2015-03-19 10:07:15.453302478 +0100
> +++ gcc/testsuite/c-c++-common/tsan/pr65400-3.c	2015-03-19 10:27:11.941515580 +0100
> @@ -0,0 +1,75 @@
> +/* PR sanitizer/65400 */
> +/* { dg-shouldfail "tsan" } */
> +/* { dg-additional-options "-fno-omit-frame-pointer -ldl" } */
> +
> +#include <pthread.h>
> +#include "tsan_barrier.h"
> +
> +static pthread_barrier_t barrier;
> +int v;
> +
> +int
> +fn1 (int a, int b, int c)
> +{
> +  int r = (a ^ b) % c;
> +  r = r * a * b + c;
> +  r = (r ^ b) % c;
> +  return r;
> +}
> +
> +int
> +fn2 (int a, int b, int c)
> +{
> +  int r = (a ^ b) % c;
> +  r = r * a * b + c;
> +  r = (r ^ b) % c;
> +  return r;
> +}
> +
> +__attribute__((noinline, noclone)) void
> +foo (void)
> +{
> +  barrier_wait (&barrier);
> +  barrier_wait (&barrier);
> +  v++;
> +}
> +
> +__attribute__((noinline, noclone)) void
> +bar (void)
> +{
> +  int (*fna) (int, int, int);
> +  int (*fnb) (int, int, int);
> +  int i;
> +  asm ("" : "=g" (fna) : "0" (fn1));
> +  asm ("" : "=g" (fnb) : "0" (fn2));
> +  for (i = 0; i < 128; i++)
> +    {
> +      fna (0, 0, i + 1);
> +      fnb (0, 0, i + 1);
> +    }
> +  foo ();
> +}
> +
> +__attribute__((noinline, noclone)) void *
> +tf (void *arg)
> +{
> +  (void) arg;
> +  bar ();
> +  return NULL;
> +}
> +
> +int
> +main ()
> +{
> +  pthread_t th;
> +  barrier_init (&barrier, 2);
> +  if (pthread_create (&th, NULL, tf, NULL))
> +    return 0;
> +  barrier_wait (&barrier);
> +  v++;
> +  barrier_wait (&barrier);
> +  pthread_join (th, NULL);
> +  return 0;
> +}
> +
> +/* { dg-output "WARNING: ThreadSanitizer: data race.*#2 _?tf" } */
> 
> 	Jakub
> 
>
diff mbox

Patch

--- gcc/tsan.c.jj	2015-01-19 09:31:24.000000000 +0100
+++ gcc/tsan.c	2015-03-19 09:51:39.086804965 +0100
@@ -680,6 +680,10 @@  instrument_gimple (gimple_stmt_iterator
       && (gimple_call_fndecl (stmt)
 	  != builtin_decl_implicit (BUILT_IN_TSAN_INIT)))
     {
+      /* All functions with function call will have exit instrumented,
+	 therefore no function calls other than __tsan_func_exit
+	 shall appear in the functions.  */
+      gimple_call_set_tail (as_a <gcall *> (stmt), false);
       if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
 	instrument_builtin_call (gsi);
       return true;
--- gcc/testsuite/c-c++-common/tsan/pr65400-3.c.jj	2015-03-19 10:07:15.453302478 +0100
+++ gcc/testsuite/c-c++-common/tsan/pr65400-3.c	2015-03-19 10:27:11.941515580 +0100
@@ -0,0 +1,75 @@ 
+/* PR sanitizer/65400 */
+/* { dg-shouldfail "tsan" } */
+/* { dg-additional-options "-fno-omit-frame-pointer -ldl" } */
+
+#include <pthread.h>
+#include "tsan_barrier.h"
+
+static pthread_barrier_t barrier;
+int v;
+
+int
+fn1 (int a, int b, int c)
+{
+  int r = (a ^ b) % c;
+  r = r * a * b + c;
+  r = (r ^ b) % c;
+  return r;
+}
+
+int
+fn2 (int a, int b, int c)
+{
+  int r = (a ^ b) % c;
+  r = r * a * b + c;
+  r = (r ^ b) % c;
+  return r;
+}
+
+__attribute__((noinline, noclone)) void
+foo (void)
+{
+  barrier_wait (&barrier);
+  barrier_wait (&barrier);
+  v++;
+}
+
+__attribute__((noinline, noclone)) void
+bar (void)
+{
+  int (*fna) (int, int, int);
+  int (*fnb) (int, int, int);
+  int i;
+  asm ("" : "=g" (fna) : "0" (fn1));
+  asm ("" : "=g" (fnb) : "0" (fn2));
+  for (i = 0; i < 128; i++)
+    {
+      fna (0, 0, i + 1);
+      fnb (0, 0, i + 1);
+    }
+  foo ();
+}
+
+__attribute__((noinline, noclone)) void *
+tf (void *arg)
+{
+  (void) arg;
+  bar ();
+  return NULL;
+}
+
+int
+main ()
+{
+  pthread_t th;
+  barrier_init (&barrier, 2);
+  if (pthread_create (&th, NULL, tf, NULL))
+    return 0;
+  barrier_wait (&barrier);
+  v++;
+  barrier_wait (&barrier);
+  pthread_join (th, NULL);
+  return 0;
+}
+
+/* { dg-output "WARNING: ThreadSanitizer: data race.*#2 _?tf" } */