-
Notifications
You must be signed in to change notification settings - Fork 151
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
Add additional meta data to improve crate experience #118
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Nicoretti
commented
Oct 26, 2018
- Add readme setting, so README.md is shown on https://crates.io/
* Add readme setting, so README.md is shown on https://crates.io/
therealprof
approved these changes
Oct 26, 2018
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.
Good catch, thanks!
bors r+ |
bors bot
added a commit
that referenced
this pull request
Oct 26, 2018
118: Add additional meta data to improve crate experience r=therealprof a=Nicoretti * Add readme setting, so README.md is shown on https://crates.io/ Co-authored-by: Nicola Coretti <[email protected]>
Build failed |
bors retry |
bors bot
added a commit
that referenced
this pull request
Oct 26, 2018
118: Add additional meta data to improve crate experience r=therealprof a=Nicoretti * Add readme setting, so README.md is shown on https://crates.io/ Co-authored-by: Nicola Coretti <[email protected]>
Build succeeded |
adamgreig
pushed a commit
that referenced
this pull request
Jan 12, 2022
118: [RFC] Keep `.stack_sizes` r=japaric a=japaric PR [rust-lang/rust#51946] will add a `-Z emit-stack-sizes` to `rustc` that can be used to (make LLVM) emit stack usage metadata. Most linkers will discard this metadata by default, however. [rust-lang/rust#51946]: rust-lang/rust#51946 This PR / RFC proposes that we modify our linker script to keep this metadata and place in an output `.stack_sizes` section (the canonical section name that LLVM uses for this metadata). This `.stack_sizes` section will be marked as `(INFO)` meaning that it won't be loaded into the Flash or RAM of the target device. The advantage of doing this is that tools that parse this information will work out of the box. See examples at the bottom. Not doing this means that users will have to mess with linker scripts and tweak the linking process to be able to access the stack usage metadata. See [this example] to get an idea of the required steps to keep the metadata information. [this example]: https://github.com/japaric/stack-sizes#example-usage # Examples Using the [`cargo stack-sizes`] subcommand to build a project and print its stack usage information. Assumes that this PR has been merged. [`cargo stack-sizes`]: https://github.com/japaric/stack-sizes ``` console $ cargo stack-sizes --bin app -v RUSTC=stack-sizes-rustc "cargo" "rustc" "--bin" "app" "--" Finished dev [unoptimized + debuginfo] target(s) in 0.01s "stack-sizes" "target/thumbv7m-none-eabi/debug/app" address stack name 0x00000416 112 <cortex_m_rt::ExceptionFrame as core::fmt::Debug>::fmt::h4362577979112554 0x0000059e 96 <<cortex_m_rt::ExceptionFrame as core::fmt::Debug>::fmt::Hex as core::fmt::Debug>::fmt::hd24e21694d63c2ec 0x0000069c 72 core::fmt::Arguments::new_v1_formatted::hc22bfa5fcec35f0e 0x00000758 48 r0::init_data::hdc91f9605f364aeb 0x00000710 40 r0::zero_bss::h24ed73bae17eec88 0x000007d8 24 core::ptr::<impl *mut T>::offset::h940561014e707589 0x000007fc 24 core::ptr::<impl *const T>::offset::h6847f0bc0fe1f0b5 0x00000820 24 core::ptr::read::ha83613479c1b8688 0x00000858 24 core::sync::atomic::compiler_fence::h89dd3725007a5019 0x00002cb0 24 core::sync::atomic::compiler_fence::h4e6eb311378b7447 0x00000668 16 UserHardFault_ 0x000007be 16 core::ptr::write_volatile::h92be4be56d39953d 0x00000840 16 core::ptr::write::h1bff45e7fbd786f1 0x00002c92 16 rust_begin_unwind 0x00000404 8 main 0x0000061c 8 Reset 0x00000684 8 SysTick 0x000006fc 8 core::ptr::drop_in_place::h96b0344554e68fcb 0x0000089c 8 core::mem::uninitialized::hea41370061be039b 0x000008aa 8 core::mem::zeroed::h87926bff0ea7d7b3 0x00000400 0 app::main::hb1f6f4352b7a35e2 0x0000069a 0 __pre_init ``` ``` console $ cargo stack-sizes --bin app --release -v RUSTC=stack-sizes-rustc "cargo" "rustc" "--bin" "app" "--release" "--" Finished release [optimized + debuginfo] target(s) in 0.01s "stack-sizes" "target/thumbv7m-none-eabi/release/app" address stack name 0x00000400 0 main 0x00000402 0 Reset 0x00000616 0 UserHardFault_ 0x00000618 0 SysTick 0x0000061a 0 __pre_init ``` --- r? @rust-embedded/cortex-m does this seem like a reasonable addition? Or do you think keeping .stack_sizes should *not* be done in this crate? Co-authored-by: Jorge Aparicio <[email protected]>
adamgreig
pushed a commit
that referenced
this pull request
Jan 12, 2022
this undoes PR #118 I found a problem with keeping this section. Turns out that the input `.stack_sizes` sections contain references to all functions compiled with `-Z emit-stack-sizes` (the section contains the addresses of all those functions after all) so keeping those section prevents the linker from removing *any* of those functions. This is not a problem today because `-Z emit-stack-sizes` is *opt-in* and only used to analyze a program. However, I am proposing a rust-lang/rust PR to build the `compiler-builtins` crate with `-Z emit-stack-sizes`. That change will cause *all* the functions in that crate to be kept in binaries that link to `cortex-m-rt`, regardless of whether the crate author uses `-Z emit-stack-sizes` or not, leading a increase in binary size of about 14 KB (`.text` section). To prevent issues with that rust-lang/rust PR I propose we undo PR #118. On the bright side, the tools that were depending on this (`cargo-stack-sizes` and `cargo-call-stack`) no longer do so in their latest releases so nothing is lost on the tooling front with this change.
adamgreig
pushed a commit
that referenced
this pull request
Jan 12, 2022
186: do not KEEP the .stack_sizes section r=therealprof a=japaric this undoes PR #118 I found a problem with keeping this section. Turns out that the input `.stack_sizes` sections contain references to all functions compiled with `-Z emit-stack-sizes` (the section contains the addresses of all those functions after all) so keeping those section prevents the linker from removing *any* of those functions. This is not a problem today because `-Z emit-stack-sizes` is *opt-in* and only used to analyze a program. However, I am proposing a rust-lang/rust PR to build the `compiler-builtins` crate with `-Z emit-stack-sizes`. That change will cause *all* the functions in that crate to be kept in binaries that link to `cortex-m-rt`, regardless of whether the crate author uses `-Z emit-stack-sizes` or not, leading a increase in binary size of about 14 KB (`.text` section). To prevent issues with that rust-lang/rust PR I propose we undo PR #118. On the bright side, the tools that were depending on this (`cargo-stack-sizes` and `cargo-call-stack`) no longer do so in their latest releases so nothing is lost on the tooling front with this change. r? @rust-embedded/cortex-m Co-authored-by: Jorge Aparicio <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.