On 2018/12/13 11:51 PM, Thomas Schwinge wrote:
> On Thu, 13 Dec 2018 23:28:49 +0800, Chung-Lin Tang<chunglin_tang@mentor.com>  wrote:
>> On 2018/12/7 6:26 AM, Julian Brown wrote:
>>> On Thu, 6 Dec 2018 22:22:46 +0000
>>> Julian Brown<julian@codesourcery.com>  wrote:
>>>
>>>> On Thu, 6 Dec 2018 21:42:14 +0100
>>>> Thomas Schwinge<thomas@codesourcery.com>  wrote:
>>>>
>>>>> [...]
>>>>> ..., where the "Invalid read of size 8" happens, and which
>>>>> eventually would try to "free (tgt)" again, via
>>>>> libgomp/target.c:gomp_unmap_tgt:
>>>>>
>>>>>       attribute_hidden void
>>>>>       gomp_unmap_tgt (struct target_mem_desc *tgt)
>>>>>       {
>>>>>         /* Deallocate on target the tgt->tgt_start .. tgt->tgt_end
>>>>> region.  */ if (tgt->tgt_end)
>>>>>           gomp_free_device_memory (tgt->device_descr, tgt->to_free);
>>>>>       
>>>>>         free (tgt->array);
>>>>>         free (tgt);
>>>>>       }
>>>>>
>>>>> Is the "free (tgt)" in libgomp/target.c:gomp_unmap_vars_async wrong,
>>>>> or something else?

I think I understand the problem now. In gomp_unmap_vars_async(), in the case of
tgt->list_count == 0 (i.e. no map arguments at all) the code should simply free the tgt
and return, while the code in goacc_async_copyout_unmap_vars() didn't handle this case
and always scheduled an asynchronous free of the tgt later, causing that valgrind error
you see.

I am still testing the attached patch, but I think it is the right fix: I reviewed what I
wrote and it seemed the way I organized things into a goacc_async_copyout_unmap_vars() routine,
including the hackish refcount++, etc. is simply unneeded. I have deleted those stuff
and consolidated things back into gomp_unmap_vars_async().

I'll update the whole patches later after complete testing, the attached is the patch atop
of the prior async patches. (the small program you gave above does pass valgrind now)

Julian, I didn't try the OG8 refcount changes, it's just too large a set of changes to
reason about in so short time, maybe later when we are prepared to fix things completely as
you noted what those patches were capable of.

Chung-Lin