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

Tue Dec 11 15:25:36 PST 2018

I guess I don't see why we need a temporary solution at all.  If we can
have logic that can be rolled into the SymbolVendor when we get it, and
makes sense there, and is also simple, why not go with it?  Failing that,
doesn't the `target symbols add` solution also work fine?

On Tue, Dec 11, 2018 at 3:24 PM Leonard Mosescu <mosescu at google.com> wrote:

> But here, we're talking about a situation where there is no EXE, only a
>> minidump.  If there is a minidump and no EXE then neither WinDbg nor VS
>> will search the minidump folder for the PDB.
>
>
> Indeed, this is key part.
>
>
>>  In my mind, the algorithm could be something like: ...
>>
>
> I'm not a big fan of this kind of magic. VS sometime does it, and while
> it's nice in the 80% of the cases I found it hard to troubleshoot when
> something goes wrong. For what it's worth, recent versions of VC++ debugger
> are more aligned with ntsd/cdb/windbg.
>
> Anyway, for this patch I think the question is: what's the simplest
> temporary solution? All of these ideas are interesting but more appropriate
> to evaluate when we start designing the PDB SymbolVendor.
>
>
>
> On Tue, Dec 11, 2018 at 3:05 PM Zachary Turner <zturner at google.com> wrote:
>
>> Only one way to know for sure, and that's to test it :)  So I did.
>>
>> Yes, it will search the directory of the EXE for the PDB.  But here,
>> we're talking about a situation where there is no EXE, only a minidump.  If
>> there is a minidump and no EXE then neither WinDbg nor VS will search the
>> minidump folder for the PDB.  Personally I think it should and I wouldn't
>> mind if that were a documented and tested feature of LLDB, because since it
>> already is established behavior to search the minidump folder for the EXE,
>> it doesn't seem hacky at all for it to also search that location for the
>> PDB.  In my mind, the algorithm could be something like:
>>
>> ```
>> if (ExeLocation = FindExecutableInMinidumpFolder()) {
>>   PdbLocation = FindPdbFromExecutable(ExeLocation)
>> } else {
>>   PdbLocation = FindPdbInMinidumpFolder();
>>   if (!PdbLocation)
>>     PdbLocation = FindPdbInSympath();
>> }
>> ```
>>
>> and that would be pretty logical and not code that we would have to
>> consider temporary.
>>
>> My main pushback here is that it's harder to remove code than it is to
>> add it, because once you add it, someone is going to depend on it and
>> complain when you try to remove it.  So if we can just establish some
>> behavior that satisfies the use case while not being temporary, I would
>> prefer to do it.
>>
>> Another option I can think of is to just run `target symbols add -s
>> foo.exe foo.pdb`.  I think we would need to have SymbolFileNativePDB check
>> the value of this setting, but using this workflow, you could just put a
>> .lldbinit file next to lldb.exe that runs this command at startup.
>>
>> On Tue, Dec 11, 2018 at 2:17 PM Adrian McCarthy <amccarth at google.com>
>> wrote:
>>
>>> I believe the PDB is searched for in the EXE directory before the symbol
>>> search path is used.  At least, that's what it used to do, back when I used
>>> VS debugger for post-mortem debugging.  It was the only sane way to ensure
>>> it would find the right version of the PDB if you didn't have a local
>>> symbol server.
>>>
>>> Yes, I understand that the security note was about DLL loading.  My
>>> point was that, in general, Windows looks in well known places first,
>>> before checking the current working directory.
>>>
>>> On Tue, Dec 11, 2018 at 2:12 PM Leonard Mosescu <mosescu at google.com>
>>> wrote:
>>>
>>>> The Windowsy thing to do is what Zach said:  Check the directory that
>>>>> contains the .dmp for the .pdb.  It's the first place the VS debugger looks
>>>>> when opening a minidump.  It's less sensitive to the user's environment.
>>>>> (Making the user change the current working directory for this could be at
>>>>> odds with other bits of the software that look relative the cwd.)
>>>>>
>>>>> https://docs.microsoft.com/en-us/visualstudio/debugger/using-dump-files?view=vs-2017#BKMK_Find_binaries__symbol___pdb__files__and_source_files
>>>>>
>>>>
>>>> Except that it doesn't :) Neither VisualStudio nor the Windows
>>>> Debuggers (windbg & co) look for PDBs in the same directory as the dump
>>>> file. The search is controlled by an explicit "symbol search path". The
>>>> link you mentioned is a bit confusing indeed (although it only claims that
>>>> the .exe's are searched in the same directory as the .dmp)
>>>>
>>>> While security is not a big issue here, note that Windows generally
>>>>> searches for DLLs in the known/expected places _before_ checking to see if
>>>>> it's in the current working directory.  This prevents a sneaky download
>>>>> from effectively replacing a DLL.  Replacing a PDB is probably less of a
>>>>> concern.
>>>>>
>>>>> https://docs.microsoft.com/en-us/windows/desktop/dlls/dynamic-link-library-search-order#standard-search-order-for-desktop-applications
>>>>>
>>>>
>>>> This is about the runtime dynamic module loader, not the debugger.
>>>>
>>>>
>>>>
>>>>
>>>> On Tue, Dec 11, 2018 at 1:28 PM Adrian McCarthy <amccarth at google.com>
>>>> wrote:
>>>>
>>>>> It's really frustrating how the email discussion doesn't always make
>>>>> it to Phabricator.
>>>>>
>>>>> The Windowsy thing to do is what Zach said:  Check the directory that
>>>>> contains the .dmp for the .pdb.  It's the first place the VS debugger looks
>>>>> when opening a minidump.  It's less sensitive to the user's environment.
>>>>> (Making the user change the current working directory for this could be at
>>>>> odds with other bits of the software that look relative the cwd.)
>>>>>
>>>>>
>>>>> https://docs.microsoft.com/en-us/visualstudio/debugger/using-dump-files?view=vs-2017#BKMK_Find_binaries__symbol___pdb__files__and_source_files
>>>>>
>>>>> While security is not a big issue here, note that Windows generally
>>>>> searches for DLLs in the known/expected places _before_ checking to see if
>>>>> it's in the current working directory.  This prevents a sneaky download
>>>>> from effectively replacing a DLL.  Replacing a PDB is probably less of a
>>>>> concern.
>>>>>
>>>>>
>>>>> https://docs.microsoft.com/en-us/windows/desktop/dlls/dynamic-link-library-search-order#standard-search-order-for-desktop-applications
>>>>>
>>>>> So I +1 Zach's proposal.
>>>>>
>>>>> On Tue, Dec 11, 2018 at 12:07 PM Leonard Mosescu <mosescu at google.com>
>>>>> wrote:
>>>>>
>>>>>> I think as combination of explicit symbol search path + something
>>>>>> similar to Microsoft's symsrv would be a good "real" solution (and yes,
>>>>>> that would be packaged as a SymbolVendor, outside SymbolFilePDB)
>>>>>>
>>>>>> For short term, I don't see a clearly superior alternative to
>>>>>> searching the current directory.
>>>>>>
>>>>>> On Tue, Dec 11, 2018 at 12:02 PM Pavel Labath <pavel at labath.sk>
>>>>>> wrote:
>>>>>>
>>>>>>> On 11/12/2018 20:34, Zachary Turner wrote:
>>>>>>> > I meant the location of the minidump.  So if you have
>>>>>>> C:\A\B\C\foo.dmp
>>>>>>> > which is the dump file for bar.exe which crashed on another
>>>>>>> machine,
>>>>>>> > then it would look for C:\A\B\C\bar.pdb.  That actually seems like
>>>>>>> > fairly intuitive behavior to me, but maybe I'm in the minority :)
>>>>>>> >
>>>>>>> > We can see what Pavel, Adrian, and others think though or if they
>>>>>>> have
>>>>>>> > any other suggestions.
>>>>>>> >
>>>>>>>
>>>>>>> It sounds like there is a precedent for searching in CWD. I don't
>>>>>>> know
>>>>>>> how useful it is (I traced it back to r185366, but it is not
>>>>>>> mentioned
>>>>>>> there specifically), but it is there, and I guess it's not
>>>>>>> completely
>>>>>>> nonsensical from the POV of a command line user.
>>>>>>>
>>>>>>> I guess we can just keep that there and not call it a hack (though,
>>>>>>> the
>>>>>>> fact that the searching happens inside SymbolFilePDB *is* a hack).
>>>>>>>
>>>>>>> Searching in the minidump directory would also make sense somewhat,
>>>>>>> but
>>>>>>> I expect you would need more plumbing for that to happen (and I
>>>>>>> don't
>>>>>>> know of a precedent for that).
>>>>>>>
>>>>>>> pl
>>>>>>>
>>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20181211/69b5fb11/attachment-0001.html>