<div dir="ltr"><div dir="ltr"><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="color:rgb(80,0,80)">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.</span></blockquote><div><br></div><div>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)</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">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.<div>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.</div></blockquote><div dir="ltr"><div><br></div><div>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.</div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Dec 12, 2018 at 1:19 PM Leonard Mosescu via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">lemo marked an inline comment as done.<br>
lemo added inline comments.<br>
<br>
<br>
================<br>
Comment at: source/Commands/CommandObjectTarget.cpp:4246<br>
           if (symbol_file) {<br>
-            ObjectFile *object_file = symbol_file->GetObjectFile();<br>
<br>
----------------<br>
note I had to bypass this check: we don't (yet) have a ObjectFilePDB so the SymbolFileNativePDB always points to the associated PE binary. <br>
<br>
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?<br>
<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D55142/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D55142/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D55142" rel="noreferrer" target="_blank">https://reviews.llvm.org/D55142</a><br>
<br>
<br>
<br>
</blockquote></div>