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

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo planned changes to this revision.
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>
----------------
int3 wrote:
> oontvoo wrote:
> > 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? 
> > 
> > 
> > in practice, how often do you see 4+ personality symbols?
> 
> none of the things I've been building so far need it :)
> 
> > it's a limitation from ld64, too - but doesn't mean have to imitate that, right?
> 
> nope, but it's a limitation of the format. I assume `-no_compact_unwind` makes ld64 use the non-compact DWARF format that doesn't have that limitation.
Ah! I think I saw the problem  (at least in this build I'm trying to get working).
The personality symbols could come from both Defined and DylibSymbol. 
So while there are only 3 uniquely named personalities , there were 4 or 5 personalities  (ie., different got indices).

I guess for this case, we could just pick the Defined personalities over the others?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107533