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 <
revi...@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
>
>
>
>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
