[Lldb-commits] [PATCH] D55142: Minidump debugging using the native PDB reader

[Leonard Mosescu via lldb-commits]

I ended up implementing the support for "target symbols add" since it's
something we needed anyway. This allowed the removal of the contentious
implicit search in the current directory.

I tried to verify this behavior, but it seems like it should already work
> out of the box?   So we're on the same page, we already do have a real
> SymbolVendor implementation, it just happens to be the *default*
> SymbolVendor implementation.  It's not the case that one doesn't exist at
> all.


There were a few missing pieces, although you're right, it works with the
default SymbolVendor as you pointed out (btw, what I meant by "real"
SymbolVendor is a "PDB specific" SymbolVendor, sorry for the confusion)

There is another option which I was just made aware of.  LLDB already has a
> setting called `target.debug-file-search-paths`.  This is basically a
> symbol path.  If you call Symbols::LocateExecutableSymbolFile, it will
> already add use this setting, and moreover it will implicitly add current
> working directory to this path.
> So, if you want this behavior in a supported way that isn't temporary, we
> should move the code for findMatchingPDBFile() out of SymbolFilePDB and
> into this function.  Then everyone is happy I think.
>

As far as I can tell, Symbols::LocateExecutableSymbolFile() is a helper
intended for specialized SymbolVendors. So yes, when we get around to build
a specialized SymbolVendorPDB we'd be able to use it but until then I don't
think that moving that logic inside SymbolFileNativePDB is appropriate.

On Wed, Dec 12, 2018 at 1:19 PM Leonard Mosescu via Phabricator <
reviews at reviews.llvm.org> wrote:

> lemo marked an inline comment as done.
> lemo added inline comments.
>
>
> ================
> Comment at: source/Commands/CommandObjectTarget.cpp:4246
>            if (symbol_file) {
> -            ObjectFile *object_file = symbol_file->GetObjectFile();
>
> ----------------
> note I had to bypass this check: we don't (yet) have a ObjectFilePDB so
> the SymbolFileNativePDB always points to the associated PE binary.
>
> the check itself seems valuable as a diagnostic but not strictly required.
> Should I add a TODO comment and/or open a bug to revisit this?
>
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D55142/new/
>
> https://reviews.llvm.org/D55142
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20181212/85361815/attachment.html>