[Openmp-dev] Possible misuse of libomptarget plugin interface

[Guilherme Valarini via Openmp-dev]

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!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/openmp-dev/attachments/20210211/a46470d9/attachment.html>