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

Sun Sep 8 23:20:14 PDT 2019

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

In D67053#1662604 <https://reviews.llvm.org/D67053#1662604>, @MaskRay wrote:

> In D67053#1662315 <https://reviews.llvm.org/D67053#1662315>, @mstorsjo wrote:
>
> > Do you have an opinion on whether I should keep the LLVMSymbolizer object around between calls?
>
>
> This is the error path and we don't expect this code to be called many times so its contribution to the overall time may be negligible. Keep the same LLVMSymbolizer will benefit when you symbolize the same object multiple times. If reusing LLVMSymbolizer does not take too many lines, you can do that.

@ruiu - Do you have any suggestion on where to store it, to have it reused in multiple calls here? I could add it as an `std::unique_ptr` in the `config` object, but I think that might end up requiring including the symbolizer header in the config header.

>> And would it be better to use `symbolizeInlinedCode` instead of `symbolizeCode`? (That requires a small patch to the symbolizer, to allow that one to operate on an in-memory object file.)
> 
> In the diagnostics I think we just need one line, so symbolizeCode should be sufficient. symbolizeInlinedCode can give you multiple frames but I'm not sure how to pretty print them.

Ok, thanks for the info!

================
Comment at: test/COFF/undefined-symbol-dwarf.s:109
+	.byte	1                       # Abbrev [1] 0xb:0xb0 DW_TAG_compile_unit
+	.byte   0                       # DW_AT_producer
+	.short	4                       # DW_AT_language
----------------
MaskRay wrote:
> Convert tabs to spaces so they will align regardless of the tabstop setting (e.g. in phabricator it is nicely rendered)
Ok, will do

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

https://reviews.llvm.org/D67053