[Openmp-commits] [openmp] b923c15 - [libomptarget] Fix a race condition in checkDeviceAndCtors

[Joachim Protze via Openmp-commits]

Author: Lechen Yu
Date: 2022-10-31T15:38:22+01:00
New Revision: b923c15d3c00c78a7b9258e79755df9587e1f0b1

URL: https://github.com/llvm/llvm-project/commit/b923c15d3c00c78a7b9258e79755df9587e1f0b1
DIFF: https://github.com/llvm/llvm-project/commit/b923c15d3c00c78a7b9258e79755df9587e1f0b1.diff

LOG: [libomptarget] Fix a race condition in checkDeviceAndCtors

When multiple threads invoke checkDeviceAndCtors, both of them may read true
from the shared variable Device.HasPendingGlobals, and then invoke initLibrary
redundantly. Therefore only protecting the access to Device.HasPendingGlobals
is not sufficient to guarantee that initLibrary is invoked just once.

To fix this race condition, we move the invocation of initLibrary into the
critical section, and remove the same lock inside initLibrary.

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

Added: 
    

Modified: 
    openmp/libomptarget/src/omptarget.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/libomptarget/src/omptarget.cpp b/openmp/libomptarget/src/omptarget.cpp
index e8b7a594cf633..5b1a11947d030 100644
--- a/openmp/libomptarget/src/omptarget.cpp
+++ b/openmp/libomptarget/src/omptarget.cpp
@@ -80,9 +80,6 @@ static int initLibrary(DeviceTy &Device) {
   int Rc = OFFLOAD_SUCCESS;
   bool SupportsEmptyImages = Device.RTL->supports_empty_images &&
                              Device.RTL->supports_empty_images() > 0;
-
-  std::lock_guard<decltype(Device.PendingGlobalsMtx)> LG(
-      Device.PendingGlobalsMtx);
   {
     std::lock_guard<decltype(PM->TrlTblMtx)> LG(PM->TrlTblMtx);
     for (auto *HostEntriesBegin : PM->HostEntriesBeginRegistrationOrder) {
@@ -320,16 +317,14 @@ bool checkDeviceAndCtors(int64_t &DeviceID, ident_t *Loc) {
   DeviceTy &Device = *PM->Devices[DeviceID];
 
   // Check whether global data has been mapped for this device
-  bool HasPendingGlobals;
   {
     std::lock_guard<decltype(Device.PendingGlobalsMtx)> LG(
         Device.PendingGlobalsMtx);
-    HasPendingGlobals = Device.HasPendingGlobals;
-  }
-  if (HasPendingGlobals && initLibrary(Device) != OFFLOAD_SUCCESS) {
-    REPORT("Failed to init globals on device %" PRId64 "\n", DeviceID);
-    handleTargetOutcome(false, Loc);
-    return true;
+    if (Device.HasPendingGlobals && initLibrary(Device) != OFFLOAD_SUCCESS) {
+      REPORT("Failed to init globals on device %" PRId64 "\n", DeviceID);
+      handleTargetOutcome(false, Loc);
+      return true;
+    }
   }
 
   return false;