[all-commits] [llvm/llvm-project] 545fcc: [OpenMP][CUDA] Fix potential program crash caused ...

Shilei Tian via All-commits all-commits at lists.llvm.org
Fri Mar 25 19:49:43 PDT 2022


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 545fcc3d842c0912db61591520bd4f760686c5a3
      https://github.com/llvm/llvm-project/commit/545fcc3d842c0912db61591520bd4f760686c5a3
  Author: Shilei Tian <i at tianshilei.me>
  Date:   2022-03-25 (Fri, 25 Mar 2022)

  Changed paths:
    M openmp/libomptarget/plugins/cuda/src/rtl.cpp

  Log Message:
  -----------
  [OpenMP][CUDA] Fix potential program crash caused by double free resources

As we mentioned in the code comments for function `ResourcePoolTy::release`,
at some point there could be two identical resources on the two sides of `Next`
mark. It is usually not an issue, unless the following case:
1. Some resources are not returned.
2. We need to iterate the pool and free the element.

That will cause double free, which is the case for event pool. Since we don't release
events hold by the data map, it can happen that the `Next` mark is not reset, and
we have two identical items in the pool. When the pool is destroyed, we will call
`cuEventDestroy` twice on the same event. In the best case, we can only observe
CUDA errors. In the worst case, it can cause internal failures in CUDART and further
crash.

This patch fixes the issue by tracking all resources that have been given using
an `unordered_set`. We don't remove it when a resource is returned. When the pool
is destroyed, we merge the pool (a `vector`) and the set. In this way, we can make
sure that the set contains all resources allocated from the device. We just need
to iterate the set and free the resource accordingly.

For now, only event pool is set to use it. Stream pool is not because we can make
sure all streams are returned when the plugin is destroyed.

Someone might be wondering, why don't we release all events hold in the data map.
That is because, plugins are determined to be destroyed *before* `libomptarget`.
If we can somehow make the plugin outlast `libomptarget`, life will be much
easier.

Reviewed By: jdoerfert

Differential Revision: https://reviews.llvm.org/D122014




More information about the All-commits mailing list