- User Since
- Sep 6 2017, 12:48 PM (66 w, 3 d)
Fri, Dec 14
I love the idea of custom dump commands, thanks Greg.
Wed, Dec 12
Mon, Dec 10
How large is the PDB file here?
Fri, Dec 7
Incorporating feedback + adding a test case
Thu, Dec 6
Wed, Dec 5
Tue, Dec 4
Fri, Nov 30
Thu, Nov 29
Wed, Nov 28
I noticed a small problem, this change breaks "lldb -c <corefile>". The inline comment explains the root cause.
Nov 12 2018
Nov 7 2018
Shouldn't we also handle the general case where raw size < virtual size? (which would naturally subsume this fix)
Nov 5 2018
Cool. Looking forward to the LLDB integration!
Greg, what do you have in mind as the next step? LLDB integration?
Nov 2 2018
Using a new, smaller test pdb (Stripped.pdb)
Nov 1 2018
Thanks Zach for your assistance with this!
Incorporating CR feedback
Oct 26 2018
Oct 25 2018
looks good. a few comments inline.
Oct 24 2018
drive by CR notes:
- does this handle forwarding imports? (it doesn't seem to from a quick glance at the code)
- can you please add, or extend the existing test to cover the transitive case? A simple dag would suffice (ex. make both dllA and dllB implicitly import dllC)
Oct 22 2018
Looks good to me, minor notes inline.
+Eric Christopher for a DWARF expert's perspective
Oct 9 2018
Yay! Nice to see this is coming along, thanks Zach!
Sep 20 2018
Hi Greg, looking at request_evaluate() I noticed that it will evaluate the
string as a lldb command if prefixed by ` .
Sep 4 2018
Aug 29 2018
Looks good (with one inline request for a comment)
I'm curious too: where did the PDB70 age create matching problems?
Aug 23 2018
Added the comment requested by zturner
It's an interesting idea, thanks! I don't object moving code around if
there's a strong case for it, but I'd like to keep the fix small and simple
for now, but it's worth considering if the current minidump loading path
will need more flexibility.
That's not changing the module list, that's changing the loaded section information. It is the dynamic loader's job to figure out what got loaded where, plus your stack trace show this happening after we've selected a plugin, not in the process of negotiating for the plugin. Clearing the section load list before setting to work seems like an appropriate thing for a selected plugin to do.
Dynamic loaders are needed for loading breakpad minidumps that are for MacOSX and iOS processes. They should also be needed for loading any minidumps that have stack traces.
Aug 22 2018
Aug 8 2018
The LLDB MI plugin didn't work very well as was quite flaky when I tested
it a while back.
Aug 7 2018
Really cool! Are you planning to add some documentation for it? (set up
Aug 6 2018
Updated the LIT file too
Incorporating feedback, thanks.
Aug 3 2018
Thanks Zach. I can't find llvm::contains() though, any pointers to it?
The new test cases did catch a real bug:
Jul 31 2018
Thanks Greg, looks good to me (a couple of inline comments left at your discretion)
Jul 30 2018
FYI, Breakpad & Crashpad will start generating the Microsoft flavor of ARM
Jul 27 2018
I never *ran LLDB tests*, not sure where they are and what they are.
Jul 25 2018
Jul 24 2018
Looks really good, a few comments inline.
The problem is that shared libraries differ on these machines and
LLDB either fails to load some libraries *or loads wrong ones*.
Jul 17 2018
Great timing! ARM support would be most welcome. Are you planning to
support the Breakpad flavor or ARM minidumps or the Microsoft one? (Mark
Mentovai just reminded me that the ARM support was added independently and
some of the structures are different)
Jul 16 2018
The problem is not returning an error from Minidump::Create() - if that was
the case this could easily be improved indeed. The two-phase initialization
is a consequence of the LLDB plugin lookup:
Jul 12 2018
Thanks Adrian for the thorough review.
Jul 11 2018
Adding a few ill-formed minidumps for testing the new checks
Regarding test for the other checks, I'll try to fabricate a few invalid minidumps (although it would obviously provide limited coverage)
Incorporating CR feedback
Jun 22 2018
I'm not sure if there is a suitable place for that function. This is
needed in "ObjectFileMachO" and two dynamic loader plugins.
However, during parsing you need to know the meaning of a "0000...0" UUID.
In a MachO file (at least based on the comments in the code) this value is
used to denote the fact that the object file has no UUID. For elf, a
"000..0" build-id is a perfectly valid identifier (and the lack of a
build-id is denoted by the absence of the build-id section). The extra
constructor argument is my way of trying to support both scenarios. The
other possibility I see is to have a some kind of a factory function for
one of the options (or both). I don't really have a preference between the
The slight complication here is that
some clients (MachO) actually use the all-zero notation to mean "no UUID
has been set". To keep this use case working, I have introduced an
additional argument to the UUID constructor, which specifies whether an
all-zero vector should be considered a valid UUID. For the usages where
the UUID data comes from a MachO file, I set this argument to false.
Jun 20 2018
Jun 13 2018
The intention in the scope of this change is just to check that the new
overload is exposed correctly through the Python API.
Jun 11 2018
Ah I see. That's because the last argument is a C++ default argument. It
looks like the convention in this file is that the error argument should be
the last non-defaulted argument.
spaces removed :)