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

Latest release of cortex-m-semihosting seems broken? #271

Closed
korken89 opened this issue Oct 15, 2020 · 10 comments
Closed

Latest release of cortex-m-semihosting seems broken? #271

korken89 opened this issue Oct 15, 2020 · 10 comments
Labels
nominated Issue nominated as discussion topic for the Embedded WG meeting

Comments

@korken89
Copy link
Contributor

After the 0.4.0 update of cortex-m-semihosting we are seeing breakage in RTIC CI pipeline as follows:

error: failed to run custom build command for `cortex-m-semihosting v0.4.0`

Caused by:
  process didn't exit successfully: `/home/emifre/Git/rtic/cortex-m-rtic/target/debug/build/cortex-m-semihosting-6c5d0dd3173d1553/build-script-build` (exit code: 101)
  --- stderr
  thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }', /home/emifre/.cargo/registry/src/github.com-1ecc6299db9ec823/cortex-m-semihosting-0.4.0/build.rs:15:14
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...
error: build failed

No idea what is actually wrong here, but it seems something broke in the latest release?

@jamesmunns
Copy link
Member

Relevant build script code:

https://github.com/rust-embedded/cortex-m/blob/master/cortex-m-semihosting/build.rs#L10-L15

CC @jonas-schievink who I think worked on the prebuild static archives most recently in #262

@jamesmunns
Copy link
Member

We should probably mirror the build.rs changes made in this commit:

73a9d29#diff-d0d98998092552a1d3259338c2c71e118a5b8343dd4703c0c7f552ada7f9cb42

@jonas-schievink
Copy link
Contributor

Seems like cargo package doesn't include the right files here? We do have tests for cortex-m-semihosting and those worked fine prior to publishing.

What if we symlink cortex-m-semihosting/bin to ../bin? Will cargo package include the files?

@jonas-schievink
Copy link
Contributor

Yanked 0.4.0

@adamgreig
Copy link
Member

@jonas-schievink, it seems to me like we could forward the inline-asm feature of c-m-sh to its c-m dependency and use the new syscall added to cortex-m, therefore removing c-m-sh's build script and prebuilt binary entirely?

@jonas-schievink
Copy link
Contributor

yeah that should work

@adamgreig
Copy link
Member

Though it will mean c-m-sh 0.4 requires c-m 0.7 (not yet released) which is unfortunate, hm.

@thalesfragoso thalesfragoso added the nominated Issue nominated as discussion topic for the Embedded WG meeting label Oct 19, 2020
bors bot added a commit that referenced this issue Oct 22, 2020
273: c-m-sh: fix missing prebuilt binaries. closes #271 r=adamgreig a=adamgreig

Adding a symlink to `../bin` allows `cargo package` to include a copy of the binary files in the resulting tarball, which means the build script is able to link them as normal.

* We include the `-lto.a` files, but don't have an LTO_PLUGIN feature and so never use them
* While this should work for all platforms to use from crates.io, it's not clear what e.g. Windows will do with the symlink when trying to use this as a path/patch dependency directly from the git repository.

If it seems useful I could probably add a LTO feature the same as cortex-m, but I'd rather wait until we can have c-m-sh just directly use cortex-m's `syscall` method and then users can enable that feature in cortex-m themselves instead.

If anyone has this set up on Windows and is able to see if you can still use this repository as a path dependency, that would be great!

Co-authored-by: Adam Greig <adam@adamgreig.com>
@bors bors bot closed this as completed in eefd672 Oct 22, 2020
@adamgreig adamgreig reopened this Oct 22, 2020
@adamgreig
Copy link
Member

Well, that didn't work so well.

          rust-lld: error: duplicate symbol: __wfi
          >>> defined at inline.rs:13 (asm/inline.rs:13)
          >>>            bin/thumbv7em-none-eabihf.o:(__wfi) in archive /tmp/rustckWdISd/libcortex_m_semihosting-33bbc22c19d1c0ab.rlib
          >>> defined at asm.s:189
          >>>            cortex-m.o:(.text.__wfi+0x1) in archive /tmp/rustckWdISd/libcortex_m-c1c501809caa9026.rlib

@nickray
Copy link

nickray commented Nov 28, 2020

What's the status on this?

If I have a library that uses cortex-m 0.7, since 0.4.0 is yanked and 0.4.1 unpublished, I get a cortex-m-semihosting that forces cortex-m 0.6, which ends up with a warning: Linking globals named 'CORE_PERIPHERALS': symbol multiply defined! and an error: failed to load bc of "cortex_m-faf0ca0469234fc4.cortex_m.2vla0p20-cgu.0.rcgu.o".

I'm a bit confused :)

@adamgreig
Copy link
Member

This should now be fixed by the new cortex-m-semihosting release v0.3.6, which supports either cortex-m 0.6 or cortex-m 0.7.

bors bot added a commit that referenced this issue Dec 2, 2020
299: Expose __syscall and add new bootstrap method r=jonas-schievink a=adamgreig

We added `__syscall` in the new inline asm, but did not expose it in the crate API, and the cortex-m-semihosting crate can't use it directly because the pre-built binaries would contain duplicate symbols (#271). This PR renames it to `__sh_syscall` (since we could imagine other different syscalls; this one is explicitly semihosting with the `bkpt 0xAB`) and exposes it in `cortex_m::asm::sh_syscall`.

This PR also adds the new methods discussed in #297 to permit sound bootstrapping, either from an MSP and RV or a vector table address.

Co-authored-by: Adam Greig <adam@adamgreig.com>
adamgreig pushed a commit that referenced this issue Jan 12, 2022
271: Get rid of #[deny(warnings)] r=adamgreig a=therealprof

This is considered harmful as the warnings may change with any compiler
release.

Signed-off-by: Daniel Egger <daniel@eggers-club.de>

Co-authored-by: Daniel Egger <daniel@eggers-club.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nominated Issue nominated as discussion topic for the Embedded WG meeting
Projects
None yet
Development

No branches or pull requests

6 participants