[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

Mon Apr 20 02:39:06 PDT 2020

labath added a comment.

In D75750#1988669 <https://reviews.llvm.org/D75750#1988669>, @kwk wrote:

> In D75750#1988330 <https://reviews.llvm.org/D75750#1988330>, @labath wrote:
>
> > The situation around finding symbols is messy enough already
>
>
> The way in which I integrated debuginfod for now is just to find source files and not yet symbols. That being said. I don't fear the status quo so much. The status quo is probably worse for symbols than it is for source files, don't you think? So with *all* the CMake integration, the hosting inside `lldb/include/lldb/Host/DebugInfoD.h` and your beloved test case,
>
> <snip huge meme>
>
> I think it is fair to say that at least some work is there that can be taken into LLDB. **As long** as I fix the retrieval of the module in `SourceManager::File::CommonInitializer`. As suggested by @jankratochvil either here or on IRC, I would like to give it a shot and try to pass down the correct module to this function. I'd say, let's see if this function can be passed a Module and if the changes are worth it. The whole part for retrieving debug information can come when the architectural changes are done.

That all sounds reasonable, and I would not really have a big problem with integrating debuginfod in this way (through @clayborg might -- you will also want to check with him). However, I have doubts about how long will it take to do the architectural changes to get symbol downloading to work (or even if it will happen). I don't want to demean the work you have done so far, but I think that stuff will be much more complicated than this.

In that light, it's not clear to me whether having the ability to download the source files without being able to get the executable and symbol files is particularly useful. It does not seem very useful to me, but maybe I am missing something.

Do you find that useful? If not, I think the changes can just as easily sit in this patch instead of in the tree. This isn't touching any areas under active development, so its not like this functionality will rot quickly.

> But then it's a piece of cake to extend `lldb/include/lldb/Host/DebugInfoD.h` with the right methods to call the debuginfod client lib.
> 
>> because one needs to understand the funky mac symbol searching mechanism, which is pretty much impossible without a mac machine.
> 
> I'm setting up my old mac to compile LLDB and I guess @jankratochvil might soon also have his own Mac. This at least puts us in a position where we can verify some of our changes.

That's great to hear. (though it's sad that is necessary)

> 
> 
>> After debuginfod, one will need to understand both, and have a linux machine with some debuginfod setup. The set of such people is likely to be empty of a long time...
> 
> I'm not sure if I understand you correctly but to me the *setup* is just to point to a machine with *your* or a hosted server. At least for OS binaries @fche2 @fche  (which is the correct one?) is making some effort to have those debuginfos and source files available and setup. That is a great start for most embedded systems with not much disk space to install all debug information I guess. Correct my if this is a wrong anticipation. Sure, I mean it will take a while before LLDB with debuginfod will make it into a distribution but hey, time flies by.

I'm not worried about having symbols for all the system binaries. But just the effort to setup a environment with a foreign operating system and learn enough about it to be able to make changes to it are enough to dissuade a lot of potential developers (this is to your credit). After you start playing around with a mac, I think you'll find that a lot of things work differently there -- it took me about four years to understand how dsyms and debug maps work (I wasn't trying to learnt it all of that time, but still...).

In D75750#1988694 <https://reviews.llvm.org/D75750#1988694>, @jankratochvil wrote:

> The current plan discussed with @kwk is to create the new `SymbolServer` abstract superclass and some its inherited implementation and move there the appropriate parts of existing `lldb/source/Symbol/LocateSymbolFile.cpp`. Current `SymbolVendor` implementations would then iterate new `SymbolServer`s by the existing `LocateExecutableSymbolFile` function. That may be enough for a patch of its own.

I'll have to see the actual patch for a definitive opinion, but I have to say that a priori I am sceptical of this direction. And yes, that should definitely be a separate patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75750