From 47e9e588083493414d0abfbe33fe3566879c219b Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Fri, 16 Sep 2022 11:23:08 -0700 Subject: [PATCH] [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. --- compiler-rt/lib/orc/macho_platform.cpp | 30 +++++++++++++- .../Darwin/x86-64/jit-re-dlopen-data-reset.S | 46 ++++++++++++++++++++++ llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp | 23 ++++++++++- .../Orc/MapperJITLinkMemoryManager.cpp | 10 ++++- 4 files changed, 105 insertions(+), 4 deletions(-) create mode 100644 compiler-rt/test/orc/TestCases/Darwin/x86-64/jit-re-dlopen-data-reset.S diff --git a/compiler-rt/lib/orc/macho_platform.cpp b/compiler-rt/lib/orc/macho_platform.cpp index 7c70999..f17230c 100644 --- a/compiler-rt/lib/orc/macho_platform.cpp +++ b/compiler-rt/lib/orc/macho_platform.cpp @@ -137,6 +137,8 @@ private: std::vector Deps; AtExitsVector AtExits; const objc_image_info *ObjCImageInfo = nullptr; + std::unordered_map> DataSectionContent; + std::unordered_map ZeroInitRanges; std::vector> ModInitsSections; std::vector> ModInitsSectionsNew; std::vector> 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()) && + "Address already registered."); + auto S = KV.second.toSpan(); + JDS->DataSectionContent[KV.second.Start.toPtr()] = + std::vector(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()] = KV.second.size(); + } else if (KV.first == "__DATA,__thread_data") { if (auto Err = registerThreadDataSection(KV.second.toSpan())) 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()); + } else if (KV.first == "__DATA,__common") { + JDS->ZeroInitRanges.erase(KV.second.Start.toPtr()); + } else if (KV.first == "__DATA,__thread_data") { if (auto Err = deregisterThreadDataSection(KV.second.toSpan())) 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 0000000..8582c9e --- /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 8338d42..e92986f 100644 --- a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp +++ b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp @@ -163,6 +163,8 @@ private: 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("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 c774e38..eb3a5ac 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 Lock(Mutex); -- 2.7.4