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 Control Flow Guard in rustbuild #68824

Merged
merged 1 commit into from
Feb 11, 2020

Conversation

ajpaverd
Copy link
Contributor

@ajpaverd ajpaverd commented Feb 4, 2020

Now that Rust supports Control Flow Guard (#68180), add a config.toml option to build the standard library with CFG enabled.

r? @nagisa

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 4, 2020
@nagisa
Copy link
Member

nagisa commented Feb 4, 2020

@bors r+ option disabled by default, no danger in adding it.

@bors
Copy link
Contributor

bors commented Feb 4, 2020

📌 Commit ede8019409194d2ce02f2e5188c706f90f504e70 has been approved by nagisa

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 4, 2020
@ajpaverd
Copy link
Contributor Author

ajpaverd commented Feb 4, 2020

Thanks for the super fast review @nagisa!

src/bootstrap/builder.rs Outdated Show resolved Hide resolved
src/bootstrap/builder.rs Outdated Show resolved Hide resolved
@nagisa
Copy link
Member

nagisa commented Feb 4, 2020

@bors r- on commentary above.

r? @Mark-Simulacrum

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 4, 2020
@nikic
Copy link
Contributor

nikic commented Feb 4, 2020

Isn't this something that should be happening through -Z build-std, similar to how it is (since recently) done for sanitizer builds?

@ajpaverd
Copy link
Contributor Author

ajpaverd commented Feb 5, 2020

Isn't this something that should be happening through -Z build-std, similar to how it is (since recently) done for sanitizer builds?

Thanks for the suggestion @nikic - I hadn't seen -Z build-std. Did I understand correctly that this approach would not require changes to rustbuild (i.e. libstd would be rebuilt with Control Flow Guard enabled when the -Z control_flow_guard flag was set for rustc)?

On the other hand, is there some way to avoid rebuilding libstd every time and always use the same version (with Control Flow Guard enabled)?

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-7 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-02-10T14:49:03.6652346Z ========================== Starting Command Output ===========================
2020-02-10T14:49:03.6669792Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/135719e1-3aa1-4232-8b1b-2e2177705451.sh
2020-02-10T14:49:03.6869039Z 
2020-02-10T14:49:03.6928806Z ##[section]Finishing: Disable git automatic line ending conversion
2020-02-10T14:49:03.6934348Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/68824/merge to s
2020-02-10T14:49:03.6936203Z Task         : Get sources
2020-02-10T14:49:03.6936233Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-10T14:49:03.6936280Z Version      : 1.0.0
2020-02-10T14:49:03.6936310Z Author       : Microsoft
---
2020-02-10T14:49:04.5516567Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-02-10T14:49:04.5604040Z ##[command]git config gc.auto 0
2020-02-10T14:49:04.5683988Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-02-10T14:49:04.5781797Z ##[command]git config --get-all http.proxy
2020-02-10T14:49:04.5922275Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/68824/merge:refs/remotes/pull/68824/merge

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me with comment resolved

src/bootstrap/builder.rs Show resolved Hide resolved
@Mark-Simulacrum
Copy link
Member

Please also squash the commits, forgot to mention that in my previous comment.

@Mark-Simulacrum
Copy link
Member

@bors r+

@ajpaverd
Copy link
Contributor Author

Thanks @Mark-Simulacrum! Is bors still waiting for me to do something here?

@nikic
Copy link
Contributor

nikic commented Feb 11, 2020

@bors r=Mark-Simulacrum

Looks like bors just missed this.

@bors
Copy link
Contributor

bors commented Feb 11, 2020

📌 Commit 87df124 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 11, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 11, 2020
…Simulacrum

Enable Control Flow Guard in rustbuild

Now that Rust supports Control Flow Guard (rust-lang#68180), add a config.toml option to build the standard library with CFG enabled.

r? @nagisa
bors added a commit that referenced this pull request Feb 11, 2020
Rollup of 8 pull requests

Successful merges:

 - #66498 (Remove unused feature gates)
 - #68816 (Tweak borrow error on `FnMut` when `Fn` is expected)
 - #68824 (Enable Control Flow Guard in rustbuild)
 - #69022 (traits: preallocate 2 Vecs of known initial size)
 - #69031 (Use `dyn Trait` more in tests)
 - #69044 (Don't run coherence twice for future-compat lints)
 - #69047 (Don't rustfmt check the vendor directory.)
 - #69055 (Clean up E0307 explanation)

Failed merges:

r? @ghost
@bors bors merged commit 87df124 into rust-lang:master Feb 11, 2020
This pull request was closed.
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.

6 participants