-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Use stack probes instead of split stacks for overflow protection on windows #17563
Conversation
There's at least one other place to fix: |
Added the rustdoc fix. |
This is still not quite right. 'probe-stack' is not needed (and doesn't do anything); llvm automatically generates stack probes on windows. |
Should be complete now. |
The test needs to be ignored / stubbed out on non-Windows platforms. An alternative is having a test of |
This is the bare minimum to stop using split stacks on Windows, fixing #13259 and #14742, by turning on stack probes for all functions and disabling compiler and runtime support for split stacks on Windows. It does not restore the out-of-stack error message, which requires more runtime work. This includes a test that the Windows TCB is no longer being clobbered, but the out-of-stack test itself is pretty weak, only testing that the program exits abnormally, not that it isn't writing to bogus memory, so I haven't truly verified that this is providing the safety we claim. A more complete solution is in #16388, which has some unresolved issues yet. cc @Zoxc @klutzy @vadimcn
This is the bare minimum to stop using split stacks on Windows, fixing #13259 and #14742, by turning on stack probes for all functions and disabling compiler and runtime support for split stacks on Windows.
It does not restore the out-of-stack error message, which requires more runtime work.
This includes a test that the Windows TCB is no longer being clobbered, but the out-of-stack test itself is pretty weak, only testing that the program exits abnormally, not that it isn't writing to bogus memory, so I haven't truly verified that this is providing the safety we claim.
A more complete solution is in #16388, which has some unresolved issues yet.
cc @Zoxc @klutzy @vadimcn