- User Since
- Sep 6 2017, 12:48 PM (45 w, 2 d)
Tue, Jul 17
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)
Mon, Jul 16
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:
Thu, Jul 12
Thanks Adrian for the thorough review.
Wed, Jul 11
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
Fri, Jun 22
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 :)
remove space before (
Adding test cases for the new overload.
Jun 8 2018
I agree, checked in binaries are not always pretty. But some coverage
depends checked in binaries (or at very least is dramatically harder to get
the same thing from source)
Doesn't the LIT based test drop the split-function case (originally
produced with PGO)?
May 2 2018
May 1 2018
Thanks for the feedback. I uploaded a new revision (incorporating some of the feedback, including an ELF test case)
Apr 30 2018
Apr 24 2018
Hi Erik, the review is still marked as requiring changes. Once that is
sorted out I'd be happy to submit this on your behalf (what is the base SVN
revision for the latest patch?)
Apr 19 2018
It looks like nobody except me is worried about the
module-without-an-object-file situation, so I guess we can try this out and
see how it goes.
Apr 18 2018
Greg/Pavel, does the latest revision look good to you? Thanks!
Apr 17 2018
Thanks for the feedback.
Apr 16 2018
Oct 23 2017
Oct 5 2017
Updating the original changes to handle reentrant calls to CommandInterpreter::IOHandlerInputComplete().
Oct 4 2017
Changed how CMAKE_BUILD_TYPE is tested to ensure we default to LLDB_CONFIGURATION_DEBUG.
Good point, I wouldn't be surprised if that was the case at some point, but right now the main llvm CMakeLists.txt has this:
Oct 3 2017
Sep 20 2017
- Added SB APIs (SBCommandInterpreter::WasInterrupted()). This allows python commands to query for interrupt requests.
Sep 19 2017
Ping? Are there any more open questions or unresolved comments or is this good to be checked in?
Sep 18 2017
Fixed the FreeBSD/Windows break : the intention was to keep Process::WillResume() and Process::DoResume() "in-sync", but this had the unfortunate consequence of breaking Process sub-classes which don't override WillResume().
Sep 15 2017
Incorporating CR feedback.
Split the SIGINT handles fixes into a stanalone patch
Sep 13 2017
@Jim: Good point, I created this bug to track this as you suggested:
Bug 34594 - Revisit the failure path for Process::Resume() to avoid leaking Stopped->Running->Stopped events
Adding test coverage
Sep 12 2017
Revised patch based on the review feedback: I looked into updating the running state after everything else succeeds, but it proved a bit awkward since 1) TrySetRunning() can fail and 2) if we check the running state then use SetRunning() instead of TrySetRunning() it opens the possibility for a race.
Sep 8 2017
Thanks everyone for the feedback.
Sep 7 2017
Thanks everyone. I don't have commit permissions, can someone please commit this on my behalf?