[PATCH] D107533: [lld-macho] Allow encode up to 4 personality symbols.

Thu Aug 5 13:58:10 PDT 2021

oontvoo marked an inline comment as not done.
oontvoo added inline comments.

================
Comment at: lld/MachO/UnwindInfoSection.cpp:285
 // There should only be a handful of unique personality pointers, so we can
 // encode them as 2-bit indices into a small array.
 template <class Ptr>
----------------
oontvoo wrote:
> oontvoo wrote:
> > int3 wrote:
> > > thakis wrote:
> > > > Due to this comment, I think you'll have to change some encoding code in addition to bumping the number too. Two bits aren't enough for 4 personalities.
> > > Yeah. To be clear, it's not enough because in addition to the 4 personalities, we need to encode the "no personality" value.
> > > 
> > > ld64 should have the same limitation too. I found some links mentioning the issue, with the suggested workaround of passing `-no_compact_unwind`: https://bugzilla.mozilla.org/show_bug.cgi?id=1289847. However https://stackoverflow.com/a/30733047/149330 mentions that it only works if you crash on every exception (which I think Rust does?)
> > > 
> > > In any case, LLD doesn't yet support this flag. But I'm wondering if the ld64-linked build in PR51262 is succeeding because of something else, e.g. perhaps dead-stripping is removing the code that requires the extra personality.
> > How about we keep it as 2-bit and change the personalityIndex to be 0-based (below)?
> > Should still be enough for now - unless we find a fifth one ...
> Ooops - crosstalking ... didn't see int3's comment

- in practice, how often do you see 4+ personalty symbols?  
- it's a limitation from ld64, too - but doesn't mean have to imitate that, right? 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107533