From 6ac38cdc70a58f0bf158118967f40b3c531d8b9e Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Mon, 28 Aug 2023 10:10:40 -0700 Subject: [PATCH 1/3] [JITLink] Truncate ELF symbol sizes to fit containing block. LLVM currently emits dubious symbol sizes for aliases. E.g. assembling the following with LLVM top-of-tree... ``` $ cat foo.s .data .globl base base: .dword 42 .size base, 8 .set alias, base+4 ``` results in both base and alias having symbol size 8, even alias starts at base + 4. This also means that alias extends past the end of the .data section in this example. We should probably teach LLVM not to do this in the future, but as a short-term fix this patch teaches JITLink to simply truncate symbols that would extend past the end of their containing block. rdar://114207607 (cherry picked from commit 6267697a4c88688a89d2d770d00d52eab8aa72c7) Conflicts: llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h (cherry picked from commit 2f53a4c56aff5bd3a880f87fda414f5123dab6b4) --- .../ExecutionEngine/JITLink/ELFLinkGraphBuilder.h | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h index 8dd176cd07f92..10eafbf83fe07 100644 --- a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h +++ b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h @@ -529,6 +529,11 @@ template Error ELFLinkGraphBuilder::graphifySymbols() { return make_error(std::move(ErrMsg)); } + // Truncate symbol if it would overflow -- ELF size fields can't be + // trusted. + uint64_t Size = + std::min(static_cast(Sym.st_size), B->getSize() - Offset); + // In RISCV, temporary symbols (Used to generate dwarf, eh_frame // sections...) will appear in object code's symbol table, and LLVM does // not use names on these temporary symbols (RISCV gnu toolchain uses @@ -536,11 +541,9 @@ template Error ELFLinkGraphBuilder::graphifySymbols() { // anonymous symbol. auto &GSym = Name->empty() - ? G->addAnonymousSymbol(*B, Offset, Sym.st_size, - false, false) - : G->addDefinedSymbol(*B, Offset, *Name, Sym.st_size, L, - S, Sym.getType() == ELF::STT_FUNC, - false); + ? G->addAnonymousSymbol(*B, Offset, Size, false, false) + : G->addDefinedSymbol(*B, Offset, *Name, Size, L, S, + Sym.getType() == ELF::STT_FUNC, false); GSym.setTargetFlags(Flags); setGraphSymbol(SymIndex, GSym); From a635a8633e1e08bd6b893af4ac5e412d545a76a7 Mon Sep 17 00:00:00 2001 From: Ben Langmuir Date: Wed, 21 Aug 2024 18:03:19 -0700 Subject: [PATCH 2/3] [jitlink] Fix cherry-pick in 6267697a4c This change fixed the size of the symbol but the check that emited the error was still using the old size. Updated to use the correct size, which will make the code in the if unreachable, but it's probably better to leave it to avoid merge conflicts. rdar://133510063 (cherry picked from commit d629b3871976677b908e1f240fb3691d796b708b) --- .../ExecutionEngine/JITLink/ELFLinkGraphBuilder.h | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h index 10eafbf83fe07..47e1b37f572f3 100644 --- a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h +++ b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h @@ -513,7 +513,14 @@ template Error ELFLinkGraphBuilder::graphifySymbols() { TargetFlagsType Flags = makeTargetFlags(Sym); orc::ExecutorAddrDiff Offset = getRawOffset(Sym, Flags); - if (Offset + Sym.st_size > B->getSize()) { + // Truncate symbol if it would overflow -- ELF size fields can't be + // trusted. + // FIXME: this makes the following error check unreachable, but it's + // left here to reduce merge conflicts. + uint64_t Size = + std::min(static_cast(Sym.st_size), B->getSize() - Offset); + + if (Offset + Size > B->getSize()) { std::string ErrMsg; raw_string_ostream ErrStream(ErrMsg); ErrStream << "In " << G->getName() << ", symbol "; @@ -529,11 +536,6 @@ template Error ELFLinkGraphBuilder::graphifySymbols() { return make_error(std::move(ErrMsg)); } - // Truncate symbol if it would overflow -- ELF size fields can't be - // trusted. - uint64_t Size = - std::min(static_cast(Sym.st_size), B->getSize() - Offset); - // In RISCV, temporary symbols (Used to generate dwarf, eh_frame // sections...) will appear in object code's symbol table, and LLVM does // not use names on these temporary symbols (RISCV gnu toolchain uses From 1234d78132027586b3699f52fa3c784de6d1a832 Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Mon, 28 Aug 2023 22:30:46 -0700 Subject: [PATCH 3/3] [JITLink] Use the union of ELF section permissions rdar://114207428. Swift is currently generating multiple .rodata sections with a combination of SHF_ALLOC and (SHF_ALLOC | SHF_WRITABLE) flags and this was tripping an assert in the ELFLinkGraphBuilder. As a temporary workaround this patch just uses the union of the requested permissions. rdar://114207428 (cherry picked from commit ead32e1f2de2dc1f35297c17a2e7911df90605b7) Conflicts: llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h (cherry picked from commit a655fbef92eed9133c66c6d6d21db5b1e5dcddcb) --- llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h index 47e1b37f572f3..8946fc31db4d8 100644 --- a/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h +++ b/llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h @@ -375,14 +375,7 @@ template Error ELFLinkGraphBuilder::graphifySections() { } } - if (GraphSec->getMemProt() != Prot) { - std::string ErrMsg; - raw_string_ostream(ErrMsg) - << "In " << G->getName() << ", section " << *Name - << " is present more than once with different permissions: " - << GraphSec->getMemProt() << " vs " << Prot; - return make_error(std::move(ErrMsg)); - } + GraphSec->setMemProt(GraphSec->getMemProt() | Prot); Block *B = nullptr; if (Sec.sh_type != ELF::SHT_NOBITS) {