amccarth marked 2 inline comments as done.
amccarth 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:
> 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?
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
