[llvm] [OpenMP] Fix crash with duplicate mapping on target directive (PR #146136)

[Julian Brown via llvm-commits]

jtb20 wrote:

> Thanks for fixing this!
> 
> I'm a little confused by what you wrote about the runtime behavior. In the code comment, you wrote:
> 
> > [...] But, the shadow pointer is only initialised on the target for
> > // the first copy, and the second copy clobbers it.  So, this condition
> > // avoids the (second) copy here if we have already set shadow pointer info.
> 
> In your PR post, you mentioned that for step 4 "the previous shadow pointer metadata is still present, so the runtime doesn't modify the target pointer a second time."
> 
> So, the runtime (before the fix) didn't do the same data copy twice, but instead overwrote the previously correct shadow pointer info with the host pointer metadata leading to the target pointer point to the host memory?

There are three copy operations that take place, and a fourth that doesn't. So we have something like:

- copy partial struct "s.mem" from host to target, pointing to host memory.
- update the s.mem pointer to point to target memory instead (and create an entry in the "States->ShadowPtrInfos" set).
- copy partial struct "s.mem" from host to target *again*, pointing to host memory.
- now, addShadowPointer sees that we have an entry in States->ShadowPtrInfos, so *doesn't* modify the s.mem pointer again. So, we try to access the host version of the pointer on the target.

Those are just the operations corresponding to the pointer/struct member "s.mem" itself, not the separately-allocated block it points to. HTH!

https://github.com/llvm/llvm-project/pull/146136