Skip to content

Commit

Permalink
[Kaleidoscope] Fix race condition in order-of-destruction between Sec…
Browse files Browse the repository at this point in the history
…tionMemoryManager and its MemoryMapper

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.
Since 47f5c54 it's triggered with GCC as well. The solution from
this patch was proposed in the existing review https://reviews.llvm.org/D107087
before, but it didn't move forward.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D154338
  • Loading branch information
weliveindetail committed Jul 5, 2023
1 parent 5f1ba3a commit 9ce0641
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 8 deletions.
3 changes: 2 additions & 1 deletion llvm/include/llvm/ExecutionEngine/SectionMemoryManager.h
Expand Up @@ -185,7 +185,8 @@ class SectionMemoryManager : public RTDyldMemoryManager {
MemoryGroup CodeMem;
MemoryGroup RWDataMem;
MemoryGroup RODataMem;
MemoryMapper &MMapper;
MemoryMapper *MMapper;
std::unique_ptr<MemoryMapper> OwnedMMapper;
};

} // end namespace llvm
Expand Down
17 changes: 10 additions & 7 deletions llvm/lib/ExecutionEngine/SectionMemoryManager.cpp
Expand Up @@ -101,7 +101,7 @@ uint8_t *SectionMemoryManager::allocateSection(
// 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) {
Expand Down Expand Up @@ -204,7 +204,7 @@ std::error_code
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();
Expand Down Expand Up @@ -234,7 +234,7 @@ void SectionMemoryManager::invalidateInstructionCache() {
SectionMemoryManager::~SectionMemoryManager() {
for (MemoryGroup *Group : {&CodeMem, &RWDataMem, &RODataMem}) {
for (sys::MemoryBlock &Block : Group->AllocatedMem)
MMapper.releaseMappedMemory(Block);
MMapper->releaseMappedMemory(Block);
}
}

Expand Down Expand Up @@ -263,11 +263,14 @@ class DefaultMMapper final : public SectionMemoryManager::MemoryMapper {
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

0 comments on commit 9ce0641

Please sign in to comment.