[PATCH] D102138: [LLD] [COFF] Fix including the personality function for DWARF EH when linking with --gc-sections

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a subscriber: MaskRay.
rnk added inline comments.


================
Comment at: lld/COFF/Driver.cpp:2240
+      Defined *d = dyn_cast_or_null<Defined>(
+          symtab->findUnderscore("__gxx_personality_v0"));
+      if (d && !d->isGCRoot) {
----------------
mstorsjo wrote:
> rnk wrote:
> > GCC produces a handful of these:
> > https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/EHPersonalities.cpp#L26
> > ```
> >       .Case("__gxx_personality_v0", EHPersonality::GNU_CXX)
> >       .Case("__gxx_personality_seh0", EHPersonality::GNU_CXX)
> >       .Case("__gxx_personality_sj0", EHPersonality::GNU_CXX_SjLj)
> >       .Case("__gcc_personality_v0", EHPersonality::GNU_C)
> >       .Case("__gcc_personality_seh0", EHPersonality::GNU_C)
> >       .Case("__gcc_personality_sj0", EHPersonality::GNU_C_SjLj)
> > ```
> > 
> > The `__gcc` variants mostly come from `__attribute__((cleanup))` usage, `_sj0` is presumably for sjlj eh, and I'm not sure about `_v0` vs. `_seh0`.
> > 
> > We could replicate the list here, or we could parse eh_frame and look for personalities. Don't we already parse eh_frame in ELF? Can we use the logic easily?
> > 
> SJLJ doesn't use unwind tables but just direct references in code, so that works as-is.
> 
> SEH is handled via xdata/pdata, which is handled correctly via either properly associative comdats or gnu-style suffixed comdats, so there the personality function is kept.
> 
> So for now, dwarf is the only one that needs this, and dwarf uses the `_v0` suffixed personality. But we could indeed check for both the gcc and gxx variants.
> 
> I'm not sure how much we'd gain by parsing the eh_frame - we only retain in the gcc/gxx personality functions if they're already pulled in by object files. Sure - if they're only referenced by a function that is discarded, we don't need to keep them...
> 
> But I'd rather (long term) split up eh_frame in similar associative comdats like xdata/pdata - although that seems to be a bit more work. (Or is there a reason not to do that - how is that handled in ELF?)
Great, let's just do the gcc and gxx variants and call it a day.

On ELF, .eh_frame is generally merged together into one section, and the linker parses it to discard the unnecessary DIE entries for discarded functions. This is not ideal, but until recently there wasn't consensus about how to represent something like an associative comdat section. And, I believe splitting .eh_frame would have high overhead, both from ELF and DWARF. Maybe @maskray can say more about the state of the art.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102138