[PATCH] D76935: [COFF] Don't treat DWARF sections as GC roots

[Reid Kleckner via Phabricator via llvm-commits]

rnk marked 4 inline comments as done.
rnk added a comment.

In D76935#1946511 <https://reviews.llvm.org/D76935#1946511>, @MaskRay wrote:

> The code change looks good, but I've got some test suggestions. I have some thoughts if COFF/MarkLive.cpp is ever made more sophisticated. In ELF, gc is for sections which are part of the load image (SHF_ALLOC) and non-SHF_ALLOC sections are generally not discarded. I don't know what is similar to SHF_ALLOC in COFF, though.


I guess the analogous flag would be IMAGE_SCN_MEM_DISCARDABLE. It is phrased in the opposite direction: sections lacking the flag are loaded, sections with the flag are not loaded into memory, and are generally sorted to the end of the image.

Strangely, LLVM MC suppresses the `D` assembler flag when printing textual assembly, and infers it from the section name:
https://github.com/llvm/llvm-project/blob/a55daa146166353236aa528546397226bee9363b/llvm/lib/MC/MCParser/COFFAsmParser.cpp#L258-L260

I realized, though, that there is one major difference in behavior. `isDWARF()` returns true for `.eh_frame` sections, but they are not discardable. They must be loaded into memory for unwinding to work. So, in this situation, GC will not work:

  void foo();
  int main() { foo(); } 
  void unused1() { foo(); } 
  void unused2() { foo(); }

Compiled like so:
`$ clang -O1 --target=x86_64-windows-gnu -c t.c -fdwarf-exceptions -ffunction-sections`

The unwind data is not separated into nice little comdat .pdata/.xdata sections, it is a single monolithic .eh_frame section:

  $ llvm-objdump -h t.o
  
  t.o:    file format coff-x86-64
  
  Sections:
  Idx Name          Size     VMA              Type
    0 .text         00000000 0000000000000000 TEXT
    1 .data         00000000 0000000000000000 DATA
    2 .bss          00000000 0000000000000000 BSS
    3 .text$main    0000001a 0000000000000000 TEXT
    4 .text$unused1 0000000e 0000000000000000 TEXT
    5 .text$unused2 0000000e 0000000000000000 TEXT
    6 .eh_frame     00000080 0000000000000000 DATA
    7 .llvm_addrsig 00000000 0000000000000000

I checked, and it has relocations against all the functions. :( Blech. I think we have to do the isDWARF check.



================
Comment at: lld/test/COFF/gc-dwarf.s:10
+
+# MAP: main
+# MAP: used
----------------
MaskRay wrote:
> It will be better to prepend `# MAP: Symbol` to make clear it is the `MAP` column.
> 
> In not-so-rare circumstances the absolute path of `%t.obj` may contain `main`.
> `main{{$}}` can prevent this pattern from matching a path component.
Yeah, unfortunately there is no other leading text on the line to match against to try to prevent that, so I left it as was. It's a bit redundant to check which symbols were included anyway. But I can make it tighter.


================
Comment at: lld/test/COFF/gc-dwarf.s:22
+	callq used
+	xorl	%eax, %eax
+	retq
----------------
MaskRay wrote:
> Optional: in ELF we tend to omit unneeded details like `xorl %eax, %eax; retq`.
> But we will keep `retq` because an empty section and a non-empty section make a difference for GNU linkers.
Sure, I based this assembly on normal compiler output. I prefer it when the assembly appears to do something semi-plausible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76935