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

Let -Crelocation-model better control -pie linking #40245

Merged
merged 1 commit into from
Mar 4, 2017

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Mar 3, 2017

Prior to this, if relocation model in the target options was "pic", as
most targets have it, then the user's -Crelocation-model had no effect
on whether -pie would be used. Only -Clink-arg=-static would have a
further override here.

Now we use context::get_reloc_model, which gives precedence to the
user's option, and if it's RelocMode::PIC we enable -pie. This is
the same test as context::is_pie_binary, except that folds across all
sess.crate_types, not just the current one.

Fixes #35061.

Prior to this, if relocation model in the target options was "pic", as
most targets have it, then the user's `-Crelocation-model` had no effect
on whether `-pie` would be used.  Only `-Clink-arg=-static` would have a
further override here.

Now we use `context::get_reloc_model`, which gives precedence to the
user's option, and if it's `RelocMode::PIC` we enable `-pie`.  This is
the same test as `context::is_pie_binary`, except that folds across all
`sess.crate_types`, not just the current one.
@rust-highfive
Copy link
Collaborator

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

@cuviper
Copy link
Member Author

cuviper commented Mar 3, 2017

I first noticed this in #37874 (comment), but #35061 is more direct.

I'd like a test for this, but I'm not sure how. It will already get exercised in run-make/relocation-model, which ensures nothing fundamentally broke here, but that doesn't actually test the mode of the resulting binary. I could perhaps call readelf -h, looking for DYN or EXEC, but this isn't portable. /usr/bin/file also calls it shared object or executable, but I'm not sure of its portability either.

@alexcrichton
Copy link
Member

@bors: r+

Nah I think it's ok to avoid a test here in this case, thanks!

@bors
Copy link
Contributor

bors commented Mar 3, 2017

📌 Commit 111fbe7 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Mar 4, 2017

⌛ Testing commit 111fbe7 with merge 27918f3...

bors added a commit that referenced this pull request Mar 4, 2017
Let `-Crelocation-model` better control `-pie` linking

Prior to this, if relocation model in the target options was "pic", as
most targets have it, then the user's `-Crelocation-model` had no effect
on whether `-pie` would be used.  Only `-Clink-arg=-static` would have a
further override here.

Now we use `context::get_reloc_model`, which gives precedence to the
user's option, and if it's `RelocMode::PIC` we enable `-pie`.  This is
the same test as `context::is_pie_binary`, except that folds across all
`sess.crate_types`, not just the current one.

Fixes #35061.
@bors
Copy link
Contributor

bors commented Mar 4, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 27918f3 to master...

@bors bors merged commit 111fbe7 into rust-lang:master Mar 4, 2017
@cuviper cuviper deleted the maybe-not-pie branch September 26, 2017 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants