[PATCH] D140663: CUDA/HIP: Use kernel name to map to symbol

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D140663#4203604 <https://reviews.llvm.org/D140663#4203604>, @tra wrote:

> It appears that this patch may be causing a use-after free when we attempt to generate kernel registration code. 
> The root cause is that the value we insert into `KernelHandles` by name is later on replaced by a different instance of the global value with the same name.
> AFAICT, the invalidation issue was present before but we accidentally avoided it because we only looked up the still-valid new entries. The dangling references were still in the map, but not accessed.

Agree. I think the reason is that the `F` we passed into `CGNVCUDARuntime::getKernelHandle` may be replaced by a new function with the same name. Luckily, the new function should be passed to `CGNVCUDARuntime::getKernelHandle` again, therefore we get a chance to update our maps.



================
Comment at: clang/lib/CodeGen/CGCUDANV.cpp:1198
+  auto Loc = KernelHandles.find(F->getName());
   if (Loc != KernelHandles.end())
     return Loc->second;
----------------
It is possible that F is replaced with a new function with the same name. In this case, we need to update our map, so add a condition `&& Loc->second == F` to the above condition.




================
Comment at: clang/lib/CodeGen/CGCUDANV.cpp:1207-1215
   auto *Var = new llvm::GlobalVariable(
       TheModule, F->getType(), /*isConstant=*/true, F->getLinkage(),
       /*Initializer=*/nullptr,
       CGM.getMangledName(
           GD.getWithKernelReferenceKind(KernelReferenceKind::Kernel)));
   Var->setAlignment(CGM.getPointerAlign().getAsAlign());
   Var->setDSOLocal(F->isDSOLocal());
----------------
Add a condition `if (Loc == KernelHandles.end())` to the above code for creating and modifying `Var`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140663/new/

https://reviews.llvm.org/D140663