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

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo 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) {
----------------
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?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102138