From 7353c9bb4a0c67350a48c3aecc6529098dbf37b2 Mon Sep 17 00:00:00 2001 From: Bjorn Pettersson Date: Fri, 20 Dec 2024 22:08:49 +0800 Subject: [PATCH 1/2] [lld][MachO] Fix symbol insertion in `transplantSymbolsAtOffset` The existing comparison does not insert symbols in the intended place. Unfortunately, it is not very clear how to test this. Suggestions appreciated. Closes #120559. --- lld/MachO/SymbolTable.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp index f0a92da8777e1..36e421b52abca 100644 --- a/lld/MachO/SymbolTable.cpp +++ b/lld/MachO/SymbolTable.cpp @@ -69,7 +69,7 @@ static void transplantSymbolsAtOffset(InputSection *fromIsec, // Ensure the symbols will still be in address order after our insertions. auto insertIt = llvm::upper_bound(toIsec->symbols, toOff, [](uint64_t off, const Symbol *s) { - return cast(s)->value < off; + return cast(s)->value > off; }); llvm::erase_if(fromIsec->symbols, [&](Symbol *s) { auto *d = cast(s); From e77e289cdcf29af5363e0ba7dad53da7ccb9a0e1 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera Date: Sat, 21 Dec 2024 02:20:01 +0800 Subject: [PATCH 2/2] Add `assert` to `transplantSymbolsAtOffset` This should help catch problems with symbol insertion. --- lld/MachO/SymbolTable.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp index 36e421b52abca..a61e60a944fb4 100644 --- a/lld/MachO/SymbolTable.cpp +++ b/lld/MachO/SymbolTable.cpp @@ -67,10 +67,15 @@ static void transplantSymbolsAtOffset(InputSection *fromIsec, InputSection *toIsec, Defined *skip, uint64_t fromOff, uint64_t toOff) { // Ensure the symbols will still be in address order after our insertions. - auto insertIt = llvm::upper_bound(toIsec->symbols, toOff, - [](uint64_t off, const Symbol *s) { - return cast(s)->value > off; - }); + auto symSucceedsOff = [](uint64_t off, const Symbol *s) { + return cast(s)->value > off; + }; + assert(std::is_partitioned(toIsec->symbols.begin(), toIsec->symbols.end(), + [symSucceedsOff, toOff](const Symbol *s) { + return !symSucceedsOff(toOff, s); + }) && + "Symbols in toIsec must be partitioned by toOff."); + auto insertIt = llvm::upper_bound(toIsec->symbols, toOff, symSucceedsOff); llvm::erase_if(fromIsec->symbols, [&](Symbol *s) { auto *d = cast(s); if (d->value != fromOff)