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

Source file names are included into a release binary even if abort upon panic is enabled #75263

Closed
dmitry-zakablukov opened this issue Aug 7, 2020 · 32 comments
Labels
I-heavy Issue: Problems and improvements with respect to binary size of generated code. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@dmitry-zakablukov
Copy link

dmitry-zakablukov commented Aug 7, 2020

Good day!

I have found, that if a library contains code that may panic (slices, unwraps, etc.), then a filename of such source file will be included in a binary. Release build doesn't change this behavior, neither debug symbols stripping do.

I have tried to turn on "abort" for panic in release profile. Even though this resulted in a smaller binary size, it doesn't wipe out source file names from the binary.

Here is a small reproducible code.

// "engine" library, file lib.rs

pub fn print_message() {
    let mut input = String::new();
    std::io::stdin().read_line(&mut input).unwrap();
}
// "main" program, file main.rs

fn main() {
    engine::print_message();
}
// main Cargo.toml

[package]
name = "symbols_test"
version = "0.1.0"
authors = ["Name Soname <[email protected]>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[profile.release]
panic = "abort"

[dependencies]
engine = { path = "../engine" }

If I build this example with a command cargo build --release on Windows, then a symbols_test.exe will contain a string engine\src\lib.rs.

==============================

So what should I do, if I want to distribute my program to the clients and do not want them to find out file names of my source files (which are a business secret)? How to strip source file names from the binary?

UPD: How to reproduce the problem on Windows:

git clone https://github.com/dmitry-zakablukov/rust-lang-issue-75263.git
cd rust-lang-issue-75263\symbols_test
cargo build --release
strings target\release\symbols_test.exe | grep "engine\src"

On *nix systems change slashes to '/' and remove '.exe' in the last command.

@ghost
Copy link

ghost commented Sep 16, 2020

dont use rust. see #40552 privacy isnt a thing they want to think or fix. paths should be less issue than the username in your binary

@jyn514 jyn514 added the I-heavy Issue: Problems and improvements with respect to binary size of generated code. label Sep 16, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 16, 2020

panic=abort will still run the panic hook I'm pretty sure, which will print out the line where the panic occurred. So it's not needlessly being added. Maybe there's some option to set a different panic hook?

@Dylan-DPC-zz Dylan-DPC-zz added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Sep 16, 2020
@Dylan-DPC-zz
Copy link

Marking this for prioritisation request as some people have privacy concerns over file names showing up in their production apps

@sfackler
Copy link
Member

Why is --remap-path-prefix not sufficient to deal with privacy concerns?

@Monadic-Cat
Copy link
Contributor

I've never heard of --remap-path-prefix until today. Testing it out, it doesn't do enough.

I went ahead and threw this into a project's .cargo/config:

[build]
rustflags = ["--remap-path-prefix=/home/jmn/Projects/mprojects/mbot=src", "--remap-path-prefix=/home/jmn/.cargo=cargo", "--remap-path-prefix=/home/jmn/.rustup=rustup"]

It did get rid of mentions of /home/jmn/Projects/mprojects/mbot, but I'm still seeing /home/jmn/.rustup/toolchains/... stuff in strings output, even after stripping the binary.

@apiraino
Copy link
Contributor

Assigning P-medium as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

@apiraino apiraino added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 17, 2020
@unpavedmop
Copy link

anyone from compiler team going to comment on this or just ignore it? workarounds dont actually work

@ssokolow
Copy link

@unpavedmop This is a piece of team infrastructure that's open to the public, not a discussion forum. The "comment" was "Marking this for prioritisation request as some people have privacy concerns over file names showing up in their production apps" and receiving a P-medium tag.

If you disagree, check out P-high to see what kinds of bugs they consider more pressing concerns.

@jyn514
Copy link
Member

jyn514 commented Feb 12, 2021

It did get rid of mentions of /home/jmn/Projects/mprojects/mbot, but I'm still seeing /home/jmn/.rustup/toolchains/... stuff in strings output, even after stripping the binary.

This sounds like the compiler version is being embedded in the binary.

I've never heard of --remap-path-prefix until today. Testing it out, it doesn't do enough.

I went ahead and threw this into a project's .cargo/config:

[build]
rustflags = ["--remap-path-prefix=/home/jmn/Projects/mprojects/mbot=src", "--remap-path-prefix=/home/jmn/.cargo=cargo", "--remap-path-prefix=/home/jmn/.rustup=rustup"]

It did get rid of mentions of /home/jmn/Projects/mprojects/mbot, but I'm still seeing /home/jmn/.rustup/toolchains/... stuff in strings output, even after stripping the binary.

I can't reproduce this.

$ cargo new hello-world
$ cd hello-world
$ echo '[profile.release]
panic = "abort"' >> Cargo.toml
$ RUSTFLAGS="--remap-path-prefix=/home/joshua/test-rustdoc/hello-world=src --remap-path-prefix=/home/joshua/.local/lib/cargo=cargo --remap-path-prefix=/home/joshua/.local/lib/rustup=rustup" cargo run --release
$ strings /home/joshua/.local/lib/cargo/target/release/hello-world | grep home
home_dir
_ZN3std3sys4unix2os8home_dir17h5d3b091b9e7ed062E
home_dir
_ZN3std3sys4unix2os8home_dir8fallback17hb3ac94fa9a85ddbaE
_ZN3std3sys4unix2os8home_dir28_$u7b$$u7b$closure$u7d$$u7d$17ha072306bde63d38dE
_ZN3std3env8home_dir17h055eda4c1c3735acE

@jyn514
Copy link
Member

jyn514 commented Feb 12, 2021

I tried using a library like in the original reproduction instructions, and if I modify remap-path-prefix slightly it still works fine:

> RUSTFLAGS="--remap-path-prefix=/home/joshua/src/rust/test-rustdoc=src --remap-path-prefix=/home/joshua/.local/lib/cargo=cargo --remap-path-prefix=/home/joshua/.local/lib/rustup=rustup" cargo run --release
   Compiling engine v0.1.0 (/home/joshua/src/rust/test-rustdoc/engine)
   Compiling hello-world v0.1.0 (/home/joshua/src/rust/test-rustdoc/hello-world)
    Finished release [optimized] target(s) in 0.52s
     Running `/home/joshua/.local/lib/cargo/target/release/hello-world`
$ strings /home/joshua/.local/lib/cargo/target/release/hello-world | grep home
home_dir
_ZN3std3sys4unix2os8home_dir17h5d3b091b9e7ed062E
home_dir
_ZN3std3sys4unix2os8home_dir8fallback17hb3ac94fa9a85ddbaE
_ZN3std3sys4unix2os8home_dir28_$u7b$$u7b$closure$u7d$$u7d$17ha072306bde63d38dE
_ZN3std3env8home_dir17h055eda4c1c3735acE

@unpavedmop
Copy link

@jyn514 how do you propose that in a cargo.toml otherwise devs cant expect to tell people use this set of flags, and its hardcoded path

@ssokolow
Copy link

ssokolow commented Feb 12, 2021

@unpavedmop You put it in your README.

Same way ripgrep instructs people to use a nightly compiler and RUSTFLAGS="-C target-cpu=native" cargo build --release --features 'simd-accel' for best performance.

It's your users' fault if they don't follow the official build instructions, same way it would be if they patched the code and introduced a privacy problem that way.

@jyn514
Copy link
Member

jyn514 commented Feb 12, 2021

Hmm, I think this is duplicate of #40552 (or the other way around).

@ssokolow

This comment has been minimized.

@ssokolow

This comment has been minimized.

@ssokolow

This comment has been minimized.

@jyn514
Copy link
Member

jyn514 commented Apr 11, 2021

@Subsentient could you please paste the exact commands you're running to replicate this? I'm running

cargo new hello-world
cd hello-world
echo '[profile.release]
panic = "abort"' >> Cargo.toml
RUSTFLAGS="--remap-path-prefix=/home/joshua/test-rustdoc/hello-world=src --remap-path-prefix=/home/joshua/.local/lib/cargo=cargo --remap-path-prefix=/home/joshua/.local/lib/rustup=rustup" cargo run --release
strings /home/joshua/.local/lib/cargo/target/release/hello-world | grep home

I've tried this on two different machines (one running Ubuntu 20.04, the other running Debian 10) and neither reproduces the issue.

@dmitry-zakablukov
Copy link
Author

@jyn514 , but what about the test in my original message? Remap flags are helpful to hide the names of the parent folders of a project. But these flags can't help to hide a relative source file name.
P.S. It is still annoying to track down all parent folders names and can be easily messed up.

@dmitry-zakablukov
Copy link
Author

@Subsentient , what story in the news are you talking about? Can you please provide a link to it?

@daxpedda
Copy link
Contributor

See #66955.
Which maybe might be why people are having re-production problems?

To summarize: there is a bug that --remap-path-prefix doesn't cause proper re-compilation and requires cargo clean to work properly.

@BurntSushi
Copy link
Member

Moderation note: Folks, please keep comments directed toward constructively resolving the problem at hand, just like you would any other issue. Further unconstructive meta commentary will be removed.

If you have questions about our moderation, please do not respond here. Instead, please email the mods at [email protected].

@jyn514
Copy link
Member

jyn514 commented Apr 11, 2021

Ok, this is related to release vs debug mode somehow, panic=abort has nothing to do with it. With a blank Cargo.toml this works properly:

$ RUSTFLAGS=--remap-path-prefix=/home/joshua/test-rustdoc/hello-world=src' --remap-path-prefix=/home/joshua/.local/lib/cargo=cargo --remap-path-prefix=/home/joshua/.local/lib/rustup=rustup' cargo run --release
$ strings target/release/hello-world | grep joshua | wc -l
0

but the same thing without --release (and without incremental) does not:

$ CARGO_INCREMENTAL=0 RUSTFLAGS=--remap-path-prefix=/home/joshua/test-rustdoc/hello-world=src' --remap-path-prefix=/home/joshua/.local/lib/cargo=cargo --remap-path-prefix=/home/joshua/.local/lib/rustup=rustup' cargo run -q
Hello, world!
$ strings target/debug/hello-world | grep joshua | wc -l
8

@dmitry-zakablukov
Copy link
Author

@Subsentient , @jyn514 , here is instruction to reproduce original problem (instruction was added to the first message).

git clone https://github.com/dmitry-zakablukov/rust-lang-issue-75263.git
cd rust-lang-issue-75263\symbols_test
cargo build --release
strings target\release\symbols_test.exe | grep "engine\src"

@jyn514
Copy link
Member

jyn514 commented Apr 12, 2021

@dmitry-zakablukov I was able to omit those symbols altogether with cargo run -Z build-std --target x86_64-unknown-linux-gnu -Zbuild-std-features=panic_immediate_abort --release. It doesn't seem to work in debug mode (maybe because it's not stripping dead code?) but I would open a separate issue for that.

Again, the reason this happens with panic=abort is because that still runs the panic hook: #75263 (comment). I would expect if you override the panic hook that might also let the compiler optimize out the strings: https://doc.rust-lang.org/std/panic/fn.set_hook.html.


I'm going to close this issue because there have been many different unrelated issues reported and most of them are being drowned in drama. I've opened #84125 for the issue with --remap-path-prefix I ran into at the end, and there are already some other related open issues, such as #66955. If you think --remap-path-prefix should be passed by default, see #40552.

@Monadic-Cat I couldn't reproduce the bug in #75263 (comment) - if you run into it again in release mode, could you please open a new issue? It might be #73167 if you have rust-src installed. Debug mode is tracked by #84125.

@jyn514 jyn514 closed this as completed Apr 12, 2021
@dmitry-zakablukov
Copy link
Author

@jyn514 , setting custom panic hook only doesn't resolve my issue.
I cannot afford to use a nightly compiler, so the workaround suggested by you is not suitable for me. How soon can I expect a panic_immediate_abort feature to be merged into stable compiler? And why this feature is not enabled even though I set a panic = "abort"?

@jyn514
Copy link
Member

jyn514 commented Apr 12, 2021

How soon can I expect a panic_immediate_abort feature to be merged into stable compiler?

I'm not sure there is a plan to stabilize this, I don't see a tracking issue.

And why this feature is not enabled even though I set a panic = "abort"?

Because they do different things. Aborting immediately is different from printing a panic message and then aborting, and is much harder to debug. In particular, it doesn't allow setting RUST_BACKTRACE=1 to see where the panic happens.

@Monadic-Cat
Copy link
Contributor

I did have rust-src installed, yeah.
Removing the rust-src component and building in release mode has indeed removed the rustup paths from the final binary, so you're probably right.

@jyn514
Copy link
Member

jyn514 commented Apr 12, 2021

I'm not sure there is a plan to stabilize this, I don't see a tracking issue.

Oh sorry I misunderstood - the tracking repo for -Z build-std is https://github.com/rust-lang/wg-cargo-std-aware.

@crumblingstatue
Copy link
Contributor

crumblingstatue commented May 1, 2021

@jyn514

This sounds like the compiler version is being embedded in the binary.

Is there a way to remove that information? Since --remap-prefix doesn't seem to affect it.

@crumblingstatue
Copy link
Contributor

A workaround I found to remove the home dir data from the .wasm binary I'm distributing:

-Z build-std generates duplicate lang item errors, but building with xargo worked.

Xargo.toml:

[dependencies]
std = {default-features=false, features=["panic_immediate_abort"]}

xargo build --target=wasm32-unknown-unknown --release

This removes all mentions of my home dir from the .wasm binary.

@jyn514
Copy link
Member

jyn514 commented May 1, 2021

-Z build-std generates duplicate lang item errors,

Can you open an issue about that with more details?

@crumblingstatue
Copy link
Contributor

Can you open an issue about that with more details?

Looks like there are already similar issues about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-heavy Issue: Problems and improvements with respect to binary size of generated code. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests