PR libgomp/51376 fix

[Alan Modra]

On Thu, Dec 01, 2011 at 12:36:08PM +0100, Jakub Jelinek wrote:
> On Thu, Dec 01, 2011 at 09:58:08PM +1030, Alan Modra wrote:
> > The GOMP_task change fixes a similar potential problem.  Bootstrapped
> > and regression tested powerpc-linux.  OK to apply?
> > 
> > 	PR libgomp/51376
> > 	* task.c (GOMP_taskwait): Don't access task->children outside of
> > 	task_lock mutex region.
> > 	(GOMP_task): Likewise.
> 
> Can't this be solved just by adding a barrier?  The access to the var
> outside of the lock has been quite intentional, to avoid locking in the
> common case where there are no children.

No, I tried that and the task-6.C testcase still failed although not
quite as readily.  I was using

if (task == NULL
    || __atomic_load_n (&task->children, MEMMODEL_ACQUIRE) == 0)

You need a release in the child as well as the acquire to ensure
proper synchronisation, and there's a window for failure between the
child clearing task->children and performing a release as part of the
mutex unlock.

Oops, on looking at this email I see I attached an old patch..  This
one avoids a segfault on trying to lock team->task_lock when there
is no team.  This one really has been bootstrapped and regression
tested successfully.

	PR libgomp/51376
	* task.c (GOMP_taskwait): Don't access task->children outside of
	task_lock mutex region.
	(GOMP_task): Likewise.

Index: libgomp/task.c
===================================================================
--- libgomp/task.c	(revision 181902)
+++ libgomp/task.c	(working copy)
@@ -116,10 +116,11 @@ GOMP_task (void (*fn) (void *), void *da
 	}
       else
 	fn (data);
-      if (task.children)
+      if (team != NULL)
 	{
 	  gomp_mutex_lock (&team->task_lock);
-	  gomp_clear_parent (task.children);
+	  if (task.children != NULL)
+	    gomp_clear_parent (task.children);
 	  gomp_mutex_unlock (&team->task_lock);
 	}
       gomp_end_task ();
@@ -290,8 +291,9 @@ GOMP_taskwait (void)
   struct gomp_task *child_task = NULL;
   struct gomp_task *to_free = NULL;
 
-  if (task == NULL || task->children == NULL)
+  if (task == NULL || team == NULL)
     return;
+
   gomp_mutex_lock (&team->task_lock);
   while (1)
     {

-- 
Alan Modra
Australia Development Lab, IBM