knurling-rs / probe-run

Run embedded programs just like native ones
Apache License 2.0
644 stars 75 forks source link

stackdump integration for better backtraces #322

Open japaric opened 2 years ago

japaric commented 2 years ago

the set of stackdump crates provide a mechanism to dump the contents of a device's memory (stackdump-capture) and analyze those stack dumps (stackdump-trace). the trace crate, in particular, can produce stack backtraces more complete than probe-run's as they include information about function arguments (see #62)

we'd like to make use of the trace crate to improve probe-run's backtraces but we'd like to avoid copying the whole stack memory from the device to the host as that's rather slow (see --measure-stack feature in early v0.3.x), which is the mechanism provided by the capture crate.

the integration work would consist of two parts

potential integration problems

diondokter commented 2 years ago

I made a branch with some initial changes here: https://github.com/tweedegolf/stackdump/tree/probe

diondokter commented 2 years ago

So, I made an implementation that works for my CLI, see the stackdump PR above. Small problem, though, is that probe-rs doesn't support reading the FPU registers in the latest release. I made a PR to fix that here: https://github.com/probe-rs/probe-rs/pull/1122

But even if that gets merged, that won't be in an official release until probe-rs 0.13. Theoretically this update isn't absolutely required. But if you miss the fpu registers, then most/all floats will not be able to be read. What do you guys think?

japaric commented 2 years ago

@diondokter nice! re: fpu registers, I think it's fine if they are not read and stackdump-capture-probe is published with a dependency on probe-rs v0.12.x. as you mentioned unwinding does not depend on the contents of the fpu registers so whenever their contents are missing probe-run could report a ? (unknown) value for those function arguments. probe-run currently does not report the contents of any register (function arguments) in the backtrace so having the data of the general purpose registers would already be a huge improvement.

Urhengulas commented 2 years ago

So, I made an implementation that works for my CLI, see the stackdump PR above. Small problem, though, is that probe-rs doesn't support reading the FPU registers in the latest release. I made a PR to fix that here: probe-rs/probe-rs#1122

But even if that gets merged, that won't be in an official release until probe-rs 0.13. Theoretically this update isn't absolutely required. But if you miss the fpu registers, then most/all floats will not be able to be read. What do you guys think?

That is great news! 🎉 Firstly, I agree with Jorge, that having non-float arguments being reported is already a good addition which is worth integrating. Secondly, speaking from past experience, it might take some time until the next probe-rs version releases, so I think we can move on without float support and add that as soon as the next probe-rs gets released.

diondokter commented 2 years ago

I have released stackdump-capture-probe: https://crates.io/crates/stackdump-capture-probe This is the adaptor that probe-run should be able to use.

I've got my own CLI as well and this is the usage: https://github.com/tweedegolf/stackdump/blob/master/cli/src/probe.rs

Printing is done like here: https://github.com/tweedegolf/stackdump/blob/master/cli/src/main.rs#L123-L163 This has some line wrapping logic because some data can be annoyingly long.