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

rust: enable Control Flow Guard on gnullvm #22325

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mati865
Copy link
Collaborator

@mati865 mati865 commented Oct 27, 2024

It works but not sure whether we should do it.

@ognevny
Copy link
Collaborator

ognevny commented Oct 28, 2024

for a future reference, why can't we enable it for other environments? it depends on support from local mingw-w64 runtime, right?

@mati865
Copy link
Collaborator Author

mati865 commented Oct 28, 2024

I didn't test it, but I suppose the whole toolchain has to support it.

mati865 and others added 2 commits October 28, 2024 18:18
@mati865 mati865 marked this pull request as ready for review October 28, 2024 17:28
@lazka
Copy link
Member

lazka commented Oct 29, 2024

Just to clarify, does this enable it

  • for building rust itself?
  • for things built with rustc?
  • for users to enable it with rustc?

and are there any differences to the official upstream builds?

@mati865
Copy link
Collaborator Author

mati865 commented Oct 29, 2024

for building rust itself?

It's enabled for libstd (equivalent of libstdc++ from GCC), that is used by Rust and things built with Rust. The comment mentions only libstd, but I think it might be enabled for the compiler as well.

for things built with rustc?

Not explicitly, it's similar to mingw-w64. Some checks will be pulled from the library, but to fully benefit from the feature (at a small performance penalty), it has to be enabled manually.

for users to enable it with rustc?

It allows enabling that option.

and are there any differences to the official upstream builds?

AFAICT upstream doesn't enable it in any provided builds.

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.

4 participants