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

Explicitly pass -no-pie #44067

Closed
wants to merge 1 commit into from
Closed

Conversation

sfackler
Copy link
Member

Some linkers (e.g. Debian and Arch's) that are configured to make PIEs
by default will make dynamically linked executables that don't actually
dynamically link to anything. Fix that by explicitly passing -no-pie in
those cases.

Closes #43647

r? @alexcrichton

Some linkers (e.g. Debian and Arch's) that are configured to make PIEs
by default will make dynamically linked executables that don't actually
dynamically link to anything. Fix that by explicitly passing -no-pie in
those cases.

Closes rust-lang#43647
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 24, 2017

📌 Commit 1d838a2 has been approved by alexcrichton

@aidanhs
Copy link
Member

aidanhs commented Aug 24, 2017

I'm 80% sure that #43647 is already fixed - comparing the link args , the one missing link flag before and after #40113 (aside from -l unwind cc #44069) is -static. Running rustc -C link-args="-static" --target x86_64-unknown-linux-musl x.rs on stable on an arch linux that reproduces the issue, produces a full static binary (as -static appears to default to -no-pie).

So I don't see what we're fixing by merging this PR. Even if -static defaulted to -pie, that's valid and we should respect the system default - fully static PIE binaries work fine.

All of this aside, I think we should postpone this PR so we can fix #44069 first.

@alexcrichton
Copy link
Member

@aidanhs do you think, though, that we shouldn't land this?

@alexcrichton alexcrichton added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 24, 2017
@aidanhs
Copy link
Member

aidanhs commented Aug 24, 2017

@alexcrichton that is correct. pie for shared executables is pretty widely accepted as A Good Thing, but the case is a bit murkier for static executables (until recently-ish I think it required some arcane incantations to get a pie static executable from gcc).

I think we should defer to the system linker and leave pie alone for static executables - most systems will end up with non-pie since that's what gcc seems to default to for -static, but I see no advantage to enforcing it (and some disadvantages, like not being able to opt-in to ASLR on a system-level).

@alexcrichton
Copy link
Member

@bors: r-

Ok @sfackler do you agree as well?

Should we close #43647

@aidanhs
Copy link
Member

aidanhs commented Aug 24, 2017

I'm going to test #43647 with the nightly tonight, I'll close if it seems fixed.

@sfackler
Copy link
Member Author

I'm happy as long as binaries that don't link to glibc or anything else dynamically don't depend on a dynamic loader.

@alexcrichton
Copy link
Member

Ok, if @aidanhs's tests come back green sounds like we can close!

@aidanhs
Copy link
Member

aidanhs commented Aug 25, 2017

Closing per #43647 (comment)

@aidanhs aidanhs closed this Aug 25, 2017
@sfackler sfackler deleted the musl-static branch August 26, 2017 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants