[compiler-rt] 47e9e58 - [ORC][ORC-RT][MachO] Reset __data and __common sections on library close.
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 16 13:40:28 PDT 2022
Author: Lang Hames
Date: 2022-09-16T13:40:19-07:00
New Revision: 47e9e588083493414d0abfbe33fe3566879c219b
URL: https://github.com/llvm/llvm-project/commit/47e9e588083493414d0abfbe33fe3566879c219b
DIFF: https://github.com/llvm/llvm-project/commit/47e9e588083493414d0abfbe33fe3566879c219b.diff
LOG: [ORC][ORC-RT][MachO] Reset __data and __common sections on library close.
If we want to be able to close and then re-open a library then we need to reset
the data section states when the library is closed. This commit updates
MachOPlatform and the ORC runtime to track __data and __common sections, and
reset the state in MachOPlatformRuntimeState::dlcloseDeinitialize.
This is only a first step to full support -- there are other data sections that
we're not capturing, and we'll probably want a more efficient representation
for the sections (rather than passing their string name over IPC), but this is
a reasonable first step.
This commit also contains a fix to MapperJITLinkMemoryManager that prevents it
from calling OnDeallocated twice in the case of an error.
Added:
compiler-rt/test/orc/TestCases/Darwin/x86-64/jit-re-dlopen-data-reset.S
Modified:
compiler-rt/lib/orc/macho_platform.cpp
llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
llvm/lib/ExecutionEngine/Orc/MapperJITLinkMemoryManager.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/orc/macho_platform.cpp b/compiler-rt/lib/orc/macho_platform.cpp
index 7c7099976421b..f17230cfdcdac 100644
--- a/compiler-rt/lib/orc/macho_platform.cpp
+++ b/compiler-rt/lib/orc/macho_platform.cpp
@@ -137,6 +137,8 @@ class MachOPlatformRuntimeState {
std::vector<JITDylibState *> Deps;
AtExitsVector AtExits;
const objc_image_info *ObjCImageInfo = nullptr;
+ std::unordered_map<void *, std::vector<char>> DataSectionContent;
+ std::unordered_map<void *, size_t> ZeroInitRanges;
std::vector<span<void (*)()>> ModInitsSections;
std::vector<span<void (*)()>> ModInitsSectionsNew;
std::vector<span<uintptr_t>> ObjCClassListSections;
@@ -340,7 +342,17 @@ Error MachOPlatformRuntimeState::registerObjectPlatformSections(
for (auto &KV : Secs) {
// FIXME: Validate section ranges?
- if (KV.first == "__DATA,__thread_data") {
+ if (KV.first == "__DATA,__data") {
+ assert(!JDS->DataSectionContent.count(KV.second.Start.toPtr<char *>()) &&
+ "Address already registered.");
+ auto S = KV.second.toSpan<char>();
+ JDS->DataSectionContent[KV.second.Start.toPtr<char *>()] =
+ std::vector<char>(S.begin(), S.end());
+ } else if (KV.first == "__DATA,__common") {
+ // fprintf(stderr, "Adding zero-init range %llx -- %llx\n",
+ // KV.second.Start.getValue(), KV.second.size());
+ JDS->ZeroInitRanges[KV.second.Start.toPtr<char *>()] = KV.second.size();
+ } else if (KV.first == "__DATA,__thread_data") {
if (auto Err = registerThreadDataSection(KV.second.toSpan<const char>()))
return Err;
} else if (KV.first == "__DATA,__objc_selrefs")
@@ -406,9 +418,17 @@ Error MachOPlatformRuntimeState::deregisterObjectPlatformSections(
// FIXME: Implement faster-path by returning immediately if JDS is being
// torn down entirely?
+ // TODO: Make library permanent (i.e. not able to be dlclosed) if it contains
+ // any Swift or ObjC. Once this happens we can clear (and no longer record)
+ // data section content, as the library could never be re-initialized.
+
for (auto &KV : Secs) {
// FIXME: Validate section ranges?
- if (KV.first == "__DATA,__thread_data") {
+ if (KV.first == "__DATA,__data") {
+ JDS->DataSectionContent.erase(KV.second.Start.toPtr<char *>());
+ } else if (KV.first == "__DATA,__common") {
+ JDS->ZeroInitRanges.erase(KV.second.Start.toPtr<char *>());
+ } else if (KV.first == "__DATA,__thread_data") {
if (auto Err =
deregisterThreadDataSection(KV.second.toSpan<const char>()))
return Err;
@@ -874,6 +894,12 @@ Error MachOPlatformRuntimeState::dlcloseDeinitialize(JITDylibState &JDS) {
moveAppendSections(JDS.ModInitsSections, JDS.ModInitsSectionsNew);
JDS.ModInitsSectionsNew = std::move(JDS.ModInitsSections);
+ // Reset data section contents.
+ for (auto &KV : JDS.DataSectionContent)
+ memcpy(KV.first, KV.second.data(), KV.second.size());
+ for (auto &KV : JDS.ZeroInitRanges)
+ memset(KV.first, 0, KV.second);
+
// Deinitialize any dependencies.
for (auto *DepJDS : JDS.Deps) {
--DepJDS->LinkedAgainstRefCount;
diff --git a/compiler-rt/test/orc/TestCases/Darwin/x86-64/jit-re-dlopen-data-reset.S b/compiler-rt/test/orc/TestCases/Darwin/x86-64/jit-re-dlopen-data-reset.S
new file mode 100644
index 0000000000000..8582c9ecad752
--- /dev/null
+++ b/compiler-rt/test/orc/TestCases/Darwin/x86-64/jit-re-dlopen-data-reset.S
@@ -0,0 +1,46 @@
+// Test that __orc_rt_macho_jit_dlopen resets the state of the data sections.
+//
+// RUN: %clang -c -o %t.main.o %p/Inputs/dlopen-dlclose-x2.S
+// RUN: %clang -c -o %t.inits.o %s
+// RUN: %llvm_jitlink \
+// RUN: -alias _dlopen=___orc_rt_macho_jit_dlopen \
+// RUN: -alias _dlclose=___orc_rt_macho_jit_dlclose \
+// RUN: %t.main.o -jd inits %t.inits.o -lmain | FileCheck %s
+
+// CHECK: entering main
+// CHECK-NEXT: X = 1, Y = 2
+// CHECK-NEXT: X = 1, Y = 2
+// CHECK-NEXT: leaving main
+
+ .section __TEXT,__text,regular,pure_instructions
+ .build_version macos, 13, 0 sdk_version 13, 0
+ .section __TEXT,__StaticInit,regular,pure_instructions
+ .p2align 4, 0x90
+_initializer:
+ movq _X(%rip), %rsi
+ addq $1, %rsi
+ movq %rsi, _X(%rip)
+ movq _Y(%rip), %rdx
+ addq $1, %rdx
+ movq %rdx, _Y(%rip)
+ leaq L_.str(%rip), %rdi
+ xorl %eax, %eax
+ jmp _printf
+
+ .section __TEXT,__cstring,cstring_literals
+L_.str:
+ .asciz "X = %zu, Y = %zu\n"
+
+ .globl _X ## @X
+.zerofill __DATA,__common,_X,8,3
+ .section __DATA,__data
+ .globl _Y ## @Y
+ .p2align 3
+_Y:
+ .quad 1 ## 0x1
+
+ .section __DATA,__mod_init_func,mod_init_funcs
+ .p2align 3
+ .quad _initializer
+
+.subsections_via_symbols
diff --git a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
index 8338d42f92a0b..e92986f1d0506 100644
--- a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
@@ -163,6 +163,8 @@ class MachOHeaderMaterializationUnit : public MaterializationUnit {
constexpr MachOHeaderMaterializationUnit::HeaderSymbol
MachOHeaderMaterializationUnit::AdditionalHeaderSymbols[];
+StringRef DataCommonSectionName = "__DATA,__common";
+StringRef DataDataSectionName = "__DATA,__data";
StringRef EHFrameSectionName = "__TEXT,__eh_frame";
StringRef ModInitFuncSectionName = "__DATA,__mod_init_func";
StringRef ObjCClassListSectionName = "__DATA,__objc_classlist";
@@ -910,8 +912,19 @@ Error MachOPlatform::MachOPlatformPlugin::registerObjectPlatformSections(
}
}
+ // Collect data sections to register.
+ StringRef DataSections[] = {DataDataSectionName, DataCommonSectionName};
+ for (auto &SecName : DataSections) {
+ if (auto *Sec = G.findSectionByName(SecName)) {
+ jitlink::SectionRange R(*Sec);
+ if (!R.empty())
+ MachOPlatformSecs.push_back({SecName, R.getRange()});
+ }
+ }
+
// If any platform sections were found then add an allocation action to call
// the registration function.
+ bool RegistrationRequired = false;
StringRef PlatformSections[] = {
ModInitFuncSectionName, ObjCClassListSectionName,
ObjCImageInfoSectionName, ObjCSelRefsSectionName,
@@ -927,6 +940,7 @@ Error MachOPlatform::MachOPlatformPlugin::registerObjectPlatformSections(
if (R.empty())
continue;
+ RegistrationRequired = true;
MachOPlatformSecs.push_back({SecName, R.getRange()});
}
@@ -939,9 +953,16 @@ Error MachOPlatform::MachOPlatformPlugin::registerObjectPlatformSections(
HeaderAddr = I->second;
}
- if (!HeaderAddr)
+ if (!HeaderAddr) {
+ // If we only found data sections and we're not done bootstrapping yet
+ // then continue -- this must be a data section for the runtime itself,
+ // and we don't need to register those.
+ if (MP.State != MachOPlatform::Initialized && !RegistrationRequired)
+ return Error::success();
+
return make_error<StringError>("Missing header for " + JD.getName(),
inconvertibleErrorCode());
+ }
// Dump the scraped inits.
LLVM_DEBUG({
diff --git a/llvm/lib/ExecutionEngine/Orc/MapperJITLinkMemoryManager.cpp b/llvm/lib/ExecutionEngine/Orc/MapperJITLinkMemoryManager.cpp
index c774e385c91e1..eb3a5acb06cc1 100644
--- a/llvm/lib/ExecutionEngine/Orc/MapperJITLinkMemoryManager.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/MapperJITLinkMemoryManager.cpp
@@ -156,8 +156,16 @@ void MapperJITLinkMemoryManager::deallocate(
Mapper->deinitialize(Bases, [this, Allocs = std::move(Allocs),
OnDeallocated = std::move(OnDeallocated)](
llvm::Error Err) mutable {
- if (Err)
+ // TODO: How should we treat memory that we fail to deinitialize?
+ // We're currently bailing out and treating it as "burned" -- should we
+ // require that a failure to deinitialize still reset the memory so that
+ // we can reclaim it?
+ if (Err) {
+ for (auto &FA : Allocs)
+ FA.release();
OnDeallocated(std::move(Err));
+ return;
+ }
{
std::lock_guard<std::mutex> Lock(Mutex);
More information about the llvm-commits
mailing list