clayborg added inline comments.
================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4175
+
+ if (object_file->GetFileSpec() != symbol_fspec) {
+ result.AppendWarning("there is a discrepancy between the module file "
----------------
amccarth wrote:
> amccarth wrote:
> > clayborg wrote:
> > > labath wrote:
> > > > This part is not good. Everywhere else except pdbs this means that we
> > > > were in fact unable to load the symbol file. What happens is that if we
> > > > cannot load the object file representing the symbol file (at [[
> > > > https://github.com/llvm/llvm-project/blob/master/lldb/source/Symbol/SymbolVendor.cpp#L48
> > > > | SymbolVendor.cpp:48 ]]), we fall back to creating a SymbolFile using
> > > > the object file of the original module (line 52).
> > > >
> > > > The effect of that is that the symbol file creation will always
> > > > succeed, the previous checks are more-or-less useless, and the only way
> > > > to check if we are really using the symbols from the file the user
> > > > specified is to compare the file specs.
> > > >
> > > > Now, PDB symbol files are abusing this fallback, and reading the
> > > > symbols from the pdb files even though they were in fact asked to read
> > > > them from the executable file. This is why this may sound like a
> > > > "discrepancy" coming from the pdb world, but everywhere else this just
> > > > means that the symbol file was not actually loaded.
> > > This could also fail if the symbol file spec was a bundle which got
> > > resolved when the module was loaded. If the user specified
> > > "/path/to/foo.dSYM" and the underlying code was able to resolve the
> > > symbol file in the dSYM bundle to
> > > "/path/to/foo.dSYM/Contents/Resources/DWARF/foo".
> > Interesting. I did not expect that fallback behavior from the API.
> >
> > > PDB symbol files are abusing this fallback
> >
> > I'm not sure there's abuse going on here. I'm not understanding something
> > about how lldb models this situation. Consider the case where the user
> > explicitly asks lldb to load symbols from a PDB:
> >
> > (lldb) target add symbols D:\my_symbols\foo.pdb
> >
> > Then `symbol_file` is the PDB and `object_file` is the executable or DLL.
> > I would expect those file specs to match only in the case where the symbols
> > are in the binary (in other words, when symbol file and object file are the
> > same file). Even ignoring the PDB case, it seems this wouldn't even work
> > in the case you mentioned above where the object file is stripped and the
> > symbol file is an unstripped copy of the object file (possibly with a
> > different name). You can also get here if the symbols were matched by UUID
> > rather than file spec. Or when the symbols were matched by the basename
> > minus some extension.
> >
> > Given that dsyms work, I must be misunderstanding something here. Can you
> > help me understand?
> >
> > What's the right thing to do here? If I just undo this refactor, then
> > we're back to silent failures when the symbols exist in a different file
> > than the object.
> In your example, the file specs do not match.
>
> 1. The old code would therefore reject the symbol file and return an error.
> 2. The new code would therefore emit a warning but proceed with the selected
> symbol file and return success.
>
> Do you want either of these behaviors or something else?
all I know is I can and have typed:
```
(lldb) target symbols add /path/to/foo.dSYM
```
And it would successfully associate it with the binary, probably because the
code on 4071 would see a UUID and accept it before it gets to this point.
What is the point of the warning? When would this happen?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73594/new/
https://reviews.llvm.org/D73594
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
