From a823ade41b66908c23403b7e73ac99d564f8a488 Mon Sep 17 00:00:00 2001 From: Alastair Houghton Date: Thu, 8 Aug 2024 18:53:17 +0100 Subject: [PATCH 1/3] [RuntimeDyld][Windows] Allocate space for dllimport things. We weren't taking account of the space we require in the stubs for things that are `dllimport`ed, and as a result we could hit the assertion failure for running out of stub space. Fix that. rdar://133473673 --- llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp | 5 ++++- .../ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp | 11 +++++++++++ .../lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.h | 7 +++++++ .../lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h | 10 ++++++++++ 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp index 7eb7da0138c97..5ac5532705dc4 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp @@ -690,9 +690,12 @@ unsigned RuntimeDyldImpl::computeSectionStubBufSize(const ObjectFile &Obj, if (!(RelSecI == Section)) continue; - for (const RelocationRef &Reloc : SI->relocations()) + for (const RelocationRef &Reloc : SI->relocations()) { if (relocationNeedsStub(Reloc)) StubBufSize += StubSize; + if (relocationNeedsDLLImportStub(Reloc)) + StubBufSize = sizeAfterAddingDLLImportStub(StubBufSize); + } } // Get section data size and alignment diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp index 25a2d8780fb56..dc5e22e496cd6 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp @@ -119,4 +119,15 @@ bool RuntimeDyldCOFF::isCompatibleFile(const object::ObjectFile &Obj) const { return Obj.isCOFF(); } +bool RuntimeDyldCOFF::relocationNeedsDLLImportStub(const RelocationRef &R) const { + object::symbol_iterator Symbol = R.getSymbol(); + Expected TargetNameOrErr = Symbol->getName(); + if (!TargetNameOrErr) + return false; + + StringRef TargetName = *TargetNameOrErr; + + return TargetName.startswith(getImportSymbolPrefix()); +} + } // namespace llvm diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.h b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.h index 41ee06c15448f..f58faebf6a004 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.h +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.h @@ -14,6 +14,7 @@ #define LLVM_RUNTIME_DYLD_COFF_H #include "RuntimeDyldImpl.h" +#include "llvm/Support/MathExtras.h" #define DEBUG_TYPE "dyld" @@ -49,6 +50,12 @@ class RuntimeDyldCOFF : public RuntimeDyldImpl { static constexpr StringRef getImportSymbolPrefix() { return "__imp_"; } + bool relocationNeedsDLLImportStub(const RelocationRef &R) const; + + unsigned sizeAfterAddingDLLImportStub(unsigned Size) const { + return alignTo(Size, PointerSize) + PointerSize; + } + private: unsigned PointerSize; uint32_t PointerReloc; diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h index e09c632842d6e..de7630b9747ea 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h @@ -455,6 +455,16 @@ class RuntimeDyldImpl { return true; // Conservative answer } + // Return true if the relocation R may require allocating a DLL import stub. + virtual bool relocationNeedsDLLImportStub(const RelocationRef &R) const { + return false; + } + + // Add the size of a DLL import stub to the buffer size + virtual unsigned sizeAfterAddingDLLImportStub(unsigned Size) const { + return Size; + } + public: RuntimeDyldImpl(RuntimeDyld::MemoryManager &MemMgr, JITSymbolResolver &Resolver) From 8be77e9ccb312b199bfb79a3249e63e2cc12e7bf Mon Sep 17 00:00:00 2001 From: Alastair Houghton Date: Thu, 8 Aug 2024 19:02:10 +0100 Subject: [PATCH 2/3] [RuntimeDyld][Windows] Eliminate a local variable. No need for `TargetName` to exist separately, really. Co-authored-by: Saleem Abdulrasool --- llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp index dc5e22e496cd6..8a307826be7fa 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp @@ -119,15 +119,14 @@ bool RuntimeDyldCOFF::isCompatibleFile(const object::ObjectFile &Obj) const { return Obj.isCOFF(); } -bool RuntimeDyldCOFF::relocationNeedsDLLImportStub(const RelocationRef &R) const { +bool RuntimeDyldCOFF::relocationNeedsDLLImportStub( + const RelocationRef &R) const { object::symbol_iterator Symbol = R.getSymbol(); Expected TargetNameOrErr = Symbol->getName(); if (!TargetNameOrErr) return false; - StringRef TargetName = *TargetNameOrErr; - - return TargetName.startswith(getImportSymbolPrefix()); + return TargetNameOrErr->startswith(getImportSymbolPrefix()); } } // namespace llvm From 3ad27ef22e2aea4ca2106c2025b69ecfba20e97a Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Tue, 13 Aug 2024 10:12:29 +1000 Subject: [PATCH 3/3] Update llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp Fix `StringRef::starts_with` use. Co-authored-by: Ben Barham --- llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp index 8a307826be7fa..73b37ee0ff331 100644 --- a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp +++ b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldCOFF.cpp @@ -126,7 +126,7 @@ bool RuntimeDyldCOFF::relocationNeedsDLLImportStub( if (!TargetNameOrErr) return false; - return TargetNameOrErr->startswith(getImportSymbolPrefix()); + return TargetNameOrErr->starts_with(getImportSymbolPrefix()); } } // namespace llvm