[PATCH] D154338: Fix race condition in order-of-destruction between SectionMemoryManager and its MemoryMapper

Stefan Gränitz via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 3 06:01:03 PDT 2023


sgraenitz created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
sgraenitz requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

SectionMemoryManager's default memory mapper used to be a global static
object. If the SectionMemoryManager itself is a global static
object, it might be destroyed after its memory mapper and thus couldn't
use it from the destructor.

The Kaleidoscope tutorial reproduced this situation with MSVC for a long time
and since 47f5c54f997a59bb2c65 <https://reviews.llvm.org/rG47f5c54f997a59bb2c65abe6b8b811f6e7553456> also with GCC. The solution from this patch was
proposed in the existing review https://reviews.llvm.org/D107087 before, but
it didn't move forward.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154338

Files:
  llvm/include/llvm/ExecutionEngine/SectionMemoryManager.h
  llvm/lib/ExecutionEngine/SectionMemoryManager.cpp


Index: llvm/lib/ExecutionEngine/SectionMemoryManager.cpp
===================================================================
--- llvm/lib/ExecutionEngine/SectionMemoryManager.cpp
+++ llvm/lib/ExecutionEngine/SectionMemoryManager.cpp
@@ -101,7 +101,7 @@
   // FIXME: Initialize the Near member for each memory group to avoid
   // interleaving.
   std::error_code ec;
-  sys::MemoryBlock MB = MMapper.allocateMappedMemory(
+  sys::MemoryBlock MB = MMapper->allocateMappedMemory(
       Purpose, RequiredSize, &MemGroup.Near,
       sys::Memory::MF_READ | sys::Memory::MF_WRITE, ec);
   if (ec) {
@@ -204,7 +204,7 @@
 SectionMemoryManager::applyMemoryGroupPermissions(MemoryGroup &MemGroup,
                                                   unsigned Permissions) {
   for (sys::MemoryBlock &MB : MemGroup.PendingMem)
-    if (std::error_code EC = MMapper.protectMappedMemory(MB, Permissions))
+    if (std::error_code EC = MMapper->protectMappedMemory(MB, Permissions))
       return EC;
 
   MemGroup.PendingMem.clear();
@@ -234,7 +234,7 @@
 SectionMemoryManager::~SectionMemoryManager() {
   for (MemoryGroup *Group : {&CodeMem, &RWDataMem, &RODataMem}) {
     for (sys::MemoryBlock &Block : Group->AllocatedMem)
-      MMapper.releaseMappedMemory(Block);
+      MMapper->releaseMappedMemory(Block);
   }
 }
 
@@ -263,11 +263,14 @@
     return sys::Memory::releaseMappedMemory(M);
   }
 };
-
-DefaultMMapper DefaultMMapperInstance;
 } // namespace
 
-SectionMemoryManager::SectionMemoryManager(MemoryMapper *MM)
-    : MMapper(MM ? *MM : DefaultMMapperInstance) {}
+SectionMemoryManager::SectionMemoryManager(MemoryMapper *UnownedMM)
+    : MMapper(UnownedMM), OwnedMMapper(nullptr) {
+  if (!MMapper) {
+    OwnedMMapper = std::make_unique<DefaultMMapper>();
+    MMapper = OwnedMMapper.get();
+  }
+}
 
 } // namespace llvm
Index: llvm/include/llvm/ExecutionEngine/SectionMemoryManager.h
===================================================================
--- llvm/include/llvm/ExecutionEngine/SectionMemoryManager.h
+++ llvm/include/llvm/ExecutionEngine/SectionMemoryManager.h
@@ -185,7 +185,8 @@
   MemoryGroup CodeMem;
   MemoryGroup RWDataMem;
   MemoryGroup RODataMem;
-  MemoryMapper &MMapper;
+  MemoryMapper *MMapper;
+  std::unique_ptr<MemoryMapper> OwnedMMapper;
 };
 
 } // end namespace llvm


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D154338.536734.patch
Type: text/x-patch
Size: 2307 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230703/4cd0d37a/attachment.bin>


More information about the llvm-commits mailing list