Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression: timeout waiting for breakpoint with cortex-m-rt 0.7.1 #426

Open
astraw opened this issue Apr 2, 2022 · 8 comments
Open

regression: timeout waiting for breakpoint with cortex-m-rt 0.7.1 #426

astraw opened this issue Apr 2, 2022 · 8 comments

Comments

@astraw
Copy link

astraw commented Apr 2, 2022

(This originally started as knurling-rs/probe-run#316. I have tracked the issue down to the cortex-m-rt 0.7 branch and so am filing a new, more specific, issue here.)

I have been trying to update the nucleo-h743zi crate to use the latest cortex-m-rt and am hitting a timeout error when probe-run attempts to start and debug the board with newly flashed firmware. It does so by setting a hardware breakpoint and then running. I assume the expected breakpoint is never hit, and so probe-run does not enter the expected state and throws a timeout error.

This timeout is a regression from cortex-m-rt 0.6.15, where the issue does not happen. Using git bisect on the archived https://github.com/rust-embedded/cortex-m-rt, I discovered that the first commit with the problematic behavior is rust-embedded/cortex-m-rt@1fd84a8. Unfortunately the relevant issues at play here are outside my familiarity and it would be slow going for me to pursue this further.

@kelleyk
Copy link

kelleyk commented Apr 7, 2022

Thanks @astraw for all of the effort so far in tracking down the culprit commit!

I've run into this issue as well. I'm using Nucleo-H745ZI-Q boards, which are very similar to the Nucleo-H743ZI2-Q boards except that the STM32H745 has a Cortex-M4F core onboard (which, for the purposes of this issue, I'm not using).

I haven't had a chance to really dig in myself, but I just noticed that in the current version of link.x.in many of the changes that rust-embedded/cortex-m-rt@1fd84a8 made have been unwound.

From some quick reading around bf99836 (which did a bunch of that unwinding), I found a few PRs/issues that look like they might be relevant:

Is it possible that this is a similar problem?

@kelleyk
Copy link

kelleyk commented Apr 8, 2022

The project where I ran into this problem was based on @antoinevg's BSP for the Nucleo boards (https://github.com/antoinevg/nucleo-h7xx and https://github.com/antoinevg/hello-nucleo-h7xx); I ran into the problem when upgrading dependencies, much like what @astraw mentioned trying to do.

I just tried something simpler---I cloned a fresh copy of https://github.com/stm32-rs/stm32h7xx-hal (master is 884a50b) and tried to build and run one of the examples.

$ cargo build --features=stm32h747cm7,rt,ethernet --example blinky
$ probe-run --chip STM32H745ZITx target/thumbv7em-none-eabihf/debug/examples/blinky

... and it works! I was using probe-run v0.2.6 (the latest that supports defmt v2) and then upgraded to probe-run v0.3.3 (the latest release), which behaves identically. I also tried checking out v0.11.0 of the HAL (the latest release), and it works just fine as well.

I think that there's something about these BSP crates that is causing trouble when we try to upgrade versions in existing projects.

@kelleyk
Copy link

kelleyk commented Apr 8, 2022

I've just followed the instructions in https://github.com/knurling-rs/app-template and that works perfectly, too. One of the steps there is to copy over memory.x (the memory layout) from the HAL crate, and I notice that that is different from the version in the project that was giving me trouble. I wonder if the trouble is because I didn't re-copy that file when upgrading the version of the stm32h7xx-hal crate that I was using.

@astraw
Copy link
Author

astraw commented Apr 8, 2022

Thank you so much. This solves the local issue for my case.

Could cortex-m-rt (or some other crate) could do a check (ideally compile-time) that the memory layout is as expected and give an explicit and clear error if not?

@adamgreig
Copy link
Member

Are you able to copy/link to the old non-working and new working memory.x? As far as I can think we haven't really changed what's required in that file between versions, so I'm surprised that's somehow breaking it unless something non-standard was there.

We do already have a bunch of checks in the linker script, so without working out what exactly was going wrong in this case it's hard to know what else to check for.

@astraw
Copy link
Author

astraw commented Apr 8, 2022

astraw/nucleo-h743zi@161aac7 is the commit which finally allowed probe-run to work without error using cortex-m-rt 0.7.1. It looks like INSERT AFTER .bss; in memory.x is the key change.

Is that the information you want? (I hope so, because these linker scripts are outside of my experience.)

@kelleyk
Copy link

kelleyk commented Apr 8, 2022

Looking around the workspace that I was using to work through this issue, I see three versions of memory.x from the outside world. I did a little bit of testing, remembering to cargo clean after modifying memory.x (per the docs). I'm using probe-run@v0.3.3.

The first one is from antoinevg/nucleo-h7xx@v0.1.2 (same as current main). This one exhibits the timeout error with probe-run, and also does not build unless I disable flip-link (Error: "MEMORY.RAM not found after scanning linker scripts").

The second one is from stm32-rs/stm32h7xx-hal@v0.9.0 (same as @v0.10.0). This one causes the timeout error with probe-run, though it builds with flip-link.

The third one is from stm32-rs/stm32h7xx-hal@v0.11.0 (same as current master). This one works correctly with probe-run and builds with flip-link.

I haven't tried to narrow down which of the differences is causing the problem, though I suspect it's related to the issues/PRs that I linked above (which are when those changes were made).

I agree with @astraw that it would be nice to have a way of reminding people that the memory.x that they (or someone producing a template for them) have copied from the HAL into their own project (as https://github.com/knurling-rs/app-template instructs) might need to be updated when they update dependencies. It shouldn't need to be fancy; just a "hey, this is revision 123 of memory.x but you're using a version of the HAL that was released with revision 124!" to let people avoid this particular footgun.

Then again, there are probably more elegant ways of addressing the problem. What do you think?

@kelleyk
Copy link

kelleyk commented Apr 8, 2022

Actually looking at the diff between the versions of memory.x in stm32h7xx-hal@{v0.9.0,v0.10.0} and @v0.11.0, the removal of INSERT AFTER .bss from the SECTIONS block at the end is the only change, which lines up with what @astraw noticed above.

I suspect that there's some interaction with the changes that were made to link.x.in in the cortex-m-rt crate, though I haven't tried testing version pairs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants