-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
[CI] Run a thumbv7m-none-eabi
binary using qemu-system-arm
[IRR-2018-embedded]
#53996
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
r? @japaric |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this, @sekineh!
This LGTM in terms of what's being tested but I'd like to see this test directly use a community maintained crate.
|
||
# hint: we could set variables per $(TARGET) basis in order to support other targets. | ||
CRATE := lm3s6965evb | ||
CRATE_URL := https://github.com/japaric/lm3s6965evb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to use the cortex-m-rt crate here since it's maintained by the WG. You would have to:
- modify that crate's memory.x file to match this one
- copy lm3s6965evb's main.rs into the examples directory of the cortex-m-rt crate
and if you add a .cargo/config file with these lines (but uncommented) to cortex-m-rt then cargo run --example qemu --target $(TARGET)
should build the example and then run it on QEMU in a single step.
Feel free to send a PR and I'll review it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do this first. (make a PR for cortex-m-rt
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When moved to cortex-m-rt, semihosting did not worked for me.
Needed some option.
HERE := $(shell pwd) | ||
|
||
|
||
# hint: we could set variables per $(TARGET) basis in order to support other targets. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qemu-system-arm
only supports full system emulation of the LM3S6965 and LM3S811 microcontrollers. They are both Cortex-M3 microcontrollers so we can test the thumbv6m-none-eabi
and thumbv7m-none-eabi
targets with either QEMU target. The thumbv7em-none-eabi
and thumbv7em-none-eabihf
targets have incompatible ISAs and they can crash QEMU.
It would be great to also test the thumbv6m-none-eabi
target here since it's an older ARM architecture. If you are up to it you can do that in a follow-up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahem, older... ;) Cortex-M0 is still very relevant, is there any support for a CM0 MCU in qemu?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@therealprof I've run 4 binaries on qemu-system-arm
. As I remember, some worked in different board setting. My theory is if an ISA (thumbv6m-none-eabi
) is roughly a subset of another (thumbv7m-none-eabi
), and there's no harmful register changes, it has chance to run successfully. I'll try with existing qemu setting for thumbv6m
(Cortex-M0) also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sekineh thumbv6m-none-eabi
is indeed a subset of thumbv7m-none-eabi
and code might run just fine on higher ISA versions. However to be on the safe(r) side you really want to test on a Cortex-M0(+) target or emulation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@therealprof these are the Cortex CPUs QEMU 3.0.0 supports:
cortex-a15
cortex-a7
cortex-a8
cortex-a9
cortex-m3
cortex-m33
cortex-m4
cortex-r5
cortex-r5f
No cortex-m0
or cortex-m4f
but you can run code compiled for thumbv6m-none-eabi
using -cpu cortex-m3
since the M0 ISA is a subset of the M3 ISA. We can't run code compiled for the thumbv7em-none-eabihf
though because the FPU instructions crash QEMU.
I'm not entirely sure if -machine
is completely orthogonal to -cpu
but I would prefer not to mix a Cortex-M3 machine
with -cpu cortex-m4
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I successfully ran the thumv6m-none-eabi
binary on qemu -cpu cortex-m3
sekineh@sekineh-VirtualBox:~/cortex-m-rt_me$ cargo run --example qemu --target thumbv6m-none-eabi
Finished dev [unoptimized + debuginfo] target(s) in 0.03s
Running `qemu-system-arm -cpu cortex-m3 -machine lm3s6965evb -nographic -semihosting-config enable=on,target=native -kernel target/thumbv6m-none-eabi/debug/examples/qemu`
x = 42
-cd $(WORK_DIR) && rm -rf $(CRATE) | ||
cd $(WORK_DIR) && bash -x $(HERE)/../git_clone_sha1.sh $(CRATE) $(CRATE_URL) $(CRATE_SHA1) | ||
cd $(WORK_DIR)/$(CRATE) && $(CARGO) build --target $(TARGET) -v | ||
cd $(WORK_DIR)/$(CRATE) && qemu-system-arm \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also test the release binary here? That would check that optimizations don't produce an invalid binary (it has happened before).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current commit does both debug and --release
.
Thanks for the review, @japaric ! |
@japaric Yeah, I'm well aware that this is possible. But it also means we won't be able to detect invalid opcodes generated by rustc which I thought was kind of the point of the exercise? |
@therealprof the main goal of this test is to check that future changes won't break linking or generate an invalid binary (e.g. a symbol disappears from the final binary due to changes in how rustc decides symbol visibility). Would you prefer that we don't test |
@japaric You don't need to run the binary to check for symbols. Of course it's fine to run the binaries but it would be a whole lot better if we could make sure that the generated code does not accidentally contain CM3 instructions or branches with targets outside of the supported range. |
thumbv7m-none-eabi
binary using qemu-system-arm
[IRR-2018-embedded]thumbv7m-none-eabi
binary using qemu-system-arm
[IRR-2018-embedded]
thumbv7m-none-eabi
binary using qemu-system-arm
[IRR-2018-embedded]thumbv7m-none-eabi
binary using qemu-system-arm
[IRR-2018-embedded]
@japaric I think I modified the patch as you've suggested. A problem I see is it does not work on my local env due to the error I'm not familiar with: stdout
stderr
|
Make writes its output to stdout and bash -x writes its output to stderr. This makes log harder to read. I feel like I want to move the logic into bash script where I can use pushd / popd. |
-cd $(WORK_DIR) && rm -rf $(CRATE) | ||
cd $(WORK_DIR) && bash -x $(HERE)/../git_clone_sha1.sh $(CRATE) $(CRATE_URL) $(CRATE_SHA1) | ||
cd $(WORK_DIR)/$(CRATE) && $(CARGO) run --target $(TARGET) --example qemu | grep "x = 42" | ||
cd $(WORK_DIR)/$(CRATE) && $(CARGO) run --target $(TARGET) --example qemu --release | grep "x = 42" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move logic l23-28 into separate file like script.sh? It will make error log clearer.
It looks like the $ grep '^target' config.toml
target = ["thumbv7m-none-eabi"]
$ ./x.py build --stage 1 src/libstd
$ rustup toolchain link stage1 build/x86_64-unknown-linux-gnu/stage1
$ cd /path/to/some/project
$ # this fails if proc_macro appears somewhere in the dependency graph
$ cargo +stage1 build --target thumbv7m-none-eabi @alexcrichton @kennytm would it be possible to tweak |
Actually, it looks like libproc_macro is in e.g. stage1 sysroot: $ cd $(rustc +stage1 --print sysroot)
$ find -name 'libproc_macro*'
./lib/libproc_macro-09d5bb2a63717cb1.so
./lib/rustlib/x86_64-unknown-linux-gnu/lib/libproc_macro-956e91f4391bd306.so But the hashes of the .so libraries don't match, which is not the case for, say, a nightly toolchain: $ cd $(rustc +nightly --print sysroot)
$ find -name 'libproc_macro*'
./lib/rustlib/x86_64-unknown-linux-gnu/lib/libproc_macro-96fdba3b331ee226.so
./lib/libproc_macro-96fdba3b331ee226.so |
@japaric Yes fulldeps is intended for the stage2 stuff. But fulldeps assumes TARGET == HOST. |
Yeah @kennytm is correct here, there's no great way to use |
OK, in that case we'll have to test against a version of cortex-m-rt that doesn't use proc macros. @sekineh, sorry for the back and forth but could you change the run-make test to include a binary Cargo project who's src/main.rs is basically this file but that uses version v0.5.4 of cortex-m-rt? The version must be exactly that one ( The syntax of the entry point will be slightly different and you'll have to copy over both .cargo/config and memory.x to this new Cargo project. You should be able to test ( |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors retry (crates.io failure) |
[CI] Run a `thumbv7m-none-eabi` binary using `qemu-system-arm` [IRR-2018-embedded] ## What's included? - Run a `thumbv7m-none-eabi` binary using `qemu-system-arm` - We are using `cortex-m-rt = "=0.5.4"` which does not use `proc_macro`. (reason: stage2 build of rustc does not work well with `proc_macro` in `run-make` phase.) - We are using GNU LD for now. ## Blocker All resolved. - ~[Waiting] `#[panic_handler]` is not available in stable.~ - [Merged] rust-lang#53619 - ~[Waiting] https://github.com/japaric/lm3s6965evb: does not compile on stable.~ - [OK] dependent crate ~`panic-abort`~ `panic-halt`: already moved to use `#[panic_handler]`. ## Update `#[panic_handler]` will be stabilized in Rust 1.30. CC @kennytm @jamesmunns @nerdyvaishali
⌛ Testing commit 35bbcf1 with merge a6242b7711bab7a987b83e869bd3abc70d99ba22... |
💔 Test failed - status-travis |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Retrying locally after |
perfectly succeeds in my environment. I'm using
|
A Cargo runner needs to be set in the test. Right now it's trying to directly run the cross compiled binary on the host. A runner can be set by (a) setting @sekineh perhaps you have the .cargo/config file locally but have not checked it in because of some .gitignore file in this repository? |
@bors r+ |
📌 Commit f872303 has been approved by |
[CI] Run a `thumbv7m-none-eabi` binary using `qemu-system-arm` [IRR-2018-embedded] ## What's included? - Run a `thumbv7m-none-eabi` binary using `qemu-system-arm` - We are using `cortex-m-rt = "=0.5.4"` which does not use `proc_macro`. (reason: stage2 build of rustc does not work well with `proc_macro` in `run-make` phase.) - We are using GNU LD for now. ## Blocker All resolved. - ~[Waiting] `#[panic_handler]` is not available in stable.~ - [Merged] #53619 - ~[Waiting] https://github.com/japaric/lm3s6965evb: does not compile on stable.~ - [OK] dependent crate ~`panic-abort`~ `panic-halt`: already moved to use `#[panic_handler]`. ## Update `#[panic_handler]` will be stabilized in Rust 1.30. CC @kennytm @jamesmunns @nerdyvaishali
☀️ Test successful - status-appveyor, status-travis |
What's included?
thumbv7m-none-eabi
binary usingqemu-system-arm
cortex-m-rt = "=0.5.4"
which does not useproc_macro
.(reason: stage2 build of rustc does not work well with
proc_macro
inrun-make
phase.)Blocker
All resolved.
[Waiting]#[panic_handler]
is not available in stable.[Waiting] https://github.com/japaric/lm3s6965evb: does not compile on stable.panic-abort
panic-halt
: already moved to use#[panic_handler]
.Update
#[panic_handler]
will be stabilized in Rust 1.30.CC @kennytm @jamesmunns @nerdyvaishali