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

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/Writer.cpp:693
         // to emit rebase opcodes for it.
-        it++;
+        skipNext = true;
         continue;
----------------
oontvoo wrote:
> int3 wrote:
> > 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
> couldn't simply copy the canonicalize() call here because we want to  call it on the "next" entry (ie., the "skipped" one) as well.
> 
> In other words this `it++` would have skipped over it .
I understand, but I meant like

```
++it;
if (auto *referentIsec = it->referent.dyn_cast<InputSection *>())
  it->referent = referentIsec->canonical();
continue;
```


================
Comment at: lld/test/MachO/fold-dwarf-lsda.s:2
+# REQUIRES: x86
+## Test for https://github.com/llvm/llvm-project/issues/63039
+
----------------



================
Comment at: lld/test/MachO/fold-dwarf-lsda.s:5
+## Use an old version to ensure we do *not* have any compact-unwind.
+# RUN: llvm-mc  -filetype=obj -triple=x86_64-apple-macos9 %s -o %t.o
+
----------------



================
Comment at: lld/test/MachO/fold-dwarf-lsda.s:43
+	.cfi_startproc
+	.cfi_personality 155, ___gxx_personality_v0
+	.cfi_lsda 16, Lexception0
----------------
can the test case be made smaller? e.g. are the `.cfi_personality` directives necessary?


================
Comment at: lld/test/MachO/fold-dwarf-lsda.s:47
+	.cfi_offset %rbp, -16
+	.cfi_def_cfa_register %rbp
+	callq	_g
----------------
`.cfi_def_cfa_register` probably isn't necessary either


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151824