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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to