[PATCH] D33155: [DWARFv5] Support FORM_strp in the line table header

[Paul Robinson via Phabricator via llvm-commits]

probinson added inline comments.


================
Comment at: lld/ELF/InputFiles.cpp:80-83
+  DwarfLine->getOrParseLineTable(LineData, 0,
+                                 Dwarf.getNumCompileUnits() > 0
+                                     ? Dwarf.getCompileUnitAtIndex(0)
+                                     : nullptr);
----------------
ruiu wrote:
> I don't fully understand this change, but it seems like if it was working before, `Dwarf.getNumCompileUnits() >0` is guaranteed, no?
No.  If you have no debug info, getNumCompileUnits() == 0, but we still go through this path.  When we discover there's no debug info, lld falls back to using FILE symbols instead.
That said, I don't have an lld test for retrieving filespecs from a DWARF v5 line table, so we could hardcode it to nullptr for now and leave a FIXME.


================
Comment at: llvm/test/DebugInfo/Inputs/dwarfdump-header-64.s:7
+# llvm-mc -triple x86_64-unknown-linux dwarfdump-header-64.s -filetype=obj \
+#         -o dwarfdump-header-64.elf-x86-64
+
----------------
aprantl wrote:
> Why do we need/want to check in the binary? Could it just be generated on the fly?
> 
> It looks like this assembler file may be general enough to also assemble for am x86_64-apple-darwin triple so we could test Mach-O support, too?
I thought checked-in binaries were usual; for example the DWARF32 version of the test is a checked-in binary.  Generating the binary on the fly has a small test-time performance cost.
I don't actually know if Mach-O would be okay with this, because section names?  I'm happy to do it that way if there's a coverage benefit.  I'll play with it and see what happens.


https://reviews.llvm.org/D33155