[lld] [lld][MachO] Fix symbol insertion in `transplantSymbolsAtOffset` (PR #120737)

[Carlo Cabrera via llvm-commits]

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/3] [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/3] 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)

>From 0db5264e5918221edec3dacd2e8c0c6328769517 Mon Sep 17 00:00:00 2001
From: Carlo Cabrera <github at carlo.cab>
Date: Sun, 22 Dec 2024 21:25:19 +0800
Subject: [PATCH 3/3] Test that assert fires in CI

---
 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 a61e60a944fb45..ad5898e10f85a3 100644
--- a/lld/MachO/SymbolTable.cpp
+++ b/lld/MachO/SymbolTable.cpp
@@ -68,7 +68,7 @@ static void transplantSymbolsAtOffset(InputSection *fromIsec,
                                       uint64_t fromOff, uint64_t toOff) {
   // Ensure the symbols will still be in address order after our insertions.
   auto symSucceedsOff = [](uint64_t off, const Symbol *s) {
-    return cast<Defined>(s)->value > off;
+    return cast<Defined>(s)->value < off;
   };
   assert(std::is_partitioned(toIsec->symbols.begin(), toIsec->symbols.end(),
                              [symSucceedsOff, toOff](const Symbol *s) {