auto merge of #17563 : brson/rust/wintcbfix, r=thestinger

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

Author: bors

src/librustc/middle/trans/context.rs

@@ -546,7 +546,7 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> {
     // but it could be enabled (with patched LLVM)
     pub fn is_split_stack_supported(&self) -> bool {
         let ref cfg = self.sess().targ_cfg;
-        cfg.os != abi::OsiOS || cfg.arch != abi::Arm
+        (cfg.os != abi::OsiOS || cfg.arch != abi::Arm) && cfg.os != abi::OsWindows
     }
 
 

src/librustdoc/lib.rs

@@ -89,47 +89,7 @@ local_data_key!(pub analysiskey: core::CrateAnalysis)
 type Output = (clean::Crate, Vec<plugins::PluginJson> );
 
 pub fn main() {
-    // Why run rustdoc in a separate task? That's a good question!
-    //
-    // We first begin our adventure at the ancient commit of e7c4fb69. In this
-    // commit it was discovered that the stack limit frobbing on windows ended
-    // up causing some syscalls to fail. This was worked around manually in the
-    // relevant location.
-    //
-    // Our journey now continues with #13259 where it was discovered that this
-    // stack limit frobbing has the ability to affect nearly any syscall. Note
-    // that the key idea here is that there is currently no knowledge as to why
-    // this is happening or how to preserve it, fun times!
-    //
-    // Now we continue along to #16275 where it was discovered that --test on
-    // windows didn't work at all! Yet curiously rustdoc worked without --test.
-    // The exact reason that #16275 cropped up is that during the expansion
-    // phase the compiler attempted to open libstd to read out its macros. This
-    // invoked the LLVMRustOpenArchive shim which in turned went to LLVM to go
-    // open a file and read it. Lo and behold this function returned an error!
-    // It was then discovered that when the same fix mentioned in #13259 was
-    // applied, the error went away. The plot thickens!
-    //
-    // Remember that rustdoc works without --test, which raises the question of
-    // how because the --test and non --test paths are almost identical. The
-    // first thing both paths do is parse and expand a crate! It turns out that
-    // the difference is that --test runs on the *main task* while the normal
-    // path runs in subtask. It turns out that running --test in a sub task also
-    // fixes the problem!
-    //
-    // So, in summary, it is unknown why this is necessary, what it is
-    // preventing, or what the actual bug is. In the meantime, this allows
-    // --test to work on windows, which seems good, right? Fun times.
-    let (tx, rx) = channel();
-    spawn(proc() {
-        std::os::set_exit_status(main_args(std::os::args().as_slice()));
-        tx.send(());
-    });
-
-    // If the task failed, set an error'd exit status
-    if rx.recv_opt().is_err() {
-        std::os::set_exit_status(std::rt::DEFAULT_ERROR_CODE);
-    }
+    std::os::set_exit_status(main_args(std::os::args().as_slice()));
 }
 
 pub fn opts() -> Vec<getopts::OptGroup> {

src/librustrt/stack.rs

@@ -200,11 +200,7 @@ pub unsafe fn record_sp_limit(limit: uint) {
         asm!("movq $0, %fs:112" :: "r"(limit) :: "volatile")
     }
     #[cfg(target_arch = "x86_64", target_os = "windows")] #[inline(always)]
-    unsafe fn target_record_sp_limit(limit: uint) {
-        // see: http://en.wikipedia.org/wiki/Win32_Thread_Information_Block
-        // store this inside of the "arbitrary data slot", but double the size
-        // because this is 64 bit instead of 32 bit
-        asm!("movq $0, %gs:0x28" :: "r"(limit) :: "volatile")
+    unsafe fn target_record_sp_limit(_: uint) {
     }
     #[cfg(target_arch = "x86_64", target_os = "freebsd")] #[inline(always)]
     unsafe fn target_record_sp_limit(limit: uint) {
@@ -228,10 +224,7 @@ pub unsafe fn record_sp_limit(limit: uint) {
         asm!("movl $0, %gs:48" :: "r"(limit) :: "volatile")
     }
     #[cfg(target_arch = "x86", target_os = "windows")] #[inline(always)]
-    unsafe fn target_record_sp_limit(limit: uint) {
-        // see: http://en.wikipedia.org/wiki/Win32_Thread_Information_Block
-        // store this inside of the "arbitrary data slot"
-        asm!("movl $0, %fs:0x14" :: "r"(limit) :: "volatile")
+    unsafe fn target_record_sp_limit(_: uint) {
     }
 
     // mips, arm - Some brave soul can port these to inline asm, but it's over
@@ -282,9 +275,7 @@ pub unsafe fn get_sp_limit() -> uint {
     }
     #[cfg(target_arch = "x86_64", target_os = "windows")] #[inline(always)]
     unsafe fn target_get_sp_limit() -> uint {
-        let limit;
-        asm!("movq %gs:0x28, $0" : "=r"(limit) ::: "volatile");
-        return limit;
+        return 1024;
     }
     #[cfg(target_arch = "x86_64", target_os = "freebsd")] #[inline(always)]
     unsafe fn target_get_sp_limit() -> uint {
@@ -318,9 +309,7 @@ pub unsafe fn get_sp_limit() -> uint {
     }
     #[cfg(target_arch = "x86", target_os = "windows")] #[inline(always)]
     unsafe fn target_get_sp_limit() -> uint {
-        let limit;
-        asm!("movl %fs:0x14, $0" : "=r"(limit) ::: "volatile");
-        return limit;
+        return 1024;
     }
 
     // mips, arm - Some brave soul can port these to inline asm, but it's over

src/libstd/rand/os.rs

@@ -136,7 +136,6 @@ mod imp {
     use os;
     use rand::Rng;
     use result::{Ok, Err};
-    use rt::stack;
     use self::libc::{DWORD, BYTE, LPCSTR, BOOL};
     use self::libc::types::os::arch::extra::{LONG_PTR};
     use slice::MutableSlice;
@@ -159,7 +158,6 @@ mod imp {
     static PROV_RSA_FULL: DWORD = 1;
     static CRYPT_SILENT: DWORD = 64;
     static CRYPT_VERIFYCONTEXT: DWORD = 0xF0000000;
-    static NTE_BAD_SIGNATURE: DWORD = 0x80090006;
 
     #[allow(non_snake_case)]
     extern "system" {
@@ -178,48 +176,12 @@ mod imp {
         /// Create a new `OsRng`.
         pub fn new() -> IoResult<OsRng> {
             let mut hcp = 0;
-            let mut ret = unsafe {
+            let ret = unsafe {
                 CryptAcquireContextA(&mut hcp, 0 as LPCSTR, 0 as LPCSTR,
                                      PROV_RSA_FULL,
                                      CRYPT_VERIFYCONTEXT | CRYPT_SILENT)
             };
 
-            // FIXME #13259:
-            // It turns out that if we can't acquire a context with the
-            // NTE_BAD_SIGNATURE error code, the documentation states:
-            //
-            //     The provider DLL signature could not be verified. Either the
-            //     DLL or the digital signature has been tampered with.
-            //
-            // Sounds fishy, no? As it turns out, our signature can be bad
-            // because our Thread Information Block (TIB) isn't exactly what it
-            // expects. As to why, I have no idea. The only data we store in the
-            // TIB is the stack limit for each thread, but apparently that's
-            // enough to make the signature valid.
-            //
-            // Furthermore, this error only happens the *first* time we call
-            // CryptAcquireContext, so we don't have to worry about future
-            // calls.
-            //
-            // Anyway, the fix employed here is that if we see this error, we
-            // pray that we're not close to the end of the stack, temporarily
-            // set the stack limit to 0 (what the TIB originally was), acquire a
-            // context, and then reset the stack limit.
-            //
-            // Again, I'm not sure why this is the fix, nor why we're getting
-            // this error. All I can say is that this seems to allow libnative
-            // to progress where it otherwise would be hindered. Who knew?
-            if ret == 0 && os::errno() as DWORD == NTE_BAD_SIGNATURE {
-                unsafe {
-                    let limit = stack::get_sp_limit();
-                    stack::record_sp_limit(0);
-                    ret = CryptAcquireContextA(&mut hcp, 0 as LPCSTR, 0 as LPCSTR,
-                                               PROV_RSA_FULL,
-                                               CRYPT_VERIFYCONTEXT | CRYPT_SILENT);
-                    stack::record_sp_limit(limit);
-                }
-            }
-
             if ret == 0 {
                 Err(IoError::last_error())
             } else {

src/test/run-pass/issue-13259-windows-tcb-trash.rs

@@ -0,0 +1,48 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+extern crate libc;
+
+#[cfg(windows)]
+mod imp {
+    use libc::{c_void, LPVOID, DWORD};
+    use libc::types::os::arch::extra::LPWSTR;
+
+    extern "system" {
+        fn FormatMessageW(flags: DWORD,
+                          lpSrc: LPVOID,
+                          msgId: DWORD,
+                          langId: DWORD,
+                          buf: LPWSTR,
+                          nsize: DWORD,
+                          args: *const c_void)
+                          -> DWORD;
+    }
+
+    pub fn test() {
+        let mut buf: [u16, ..50] = [0, ..50];
+        let ret = unsafe {
+            FormatMessageW(0x1000, 0 as *mut c_void, 1, 0x400,
+                           buf.as_mut_ptr(), buf.len() as u32, 0 as *const c_void)
+        };
+        // On some 32-bit Windowses (Win7-8 at least) this will fail with segmented
+        // stacks taking control of pvArbitrary
+        assert!(ret != 0);
+    }
+}
+
+#[cfg(not(windows))]
+mod imp {
+    pub fn test() { }
+}
+
+fn main() {
+    imp::test()
+}

src/test/run-pass/out-of-stack.rs

@@ -42,11 +42,17 @@ fn main() {
         let silent = Command::new(args[0].as_slice()).arg("silent").output().unwrap();
         assert!(!silent.status.success());
         let error = String::from_utf8_lossy(silent.error.as_slice());
-        assert!(error.as_slice().contains("has overflowed its stack"));
+        // FIXME #17562: Windows is using stack probes and isn't wired up to print an error
+        if !cfg!(windows) {
+            assert!(error.as_slice().contains("has overflowed its stack"));
+        }
 
         let loud = Command::new(args[0].as_slice()).arg("loud").output().unwrap();
         assert!(!loud.status.success());
         let error = String::from_utf8_lossy(silent.error.as_slice());
-        assert!(error.as_slice().contains("has overflowed its stack"));
+        // FIXME #17562: Windows is using stack probes and isn't wired up to print an error
+        if !cfg!(windows) {
+            assert!(error.as_slice().contains("has overflowed its stack"));
+        }
     }
 }