diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h index 54dbd6c9a74f14f..411817839d908eb 100644 --- a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h +++ b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h @@ -413,19 +413,6 @@ class Symbol { setCallable(IsCallable); } - static Symbol &constructCommon(BumpPtrAllocator &Allocator, Block &Base, - StringRef Name, orc::ExecutorAddrDiff Size, - Scope S, bool IsLive) { - assert(!Name.empty() && "Common symbol name cannot be empty"); - assert(Base.isDefined() && - "Cannot create common symbol from undefined block"); - assert(static_cast(Base).getSize() == Size && - "Common symbol size should match underlying block size"); - auto *Sym = Allocator.Allocate(); - new (Sym) Symbol(Base, 0, Name, Size, Linkage::Weak, S, IsLive, false); - return *Sym; - } - static Symbol &constructExternal(BumpPtrAllocator &Allocator, Addressable &Base, StringRef Name, orc::ExecutorAddrDiff Size, Linkage L, @@ -1127,22 +1114,6 @@ class LinkGraph { return Sym; } - /// Convenience method for adding a weak zero-fill symbol. - Symbol &addCommonSymbol(StringRef Name, Scope S, Section &Section, - orc::ExecutorAddr Address, orc::ExecutorAddrDiff Size, - uint64_t Alignment, bool IsLive) { - assert(llvm::count_if(defined_symbols(), - [&](const Symbol *Sym) { - return Sym->getName() == Name; - }) == 0 && - "Duplicate defined symbol"); - auto &Sym = Symbol::constructCommon( - Allocator, createBlock(Section, Size, Address, Alignment, 0), Name, - Size, S, IsLive); - Section.addSymbol(Sym); - return Sym; - } - /// Add an anonymous symbol. Symbol &addAnonymousSymbol(Block &Content, orc::ExecutorAddrDiff Offset, orc::ExecutorAddrDiff Size, bool IsCallable, diff --git a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp index 5198a9a8ed02e00..24c122b35c61953 100644 --- a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp @@ -454,9 +454,11 @@ Expected COFFLinkGraphBuilder::createDefinedSymbol( object::COFFSymbolRef Symbol, const object::coff_section *Section) { if (Symbol.isCommon()) { // FIXME: correct alignment - return &G->addCommonSymbol(SymbolName, Scope::Default, getCommonSection(), - orc::ExecutorAddr(), Symbol.getValue(), - Symbol.getValue(), false); + return &G->addDefinedSymbol( + G->createZeroFillBlock(getCommonSection(), Symbol.getValue(), + orc::ExecutorAddr(), Symbol.getValue(), 0), + 0, SymbolName, Symbol.getValue(), Linkage::Strong, Scope::Default, + false, false); } if (Symbol.isAbsolute()) return &G->addAbsoluteSymbol(SymbolName, diff --git a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h index b6c2b746c7ec8a9..d21dd606dc1979c 100644 --- a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h +++ b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h @@ -402,9 +402,10 @@ template Error ELFLinkGraphBuilder::graphifySymbols() { // Handle common symbols specially. if (Sym.isCommon()) { - Symbol &GSym = G->addCommonSymbol(*Name, Scope::Default, - getCommonSection(), orc::ExecutorAddr(), - Sym.st_size, Sym.getValue(), false); + Symbol &GSym = G->addDefinedSymbol( + G->createZeroFillBlock(getCommonSection(), Sym.st_size, + orc::ExecutorAddr(), Sym.getValue(), 0), + 0, *Name, Sym.st_size, Linkage::Strong, Scope::Default, false, false); setGraphSymbol(SymIndex, GSym); continue; } diff --git a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp index 37d28b10b8fc71f..3066afe90f6c75d 100644 --- a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp @@ -217,19 +217,29 @@ void JITLinkerBase::applyLookupResult(AsyncLookupResult Result) { assert(!Sym->getAddress() && "Symbol already resolved"); assert(!Sym->isDefined() && "Symbol being resolved is already defined"); auto ResultI = Result.find(Sym->getName()); - if (ResultI != Result.end()) + if (ResultI != Result.end()) { Sym->getAddressable().setAddress( orc::ExecutorAddr(ResultI->second.getAddress())); - else + Sym->setLinkage(ResultI->second.getFlags().isWeak() ? Linkage::Weak + : Linkage::Strong); + } else assert(Sym->isWeaklyReferenced() && "Failed to resolve non-weak reference"); } LLVM_DEBUG({ dbgs() << "Externals after applying lookup result:\n"; - for (auto *Sym : G->external_symbols()) + for (auto *Sym : G->external_symbols()) { dbgs() << " " << Sym->getName() << ": " - << formatv("{0:x16}", Sym->getAddress().getValue()) << "\n"; + << formatv("{0:x16}", Sym->getAddress().getValue()); + switch (Sym->getLinkage()) { + case Linkage::Strong: + break; + case Linkage::Weak: + dbgs() << " (weak)"; + } + dbgs() << "\n"; + } }); } diff --git a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp index 80908d1ed7c354a..ff3ecf9af6ddda5 100644 --- a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp @@ -350,11 +350,13 @@ Error MachOLinkGraphBuilder::graphifyRegularSymbols() { if (!NSym.Name) return make_error("Anonymous common symbol at index " + Twine(KV.first)); - NSym.GraphSymbol = &G->addCommonSymbol( - *NSym.Name, NSym.S, getCommonSection(), orc::ExecutorAddr(), - orc::ExecutorAddrDiff(NSym.Value), - 1ull << MachO::GET_COMM_ALIGN(NSym.Desc), - NSym.Desc & MachO::N_NO_DEAD_STRIP); + NSym.GraphSymbol = &G->addDefinedSymbol( + G->createZeroFillBlock(getCommonSection(), + orc::ExecutorAddrDiff(NSym.Value), + orc::ExecutorAddr(), + 1ull << MachO::GET_COMM_ALIGN(NSym.Desc), 0), + 0, *NSym.Name, orc::ExecutorAddrDiff(NSym.Value), Linkage::Strong, + NSym.S, false, NSym.Desc & MachO::N_NO_DEAD_STRIP); } else { if (!NSym.Name) return make_error("Anonymous external symbol at " diff --git a/llvm/lib/ExecutionEngine/Orc/Core.cpp b/llvm/lib/ExecutionEngine/Orc/Core.cpp index 228a50a171c688e..60c1668af1bd558 100644 --- a/llvm/lib/ExecutionEngine/Orc/Core.cpp +++ b/llvm/lib/ExecutionEngine/Orc/Core.cpp @@ -970,10 +970,9 @@ Error JITDylib::resolve(MaterializationResponsibility &MR, SymbolsInErrorState.insert(KV.first); else { auto Flags = KV.second.getFlags(); - Flags &= ~(JITSymbolFlags::Weak | JITSymbolFlags::Common); + Flags &= ~JITSymbolFlags::Common; assert(Flags == - (SymI->second.getFlags() & - ~(JITSymbolFlags::Weak | JITSymbolFlags::Common)) && + (SymI->second.getFlags() & ~JITSymbolFlags::Common) && "Resolved flags should match the declared flags"); Worklist.push_back( @@ -2909,13 +2908,13 @@ Error ExecutionSession::OL_notifyResolved(MaterializationResponsibility &MR, }); #ifndef NDEBUG for (auto &KV : Symbols) { - auto WeakFlags = JITSymbolFlags::Weak | JITSymbolFlags::Common; auto I = MR.SymbolFlags.find(KV.first); assert(I != MR.SymbolFlags.end() && "Resolving symbol outside this responsibility set"); assert(!I->second.hasMaterializationSideEffectsOnly() && "Can't resolve materialization-side-effects-only symbol"); - assert((KV.second.getFlags() & ~WeakFlags) == (I->second & ~WeakFlags) && + assert((KV.second.getFlags() & ~JITSymbolFlags::Common) == + (I->second & ~JITSymbolFlags::Common) && "Resolving symbol with incorrect flags"); } #endif diff --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp index aa273db238f56f1..7b4da052ed368bb 100644 --- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp +++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp @@ -218,6 +218,8 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext { Flags |= JITSymbolFlags::Callable; if (Sym->getScope() == Scope::Default) Flags |= JITSymbolFlags::Exported; + if (Sym->getLinkage() == Linkage::Weak) + Flags |= JITSymbolFlags::Weak; InternedResult[InternedName] = JITEvaluatedSymbol(Sym->getAddress().getValue(), Flags); diff --git a/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp index 5d1c9afc560abe1..e55a607cba1da12 100644 --- a/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp +++ b/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp @@ -270,17 +270,21 @@ Error RTDyldObjectLinkingLayer::onObjLoad( auto InternedName = getExecutionSession().intern(KV.first); auto Flags = KV.second.getFlags(); - - // Override object flags and claim responsibility for symbols if - // requested. - if (OverrideObjectFlags || AutoClaimObjectSymbols) { - auto I = R.getSymbols().find(InternedName); - - if (OverrideObjectFlags && I != R.getSymbols().end()) + auto I = R.getSymbols().find(InternedName); + if (I != R.getSymbols().end()) { + // Override object flags and claim responsibility for symbols if + // requested. + if (OverrideObjectFlags) Flags = I->second; - else if (AutoClaimObjectSymbols && I == R.getSymbols().end()) - ExtraSymbolsToClaim[InternedName] = Flags; - } + else { + // RuntimeDyld/MCJIT's weak tracking isn't compatible with ORC's. Even + // if we're not overriding flags in general we should set the weak flag + // according to the MaterializationResponsibility object symbol table. + if (I->second.isWeak()) + Flags |= JITSymbolFlags::Weak; + } + } else if (AutoClaimObjectSymbols) + ExtraSymbolsToClaim[InternedName] = Flags; Symbols[InternedName] = JITEvaluatedSymbol(KV.second.getAddress(), Flags); } diff --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_common_symbol.s b/llvm/test/ExecutionEngine/JITLink/X86/COFF_common_symbol.s index 2d4ad30f94d8d05..cd1401ebd33a35c 100644 --- a/llvm/test/ExecutionEngine/JITLink/X86/COFF_common_symbol.s +++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_common_symbol.s @@ -6,7 +6,7 @@ # # CHECK: Creating graph symbols... # CHECK: 7: Creating defined graph symbol for COFF symbol "var" in (common) (index: 0) -# CHECK-NEXT: 0x0 (block + 0x00000000): size: 0x00000004, linkage: weak, scope: default, dead - var +# CHECK-NEXT: 0x0 (block + 0x00000000): size: 0x00000004, linkage: strong, scope: default, dead - var .text