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

QEMU debugging updates #239

Merged
merged 2 commits into from
May 24, 2020
Merged

QEMU debugging updates #239

merged 2 commits into from
May 24, 2020

Conversation

blelem
Copy link

@blelem blelem commented May 17, 2020

Addressing a few issues:
Fixes #237, Fixes #234, Fixes #217

The biggest issue this PR is addressing is correcting the "debugging QEMU with GDB" section that really didn't work with latest ARM toolchain (on my setup), because of two things:

  • gdb didn't map the rust libcore functions properly. For example, after connecting to a QEMU instance, it did not show the current PC to be on the Reset function, but some random method.
  • gdb didn't step over (next), after breaking at main. It just ran to the end of the program.

The user is now instructed to disable the LDD and use the GNU ARM linker instead, which addresses the issues with the mapping.

And instead of doing a simple, break main which did not allow the user to step over after hitting the breakpoint, the user is now guided to do a more complicated,
break hello::__cortex_m_rt_main. After hitting that breakpoint, the next and step commands are working as one would expect.

I am no expert in embedded rust, so the proposed solution may not be optimal, but I think they are better than the current instructions provided by the book which are not working at all on my setup. (Windows 10, with latest versions of QEMU, ARM toolchain, Rust)

@blelem blelem requested a review from a team as a code owner May 17, 2020 07:51
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @thejpster (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-resources labels May 17, 2020
@blelem blelem mentioned this pull request May 17, 2020
...
rustflags = [
# LLD (shipped with the Rust toolchain) is used as the default linker
#"-C", "link-arg=-Tlink.x", <--- Comment this line
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still required to have link-arg=-Tlink.x, otherwise the linker script isn't used at all. The user would have to just add linker=arm-none-eabi-ld while keeping link-arg=-Tlink.x.

That said, I'm not sure why changing linker helps in this case anyway, which might be worth a little more investigation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen a handful of bugs where LLD messes up debuginfo sections, but I don't think we should recommend GNU LD by default because of that. LLD is the default Rust linker for thumb targets.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how much the debug info is messed up. The application compiled with LLD works, stepping through the code with gdb works, at least for that minimalist hello world.
So perhaps it's enough for the time being to just signal in the book the bug with smtg like:

once connected to the remote qemu, gdb should indicate that it is paused at the reset() function, waiting for your input.
Reset () at $REGISTRY/cortex-m-rt-0.6.1/src/lib.rs:473

However in some setup, gdb may instead emit some warnings like : core::num::bignum::Big32x40::mul_small () at src/libcore/num/bignum.rs:254 254 src/libcore/num/bignum.rs: No such file or directory.
You can safely ignore those warnings.

I can file a bug for that linker issue, I would appreciate your help if you can suggest where the bug should be filed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR, removing the paragraph recommanding to switch to the GNU LD.

I added some text hinting at the possible debuginfo mix-up glitch, and that it can be safely ignored.

@@ -488,11 +515,11 @@ This reset handler will eventually call our main function. Let's skip all the
way there using a breakpoint and the `continue` command:

```console
break main
break hello::__cortex_m_rt_main
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, the name __cortex_m_rt_main is an implementation detail of cortex-m-rt and probably shouldn't be relied on for this. I'm not sure if there's any other symbol we could use for a breakpoint, though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If exposing that implementation detail is not desired, I believe this slightly more verbose version would work well for first time users:

list main

This will show the source code, from the file examples/hello.rs

6       extern crate panic_halt;
7
8       use cortex_m_rt::entry;
9       use cortex_m_semihosting::{debug, hprintln};
10
11      #[entry]
12      fn main() -> ! {
13          hprintln!("Hello, world!").unwrap();
14
15          // exit QEMU

We now would like to add a breakpoint just before the "Hello, world!", which is on line 13. We do that with the break command:

break 13

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR, replacing setting up a breakpoint at the main symbol with a gentle "set your breakpoint on line 13". I think this way of doing will work well for people following this text.

@adamgreig
Copy link
Member

Thanks for this PR! Generally your changes look good but I want to be sure of what's going on before merging the recommendation to use GCC LD or that specific breakpoint name. Others from @rust-embedded/cortex-m might have a better idea...

Copy link
Member

@adamgreig adamgreig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update!

bors merge

@bors
Copy link
Contributor

bors bot commented May 24, 2020

Build succeeded:

@bors bors bot merged commit 1e6247e into rust-embedded:master May 24, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 26, 2020
Update books

## reference

7 commits in 892b928b565e35d25b6f9c47faee03b94bc41489..becdca9477c9eafa96a4eea5156fe7a2730d9dd2
2020-05-11 11:13:51 -0700 to 2020-05-21 21:08:02 +0100
- Update tuple index token. (rust-lang/reference#814)
- Fixes minor errors (rust-lang/reference#818)
- Update that macros can be deprecated. (rust-lang/reference#813)
- work on char/str descriptions (rust-lang/reference#809)
- cfg_attr needs a valid predicate (rust-lang/reference#812)
- Account for removal of UB in float-to-int casts (rust-lang/reference#810)
- Fix stray plus signs. (rust-lang/reference#811)

## book

6 commits in 6247be15a7f7509559f7981ee2209b9e0cc121df..e8a4714a9d8a6136a59b8e63544e149683876e36
2020-05-03 10:55:09 -0500 to 2020-05-25 10:29:27 -0500
- code is 1024 now, not 512
- Clean up install a bit
- We don't need build.sh anymore
- Fix CI status in README
- Port to github actions (rust-lang/book#2337)
- operating system -&gt; allocator

## rust-by-example

5 commits in ab072b14393cbd9e8a1d1d75879bf51e27217bbb..7aa82129aa23e7e181efbeb8da03a2a897ef6afc
2020-05-09 08:46:39 -0300 to 2020-05-25 14:54:26 -0300
- Person of age 0 is alive (rust-lang/rust-by-example#1348)
- Gramatical fix in std/rc.md (rust-lang/rust-by-example#1347)
- Capture example should use String (rust-lang/rust-by-example#1331)
- Fix empty bound examples (rust-lang/rust-by-example#1343)
- Fix an inline comment in macros/repeat.md (rust-lang/rust-by-example#1344)

## edition-guide

1 commits in 49270740c7a4bff2763e6bc730b191d45b7d5167..0a8ab5046829733eb03df0738c4fafaa9b36b348
2020-05-11 08:50:29 -0500 to 2020-05-18 08:34:23 -0500
- Changes for Rust 1.32 & setup for edition-next (rust-lang/edition-guide#213)

## embedded-book

3 commits in 366c50a03bed928589771eba8a6f18e0c0c01d23..5555a97f04ad7974ac6fb8fb47c267c4274adf4a
2020-05-07 09:04:42 +0000 to 2020-05-25 18:00:51 +0000
- Remove reference to const-fn feature of cortex-m. Closes rust-embedded/book#242.  (rust-embedded/book#243)
- Spelling: Appplication -&gt; Application  (rust-embedded/book#241)
- QEMU debugging updates  (rust-embedded/book#239)
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 26, 2020
Update books

## reference

7 commits in 892b928b565e35d25b6f9c47faee03b94bc41489..becdca9477c9eafa96a4eea5156fe7a2730d9dd2
2020-05-11 11:13:51 -0700 to 2020-05-21 21:08:02 +0100
- Update tuple index token. (rust-lang/reference#814)
- Fixes minor errors (rust-lang/reference#818)
- Update that macros can be deprecated. (rust-lang/reference#813)
- work on char/str descriptions (rust-lang/reference#809)
- cfg_attr needs a valid predicate (rust-lang/reference#812)
- Account for removal of UB in float-to-int casts (rust-lang/reference#810)
- Fix stray plus signs. (rust-lang/reference#811)

## book

6 commits in 6247be15a7f7509559f7981ee2209b9e0cc121df..e8a4714a9d8a6136a59b8e63544e149683876e36
2020-05-03 10:55:09 -0500 to 2020-05-25 10:29:27 -0500
- code is 1024 now, not 512
- Clean up install a bit
- We don't need build.sh anymore
- Fix CI status in README
- Port to github actions (rust-lang/book#2337)
- operating system -&gt; allocator

## rust-by-example

5 commits in ab072b14393cbd9e8a1d1d75879bf51e27217bbb..7aa82129aa23e7e181efbeb8da03a2a897ef6afc
2020-05-09 08:46:39 -0300 to 2020-05-25 14:54:26 -0300
- Person of age 0 is alive (rust-lang/rust-by-example#1348)
- Gramatical fix in std/rc.md (rust-lang/rust-by-example#1347)
- Capture example should use String (rust-lang/rust-by-example#1331)
- Fix empty bound examples (rust-lang/rust-by-example#1343)
- Fix an inline comment in macros/repeat.md (rust-lang/rust-by-example#1344)

## edition-guide

1 commits in 49270740c7a4bff2763e6bc730b191d45b7d5167..0a8ab5046829733eb03df0738c4fafaa9b36b348
2020-05-11 08:50:29 -0500 to 2020-05-18 08:34:23 -0500
- Changes for Rust 1.32 & setup for edition-next (rust-lang/edition-guide#213)

## embedded-book

3 commits in 366c50a03bed928589771eba8a6f18e0c0c01d23..5555a97f04ad7974ac6fb8fb47c267c4274adf4a
2020-05-07 09:04:42 +0000 to 2020-05-25 18:00:51 +0000
- Remove reference to const-fn feature of cortex-m. Closes rust-embedded/book#242.  (rust-embedded/book#243)
- Spelling: Appplication -&gt; Application  (rust-embedded/book#241)
- QEMU debugging updates  (rust-embedded/book#239)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-resources
Projects
None yet
6 participants