diff --git a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h --- a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h +++ b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h @@ -1051,8 +1051,8 @@ /// added to the cache manually (descending offset order must be /// preserved), or the cache can be set to None and rebuilt by /// splitBlock on the next call. - Block &splitBlock(Block &B, size_t SplitIndex, - SplitBlockCache *Cache = nullptr); + Expected splitBlock(Block &B, size_t SplitIndex, + SplitBlockCache *Cache = nullptr); /// Add an external symbol. /// Some formats (e.g. ELF) allow Symbols to have sizes. For Symbols whose diff --git a/llvm/lib/ExecutionEngine/JITLink/EHFrameSupport.cpp b/llvm/lib/ExecutionEngine/JITLink/EHFrameSupport.cpp --- a/llvm/lib/ExecutionEngine/JITLink/EHFrameSupport.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/EHFrameSupport.cpp @@ -113,9 +113,10 @@ } uint64_t BlockSize = BlockReader.getOffset() - RecordStartOffset; - auto &NewBlock = G.splitBlock(B, BlockSize); - (void)NewBlock; - LLVM_DEBUG(dbgs() << " Extracted " << NewBlock << "\n"); + auto NewBlock = G.splitBlock(B, BlockSize); + if (!NewBlock) + return NewBlock.takeError(); + LLVM_DEBUG(dbgs() << " Extracted " << *NewBlock << "\n"); } } diff --git a/llvm/lib/ExecutionEngine/JITLink/JITLink.cpp b/llvm/lib/ExecutionEngine/JITLink/JITLink.cpp --- a/llvm/lib/ExecutionEngine/JITLink/JITLink.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/JITLink.cpp @@ -150,8 +150,8 @@ B->~Block(); } -Block &LinkGraph::splitBlock(Block &B, size_t SplitIndex, - SplitBlockCache *Cache) { +Expected LinkGraph::splitBlock(Block &B, size_t SplitIndex, + SplitBlockCache *Cache) { assert(SplitIndex > 0 && "splitBlock can not be called with SplitIndex == 0"); @@ -213,7 +213,12 @@ // Transfer all symbols with offset less than SplitIndex to NewBlock. while (!BlockSymbols.empty() && BlockSymbols.back()->getOffset() < SplitIndex) { - BlockSymbols.back()->setBlock(NewBlock); + auto *Sym = BlockSymbols.back(); + // If there are symbols span across the split boundary, return error. + if (Sym->getOffset() + Sym->getSize() > SplitIndex) + return make_error( + "symbol span across the split boundary"); + Sym->setBlock(NewBlock); BlockSymbols.pop_back(); } diff --git a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp --- a/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp +++ b/llvm/lib/ExecutionEngine/JITLink/MachOLinkGraphBuilder.cpp @@ -317,7 +317,9 @@ Data ? G->createContentBlock(GraphSec, ArrayRef(Data, Size), Address, Alignment, 0) : G->createZeroFillBlock(GraphSec, Size, Address, Alignment, 0); - auto &Sym = G->addAnonymousSymbol(B, 0, Size, false, IsLive); + // Create section start symbol with size 0 so the start symbol will not block + // splitBlock if needs to be performed. + auto &Sym = G->addAnonymousSymbol(B, 0, 0, false, IsLive); auto SecI = IndexToSection.find(SecIndex); assert(SecI != IndexToSection.end() && "SecIndex invalid"); auto &NSec = SecI->second; @@ -764,14 +766,16 @@ LinkGraph::SplitBlockCache C; for (unsigned I = 0; I != NumBlocks; ++I) { - auto &CURec = G.splitBlock(*B, CURecordSize, &C); + auto CURec = G.splitBlock(*B, CURecordSize, &C); + if (!CURec) + return CURec.takeError(); bool AddedKeepAlive = false; - for (auto &E : CURec.edges()) { + for (auto &E : CURec->edges()) { if (E.getOffset() == 0) { LLVM_DEBUG({ dbgs() << " Updating compact unwind record at " - << formatv("{0:x16}", CURec.getAddress()) << " to point to " + << formatv("{0:x16}", CURec->getAddress()) << " to point to " << (E.getTarget().hasName() ? E.getTarget().getName() : StringRef()) << " (at " << formatv("{0:x16}", E.getTarget().getAddress()) @@ -781,25 +785,25 @@ if (E.getTarget().isExternal()) return make_error( "Error adding keep-alive edge for compact unwind record at " + - formatv("{0:x}", CURec.getAddress()) + ": target " + + formatv("{0:x}", CURec->getAddress()) + ": target " + E.getTarget().getName() + " is an external symbol"); auto &TgtBlock = E.getTarget().getBlock(); auto &CURecSym = - G.addAnonymousSymbol(CURec, 0, CURecordSize, 0, false); + G.addAnonymousSymbol(*CURec, 0, CURecordSize, 0, false); TgtBlock.addEdge(Edge::KeepAlive, 0, CURecSym, 0); AddedKeepAlive = true; } else if (E.getOffset() != PersonalityEdgeOffset && E.getOffset() != LSDAEdgeOffset) - return make_error("Unexpected edge at offset " + - formatv("{0:x}", E.getOffset()) + - " in compact unwind record at " + - formatv("{0:x}", CURec.getAddress())); + return make_error( + "Unexpected edge at offset " + formatv("{0:x}", E.getOffset()) + + " in compact unwind record at " + + formatv("{0:x}", CURec->getAddress())); } if (!AddedKeepAlive) return make_error( "Error adding keep-alive edge for compact unwind record at " + - formatv("{0:x}", CURec.getAddress()) + + formatv("{0:x}", CURec->getAddress()) + ": no outgoing target edge at offset 0"); } } diff --git a/llvm/unittests/ExecutionEngine/JITLink/LinkGraphTests.cpp b/llvm/unittests/ExecutionEngine/JITLink/LinkGraphTests.cpp --- a/llvm/unittests/ExecutionEngine/JITLink/LinkGraphTests.cpp +++ b/llvm/unittests/ExecutionEngine/JITLink/LinkGraphTests.cpp @@ -10,6 +10,7 @@ #include "llvm/ExecutionEngine/JITLink/JITLink.h" #include "llvm/Support/Endian.h" #include "llvm/Support/Memory.h" +#include "llvm/Testing/Support/Error.h" #include "gtest/gtest.h" using namespace llvm; @@ -513,23 +514,24 @@ B1.addEdge(Edge::FirstRelocation, 12, ES4, 0); // Split B1. - auto &B2 = G.splitBlock(B1, 8); + auto B2 = G.splitBlock(B1, 8); + EXPECT_THAT_EXPECTED(B2, Succeeded()); // Check that the block addresses and content matches what we would expect. EXPECT_EQ(B1.getAddress(), 0x1008U); EXPECT_EQ(B1.getContent(), BlockContent.slice(8)); - EXPECT_EQ(B2.getAddress(), 0x1000U); - EXPECT_EQ(B2.getContent(), BlockContent.slice(0, 8)); + EXPECT_EQ(B2->getAddress(), 0x1000U); + EXPECT_EQ(B2->getContent(), BlockContent.slice(0, 8)); // Check that symbols in B1 were transferred as expected: // We expect S1 and S2 to have been transferred to B2, and S3 and S4 to have // remained attached to B1. Symbols S3 and S4 should have had their offsets // slid to account for the change in address of B2. - EXPECT_EQ(&S1.getBlock(), &B2); + EXPECT_EQ(&S1.getBlock(), &B2.get()); EXPECT_EQ(S1.getOffset(), 0U); - EXPECT_EQ(&S2.getBlock(), &B2); + EXPECT_EQ(&S2.getBlock(), &B2.get()); EXPECT_EQ(S2.getOffset(), 4U); EXPECT_EQ(&S3.getBlock(), &B1); @@ -550,13 +552,28 @@ EXPECT_EQ(E2->getOffset(), 4U); } - EXPECT_EQ(llvm::size(B2.edges()), 2); - if (size(B2.edges()) == 2) { - auto *E1 = &*B2.edges().begin(); - auto *E2 = &*(B2.edges().begin() + 1); + EXPECT_EQ(llvm::size(B2->edges()), 2); + if (size(B2->edges()) == 2) { + auto *E1 = &*B2->edges().begin(); + auto *E2 = &*(B2->edges().begin() + 1); if (E2->getOffset() < E1->getOffset()) std::swap(E1, E2); EXPECT_EQ(E1->getOffset(), 0U); EXPECT_EQ(E2->getOffset(), 4U); } } + +TEST(LinkGraphTest, SplitBlockError) { + LinkGraph G("foo", Triple("x86_64-apple-darwin"), 8, support::little, + getGenericEdgeKindName); + auto &Sec = G.createSection("__data", MemProt::Read | MemProt::Write); + + // Create the block to split. + auto &B1 = G.createContentBlock(Sec, BlockContent, 0x1000, 8, 0); + + // Add anonymous symbol to the block that mark the entire block. + G.addDefinedSymbol(B1, 0, "Sym", BlockContent.size(), Linkage::Strong, + Scope::Default, false, false); + auto B2 = G.splitBlock(B1, 8); + EXPECT_THAT_EXPECTED(B2, Failed()); +}