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

Enable /LARGEADDRESSAWARE & /SAFESEH for i686-pc-windows-msvc #31579

Merged
merged 2 commits into from
Feb 13, 2016

Conversation

ollie27
Copy link
Member

@ollie27 ollie27 commented Feb 11, 2016

/LARGEADDRESSAWARE is already enabled for i686-pc-windows-gnu so we should probably be consistent.
https://msdn.microsoft.com/en-us/library/wz223b1z.aspx

/SAFESEH is a good thing to enable by default.
https://msdn.microsoft.com/en-us/library/9a89h429.aspx

@rust-highfive
Copy link
Collaborator

r? @arielb1

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

@alexcrichton
Copy link
Member

Thanks for the PR! Some thoughts of mine:

  • The i686-pc-windows-msvc target is basically nightly-only right now, so I think we've got room for subtle breakage here and there. This may cause problems, but it's probably good to weed them out!
  • I'm not actually sure what "large address aware" implies? According to the documentation it means "can handle addresses larger than 2 gigabytes", but if we're already passing it for the GNU target then seems fine to me!
  • Similarly, I don't actually know what "safe exception handling" is, and it looks like this basically just verifies that the whole image is safeseh compatible, but it also seems like it doesn't hurt! In the past Gecko has had problems with this, but this seems like good impetus to weed out problems and/or ensure that we're generally always compatible.

I think that this probably has the most implications in linking in native code to a Rust binary or Rust library, but we already do some stuff like that for Unix. All-in-all this looks good to me, thanks @ollie27!

@bors: r+ c3320c0

cc @rust-lang/tools (just as a heads up)

@alexcrichton alexcrichton assigned alexcrichton and unassigned arielb1 Feb 11, 2016
bors added a commit that referenced this pull request Feb 13, 2016
/LARGEADDRESSAWARE is already enabled for i686-pc-windows-gnu so we should probably be consistent.
https://msdn.microsoft.com/en-us/library/wz223b1z.aspx

/SAFESEH is a good thing to enable by default.
https://msdn.microsoft.com/en-us/library/9a89h429.aspx
@bors
Copy link
Contributor

bors commented Feb 13, 2016

⌛ Testing commit c3320c0 with merge 5367776...

@bors bors merged commit c3320c0 into rust-lang:master Feb 13, 2016
@ollie27 ollie27 deleted the msvc_link branch May 2, 2016 18:07
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