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

Update the Rust toolchain to 2021-03-25. #280

Merged
merged 2 commits into from
Mar 31, 2021

Conversation

jrvanwhy
Copy link
Collaborator

This includes rust-lang/rust#82141, which is needed in order to add ARM support to libtock_runtime. It also allows us to use the unsafe_op_in_unsafe_fn lint.

I went ahead and turned on unsafe_op_in_unsafe_fn for the Tock 2.0 crates, as I will use that setting in upcoming PRs (in addition to warning on one pattern, it also allows you to use unsafe {} in unsafe functions).

The toolchain update caused a change in the ELF file generation, which caused elf2tab to identify the wrong flash range for the process binary. To solve this, I moved start out of .text and merged it into .crt0_header to form a new .start section. I believe the issue has to do with execute bits on the sections, but it was nontrivial to fix in elf2tab.

The remainder of the changes are a result of additional lints added in the new toolchain version.

This includes rust-lang/rust#82141, which is needed in order to add ARM support to libtock_runtime. It also allows us to use the unsafe_op_in_unsafe_fn lint.
@jrvanwhy jrvanwhy added the significant Indicates a PR is significant as defined by the code review policy. label Mar 30, 2021
@jrvanwhy
Copy link
Collaborator Author

CI failure will be fixed when #279 is merged. I'll re-run the CI then.

Copy link
Contributor

@hudson-ayers hudson-ayers left a comment

Choose a reason for hiding this comment

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

This looks good, though I am curious why you got rid of .exidx in this PR? I am guessing we just weren't using it anyway so its nice to simplify the linker script?

@jrvanwhy
Copy link
Collaborator Author

This looks good, though I am curious why you got rid of .exidx in this PR? I am guessing we just weren't using it anyway so its nice to simplify the linker script?

Good catch, I missed that when I wrote the PR description! After the toolchain update, the .exidx section was overlapping another section we needed (I think it was .bss), resulting in linker errors. This doesn't feel like the right fix, but I was unable to find another working solution.

@alistair23
Copy link
Collaborator

bors r+

@alistair23
Copy link
Collaborator

The CI failure looks unrelated, I'm going to manually merge this as I don't think a toolchain update is significative (in the sense we should wait a week for comments)

@alistair23 alistair23 merged commit c436ffe into tock:master Mar 31, 2021
@bors
Copy link
Contributor

bors bot commented Mar 31, 2021

Already running a review

bors bot added a commit that referenced this pull request Apr 6, 2021
282: Overhaul RawSyscalls to work with ARM, -Zmiri-track-raw-pointers, the revised Yield, and Exit. r=alistair23 a=jrvanwhy

1. RawSyscalls now supports Yield's new return value semantics as well as the `yield-no-wait` variant of Yield.
2. RawSyscalls now supports Exit.
3. RawSyscalls is now usable in Miri with the `-Zmiri-track-raw-pointers` flag.
4. RawSyscalls can now be implemented on ARM: previously, `class` was a runtime value, but it needs to be an immediate value on ARM.
5. The explanation for the design of RawSyscalls was completely overhauled. Instead of listing a bunch of "design considerations" with no connection to the final design, it shows how testing and efficiency considerations lead to its design. I hope the new description is more amenable to discussion. The new design should be a bit more future-proof than the previous design as well.

The new features added have been stabilized, and can be removed after #280 is merged.

Co-authored-by: Johnathan Van Why <[email protected]>
@jrvanwhy jrvanwhy deleted the toolchain-update branch July 11, 2021 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
significant Indicates a PR is significant as defined by the code review policy.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants