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

Carlo Cabrera via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 20 11:53:23 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 cbadcfb0a80eba996d988e45e17def1629c8f2a0 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 | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/lld/MachO/SymbolTable.cpp b/lld/MachO/SymbolTable.cpp
index 36e421b52abcac..64e4658c4c1ebb 100644
--- a/lld/MachO/SymbolTable.cpp
+++ b/lld/MachO/SymbolTable.cpp
@@ -15,6 +15,7 @@
 #include "SyntheticSections.h"
 #include "lld/Common/ErrorHandler.h"
 #include "lld/Common/Memory.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Demangle/Demangle.h"
 
 using namespace llvm;
@@ -67,10 +68,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 compare = [](uint64_t off, const Symbol *s) {
+    return cast<Defined>(s)->value > off;
+  };
+  assert(std::is_partitioned(toIsec->symbols.begin(), toIsec->symbols.end(),
+                             [toOff, compare](const Symbol *s) {
+                               return !compare(toOff, s);
+                             }) &&
+         "Symbols in toIsec must be partitioned by toOff.");
+  auto insertIt = llvm::upper_bound(toIsec->symbols, toOff, compare);
   llvm::erase_if(fromIsec->symbols, [&](Symbol *s) {
     auto *d = cast<Defined>(s);
     if (d->value != fromOff)
@@ -91,6 +97,11 @@ static void transplantSymbolsAtOffset(InputSection *fromIsec,
     }
     return true;
   });
+  assert(llvm::is_sorted(toIsec->symbols,
+                         [](const Defined *s, const Defined *t) {
+                           return s->value < t->value;
+                         }) &&
+         "Symbols should still be sorted at exit.");
 }
 
 Defined *SymbolTable::addDefined(StringRef name, InputFile *file,



More information about the llvm-commits mailing list