On Tue, Jan 12, 2016 at 03:23:32PM +0100, Jakub Jelinek wrote:
> But looking at GOMP_PLUGIN_target_task_completion, I see we have a bug in
> there,
> gomp_mutex_lock (&team->task_lock);
> if (ttask->state == GOMP_TARGET_TASK_READY_TO_RUN)
> {
> ttask->state = GOMP_TARGET_TASK_FINISHED;
> gomp_mutex_unlock (&team->task_lock);
> }
> ttask->state = GOMP_TARGET_TASK_FINISHED;
> gomp_target_task_completion (team, task);
> gomp_mutex_unlock (&team->task_lock);
> there was meant to be I think return; after the first unlock, otherwise
> it doubly unlocks the same lock, and performs gomp_target_task_completion
> without the lock held, which may cause great havoc.
I've bootstrapped/regtested this on x86_64-linux and i686-linux (no
offloading), and regtested on x86_64-linux -> x86_64-intelmicemul-linux
offloading, and then tested also with sleep (1) added in between gomp_mutex_lock
and preceeding gomp_target_task_fn call, both without and with the fix.
Without the fix with the extra sleeps, 3 target-3*.c tests FAILed (crashed,
hanged forever), with the fix everything was ok.
Installed on the trunk.
2016-01-15 Jakub Jelinek <ja...@redhat.com>
* task.c (GOMP_PLUGIN_target_task_completion): Add missing return.
--- libgomp/task.c.jj 2016-01-04 14:38:59.000000000 +0100
+++ libgomp/task.c 2016-01-15 00:11:08.851909133 +0100
@@ -579,6 +579,7 @@ GOMP_PLUGIN_target_task_completion (void
{
ttask->state = GOMP_TARGET_TASK_FINISHED;
gomp_mutex_unlock (&team->task_lock);
+ return;
}
ttask->state = GOMP_TARGET_TASK_FINISHED;
gomp_target_task_completion (team, task);
Jakub
