Page MenuHomePhabricator

Add "dump" command as a custom "process plugin" subcommand when ProcessMinidump is used.
ClosedPublic

Authored by clayborg on Dec 14 2018, 3:15 PM.

Details

Summary

Each process plug-in can create its own custom commands. I figured it would be nice to be able to dump things from the minidump file from the lldb command line, so I added the start of the some custom commands.

Currently you can dump:

  • minidump stream directory
  • all linux specifc streams, most of which are strings
  • each linux stream individually if desired, or all with --linux

The idea is we can expand the command set to dump more things, search for data in the core file, and much more. This patch gets us started.

Diff Detail

Event Timeline

clayborg created this revision.Dec 14 2018, 3:15 PM

This sounds like a good use-case for a lit / FileCheck test. Could you add some simple tests that run lldb with -c on a static minidump with known information inside that we don't change, then run these commands and check the output?

This sounds like a good use-case for a lit / FileCheck test. Could you add some simple tests that run lldb with -c on a static minidump with known information inside that we don't change, then run these commands and check the output?

Yep, I will custom produce some output. I have a minidump generator if you ever need something generated for a test.

lemo accepted this revision.Dec 14 2018, 5:35 PM

I love the idea of custom dump commands, thanks Greg.

The changes look good to me (I agree with Zach suggestion to add lit tests)

source/Plugins/Process/minidump/MinidumpParser.cpp
706

use a more explicit text, ex "unknown stream type" ?

source/Plugins/Process/minidump/ProcessMinidump.cpp
495

I'm not a fan of "embedded" macro definitions - it may suggests that they are scope bound why they are not. I'd move them above the class definition.

This revision is now accepted and ready to land.Dec 14 2018, 5:35 PM

+1 for tests. Other than that, the code seems fairly straight-forward, though it could be brought closer to llvm style (e.g., by using more StringRefs in favour of raw c strings).

source/Plugins/Process/minidump/MinidumpParser.cpp
709–714

Why not just have an accessor which returns (a const reference to) the list of streams. Then the users can iterate over this any way they see fit (and range-based loops are easier to read than lambda callbacks).

source/Plugins/Process/minidump/MinidumpParser.h
93

Return llvm::StringRef here.

96–99

inconsistent spelling of const references (const T & vs T const &). I think the first version is more prevalent in both lldb and llvm.

source/Plugins/Process/minidump/ProcessMinidump.cpp
561

static_cast

clayborg updated this revision to Diff 178549.Dec 17 2018, 3:49 PM

Fixed all requested changes. Added a complete test in lit.

clayborg marked 6 inline comments as done.Dec 17 2018, 3:50 PM
This revision was automatically updated to reflect the committed changes.