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

[Jez Ng via Phabricator via llvm-commits]

int3 added a comment.

+1 to what @smeenai said

Also re your commit message:

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

Is this actually a problem? Does the unwinder not like it when otherwise identical functions at two separate addresses share the same gcc_except_table?



================
Comment at: lld/MachO/Writer.cpp:680-681
+      // 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 *>())
----------------
this is now being done in `scanSymbols` (I'm aware this was from an existing comment)


================
Comment at: lld/MachO/Writer.cpp:693
         // to emit rebase opcodes for it.
-        it++;
+        skipNext = true;
         continue;
----------------
I think just copypasting the call to `canonicalize()` here would be cleaner -- no need for the `skipNext` state + would take fewer lines of code too


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151824/new/

https://reviews.llvm.org/D151824