https://github.com/weliveindetail created https://github.com/llvm/llvm-project/pull/70793
I actually ran into it with a downstream fork: ``` llvm::Error::fatalUncheckedError(), llvm-project\llvm\lib\Support\Error.cpp, line 117 ObjectFilePECOFF::AppendFromCOFFSymbolTable(), llvm-project\lldb\source\Plugins\ObjectFile\PECOFF\ObjectFilePECOFF.cpp, line 806 ObjectFilePECOFF::ParseSymtab(), llvm-project\lldb\source\Plugins\ObjectFile\PECOFF\ObjectFilePECOFF.cpp, line 777 ``` If logging is disabled `LLDB_LOG_ERROR` calls `llvm::consumeError()`, which marks the error as checked. All `llvm::Error`s must be checked before destruction. This patch fixes one more such case in `ObjectFileCOFF::ParseSymtab()`. From 923cabecd8486514f00c9cf81263169e5b263ef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graen...@gmail.com> Date: Wed, 18 Oct 2023 14:27:51 +0200 Subject: [PATCH 1/4] [llvm-jitlink] Avoid assertion failure in make_error If GOTSym is not defined this failed with: ``` Assertion failed: (Base->isDefined() && "Not a defined symbol"), function getBlock, file JITLink.h, line 554. ``` --- llvm/tools/llvm-jitlink/llvm-jitlink-elf.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/llvm/tools/llvm-jitlink/llvm-jitlink-elf.cpp b/llvm/tools/llvm-jitlink/llvm-jitlink-elf.cpp index 7881660d1a73851..2adf6df9b7615e9 100644 --- a/llvm/tools/llvm-jitlink/llvm-jitlink-elf.cpp +++ b/llvm/tools/llvm-jitlink/llvm-jitlink-elf.cpp @@ -55,7 +55,11 @@ static Expected<Symbol &> getELFStubTarget(LinkGraph &G, Block &B) { if (!E) return E.takeError(); auto &GOTSym = E->getTarget(); - if (!GOTSym.isDefined() || !isELFGOTSection(GOTSym.getBlock().getSection())) + if (!GOTSym.isDefined()) + return make_error<StringError>( + "Stubs entry in " + G.getName() + " does not point to GOT entry", + inconvertibleErrorCode()); + if (!isELFGOTSection(GOTSym.getBlock().getSection())) return make_error<StringError>( "Stubs entry in " + G.getName() + ", \"" + GOTSym.getBlock().getSection().getName() + From c07c00bc4dc11430b965cdcade028d9375cf8cbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graen...@gmail.com> Date: Wed, 18 Oct 2023 14:30:11 +0200 Subject: [PATCH 2/4] [JITLink] Fix typos: symobls -> symbols (NFC) --- llvm/include/llvm-c/Orc.h | 2 +- llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h | 2 +- llvm/include/llvm/ExecutionEngine/JITLink/aarch64.h | 2 +- llvm/include/llvm/ExecutionEngine/JITLink/loongarch.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/llvm/include/llvm-c/Orc.h b/llvm/include/llvm-c/Orc.h index a40b17b712fb3f2..3d17073dfdd8d41 100644 --- a/llvm/include/llvm-c/Orc.h +++ b/llvm/include/llvm-c/Orc.h @@ -346,7 +346,7 @@ typedef struct LLVMOrcOpaqueLookupState *LLVMOrcLookupStateRef; * into. * * The JDLookupFlags argument can be inspected to determine whether the original - * lookup included non-exported symobls. + * lookup included non-exported symbols. * * Finally, the LookupSet argument contains the set of symbols that could not * be found in JD already (the set of generation candidates). diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h index fb758f7a66cf55f..8a019492c12d593 100644 --- a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h +++ b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h @@ -1862,7 +1862,7 @@ Error makeAlignmentError(llvm::orc::ExecutorAddr Loc, uint64_t Value, int N, const Edge &E); /// Creates a new pointer block in the given section and returns an -/// Anonymous symobl pointing to it. +/// Anonymous symbol pointing to it. /// /// The pointer block will have the following default values: /// alignment: PointerSize diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/aarch64.h b/llvm/include/llvm/ExecutionEngine/JITLink/aarch64.h index 8d3f29b545f21ad..27a90ebef3d6d6a 100644 --- a/llvm/include/llvm/ExecutionEngine/JITLink/aarch64.h +++ b/llvm/include/llvm/ExecutionEngine/JITLink/aarch64.h @@ -618,7 +618,7 @@ extern const char NullPointerContent[PointerSize]; extern const char PointerJumpStubContent[12]; /// Creates a new pointer block in the given section and returns an -/// Anonymous symobl pointing to it. +/// Anonymous symbol pointing to it. /// /// If InitialTarget is given then an Pointer64 relocation will be added to the /// block pointing at InitialTarget. diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/loongarch.h b/llvm/include/llvm/ExecutionEngine/JITLink/loongarch.h index bec657723a38cf9..26351ed9971cc46 100644 --- a/llvm/include/llvm/ExecutionEngine/JITLink/loongarch.h +++ b/llvm/include/llvm/ExecutionEngine/JITLink/loongarch.h @@ -280,7 +280,7 @@ inline ArrayRef<char> getStubBlockContent(LinkGraph &G) { } /// Creates a new pointer block in the given section and returns an -/// Anonymous symobl pointing to it. +/// Anonymous symbol pointing to it. /// /// If InitialTarget is given then an Pointer64 relocation will be added to the /// block pointing at InitialTarget. From 1767f99cb93fd3d1f9bf84765564d069cd19a9ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graen...@gmail.com> Date: Wed, 18 Oct 2023 14:49:32 +0200 Subject: [PATCH 3/4] [JITLink][AArch32] Wire up GOT/PLT and test existing implementation of ThumbV7 stubs --- .../llvm/ExecutionEngine/JITLink/aarch32.h | 31 ++++++++++--- .../ExecutionEngine/JITLink/ELF_aarch32.cpp | 5 ++- llvm/lib/ExecutionEngine/JITLink/aarch32.cpp | 32 +++++++++++-- .../JITLink/AArch32/ELF_thumb_stubs.s | 45 +++++++++++++++++++ 4 files changed, 101 insertions(+), 12 deletions(-) create mode 100644 llvm/test/ExecutionEngine/JITLink/AArch32/ELF_thumb_stubs.s diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h index 3f36b53d6684a79..d92b2aea5745397 100644 --- a/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h +++ b/llvm/include/llvm/ExecutionEngine/JITLink/aarch32.h @@ -271,21 +271,37 @@ inline Error applyFixup(LinkGraph &G, Block &B, const Edge &E, llvm_unreachable("Relocation must be of class Data, Arm or Thumb"); } -/// Stubs builder for a specific StubsFlavor +/// Global Offset Table builder +class GOTTableManager : public TableManager<GOTTableManager> { +public: + static StringRef getSectionName() { return "$__GOT"; } + + bool visitEdge(LinkGraph &G, Block *B, Edge &E); + Symbol &createEntry(LinkGraph &G, Symbol &Target); + +private: + Section &getGOTSection(LinkGraph &G) { + if (!GOTSection) + GOTSection = &G.createSection(getSectionName(), orc::MemProt::Read); + return *GOTSection; + } + + Section *GOTSection = nullptr; +}; + +/// PLT stubs builder for a specific StubsFlavor /// /// Right now we only have one default stub kind, but we want to extend this /// and allow creation of specific kinds in the future (e.g. branch range /// extension or interworking). /// -/// Let's keep it simple for the moment and not wire this through a GOT. -/// template <StubsFlavor Flavor> -class StubsManager : public TableManager<StubsManager<Flavor>> { +class PLTTableManager : public TableManager<PLTTableManager<Flavor>> { public: - StubsManager() = default; + PLTTableManager(GOTTableManager &GOT) : GOT(GOT) {} /// Name of the object file section that will contain all our stubs. - static StringRef getSectionName() { return "__llvm_jitlink_STUBS"; } + static StringRef getSectionName() { return "$__STUBS"; } /// Implements link-graph traversal via visitExistingEdges(). bool visitEdge(LinkGraph &G, Block *B, Edge &E) { @@ -328,12 +344,13 @@ class StubsManager : public TableManager<StubsManager<Flavor>> { return *StubsSection; } + GOTTableManager &GOT; Section *StubsSection = nullptr; }; /// Create a branch range extension stub with Thumb encoding for v7 CPUs. template <> -Symbol &StubsManager<Thumbv7>::createEntry(LinkGraph &G, Symbol &Target); +Symbol &PLTTableManager<Thumbv7>::createEntry(LinkGraph &G, Symbol &Target); } // namespace aarch32 } // namespace jitlink diff --git a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp index 23946c7de9adbff..2ec9c238b2d14b7 100644 --- a/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/ELF_aarch32.cpp @@ -214,8 +214,9 @@ template <aarch32::StubsFlavor Flavor> Error buildTables_ELF_aarch32(LinkGraph &G) { LLVM_DEBUG(dbgs() << "Visiting edges in graph:\n"); - aarch32::StubsManager<Flavor> PLT; - visitExistingEdges(G, PLT); + aarch32::GOTTableManager GOT; + aarch32::PLTTableManager<Flavor> PLT(GOT); + visitExistingEdges(G, GOT, PLT); return Error::success(); } diff --git a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp index 4aed64966654420..b31dd5c22b2eb03 100644 --- a/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/aarch32.cpp @@ -619,6 +619,31 @@ Error applyFixupThumb(LinkGraph &G, Block &B, const Edge &E, } } +static Symbol &createAnonymousPointer(LinkGraph &G, Section &PointerSection, + Symbol *InitialTarget, + uint64_t InitialAddend = 0) { + // TODO: Do we have an alignment of 4 in ARM and if so how do we distinguish + // it at this place? + constexpr uint8_t AlignmentThumb = 2; + constexpr orc::ExecutorAddr MaxAddrThumb{~uint32_t(AlignmentThumb - 1)}; + constexpr uint64_t PointerSize = 4; + constexpr char NullPointerContent[] { 0x00, 0x00, 0x00, 0x00 }; + auto &B = G.createContentBlock(PointerSection, NullPointerContent, + MaxAddrThumb, AlignmentThumb, 0); + if (InitialTarget) + B.addEdge(Data_Pointer32, 0, *InitialTarget, InitialAddend); + return G.addAnonymousSymbol(B, 0, PointerSize, false, false); +} + +bool GOTTableManager::visitEdge(LinkGraph &G, Block *B, Edge &E) { + return false; +} + +Symbol &GOTTableManager::createEntry(LinkGraph &G, Symbol &Target) { + // Add new block in GOT section and return an anonymous symbol pointing to it. + return createAnonymousPointer(G, getGOTSection(G), &Target); +} + const uint8_t Thumbv7ABS[] = { 0x40, 0xf2, 0x00, 0x0c, // movw r12, #0x0000 ; lower 16-bit 0xc0, 0xf2, 0x00, 0x0c, // movt r12, #0x0000 ; upper 16-bit @@ -626,7 +651,7 @@ const uint8_t Thumbv7ABS[] = { }; template <> -Symbol &StubsManager<Thumbv7>::createEntry(LinkGraph &G, Symbol &Target) { +Symbol &PLTTableManager<Thumbv7>::createEntry(LinkGraph &G, Symbol &Target) { constexpr uint64_t Alignment = 4; Block &B = addStub(G, Thumbv7ABS, Alignment); LLVM_DEBUG({ @@ -636,8 +661,9 @@ Symbol &StubsManager<Thumbv7>::createEntry(LinkGraph &G, Symbol &Target) { checkRegister<Thumb_MovtAbs>(StubPtr + 4, Reg12) && "Linker generated stubs may only corrupt register r12 (IP)"); }); - B.addEdge(Thumb_MovwAbsNC, 0, Target, 0); - B.addEdge(Thumb_MovtAbs, 4, Target, 0); + Symbol &GOTEntry = GOT.getEntryForTarget(G, Target); + B.addEdge(Thumb_MovwAbsNC, 0, GOTEntry, 0); + B.addEdge(Thumb_MovtAbs, 4, GOTEntry, 0); Symbol &Stub = G.addAnonymousSymbol(B, 0, B.getSize(), true, false); Stub.setTargetFlags(ThumbSymbol); return Stub; diff --git a/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_thumb_stubs.s b/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_thumb_stubs.s new file mode 100644 index 000000000000000..18d06bbded2e128 --- /dev/null +++ b/llvm/test/ExecutionEngine/JITLink/AArch32/ELF_thumb_stubs.s @@ -0,0 +1,45 @@ +# RUN: rm -rf %t && mkdir -p %t +# RUN: llvm-mc -triple=thumbv7-linux-gnueabi -arm-add-build-attributes \ +# RUN: -filetype=obj -filetype=obj -o %t/elf_stubs.o %s +# RUN: llvm-jitlink -noexec -slab-address 0x76ff0000 \ +# RUN: -slab-allocate 10Kb -slab-page-size 4096 \ +# RUN: -abs external_func=0x76bbe880 \ +# RUN: -check %s %t/elf_stubs.o + + .text + .syntax unified + +# Check that calls/jumps to external functions trigger the generation of stubs +# and GOT entries. The GOT entry contains the absolute address of the external +# function. The stub loads it and branches there. +# +# jitlink-check: *{4}(got_addr(elf_stubs.o, external_func)) = external_func +# jitlink-check: decode_operand(test_external_call, 2) = stub_addr(elf_stubs.o, external_func) - next_pc(test_external_call) +# jitlink-check: decode_operand(test_external_jump, 0) = stub_addr(elf_stubs.o, external_func) - next_pc(test_external_jump) + .globl test_external_call + .type test_external_call,%function + .p2align 1 + .code 16 + .thumb_func +test_external_call: + bl external_func + .size test_external_call, .-test_external_call + + .globl test_external_jump + .type test_external_call,%function + .p2align 1 + .code 16 + .thumb_func +test_external_jump: + b external_func + .size test_external_jump, .-test_external_jump + +# Empty main function for jitlink to be happy + .globl main + .type main,%function + .p2align 1 + .code 16 + .thumb_func +main: + bx lr + .size main, .-main From 5091d1ffec05ec65e4e762d2bea8735d0de7b6fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graen...@gmail.com> Date: Tue, 31 Oct 2023 13:12:21 +0100 Subject: [PATCH 4/4] [lldb] Fix missing comsumeError() with LLDB_LOG in ObjectFileCOFF/PECOFF --- lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp | 6 +++--- .../Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp | 9 ++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp b/lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp index 03c454bf3efab14..a7ad5d27b237f12 100644 --- a/lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp +++ b/lldb/source/Plugins/ObjectFile/COFF/ObjectFileCOFF.cpp @@ -271,9 +271,9 @@ void ObjectFileCOFF::ParseSymtab(lldb_private::Symtab &symtab) { const auto COFFSymRef = m_object->getCOFFSymbol(SymRef); Expected<StringRef> NameOrErr = SymRef.getName(); - if (auto error = NameOrErr.takeError()) { - LLDB_LOG(log, "ObjectFileCOFF: failed to get symbol name: {0}", - llvm::fmt_consume(std::move(error))); + if (!NameOrErr) { + LLDB_LOG_ERROR(log, NameOrErr.takeError(), + "ObjectFileCOFF: failed to get symbol name: {0}"); continue; } diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp index 7fb10a69391c566..be0020cad5bee8e 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp @@ -791,11 +791,10 @@ void ObjectFilePECOFF::AppendFromCOFFSymbolTable( for (const auto &sym_ref : m_binary->symbols()) { const auto coff_sym_ref = m_binary->getCOFFSymbol(sym_ref); auto name_or_error = sym_ref.getName(); - if (auto err = name_or_error.takeError()) { - LLDB_LOG(log, - "ObjectFilePECOFF::AppendFromCOFFSymbolTable - failed to get " - "symbol table entry name: {0}", - llvm::fmt_consume(std::move(err))); + if (!name_or_error) { + LLDB_LOG_ERROR(log, name_or_error.takeError(), + "ObjectFilePECOFF::AppendFromCOFFSymbolTable - failed to " + "get symbol table entry name: {0}"); continue; } const llvm::StringRef sym_name = *name_or_error; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits