diff mbox

Fix sporadic failure in g++.dg/tsan/aligned_vs_unaligned_race.C

Message ID F96E10AE-1B00-4710-B3C4-449DABB1F71E@comcast.net
State New
Headers show

Commit Message

Mike Stump Jan. 6, 2015, 9:28 p.m. UTC 
On Jan 5, 2015, at 5:06 PM, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
> 
> I tried to elaborate your idea a bit, and used proper atomics.
> It is necessary that the logic of the step function is completely invisible to tsan.

> Therefore it should not call sleep,

Again, sleep can’t fix race conditions, so therefore tsan can never use it as an indication that no race exists.  If it ever did, that would be a bug.  The only case where that would not be true, would be a hard real time analysis verifier, and, when we get one, sleep can be so modified.

> because that can be intercepted, even if the code is not instrumented.

I don’t see the relevance.

> Unfortunately there is no __attribute__((no_sanitize_thread))

So, is that any harder to add then:

?  Doesn’t look to be much an issue, but, I’m not a tsan code-gen person, so they might have to weigh in.

> So some tests start to fail.
> 
> FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C   -O0  execution test
> FAIL: g++.dg/tsan/aligned_vs_unaligned_race.C   -O2  execution test

I don’t see this when I do this:

__attribute__((no_sanitize_thread))
void step (int i)
{
  while (__atomic_load_n (&serial, __ATOMIC_ACQUIRE) != i - 1) ;
  __atomic_store_n (&serial, i, __ATOMIC_RELEASE);
}

to step.

Any issues just adding no_sanitize_thread to let us do what we want to do?

And the last point, should we add a sched_yield or a pthread_yield to the busy loop to be nice:

__attribute__((no_sanitize_thread))
void step (int i)
{
  while (__atomic_load_n (&serial, __ATOMIC_ACQUIRE) != i - 1)
    sched_yield ();
  __atomic_store_n (&serial, i, __ATOMIC_RELEASE);
}

?  One a single core, this seems like it would run faster.
diff mbox

Patch

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index ce141b6..58bdc9e 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -766,6 +766,9 @@  const struct attribute_spec c_common_attribute_table[] =
   { "no_sanitize_address",    0, 0, true, false, false,
 			      handle_no_sanitize_address_attribute,
 			      false },
+  { "no_sanitize_thread",     0, 0, true, false, false,
+			      handle_no_sanitize_address_attribute,
+			      false },
   { "no_sanitize_undefined",  0, 0, true, false, false,
 			      handle_no_sanitize_undefined_attribute,
 			      false },
diff --git a/gcc/tsan.c b/gcc/tsan.c
index 678fcdc..cafb150 100644
--- a/gcc/tsan.c
+++ b/gcc/tsan.c
@@ -758,7 +758,9 @@  public:
   opt_pass * clone () { return new pass_tsan (m_ctxt); }
   virtual bool gate (function *)
 {
-  return (flag_sanitize & SANITIZE_THREAD) != 0;
+  return ((flag_sanitize & SANITIZE_THREAD) != 0
+	  && !lookup_attribute ("no_sanitize_thread",
+                                DECL_ATTRIBUTES (current_function_decl)));
 }
 
   virtual unsigned int execute (function *) { return tsan_pass (); }
@@ -798,7 +800,9 @@  public:
   /* opt_pass methods: */
   virtual bool gate (function *)
     {
-      return (flag_sanitize & SANITIZE_THREAD) != 0 && !optimize;
+      return ((flag_sanitize & SANITIZE_THREAD) != 0 && !optimize
+	      && !lookup_attribute ("no_sanitize_thread",
+				    DECL_ATTRIBUTES (current_function_decl)));
     }
 
   virtual unsigned int execute (function *) { return tsan_pass (); }