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

[mstorsjo]

Summary

Since c579a5b1d92a9bc2046d00ee2d427832e0f5ddec we don't traverse
.eh_frame when doing GC. But the exception handling personality
function needs to be included, and is only referenced from within
.eh_frame.

Longterm, we probably would want to split .eh_frame into associative
COMDATs just like we do for .xdata/.pdata (and then include them in
the GC traversal), but for now, just try to pull in the name of the
known personality function in mingw DWARF EH.

[rnk]

lld/COFF/Driver.cpp

2240

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?

[mstorsjo]

lld/COFF/Driver.cpp

2240

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

[rnk]

lld/COFF/Driver.cpp

2240

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.

[MaskRay]

.eh_frame consists of FDE (text section contribution) and CIE (common part shared by multiple FDE).
It needs to be monolithic in -ffunction-sections mode because:

• A section header has a large overhead (sizeof(Elf64_Shdr)=64).
• A CIE must precede a FDE which references it (CIE Pointer in a FDE is a non-zero unsigned integer). The input section order in one object file may not match the output section order, breaking this requirement. (I am surprised that mingw can have fragmented .eh_frame$

.eh_frame pieces can be stale (GC, ICF), so ld.lld has a pass merging .eh_frame pieces and discarded unneeded pieces.

When GC is concerned: ld.lld computes --gc-sections (markLive<ELFT>()) before combining .eh_frame pices (combineEHSections).
So we now have a phase ordering problem.
Since GC runs earlier, we have to conservatively assume all .eh_frame pieces live.
However, we don't necessarily retain everything referenced by .eh_frame pieces.
See the comment in MarkLive.cpp:scanEHFrameSection and MarkLive<ELFT>::resolveReloc: we retain most referenced symbols live except
a symbol defined in an executable non-linkorder-non-group section.

In ELF, you may find test/ELF/eh-frame-gc{,2}.s and their history useful.

[MaskRay]

lld/COFF/Driver.cpp

2240

Does mingw still use sjlj? GCC used to have an option -fsjlj-exceptions which has been removed for many years now.

lld/test/COFF/gc-dwarf-eh.s

38

Is .cfi_personality supported? Would be good to use correct content. The personality (__gxx_personality_v0) relocation precedes the text section symbols.

[mati865]

lld/COFF/Driver.cpp

2240

There are mingw-w64 toolchains out there that still use SJLJ for 32-bit builds but most of them has switched to DWARF already. For 64-bit builds AFAIK every popular toolchain uses SEH.

[mstorsjo]

In D102138#2752049, @MaskRay wrote:

.eh_frame consists of FDE (text section contribution) and CIE (common part shared by multiple FDE).
It needs to be monolithic in -ffunction-sections mode because:

• A section header has a large overhead (sizeof(Elf64_Shdr)=64).
• A CIE must precede a FDE which references it (CIE Pointer in a FDE is a non-zero unsigned integer). The input section order in one object file may not match the output section order, breaking this requirement. (I am surprised that mingw can have fragmented .eh_frame$

Thanks for the info! No I'm not saying that mingw can have fragmented .eh_frame$, that was just my initial assumption for how to implement it, mirroring the SEH case. But now it does indeed sound like that wouldn't be the right thing to do, so that saves me some amount of wasted time. Thanks!

.eh_frame pieces can be stale (GC, ICF), so ld.lld has a pass merging .eh_frame pieces and discarded unneeded pieces.

When GC is concerned: ld.lld computes --gc-sections (markLive<ELFT>()) before combining .eh_frame pices (combineEHSections).
So we now have a phase ordering problem.
Since GC runs earlier, we have to conservatively assume all .eh_frame pieces live.
However, we don't necessarily retain everything referenced by .eh_frame pieces.
See the comment in MarkLive.cpp:scanEHFrameSection and MarkLive<ELFT>::resolveReloc: we retain most referenced symbols live except
a symbol defined in an executable non-linkorder-non-group section.

In ELF, you may find test/ELF/eh-frame-gc{,2}.s and their history useful.

Thanks! That sounds like it would be the way to go, but it also sounds like a bit more effort than I was expecting to spend on it, so in that case, I think this particular approach might be good enough for now - dwarf EH on mingw isn't used on the most important targets anyway.

lld/test/COFF/gc-dwarf-eh.s

38

I can try to give it a shot to rewrite this using proper CFI directives. This way it felt a bit more clear exactly what the object file looks like though (although I could reorder things to fake a real .eh_frame a bit better too).

[mstorsjo]

Checking for both __gxx_personality_v0 and __gcc_personality_v0. Reworded the comment to clarify what the proper full solution would be, changed the testcase to use .cfi directives instead of a fake .eh_frame section.

[rnk]

lgtm

lld/COFF/Driver.cpp

if (config->callGraphProfileSort) {
     readCallGraphsFromObjectFiles();
   }
 
   // Handle /print-symbol-order.
   if (auto *arg = args.getLastArg(OPT_print_symbol_order))
     config->printSymbolOrder = arg->getValue();
 
   // Identify unreferenced COMDAT sections.
-  if (config->doGC)
+  if (config->doGC) {
+    if (config->mingw) {
+      // markLive doesn't traverse .eh_frame, but the personality function is
+      // only reached that way. The proper solution would be to parse and
+      // traverse the .eh_frame section, like the ELF linker does.
+      // For now, just manually try to retain the known possible personality
+      // functions. This doesn't bring in more object files, but only marks
+      // functions that already have been included to be retained.
+      for (const char *n : {"__gxx_personality_v0", "__gcc_personality_v0"}) {
+        Defined *d = dyn_cast_or_null<Defined>(symtab->findUnderscore(n));
+        if (d && !d->isGCRoot) {
+          d->isGCRoot = true;
+          config->gcroot.push_back(d);
+        }
+      }
+    }
+
     markLive(symtab->getChunks());
+  }
 
   // Needs to happen after the last call to addFile().
   convertResources();
 
   // Identify identical COMDAT sections to merge them.
   if (config->doICF != ICFLevel::None) {
     findKeepUniqueSections();
     doICF(symtab->getChunks(), config->doICF);

lld/test/COFF/gc-dwarf-eh.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -triple=i686-windows-gnu %s -filetype=obj -o %t.obj
+# RUN: lld-link -lldmingw -lldmap:%t.map -out:%t.exe -opt:ref -entry:main %t.obj -verbose 2>&1 | FileCheck %s
+# RUN: FileCheck %s --check-prefix=MAP --input-file=%t.map
+
+# CHECK: Discarded _unused
+
+# MAP: In Symbol
+# MAP: gc-dwarf-eh.s.tmp.obj:(.text)
+# MAP: {{ ___gxx_personality_v0$}}
+
+  .def  _main; .scl 2; .type  32; .endef
+  .section  .text,"xr",one_only,_main
+  .globl  _main
+  .cfi_startproc
+  .cfi_personality 0, ___gxx_personality_v0
+_main:
+  xorl  %eax, %eax
+  ret
+  .cfi_endproc
+
+  .def  ___gxx_personality_v0; .scl 2; .type  32; .endef
+  .section  .text,"xr",one_only,___gxx_personality_v0
+  .globl  ___gxx_personality_v0
+___gxx_personality_v0:
+  ret
+
+  .def  _unused; .scl 2; .type  32; .endef
+  .section  .text,"xr",one_only,_unused
+  .globl  _unused
+  .cfi_startproc
+  .cfi_personality 0, ___gxx_personality_v0
+_unused:
+  ret
+  .cfi_endproc