[PATCH] D132845: [MachO] Don't fold compact unwind entries with LSDA

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.


================
Comment at: lld/MachO/UnwindInfoSection.cpp:466
            cuEntries[*foldBegin].encoding == cuEntries[*foldEnd].encoding &&
+           !cuEntries[*foldBegin].lsda && !cuEntries[*foldEnd].lsda &&
+           // If we've gotten to this point, we don't have an LSDA, which should
----------------
thevinster wrote:
> In what scenario would we have compact unwind entries //without// LSDA? I'm wondering if this is now effectively a condition that will never be hit and thus dead code now. 
It's pretty common to have compact unwind entries without LSDA :) You only need LSDA when you have language-specific actions to take during unwinding, e.g. running destructors or a catch statement. In the test below, you'll notice that `g` and `h` have indices 0 and 1 in the LSDA descriptors but 1 and 2 in the second level indices; that's because `f` has the compact unwind entry at index 0 but no LSDA.


================
Comment at: lld/test/MachO/compact-unwind-lsda-folding.s:2
+## Verify that the compact unwind entries for two functions with identical
+## unwind information and LSDA aren't folded together; see the comment in
+## UnwindInfoSectionImpl::finalize for why.
----------------
thevinster wrote:
> Should we also have a test for when they //can// be folded? 
`compact-unwind-generated.test` handles that via `tools/validate-unwind-info.py`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132845