[lld] [lld][MachO] Fix symbol insertion in `transplantSymbolsAtOffset` (PR #120737)
Carlo Cabrera via llvm-commits
llvm-commits at lists.llvm.org
Sat Dec 21 07:35:51 PST 2024
https://github.com/carlocab updated https://github.com/llvm/llvm-project/pull/120737
>From 7353c9bb4a0c67350a48c3aecc6529098dbf37b2 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
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 f0a92da8777e13..36e421b52abcac 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<Defined>(s)->value < off;
+ return cast<Defined>(s)->value > off;
});
llvm::erase_if(fromIsec->symbols, [&](Symbol *s) {
auto *d = cast<Defined>(s);
>From e77e289cdcf29af5363e0ba7dad53da7ccb9a0e1 Mon Sep 17 00:00:00 2001
From: Carlo Cabrera <github at carlo.cab>
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 36e421b52abcac..a61e60a944fb45 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<Defined>(s)->value > off;
- });
+ auto symSucceedsOff = [](uint64_t off, const Symbol *s) {
+ return cast<Defined>(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<Defined>(s);
if (d->value != fromOff)
More information about the llvm-commits
mailing list