[PATCH] D113717: [Symbolizer][Debuginfo] Add debuginfod client to llvm-symbolizer.

Fri Dec 10 11:47:36 PST 2021

noajshu added a comment.

In D113717#3185263 <https://reviews.llvm.org/D113717#3185263>, @thakis wrote:

> In D113717#3180177 <https://reviews.llvm.org/D113717#3180177>, @noajshu wrote:
>
>> @thakis Thanks for the suggestion. Although we may know the `--obj`s which are going to be fed to `llvm-symbolizer` in advance, so we could run a "pull symbols" step just for those, we wouldn't know in advance which shared libraries the input (e.g. backtrace) will touch. So I think to make it work you'd need three steps -- symbolize and figure out what debuginfo is missing, then run the debuginfod fetch to download, and then symbolize again.
>
> You could have a tool that pulls symbols for a binary and all the shared libraries it links, right?

yes, see D112759 <https://reviews.llvm.org/D112759>

> A pattern I've seen over the years is that since we're all very busy, as soon as there's an easy explanation for why something is slow, people will jump to that explanation if something is slow. After this change, if someone says "llvm-symbolizer took several seconds", people will just say "well it probably pulled stuff off the network" instead of actually investigating.
>
> If the concern of pulling stuff from the network is separated into its own binary, it's impossible to get confused about this.

Hi Nico, I just added you as a reviewer of D115500 <https://reviews.llvm.org/D115500>, which changes the build so that LLVM  builds without libcurl by default, even if libcurl is present. This way there is no confusion.

In D113717#3186152 <https://reviews.llvm.org/D113717#3186152>, @dblaikie wrote:

> In D113717#3185471 <https://reviews.llvm.org/D113717#3185471>, @thakis wrote:
>
>> Turns out this isn't even just theoretical: This _did_ add a dependency on libcurl to lld:
>>
>>   //lld/tools/lld:lld has an assert_no_deps entry:
>>     //llvm/lib/Debuginfod:Debuginfod
>>   which fails for the dependency path:
>>     //lld/tools/lld:lld ->
>>     //lld/COFF:COFF ->
>>     //llvm/lib/DebugInfo/Symbolize:Symbolize ->
>>     //llvm/lib/Debuginfod:Debuginfod
>>
>> The linker shouldn't do http requests, obviously. Let's revert this for now until at least the lld dep is figured out.
>
> I'm not 100% sure that it's obvious the linker shouldn't make http requests if it can be used to improve diagnostics.
>
> Though most of lld's symbolizing is on object files and I don't think it's expected that debug info for unlinked object files would be in a symbol server - but perhaps there are cases where lld might want to symbolize an address in a .so where symbolizing might still be useful (though arguably so - if it's system installed it's probably not super relevant to the user to know what source files the symbol came from - they're just dealing with the system library, they aren't the authors of it - and if it's something they authored it probably has local debug info that doesn't need looking up)
>
> @noajshu Could you look into if/how lld uses the symbolizer and if there are use cases where it would have an positive effect on the user experience (improved diagnostic quality, etc) to lookup debug info on the symbol server? If there are no such cases, maybe having a separate entry point into the symbolizing library that statically does not use debuginfod would be good. The library dependency would still be there, but if a caller only uses the non-debuginfod entry point, nothing from the debuginfod library would be pulled in at link time.

Thanks for all these comments and sorry for the late response! I will respond more thoroughly to the points above soon. To avoid confusion I wanted to quickly mention something. 
I believe there is an error in the GN build configuration which causes lld's dependency on Debuginfod.
Specifically the file
llvm/utils/gn/secondary/lld/COFF/BUILD.gn
adds the dependency on DebugInfo/Symbolize, but this is not in
lld/COFF/CMakeLists.txt
Perhaps this is gn line is leftover from a previous dependency which was updated in cmake but never updated in gn?
Removing it seems to resolve the issue building lld with gn when both this diff and rG2586c23b <https://reviews.llvm.org/rG2586c23bae04c4d898c3d544ab51d2846a930930> are applied.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113717