Possible misuse of libomptarget plugin interface

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!

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