This repository was archived by the owner on Jan 24, 2022. It is now read-only.
do not KEEP the .stack_sizes section#186
Merged
Merged
Conversation
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.
Member
Author
PR in question: rust-lang/rust#59401 |
Contributor
|
bors r+ |
bors Bot
added a commit
that referenced
this pull request
Mar 24, 2019
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 <jorge@japaric.io>
Contributor
Build failed |
Member
Author
|
compiletest v0.3.19 doesn't compile with the latest nightly. We can either wait or mark the nightly build as |
Contributor
|
@japaric I'm feeling that is the only right thing to do. Using CI on nightly is a total gamble and waste of time. |
Centril
added a commit
to Centril/rust
that referenced
this pull request
Mar 29, 2019
…es, r=alexcrichton bootstrap: build crates under libtest with -Z emit-stack-sizes Please see the comment in the diff for the rationale. This change adds a `.stack_sizes` linker section to `libcompiler_builtins.rlib` but this section is discarded by the linker by default so it won't affect the binary size of most programs. It will, however, negatively affect the binary size of programs that link to a recent release of the `cortex-m-rt` crate because of the linker script that crate provides, but I have proposed a PR (rust-embedded/cortex-m-rt#186) to solve the problem (which I originally introduced :-)). This change does increase the size of the `libcompiler_builtins.rlib` artifact we distribute but the increase is in the order of (a few) KBs. r? @alexcrichton
Contributor
|
@japaric Rebase with master to get CI to pass |
Member
Author
|
@korken89 there are no merge conflicts; bors should work bors r=therealprof |
bors Bot
added a commit
that referenced
this pull request
Apr 2, 2019
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 <jorge@japaric.io>
Contributor
Build succeeded |
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
this undoes PR #118
I found a problem with keeping this section. Turns out that the input
.stack_sizessections contain references to all functions compiled with-Z emit-stack-sizes(the section contains the addresses of all those functionsafter all) so keeping those section prevents the linker from removing any of
those functions. This is not a problem today because
-Z emit-stack-sizesisopt-in and only used to analyze a program.
However, I am proposing a rust-lang/rust PR to build the
compiler-builtinscrate with
-Z emit-stack-sizes. That change will cause all the functions inthat crate to be kept in binaries that link to
cortex-m-rt, regardless ofwhether the crate author uses
-Z emit-stack-sizesor not, leading a increasein binary size of about 14 KB (
.textsection).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-sizesandcargo-call-stack) no longer do so in their latest releases so nothing is loston the tooling front with this change.
r? @rust-embedded/cortex-m