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

[Zachary Turner via lldb-commits]

Well, Visual Studio also supports remote debugging, and searching next to
the .dmp is just one of several places it looks.  And LLDB also supports
local debugging, and so looking next to the .dmp file, being consistent
with Visual Studio, will be the expected behavior for people using LLDB in
local debugging scenarios, which is one of the supported use cases.

That said, I'm also fine with the latest patch.

On Thu, Dec 13, 2018 at 10:32 AM Leonard Mosescu <mosescu at google.com> wrote:

> What's the consensus?
>
> Personally I think that, even considering the potential issue that Paval
> pointed out, the "target symbols add ..." is the most conservative approach
> in terms of introducing new behavior. I'm fine with the current directory
> lookup as well (the original change) since it's consistent with DWARF
> lookup.
> (the only choice I'm less excited about is the implicit lookup next to the
> .dmp or .dll/.exe - it's highly specific to a local debugging scenario
> which may be appropriate for something like an IDE, ex. VisualStudio but
> not for a general purpose debugger)
>
> So my preference is for the latest patch - what does everyone else think?
>
> On Thu, Dec 13, 2018 at 4:03 AM Zachary Turner <zturner at google.com> wrote:
>
>> On irc earlier i was talking about this with Greg and he said it should
>> be fine in his opinion. I’ll point him to this review in the morning so he
>> can comment
>> On Thu, Dec 13, 2018 at 3:30 AM Pavel Labath via Phabricator <
>> reviews at reviews.llvm.org> wrote:
>>
>>> labath added inline comments.
>>>
>>>
>>> ================
>>> Comment at: source/Commands/CommandObjectTarget.cpp:4246
>>>            if (symbol_file) {
>>> -            ObjectFile *object_file = symbol_file->GetObjectFile();
>>>
>>> ----------------
>>> lemo wrote:
>>> > 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?
>>> I not sure this is a good idea. Isn't this the only way of providing
>>> feedback about whether the symbols were actually added? If we are unable to
>>> load the symbol file specified (perhaps because the user made a typo, or
>>> the file is corrupted), then the symbol vendor will just create a default
>>> SymbolFile backed by the original object file. Doesn't that mean this will
>>> basically always return true now?
>>>
>>> I think this is strictly worse that the previous solution as it lets the
>>> objectless-symbol-file hack leak out of SymbolFilePDB.
>>>
>>>
>>> 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/20181213/67297263/attachment.html>