[Openmp-dev] Possible misuse of libomptarget plugin interface

[Johannes Doerfert via Openmp-dev]

I accidentally dropped the list in my earlier reply.

The conclusion is this is a bug, proposed fix is here:
   https://reviews.llvm.org/D96667

~ Johannes


On 2/11/21 4:19 PM, Guilherme Valarini via Openmp-dev wrote:
> Hi everyone,
>
> I am part of a team that is currently developing a new device target for
> libomptarget. We came across some strange behavior from the target agnostic
> layer and through some digging we have found two possible misuses of the
> plugin interface for the async functions (quoted below). In both cases, the
> address to a local variable is passed as the host data to be copied to the
> target device, but since this function can be asynchronously executed, such
> variables can get out of scope when the actual submission is done.
>
> Is there anyone that could check if this is an actual issue with the target
> agnostic layer? I could have sent a bug report but since I actually do not
> know if this is actually a problem, I thought it would be better to report
> it here first .
>
> File: openmp/libomptarget/src/omptarget.cpp:419
>
>> if (arg_types[i] & OMP_TGT_MAPTYPE_PTR_AND_OBJ && !IsHostPtr) {
>>    DP("Update pointer (" DPxMOD ") -> [" DPxMOD "]\n",
>>        DPxPTR(PointerTgtPtrBegin), DPxPTR(TgtPtrBegin));
>>    uint64_t Delta = (uint64_t)HstPtrBegin - (uint64_t)HstPtrBase;
>>    void *TgtPtrBase = (void *)((uint64_t)TgtPtrBegin - Delta);
>>    int rt = Device.submitData(PointerTgtPtrBegin, &TgtPtrBase,
>>                                sizeof(void *), async_info_ptr);
>>    if (rt != OFFLOAD_SUCCESS) {
>>      REPORT("Copying data to device failed.\n");
>>      return OFFLOAD_FAIL;
>>    }
>>    // create shadow pointers for this entry
>>    Device.ShadowMtx.lock();
>>    Device.ShadowPtrMap[Pointer_HstPtrBegin] = {
>>        HstPtrBase, PointerTgtPtrBegin, TgtPtrBase};
>>    Device.ShadowMtx.unlock();
>> }
>>
> File: openmp/libomptarget/src/omptarget.cpp:1141
>
>> DP("Parent lambda base " DPxMOD "\n", DPxPTR(TgtPtrBase));
>> uint64_t Delta = (uint64_t)HstPtrBegin - (uint64_t)HstPtrBase;
>> void *TgtPtrBegin = (void *)((uintptr_t)TgtPtrBase + Delta);
>> void *PointerTgtPtrBegin = Device.getTgtPtrBegin(
>>      HstPtrVal, ArgSizes[I], IsLast, false, IsHostPtr);
>> if (!PointerTgtPtrBegin) {
>>    DP("No lambda captured variable mapped (" DPxMOD ") - ignored\n",
>>        DPxPTR(HstPtrVal));
>>    continue;
>> }
>> if (PM->RTLs.RequiresFlags & OMP_REQ_UNIFIED_SHARED_MEMORY &&
>>      TgtPtrBegin == HstPtrBegin) {
>>    DP("Unified memory is active, no need to map lambda captured"
>>        "variable (" DPxMOD ")\n",
>>        DPxPTR(HstPtrVal));
>>    continue;
>> }
>> DP("Update lambda reference (" DPxMOD ") -> [" DPxMOD "]\n",
>>      DPxPTR(PointerTgtPtrBegin), DPxPTR(TgtPtrBegin));
>> Ret = Device.submitData(TgtPtrBegin, &PointerTgtPtrBegin,
>>                          sizeof(void *), AsyncInfo);
>> if (Ret != OFFLOAD_SUCCESS) {
>>    REPORT("Copying data to device failed.\n");
>>    return OFFLOAD_FAIL;
>> }
>
> Note: We already came up with a way of fixing the first case by first
> storing TgtPtrBase in the shadow map and passing a pointer to the correct
> map member (which will live long enough for the async function to complete)
> instead of the local variable. But the second case is a little bit trickier
> to fix without any additional data added to the target agnostic structures.
>
> Thanks in advance!
>
>
> _______________________________________________
> Openmp-dev mailing list
> Openmp-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/openmp-dev