[PATCH] D151824: [lld-macho]Fixed bug folding LSDA incorrectly

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo created this revision.
oontvoo added a reviewer: int3.
Herald added projects: lld-macho, All.
Herald added a reviewer: lld-macho.
oontvoo requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Details:

See bug report:  https://github.com/llvm/llvm-project/issues/63039

With assertion turned-off, lld would produce "bad" binary in which the gcc_except_table* got folded but not the related functions:

  00000000000006d8 s GCC_except_table1        <<<< These two got merged!
  00000000000006d8 s GCC_except_table2
  0000000000000610 T __Z1fv
  00000000000005e0 T __Z1gv
  0000000000000650 T __Z2f2v                  <<< But not f or f2
                   U __ZTIi
                   U ___cxa_allocate_exception
                   U ___cxa_begin_catch
                   U ___cxa_end_catch
                   U ___cxa_throw
                   U ___gxx_personality_v0
  0000000000003020 d __dyld_private
                   U dyld_stub_binder


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151824

Files:
  lld/MachO/Writer.cpp


Index: lld/MachO/Writer.cpp
===================================================================
--- lld/MachO/Writer.cpp
+++ lld/MachO/Writer.cpp
@@ -672,13 +672,25 @@
     if (isec->shouldOmitFromOutput())
       continue;
 
+    bool skipNext = false;
     for (auto it = isec->relocs.begin(); it != isec->relocs.end(); ++it) {
       lld::macho::Reloc &r = *it;
+
+      // Canonicalize the referent so that later accesses in Writer won't
+      // have to worry about it. Perhaps we should do this for Defined::isec
+      // too...
+      if (auto *referentIsec = r.referent.dyn_cast<InputSection *>())
+        r.referent = referentIsec->canonical();
+
+      if (skipNext) {
+        skipNext = false;
+        continue;
+      }
       if (target->hasAttr(r.type, RelocAttrBits::SUBTRAHEND)) {
         // Skip over the following UNSIGNED relocation -- it's just there as the
         // minuend, and doesn't have the usual UNSIGNED semantics. We don't want
         // to emit rebase opcodes for it.
-        it++;
+        skipNext = true;
         continue;
       }
       if (auto *sym = r.referent.dyn_cast<Symbol *>()) {
@@ -688,11 +700,6 @@
         if (!isa<Undefined>(sym) && validateSymbolRelocation(sym, isec, r))
           prepareSymbolRelocation(sym, isec, r);
       } else {
-        // Canonicalize the referent so that later accesses in Writer won't
-        // have to worry about it. Perhaps we should do this for Defined::isec
-        // too...
-        auto *referentIsec = r.referent.get<InputSection *>();
-        r.referent = referentIsec->canonical();
         if (!r.pcrel) {
           if (config->emitChainedFixups)
             in.chainedFixups->addRebase(isec, r.offset);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D151824.527143.patch
Type: text/x-patch
Size: 1714 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230531/cb928db0/attachment.bin>