[PATCH] D67053: [LLD] [COFF] Resolve source locations for undefined references using dwarf

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo marked an inline comment as done.
mstorsjo added inline comments.


================
Comment at: test/COFF/undefined-symbol-dwarf.s:15
+# CHECK-NEXT: >>> referenced by {{.*}}.o:(myptr)
+
+	.text
----------------
MaskRay wrote:
> mstorsjo wrote:
> > MaskRay wrote:
> > > clang -S output is verbose. Many directives in this file are not significant and they should be deleted to improve readability. The relocations are not resolved. Does that cause any problem?
> > > clang -S output is verbose. Many directives in this file are not significant and they should be deleted to improve readability.
> > 
> > Yes, I guess I could trim out some. This is also built with -O0 to make sure the inline function ends up in a separate comdat (`.section .text$_ZN1A5afuncEv` below) to test that resolving references in different sections work, but I guess I could use some optimization with `-fno-inline` or something similar to keep them separate.
> > 
> > However, I'm a little weary of doing too much manual cleanups of it, if it would make the debug info mismatch the actual code. I'm not very proficient in DWARF so I don't know for sure exactly what needs to match what here, and which of the debug sections could be removed without breaking it.
> > 
> > > The relocations are not resolved. Does that cause any problem?
> > 
> > Um, what do you mean? The errors about undefined symbols? That's the whole point of the test; to have undefined references, that we try to resolve back to source line numbers using the debug info.
> Can you take a look at test/ELF/debug-line-str.s and test/ELF/debug-line-obj.s? They are still a bit verbose in my opinion but is much conciser than this file..
> 
> .debug* sections (DWARF) in object files contains relocations (e.g. `.secrel32  .Linfo_string1`). They have to be resolved so that they can be symbolized by lib/DebugInfo/Symbolize. In the ELF port of lld, ELF/DWARF.cpp provides the relocation resolver functionality.
> 
> Does COFF need a counterpart? To be honest I know very little about COFF. I'll make some investigation into this. Meanwhile, can you test this patch on some larger programs, DLLs with multiple object files, etc?
There are a few directives left I could strip out, and apparently it's ok to remove most of the `.debug_str` section, and some comments. With that I don't think I would be too far off from the ELF debug-line tests.

My test case has two text sections, just like debug-line-obj.s, but I guess I could try to transcribe that one to COFF for a smaller testcase of the same situation. 

I tested it on a larg DLL (with lots of object files) and seems to work fine there as well. (I don't see how the number of object files would matter though as only one object file is given to Symbolizer at a time.)

However it doesn't seem to work with debug info generated by GCC, so I guess I need to look into what's differing there.

The libObject RelocationResolver.cpp seems to be involved to some part for resolving relocations at least, but I'm not sure if that's enough for handling it in general, or if something like what is done for ELF is needed here.


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

https://reviews.llvm.org/D67053