[eRFC] add -Z emit-stack-sizes #51946
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
rust-highfive
added
the
S-waiting-on-review
|
I have no objection to this feature per se, as long as we create a suitable tracking issue. |
stokhos
added
S-waiting-on-author
|
Update: the problem with the thumb targets that I originally reported has fixed itself after the last LLVM upgrade (yay!). Also, I tested changing the name of the section to .debug_stacksizes and found out the following:
The rename seems like an overall improvement to me but it does require patching our LLVM fork, which I know we tend to avoid. Should we do the rename or not? |
|
Ping from triage! @japaric we haven't heard from you for awhile, will you have time to work on this PR? |
|
@stokhos I'm waiting on feedback on my latest comment regarding doing or not the section rename in the LLVM source code. @nikomatsakis Given that there has been no comment from other members of the compiler team can I interpret your comment as "I'll r+ this once this gets a tracking issue"? |
pietroalbini
added
S-waiting-on-review
|
Ping from triage @nikomatsakis! This PR needs your review. |
Yes, sure. Has this been done? EDIT: Actually, an FCP is the right thing to do here. Not sure why I didn't just do it right away, sorry! |
nikomatsakis
added
the
T-compiler
|
@rfcbot fcp merge I propose that we merge this PR (once a suitable tracking issue has been created). This PR exposes LLVM's ability to report the stack usage of each function through the unstable / |
|
Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged teams:
No concerns currently listed. Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
rfcbot
added
proposed-final-comment-period
Answering myself here: I think it would be best to stick to the default, hard coded value for now (what this PR does as it is). It would be cleaner to add an API that lets you rename the section to LLVM, and then we can add a rustc flag that does the rename.
It's OK! Thanks for starting the FCP. |
TimNN
added
S-waiting-on-team
|
@Zoxc @michaelwoerister @nagisa @pnkfelix ping This is a friendly reminder to check your FCP checkboxes. |
rfcbot
added
final-comment-period
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
pnkfelix
removed
the
S-waiting-on-team
|
This should be good to go, right? |
|
@sfackler: there's one more day of the final comment period to go, but once this has been rebased, it should be ready to merge. |
TimNN
added
the
S-blocked
|
The final comment period, with a disposition to merge, as per the review above, is now complete. |
bors
added a commit
that referenced
this pull request
Sep 25, 2018
[eRFC] add -Z emit-stack-sizes # What This PR exposes LLVM's ability to report the stack usage of each function through the unstable / experimental `-Z emit-stack-sizes` flag. # Motivation The end goal is to enable whole program analysis of stack usage to prove absence of stack overflows at compile time. Such property is important in systems that lack a MMU / MPU and where stack overflows can corrupt memory. And in systems that have protection against stack overflows such proof can be used to opt out of runtime checks (e.g. stack probes or the MPU). Such analysis requires the call graph of the program, which can be obtained from MIR, and the stack usage of each function in the program. Precise information about the later later can only be obtained from LLVM as it depends on the optimization level and optimization options like LTO. This PR does **not** attempt to add the ability to perform such whole program analysis to rustc; it simply does the minimal amount of work to enable such analysis to be implemented out of tree. # Implementation This PR exposes a way to set LLVM's `EmitStackSizeSection` option from the command line. The option is documented [here]; the documentation is copied below for convenience and posteriority: [here]: https://llvm.org/docs/CodeGenerator.html#emitting-function-stack-size-information > A section containing metadata on function stack sizes will be emitted when > TargetLoweringObjectFile::StackSizesSection is not null, and TargetOptions::EmitStackSizeSection > is set (-stack-size-section). The section will contain an array of pairs of function symbol values > (pointer size) and stack sizes (unsigned LEB128). The stack size values only include the space > allocated in the function prologue. Functions with dynamic stack allocations are not included. Where the LLVM feature is not available (e.g. LLVM version < 6.0) or can't be applied (e.g. the output format doesn't support sections e.g. .wasm files) the flag does nothing -- i.e. no error or warning is emitted. # Example usage ``` console $ cargo new --bin hello && cd $_ $ cat >src/main.rs <<'EOF' use std::{mem, ptr}; fn main() { registers(); stack(); } #[inline(never)] fn registers() { unsafe { // values loaded into registers ptr::read_volatile(&(0u64, 1u64)); } } #[inline(never)] fn stack() { unsafe { // array allocated on the stack let array: [i32; 4] = mem::uninitialized(); for elem in &array { ptr::read_volatile(&elem); } } } EOF $ # we need a custom linking step to preserve the .stack_sizes section $ # (see unresolved questions for a solution that doesn't require custom linking) $ cat > keep-stack-sizes.x <<'EOF' SECTIONS { .stack_sizes : { KEEP(*(.stack_sizes)); } } EOF $ cargo rustc --release -- \ -Z emit-stack-sizes \ -C link-arg=-Wl,-Tkeep-stack-sizes.x \ -C link-arg=-N $ size -A target/release/hello | grep stack_sizes .stack_sizes 117 185136 ``` Then a tool like [`stack-sizes`] can be used to print the information in human readable format [`stack-sizes`]: https://github.com/japaric/stack-sizes/#stack-sizes ``` console $ stack-sizes target/release/hello address size name 0x000000000004b0 0 core::array::<impl core::iter::traits::IntoIterator for &'a [T; _]>::into_iter::ha50e6661c0ec84aa 0x000000000004c0 8 std::rt::lang_start::ha02aea783e0e1b3e 0x000000000004f0 8 std::rt::lang_start::{{closure}}::h5115b527d5244952 0x00000000000500 8 core::ops::function::FnOnce::call_once::h6bfa1076da82b0fb 0x00000000000510 0 core::ptr::drop_in_place::hb4de82e57787bc70 0x00000000000520 8 hello::main::h08bb6cec0556bd66 0x00000000000530 0 hello::registers::h9d058a5d765ec1d2 0x00000000000540 24 hello::stack::h88c8cb66adfdc6f3 0x00000000000580 8 main 0x000000000005b0 0 __rust_alloc 0x000000000005c0 0 __rust_dealloc 0x000000000005d0 0 __rust_realloc 0x000000000005e0 0 __rust_alloc_zeroed ``` # Stability Like `-Z sanitize` this is a re-export of an LLVM feature. To me knowledge, we don't have a policy about stabilization of such features as they are incompatible with, or demand extra implementation effort from, alternative backends (e.g. cranelift). As such this feature will remain experimental / unstable for the foreseeable future. # Unresolved questions ## Section name Should we rename the `.stack_sizes` section to `.debug_stacksizes`? With the former name linkers will strip the section unless told otherwise using a linker script, which means getting this information requires both knowledge about linker scripts and a custom linker invocation (see example above). If we use the `.debug_stacksizes` name (I believe) linkers will always keep the section, which means `-Z emit-stack-sizes` is the only thing required to get the stack usage information. # ~TODOs~ ~Investigate why this doesn't work with the `thumb` targets. I get the LLVM error shown below:~ ``` console $ cargo new --lib foo && cd $_ $ echo '#![no_std] pub fn foo() {}' > src/lib.rs $ cargo rustc --target thumbv7m-none-eabi -- -Z emit-stack-sizes LLVM ERROR: unsupported relocation on symbol ``` ~which sounds like it might be related to the `relocation-model` option. Maybe `relocation-model = static` is not supported for some reason?~ This fixed itself after the LLVM upgrade. --- r? @nikomatsakis cc @rust-lang/compiler @perlindgren @whitequark
|
💔 Test failed - status-travis |
bors
added
S-waiting-on-review
|
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 |
kennytm
added
S-waiting-on-author
I'll update the docs and make the test ignore mac. |
that makes the output .stack_sizes section non-allocatable when linking with either GNU LD or LLD
japaric
force-pushed
the
emit-stack-sizes
branch
from
4b89458
to
531e356
Compare
|
@bors r=nikomatsakis |
|
📌 Commit 531e356 has been approved by |
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Sep 26, 2018
[eRFC] add -Z emit-stack-sizes # What This PR exposes LLVM's ability to report the stack usage of each function through the unstable / experimental `-Z emit-stack-sizes` flag. # Motivation The end goal is to enable whole program analysis of stack usage to prove absence of stack overflows at compile time. Such property is important in systems that lack a MMU / MPU and where stack overflows can corrupt memory. And in systems that have protection against stack overflows such proof can be used to opt out of runtime checks (e.g. stack probes or the MPU). Such analysis requires the call graph of the program, which can be obtained from MIR, and the stack usage of each function in the program. Precise information about the later later can only be obtained from LLVM as it depends on the optimization level and optimization options like LTO. This PR does **not** attempt to add the ability to perform such whole program analysis to rustc; it simply does the minimal amount of work to enable such analysis to be implemented out of tree. # Implementation This PR exposes a way to set LLVM's `EmitStackSizeSection` option from the command line. The option is documented [here]; the documentation is copied below for convenience and posteriority: [here]: https://llvm.org/docs/CodeGenerator.html#emitting-function-stack-size-information > A section containing metadata on function stack sizes will be emitted when > TargetLoweringObjectFile::StackSizesSection is not null, and TargetOptions::EmitStackSizeSection > is set (-stack-size-section). The section will contain an array of pairs of function symbol values > (pointer size) and stack sizes (unsigned LEB128). The stack size values only include the space > allocated in the function prologue. Functions with dynamic stack allocations are not included. Where the LLVM feature is not available (e.g. LLVM version < 6.0) or can't be applied (e.g. the output format doesn't support sections e.g. .wasm files) the flag does nothing -- i.e. no error or warning is emitted. # Example usage ``` console $ cargo new --bin hello && cd $_ $ cat >src/main.rs <<'EOF' use std::{mem, ptr}; fn main() { registers(); stack(); } #[inline(never)] fn registers() { unsafe { // values loaded into registers ptr::read_volatile(&(0u64, 1u64)); } } #[inline(never)] fn stack() { unsafe { // array allocated on the stack let array: [i32; 4] = mem::uninitialized(); for elem in &array { ptr::read_volatile(&elem); } } } EOF $ # we need a custom linking step to preserve the .stack_sizes section $ # (see unresolved questions for a solution that doesn't require custom linking) $ cat > keep-stack-sizes.x <<'EOF' SECTIONS { .stack_sizes : { KEEP(*(.stack_sizes)); } } EOF $ cargo rustc --release -- \ -Z emit-stack-sizes \ -C link-arg=-Wl,-Tkeep-stack-sizes.x \ -C link-arg=-N $ size -A target/release/hello | grep stack_sizes .stack_sizes 117 185136 ``` Then a tool like [`stack-sizes`] can be used to print the information in human readable format [`stack-sizes`]: https://github.com/japaric/stack-sizes/#stack-sizes ``` console $ stack-sizes target/release/hello address size name 0x000000000004b0 0 core::array::<impl core::iter::traits::IntoIterator for &'a [T; _]>::into_iter::ha50e6661c0ec84aa 0x000000000004c0 8 std::rt::lang_start::ha02aea783e0e1b3e 0x000000000004f0 8 std::rt::lang_start::{{closure}}::h5115b527d5244952 0x00000000000500 8 core::ops::function::FnOnce::call_once::h6bfa1076da82b0fb 0x00000000000510 0 core::ptr::drop_in_place::hb4de82e57787bc70 0x00000000000520 8 hello::main::h08bb6cec0556bd66 0x00000000000530 0 hello::registers::h9d058a5d765ec1d2 0x00000000000540 24 hello::stack::h88c8cb66adfdc6f3 0x00000000000580 8 main 0x000000000005b0 0 __rust_alloc 0x000000000005c0 0 __rust_dealloc 0x000000000005d0 0 __rust_realloc 0x000000000005e0 0 __rust_alloc_zeroed ``` # Stability Like `-Z sanitize` this is a re-export of an LLVM feature. To me knowledge, we don't have a policy about stabilization of such features as they are incompatible with, or demand extra implementation effort from, alternative backends (e.g. cranelift). As such this feature will remain experimental / unstable for the foreseeable future. # Unresolved questions ## Section name Should we rename the `.stack_sizes` section to `.debug_stacksizes`? With the former name linkers will strip the section unless told otherwise using a linker script, which means getting this information requires both knowledge about linker scripts and a custom linker invocation (see example above). If we use the `.debug_stacksizes` name (I believe) linkers will always keep the section, which means `-Z emit-stack-sizes` is the only thing required to get the stack usage information. # ~TODOs~ ~Investigate why this doesn't work with the `thumb` targets. I get the LLVM error shown below:~ ``` console $ cargo new --lib foo && cd $_ $ echo '#![no_std] pub fn foo() {}' > src/lib.rs $ cargo rustc --target thumbv7m-none-eabi -- -Z emit-stack-sizes LLVM ERROR: unsupported relocation on symbol ``` ~which sounds like it might be related to the `relocation-model` option. Maybe `relocation-model = static` is not supported for some reason?~ This fixed itself after the LLVM upgrade. --- r? @nikomatsakis cc @rust-lang/compiler @perlindgren @whitequark
|
☀️ Test successful - status-appveyor, status-travis |
bors bot
added a commit
to rust-embedded/cortex-m-rt
that referenced
this pull request
Sep 28, 2018
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
to rust-embedded/cortex-m
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]>
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.
What
This PR exposes LLVM's ability to report the stack usage of each function through the unstable /
experimental
-Z emit-stack-sizesflag.Motivation
The end goal is to enable whole program analysis of stack usage to prove absence of stack overflows
at compile time. Such property is important in systems that lack a MMU / MPU and where stack
overflows can corrupt memory. And in systems that have protection against stack overflows such proof
can be used to opt out of runtime checks (e.g. stack probes or the MPU).
Such analysis requires the call graph of the program, which can be obtained from MIR, and the stack
usage of each function in the program. Precise information about the later later can only be
obtained from LLVM as it depends on the optimization level and optimization options like LTO.
This PR does not attempt to add the ability to perform such whole program analysis to rustc;
it simply does the minimal amount of work to enable such analysis to be implemented out of tree.
Implementation
This PR exposes a way to set LLVM's
EmitStackSizeSectionoption from the command line. The optionis documented here; the documentation is copied below for convenience and posteriority:
Where the LLVM feature is not available (e.g. LLVM version < 6.0) or can't be applied (e.g. the
output format doesn't support sections e.g. .wasm files) the flag does nothing -- i.e. no error or
warning is emitted.
Example usage
Then a tool like
stack-sizescan be used to print the information in human readable formatStability
Like
-Z sanitizethis is a re-export of an LLVM feature. To me knowledge, we don't have a policyabout stabilization of such features as they are incompatible with, or demand extra implementation
effort from, alternative backends (e.g. cranelift). As such this feature will remain experimental /
unstable for the foreseeable future.
Unresolved questions
Section name
Should we rename the
.stack_sizessection to.debug_stacksizes?With the former name linkers will strip the section unless told otherwise using a linker script,
which means getting this information requires both knowledge about linker scripts and a custom
linker invocation (see example above).
If we use the
.debug_stacksizesname (I believe) linkers will always keep the section, which means-Z emit-stack-sizesis the only thing required to get the stack usage information.TODOsInvestigate why this doesn't work with thethumbtargets. I get the LLVM error shown below:which sounds like it might be related to therelocation-modeloption. Mayberelocation-model = staticis not supported for some reason?This fixed itself after the LLVM upgrade.
r? @nikomatsakis
cc @rust-lang/compiler @perlindgren @whitequark