Make register output configurable

[Gankra]

Our minidump-processor APIs are inconsistent with registers:

• programmatically, we restore general purpose registers for every frame
• in human output, we also print general purpose registers for every frame
• in json output, we only print the registers for the top frame of crashing_thread (and not even for the same copy of that thread in the thread list)

I think these all reasonable defaults (though the last one is purely a "what we did before, don't question it" thing. But json output shouldn't ever be strictly less powerful than --human output, so it should at least be possible to get full registers in json.

I think this should be handled by configuring what the programmatic interface produces, and the printing formats just faithfully reproducing that. This would be done by adding a new RestoreRegisters enum in ProcessorOptions. Note that generating the registers is a necessary side-effect of stackwalking, so this basically means erasing the output after stackwalking!

I propose the following options for RestoreRegisters:

• None (never emit registers)
• CrashingFrame (only emit registers for the top frame of the crashing thread) (--json's ~current behaviour)
• CrashingThread (only emit registers for the crashing thread)
• All (always emit registers) (--human's current behaviour)

RestoreRegisters::All should be the default.

This will change the output in four ways:

• anyone programmatically invoking print_json will now get full registers (good?)
• --cyborg output will have to pick one consistent RestoreRegisters mode (I propose defaulting to RestoreRegisters::All) (good?)
• --json output will now also include registers in the top-crashing-frame in the thread list (instead of just in the crashing_thread clone) (fine?)
• every stack frame in json output will now include a registers field (which will usually be null in the default --json config) (kinda sucks, but meh)

steps:

• add a #[non-exhaustive] enum to ProcessorOptions: https://github.com/luser/rust-minidump/blob/2dc7ecc384f8ed123387cace30157c5288b6b5f3/minidump-processor/src/processor.rs#L22-L25

• check that value roughly at this point in the processor: https://github.com/luser/rust-minidump/blob/2dc7ecc384f8ed123387cace30157c5288b6b5f3/minidump-processor/src/processor.rs#L263

  • if you determine registers should be omitted, set frame.context.validity = MinidumpContextValidity::Some(HashSet::new()) (mark all registers as invalid)

• change json_registers to detect an "empty" validity, and have it emit null (Option::None) in that case: https://github.com/luser/rust-minidump/blob/2dc7ecc384f8ed123387cace30157c5288b6b5f3/minidump-processor/src/process_state.rs#L317

• use json_registers to unconditionally output a registers field in every thread (and remove the hack that adds it to the crashing_thread below?): https://github.com/luser/rust-minidump/blob/2dc7ecc384f8ed123387cace30157c5288b6b5f3/minidump-processor/src/process_state.rs#L774

• add a --restore-regs=... flag to minidump-stackwalk, using the --verbose as a template: https://github.com/luser/rust-minidump/blob/2dc7ecc384f8ed123387cace30157c5288b6b5f3/minidump-stackwalk/src/main.rs#L80-L92

  • include an extra "default" option which reproduces the current inconsistent behaviour between --json and --human (--cyborg should use --human's setting)
  • document that "default" is allowed to become more verbose, but will never become less verbose (we won't suddenly stop emitting registers in places we were before)
  • see also extra-json for how we set ProcessorOptions

[Gankra]

this interface design is also desirable for shipping #263

[luser]

Note that the current design tried to strike a balance between utility and privacy concerns: obviously the registers from the exception context are incredibly valuable for investigating crashes! Register contents need to be treated as sensitive, however, since they can contain arbitrary program contents up to and including passwords and encryption keys.

As I noted in #365, the fallback approach of "load a Windows minidump in an actual debugger" is compelling enough that I had never been too worried about this sort of thing. I did do some exploratory work around building a tool that could load a minidump and speak the GDB remote protocol, with the desired end goal of being able to debug Linux minidumps in GDB and macOS minidumps in LLDB, but it was a bunch of work and it stalled out. In the intervening years other folks have produced more full-featured GDB remote protocol implementations in Rust, it's probably worth revisiting.

[Gankra]

See also: https://bugzilla.mozilla.org/show_bug.cgi?id=1745993

[Gankra]

I suppose cc @willkg since this is relevant to things you are thinking about right now

[willkg]

It is! I wrote up bug 1745993 covering my "omg! thar' be exciting developments and changes and features and enhancements in the stackwalker and it'll no longer be standing still like a beached ship filled with shrieking eels!" concerns.