2016-01-16  Torvald Riegel  <triegel@redhat.com>

	* method-gl.cc (gl_wt_dispatch::trycommit): Ensure proxy privatization

	safety.

	* method-ml.cc (ml_wt_dispatch::trycommit): Likewise.

	* libitm/testsuite/libitm.c/priv-1.c: New.

--- libitm/method-gl.cc

+++ libitm/method-gl.cc

@@ -291,12 +291,18 @@ public:

         // See begin_or_restart() for why we need release memory order here.

 	v = gl_mg::clear_locked(v) + 1;

 	o_gl_mg.orec.store(v, memory_order_release);

-

-	// Need to ensure privatization safety. Every other transaction must

-	// have a snapshot time that is at least as high as our commit time

-	// (i.e., our commit must be visible to them).

-	priv_time = v;

       }

+

+    // Need to ensure privatization safety. Every other transaction must have

+    // a snapshot time that is at least as high as our commit time (i.e., our

+    // commit must be visible to them).  Because of proxy privatization, we

+    // must ensure that even if we are a read-only transaction.  See

+    // ml_wt_dispatch::trycommit() for details: We can't get quite the same

+    // set of problems because we just use one orec and thus, for example,

+    // there cannot be concurrent writers -- but we can still get pending

+    // loads to privatized data when not ensuring privatization safety, which

+    // is problematic if the program unmaps the privatized memory.

+    priv_time = v;

     return true;

   }

--- libitm/method-ml.cc

+++ libitm/method-ml.cc

@@ -513,6 +513,21 @@ public:

     if (!tx->writelog.size())

       {

         tx->readlog.clear();

+        // We still need to ensure privatization safety, unfortunately.  While

+        // we cannot have privatized anything by ourselves (because we are not

+        // an update transaction), we can have observed the commits of

+        // another update transaction that privatized something.  Because any

+        // commit happens before ensuring privatization, our snapshot and

+        // commit can thus have happened before ensuring privatization safety

+        // for this commit/snapshot time.  Therefore, before we can return to

+        // nontransactional code that might use the privatized data, we must

+        // ensure privatization safety for our snapshot time.

+        // This still seems to be better than not allowing use of the

+        // snapshot time before privatization safety has been ensured because

+        // we at least can run transactions such as this one, and in the

+        // meantime the transaction producing this commit time might have

+        // finished ensuring privatization safety for it.

+        priv_time = tx->shared_state.load(memory_order_relaxed);

         return true;

       }

--- /dev/null

+++ libitm/testsuite/libitm.c/priv-1.c

@@ -0,0 +1,117 @@

+/* Quick stress test for proxy privatization.  */

+

+/* We need to use a TM method that has to enforce privatization safety

+   explicitly.  */

+/* { dg-set-target-env-var ITM_DEFAULT_METHOD "ml_wt" } */

+/* { dg-options "-std=gnu11" } */

+

+#include <stdlib.h>

+#include <stdio.h>

+#include <pthread.h>

+

+/* Make them likely to be mapped to different orecs.  */

+#define ALIGN __attribute__((aligned (256)))

+/* Don't make these static to work around PR 68591.  */

+int x ALIGN;

+int *ptr ALIGN;

+int *priv_ptr ALIGN;

+int priv_value ALIGN;

+int barrier ALIGN = 0;

+const int iters = 100;

+

+static void arrive_and_wait (int expected_value)

+{

+  int now = __atomic_add_fetch (&barrier, 1, __ATOMIC_ACQ_REL);

+  while (now < expected_value)

+    __atomic_load (&barrier, &now, __ATOMIC_ACQUIRE);

+}

+

+static void __attribute__((transaction_pure,noinline)) delay (int i)

+{

+  for (volatile int v = 0; v < i; v++);

+}

+

+/* This tries to catch a case in which proxy privatization safety is not

+   ensured by privatization_user.  Specifically, it's access to the value

+   of it's transactional snapshot of ptr must read from an uncommitted write

+   by writer; thus, writer must still be active but must have read ptr before

+   proxy can privatize *ptr by assigning to ptr.

+   We try to make this interleaving more likely by delaying the commit of

+   writer and the start of proxy.  */

+static void *writer (void *dummy __attribute__((unused)))

+{

+  for (int i = 0; i < iters; i++)

+    {

+      /* Initialize state in each round.  */

+      x = 0;

+      ptr = &x;

+      priv_ptr = NULL;

+      int wrote = 1;

+      arrive_and_wait (i * 6 + 3);

+      /* Interference by another writer.  Has a conflict with the proxy

+	 privatizer.  */

+      __transaction_atomic

+	{

+	  if (ptr != NULL)

+	    *ptr = 1;

+	  else

+	    wrote = 0;

+	  delay (2000000);

+	}

+      arrive_and_wait (i * 6 + 6);

+      /* If the previous transaction committed first, wrote == 1 and x == 1;

+	 otherwise, if the proxy came first, wrote == 0 and priv_value == 0.

+       */

+      if (wrote != priv_value)

+	abort ();

+    }

+  return NULL;

+}

+

+static void *proxy (void *dummy __attribute__((unused)))

+{

+  for (int i = 0; i < iters; i++)

+    {

+      arrive_and_wait (i * 6 + 3);

+      delay(1000000);

+      __transaction_atomic

+	{

+	  /* Hand-off to privatization-user and its read-only transaction and

+	     subsequent use of privatization.  */

+	  priv_ptr = ptr;

+	  ptr = NULL;

+	}

+      arrive_and_wait (i * 6 + 6);

+    }

+  return NULL;

+}

+

+static void *privatization_user (void *dummy __attribute__((unused)))

+{

+  for (int i = 0; i < iters; i++)

+    {

+      arrive_and_wait (i * 6 + 3);

+      /* Spin until we have gotten a pointer from the proxy.  Then access

+	 the value pointed to nontransactionally.  */

+      int *p = NULL;

+      while (p == NULL)

+	__transaction_atomic { p = priv_ptr; }

+      priv_value = *p;

+      arrive_and_wait (i * 6 + 6);

+    }

+  return NULL;

+}

+

+int main()

+{

+  pthread_t p[3];

+

+  pthread_create (p+0, NULL, writer, NULL);

+  pthread_create (p+1, NULL, proxy, NULL);

+  pthread_create (p+2, NULL, privatization_user, NULL);

+

+  for (int i = 0; i < 3; ++i)

+    pthread_join  (p[i], NULL);

+

+  return 0;

+}