Implement --no-reset mode for live attach to a running device #414

whitequark · 2023-06-21T02:00:28Z

Fixes #354. Includes some related cleanups.

In my testing this works quite well, including backtraces. The one thing I haven't tested is really flooding defmt with messages, but it probably works fine.

Read the vector table (instead of looking at current PC/SP).

Urhengulas

Looks great. I added a few suggestions and questions.

PS. you are allowed to disagree with the suggestions.

src/backtrace/unwind.rs

src/cli.rs

src/main.rs

Urhengulas · 2023-06-22T10:58:13Z

src/backtrace/mod.rs

@@ -130,7 +130,7 @@ impl Outcome {
            Outcome::StackOverflow => log::error!("the program has overflowed its stack"),
            Outcome::HardFault => log::error!("the program panicked"),
            Outcome::Ok => log::info!("device halted without error"),
-            Outcome::CtrlC => log::info!("device halted by user"),
+            Outcome::CtrlC => log::info!("interrupted by user"),


Suggested change

Outcome::CtrlC => log::info!("interrupted by user"),

Outcome::CtrlC => log::info!("device interrupted by user"),

I changed it to the current message because it's the probe that was interrupted, not the device -- the device keeps running if you do --no-reset.

Maybe we can actually have two different messages then?

Suggested change

Outcome::CtrlC => log::info!("interrupted by user"),

Outcome::CtrlC if opts.no_reset == true => log::info!("interrupted by user; program keeps running"),

Outcome::CtrlC if opts.no_reset == false => log::info!("device halted by by user"),

I am not sure if opts is in scope here.

It's not, but we can just add it as an argument:

fn log(&self, no_reset: bool)

Urhengulas · 2023-06-22T11:56:59Z

src/cortexm.rs

    pub initial_stack_pointer: u32,
+    // entry 1: Reset vector
+    pub reset: u32,


There was some discussion if we can extract the reset vector from the elf or not and how to make this as stable as possible. Please see #383 (comment). Do you think we can always™️ extract the reset vector the way you implemented it?

From the linked discussion:

It could be that the Flash memory at 0x2000_0000 is aliased at address 0x0000_0000 but I have not checked.

I think that's a typo: the Flash memory is (on STM32s for example; in principe it could be anywhere the instruction fetches are legal) at 0x0800_0000 and the SRAM is (always) at 0x2000_0000.

However, that aside, I looked it up in the ARMv7-M ARM and it seems that I misremembered some details and a much more reliable way would be to read the IVT address from the VTOR register on attach and then use that. I can implement that later.

I can implement that later.

That would be nice. Maybe in a follow up PR.

In any case I we will need to test the current PR with the rp pico, because that one seems to behave a bit differently than many other microcontrollers because of the bootloader.

I can do the testing. Just need to set it up.

whitequark · 2023-06-22T17:26:55Z

src/main.rs

    }

    // run program and print logs until there is an exception
-    start_program(core, elf)?;
+    attach_to_program(core, elf)?;


Gotta update it here too:

Suggested change

attach_to_program(core, elf)?;

attack_the_program(core, elf)?;

Oh no 😆 I am sorry. This was meant to be a joke 🙈 Please change it back to attach_to_program 🤣

Co-authored-by: Johann Hemmann <[email protected]>

Urhengulas · 2023-07-18T11:00:39Z

@whitequark What is the status? 😄

whitequark · 2023-07-18T11:22:11Z

@Urhengulas I've been taking care of my health. ETA is this weekend.

Urhengulas · 2023-07-19T10:06:04Z

@Urhengulas I've been taking care of my health

That is very good! 🍀
Sorry, I did not want to pressure you

Urhengulas · 2023-09-27T15:10:27Z

@whitequark friendly ping 🛎️

whitequark · 2023-09-27T16:00:12Z

I'm doing way better! I think I'll do this Saturday.

Urhengulas · 2023-09-28T12:15:20Z

I'm doing way better! I think I'll do this Saturday.

That's great to hear!

whitequark · 2023-11-28T10:07:16Z

Sorry, I don't think I'll find time to work on this any time soon as I've not had a chance to use probe-rs in months. Feel free to do with the PR whatever you like.

whitequark

It looks like some of my responses never went through.

whitequark · 2023-06-22T17:27:48Z

src/main.rs

+    log::debug!("detaching from program");
+
+    if let Some(rtt_buffer_address) = elf.rtt_buffer_address() {
+        set_rtt_blocking_mode(core, rtt_buffer_address, false)?;


Oh yeah this is true.

whitequark · 2023-06-22T17:28:30Z

src/main.rs

+        set_rtt_blocking_mode(core, rtt_buffer_address, false)?;
+    }
+
+    core.clear_hw_breakpoint(cortexm::clear_thumb_bit(elf.vector_table.hard_fault).into())?;


It's some code I moved around--I didn't introduce it. For some reason (I didn't investigate closely) probe-run sets a breakpoint on HardFault. Probably to display a nicer error if you hit one?

whitequark · 2023-06-23T12:43:09Z

src/main.rs


    match (core.available_breakpoint_units()?, elf.rtt_buffer_address()) {
        (0, Some(_)) => bail!("RTT not supported on device without HW breakpoints"),
        (0, None) => log::warn!("device doesn't support HW breakpoints; HardFault will NOT make `probe-run` exit with an error code"),
-        (_, Some(rtt_buffer_address)) => set_rtt_to_blocking(core, elf.main_fn_address(), rtt_buffer_address)?,
+        (_, Some(rtt_buffer_address)) =>
+            set_rtt_blocking_mode(core, rtt_buffer_address, true)?,


It looks like #[pre_init] specifies any access to memory as UB, so it's not really "life before main".

whitequark added 3 commits June 21, 2023 01:58

backtrace: enhance debug logging for failure cases.

f285238

Make analyze_vector_table() do what it says on the tin.

8e718ef

Read the vector table (instead of looking at current PC/SP).

Remove analyze_vector_table() as the ELF file has that information.

f01365b

whitequark mentioned this pull request Jun 21, 2023

Support for viewing defmt RTT logs without resetting device #354

Open

Implement --no-reset mode for live attach to a running device.

39a470b

whitequark force-pushed the live-attach branch from a2af87a to 39a470b Compare June 21, 2023 02:04

Urhengulas suggested changes Jun 22, 2023

View reviewed changes

Urhengulas reviewed Jun 22, 2023

View reviewed changes

whitequark commented Jun 22, 2023

View reviewed changes

Apply suggestions from code review

c867264

Co-authored-by: Johann Hemmann <[email protected]>

Urhengulas added 2 commits July 18, 2023 13:02

Merge branch 'main' into live-attach

14ef801

Fix main.rs

5b423b1

whitequark commented Nov 28, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement --no-reset mode for live attach to a running device #414

Implement --no-reset mode for live attach to a running device #414

whitequark commented Jun 21, 2023

Urhengulas left a comment

Urhengulas Jun 22, 2023

whitequark Jun 22, 2023

Urhengulas Jun 23, 2023 •

edited

Loading

Urhengulas Jun 23, 2023

Urhengulas Jun 22, 2023

whitequark Jun 22, 2023

Urhengulas Jun 23, 2023

Urhengulas Jun 23, 2023

whitequark Jun 22, 2023

Urhengulas Jun 23, 2023

Urhengulas commented Jul 18, 2023

whitequark commented Jul 18, 2023

Urhengulas commented Jul 19, 2023

Urhengulas commented Sep 27, 2023

whitequark commented Sep 27, 2023

Urhengulas commented Sep 28, 2023

whitequark commented Nov 28, 2023

whitequark left a comment

whitequark Jun 22, 2023

whitequark Jun 22, 2023

whitequark Jun 23, 2023

	Outcome::CtrlC => log::info!("interrupted by user"),
	Outcome::CtrlC => log::info!("device interrupted by user"),

	Outcome::CtrlC => log::info!("interrupted by user"),
	Outcome::CtrlC if opts.no_reset == true => log::info!("interrupted by user; program keeps running"),
	Outcome::CtrlC if opts.no_reset == false => log::info!("device halted by by user"),

	attach_to_program(core, elf)?;
	attack_the_program(core, elf)?;

Implement --no-reset mode for live attach to a running device #414

Are you sure you want to change the base?

Implement --no-reset mode for live attach to a running device #414

Conversation

whitequark commented Jun 21, 2023

Urhengulas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Urhengulas Jun 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Urhengulas commented Jul 18, 2023

whitequark commented Jul 18, 2023

Urhengulas commented Jul 19, 2023

Urhengulas commented Sep 27, 2023

whitequark commented Sep 27, 2023

Urhengulas commented Sep 28, 2023

whitequark commented Nov 28, 2023

whitequark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Urhengulas Jun 23, 2023 •

edited

Loading