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

Do not try to attach to RTT if the elf doesn't advertise using RTT #1865

Closed
ia0 opened this issue Nov 7, 2023 · 9 comments · Fixed by #1919
Closed

Do not try to attach to RTT if the elf doesn't advertise using RTT #1865

ia0 opened this issue Nov 7, 2023 · 9 comments · Fixed by #1919
Labels
enhancement New feature or request

Comments

@ia0
Copy link
Contributor

ia0 commented Nov 7, 2023

Is your feature request related to a problem? Please describe.
As described in more details in #1863, if I compile my program without defmt-rtt then probe-rs will try for an excessively long time to connect to the RTT, while probe-run used to check the binary to see if it was compiled with RTT support.

Describe the solution you'd like
I'd like probe-rs to also check the binary to guess whether RTT should be used or not (and when not, just look until the device halts, e.g. with a semi-hosting command).

Describe alternatives you've considered
I've considered using defmt-rtt for my production binaries, but I don't like that too much.

@ia0 ia0 added the enhancement New feature or request label Nov 7, 2023
ia0 added a commit to google/wasefire that referenced this issue Nov 7, 2023
This was broken by #246 because `probe-rs` doesn't consider normal
breakpoints as semi-hosting exit command.

Note that `./scripts/hwci.sh nordic` is currently slow with `--release`
because of probe-rs/probe-rs#1865
@Yatekii
Copy link
Member

Yatekii commented Nov 7, 2023

probe-rs run checks the binary just like probe-run. The only difference is that we scan the memory for 10 seconds if we can't find the SEGGER_RTT block because that's a valid usecase of RTT in the original standard, if you do not have any debug symbols.

I am happy to change this behavior or at least enable this only via a flag, because I think most people have ELF debug info if RTT is enabled.

@ia0
Copy link
Contributor Author

ia0 commented Nov 7, 2023

probe-rs run checks the binary just like probe-run.

What is checked exactly? probe-run checks the ELF, not the runtime memory.

I think most people have ELF debug info if RTT is enabled.

Exactly, so it would make sense to not try to connect to RTT if the ELF doesn't have debug symbols. That would be a first simple check. This check could be further improved by checking for an _SEGGER_RTT symbol in the ELF like probe-run does, but I don't need this and would be fine if it's not implemented.

@Yatekii
Copy link
Member

Yatekii commented Nov 7, 2023

We check the ELF symbols :) That's something we did long before probe-run. We are just compatible with the Segger RTT standard while they went a more opinionated route.

How our RTT impl goes about this is like so:

  1. Check the ELF for the SEGGER_RTT symbol.
  2. If it's not there, scan the live chip memory if we can maybe find the RTT header.

We could make step 2 optional. Or we deprecate it alltogether in probe-rs run.

@ia0
Copy link
Contributor Author

ia0 commented Nov 7, 2023

Oh I see, so the only difference is that probe-run checks for _SEGGER_RTT which is what defmt produces, while probe-rs checks for SEGGER_RTT. I see a few options to solve my problem then:

  1. Make defmt-rtt produce SEGGER_RTT (either in addition to or instead of _SEGGER_RTT) based on a feature to preserve backward compatibility.
  2. Make probe-rs also check for _SEGGER_RTT in additional to SEGGER_RTT, possibly gated by a feature but this doesn't seem necessary.
  3. Make probe-rs skip step 2 either based on a flag and/or based on whether the presence of debugging symbols in the ELF.

@Yatekii
Copy link
Member

Yatekii commented Nov 7, 2023

Sorry, I was imprecise in my answer, the header symbol value in memory is SEGGER_RTT, the DWARF symbol in the ELF is _SEGGER_RTT. You can find the code here.

"_SEGGER_RTT" => {

The only real fix here is 3. Because you compile in release mode - seemingly without debug symbols (you could ofc. generate them :)) - we cannot find the symbol and we start looking in RAM. Which doesn't have the header either (because you don't initialize it in release?) or it cannot find it within 10 secs. Dependent on memory size and where you start the scan this will take forever to find :D Maybe we should just patch this in the RTT code so there is a connect function that does not perform the scan if no symbol is found and use this. But maybe the flag would actually be a great choice so people that want to run without the debug info can still use it.

@ia0
Copy link
Contributor Author

ia0 commented Nov 7, 2023

I see, indeed only (3) is an option.

I tried generating debug symbols but same issue. It looks to me that probe-rs doesn't check whether there are symbols but _SEGGER_RTT is absent as a signal that it should not scan the memory.

Maybe we should just patch this in the RTT code so there is a connect function that does not perform the scan if no symbol is found and use this.

Yes that would be the behavior of probe-run as far as I understand. What do you mean by "patch this in the RTT code"?

But maybe the flag would actually be a great choice so people that want to run without the debug info can still use it.

Yes, I don't mind setting a flag to only rely on the debug symbols (my probe-rs command line is generated anyway so it has no cost on my side and will always be set). I never want to scan the memory, I always set the symbol when I use RTT.

@Yatekii
Copy link
Member

Yatekii commented Nov 7, 2023

It looks to me that probe-rs doesn't check whether there are symbols but _SEGGER_RTT is absent as a signal that it should not scan the memory.

That is correct.

Yes that would be the behavior of probe-run as far as I understand.

Yup.

What do you mean by "patch this in the RTT code"?

I mean in the rtt module in probe-rs we could add this :)

Yes, I don't mind setting a flag to only rely on the debug symbols

I would make it so it needs to flag for the additional memory scan and otherwise just scans the ELF.

@ia0
Copy link
Contributor Author

ia0 commented Nov 7, 2023

Perfect, thanks a lot!

@Yatekii
Copy link
Member

Yatekii commented Nov 7, 2023

YW!

kofls pushed a commit to kofls/wasefire that referenced this issue Nov 27, 2023
This was broken by google#246 because `probe-rs` doesn't consider normal
breakpoints as semi-hosting exit command.

Note that `./scripts/hwci.sh nordic` is currently slow with `--release`
because of probe-rs/probe-rs#1865
ia0 added a commit to ia0/probe-rs that referenced this issue Dec 1, 2023
This changes the default of scanning to not scanning. This should hopefully not
impact many users since most tools define a `_SEGGER_RTT` symbol in the ELF, in
which case only that particular address is checked.

Fixes probe-rs#1865
ia0 added a commit to ia0/probe-rs that referenced this issue Dec 1, 2023
This changes the default of scanning to not scanning. This should hopefully not
impact many users since most tools define a `_SEGGER_RTT` symbol in the ELF, in
which case only that particular address is checked.

Fixes probe-rs#1865
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants