Rollup merge of #100984 - ChrisDenton:reinstate-init, r=Mark-Simulacrum

Reinstate preloading of some dll imports

I've now come around to the conclusion that there is a justification for pre-loading the synchronization functions `WaitOnAddress` and `WakeByAddressSingle`. I've found this to have a particularly impact in testing frameworks that may have short lived processes which immediately spawn lots of threads.

Also, because pre-main initializers imply a single-threaded environment, we can switch back to using relaxed atomics which might be a minor perf improvement on some platforms (though I doubt it's particularly notable).

r? ``@Mark-Simulacrum`` and sorry for the churn here.

For convenience I'll summarise previous issues with preloading and the solutions that are included in this PR (if any):

**Issue:** User pre-main initializers may be run before std's
**Solution:** The std now uses initializers that are guaranteed to run earlier than the old initializers. A note is also added that users should not copy std's behaviour if they want to ensure they run their initializers after std.

**Issue:** Miri does not understand pre-main initializers.
**Solution:** For miri only, run the function loading lazily instead.

**Issue:** We should ideally use `LoadLibrary` to get "api-ms-win-core-synch-l1-2-0". Only "ntdll" and "kernel32" are guaranteed to always be loaded.
**Solution:** None. We can't use `LoadLibrary` pre-main. However, in the past `GetModuleHandle` has always worked in practice so this should hopefully not be a problem.

If/when Windows 7 support is dropped, we can finally remove all this for good and just use normal imports.

diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs
index ef3f6a9ba175545c9b10c1a72df432989feb6121..891d7e855f0b9a218f30ca013451195a66380c64 100644 (file)
--- a/library/std/src/sys/windows/c.rs
+++ b/library/std/src/sys/windows/c.rs
@@ -228,8 +228,6 @@ fn clone(&self) -> Self {
 pub const IPV6_DROP_MEMBERSHIP: c_int = 13;
 pub const MSG_PEEK: c_int = 0x2;
 
-pub const LOAD_LIBRARY_SEARCH_SYSTEM32: u32 = 0x800;
-
 #[repr(C)]
 #[derive(Copy, Clone)]
 pub struct linger {
@@ -1032,7 +1030,6 @@ pub fn CreateFileW(
     pub fn GetProcAddress(handle: HMODULE, name: LPCSTR) -> *mut c_void;
     pub fn GetModuleHandleA(lpModuleName: LPCSTR) -> HMODULE;
     pub fn GetModuleHandleW(lpModuleName: LPCWSTR) -> HMODULE;
-    pub fn LoadLibraryExA(lplibfilename: *const i8, hfile: HANDLE, dwflags: u32) -> HINSTANCE;
 
     pub fn GetSystemTimeAsFileTime(lpSystemTimeAsFileTime: LPFILETIME);
     pub fn GetSystemInfo(lpSystemInfo: LPSYSTEM_INFO);

diff --git a/library/std/src/sys/windows/compat.rs b/library/std/src/sys/windows/compat.rs
index 9c8ddc3aa1d256f6ecbd43d067b388c54c8129b9..7dff81ecb8ddeebb6e4d938dad632c198727dc78 100644 (file)
--- a/library/std/src/sys/windows/compat.rs
+++ b/library/std/src/sys/windows/compat.rs
@@ -21,9 +21,52 @@
 
 use crate::ffi::{c_void, CStr};
 use crate::ptr::NonNull;
-use crate::sync::atomic::{AtomicBool, Ordering};
+use crate::sync::atomic::Ordering;
 use crate::sys::c;
 
+// This uses a static initializer to preload some imported functions.
+// The CRT (C runtime) executes static initializers before `main`
+// is called (for binaries) and before `DllMain` is called (for DLLs).
+//
+// It works by contributing a global symbol to the `.CRT$XCT` section.
+// The linker builds a table of all static initializer functions.
+// The CRT startup code then iterates that table, calling each
+// initializer function.
+//
+// NOTE: User code should instead use .CRT$XCU to reliably run after std's initializer.
+// If you're reading this and would like a guarantee here, please
+// file an issue for discussion; currently we don't guarantee any functionality
+// before main.
+// See https://docs.microsoft.com/en-us/cpp/c-runtime-library/crt-initialization?view=msvc-170
+#[used]
+#[link_section = ".CRT$XCT"]
+static INIT_TABLE_ENTRY: unsafe extern "C" fn() = init;
+
+/// Preload some imported functions.
+///
+/// Note that any functions included here will be unconditionally loaded in
+/// the final binary, regardless of whether or not they're actually used.
+///
+/// Therefore, this should be limited to `compat_fn_optional` functions which
+/// must be preloaded or any functions where lazier loading demonstrates a
+/// negative performance impact in practical situations.
+///
+/// Currently we only preload `WaitOnAddress` and `WakeByAddressSingle`.
+unsafe extern "C" fn init() {
+    // In an exe this code is executed before main() so is single threaded.
+    // In a DLL the system's loader lock will be held thereby synchronizing
+    // access. So the same best practices apply here as they do to running in DllMain:
+    // https://docs.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-best-practices
+    //
+    // DO NOT do anything interesting or complicated in this function! DO NOT call
+    // any Rust functions or CRT functions if those functions touch any global state,
+    // because this function runs during global initialization. For example, DO NOT
+    // do any dynamic allocation, don't call LoadLibrary, etc.
+
+    // Attempt to preload the synch functions.
+    load_synch_functions();
+}
+
 /// Helper macro for creating CStrs from literals and symbol names.
 macro_rules! ansi_str {
     (sym $ident:ident) => {{
@@ -75,20 +118,6 @@ pub unsafe fn new(name: &CStr) -> Option<Self> {
         NonNull::new(module).map(Self)
     }
 
-    /// Load the library (if not already loaded)
-    ///
-    /// # Safety
-    ///
-    /// The module must not be unloaded.
-    pub unsafe fn load_system_library(name: &CStr) -> Option<Self> {
-        let module = c::LoadLibraryExA(
-            name.as_ptr(),
-            crate::ptr::null_mut(),
-            c::LOAD_LIBRARY_SEARCH_SYSTEM32,
-        );
-        NonNull::new(module).map(Self)
-    }
-
     // Try to get the address of a function.
     pub fn proc_address(self, name: &CStr) -> Option<NonNull<c_void>> {
         // SAFETY:
@@ -182,14 +211,10 @@ pub mod $symbol {
 
                 #[inline(always)]
                 pub fn option() -> Option<F> {
-                    let f = PTR.load(Ordering::Acquire);
-                    if !f.is_null() { Some(unsafe { mem::transmute(f) }) } else { try_load() }
-                }
-
-                #[cold]
-                fn try_load() -> Option<F> {
-                    $load_functions;
-                    NonNull::new(PTR.load(Ordering::Acquire)).map(|f| unsafe { mem::transmute(f) })
+                    // Miri does not understand the way we do preloading
+                    // therefore load the function here instead.
+                    #[cfg(miri)] $load_functions;
+                    NonNull::new(PTR.load(Ordering::Relaxed)).map(|f| unsafe { mem::transmute(f) })
                 }
             }
         )+
@@ -205,17 +230,14 @@ fn try_load() -> Option<()> {
 
         // Try loading the library and all the required functions.
         // If any step fails, then they all fail.
-        let library = unsafe { Module::load_system_library(MODULE_NAME) }?;
+        let library = unsafe { Module::new(MODULE_NAME) }?;
         let wait_on_address = library.proc_address(WAIT_ON_ADDRESS)?;
         let wake_by_address_single = library.proc_address(WAKE_BY_ADDRESS_SINGLE)?;
 
-        c::WaitOnAddress::PTR.store(wait_on_address.as_ptr(), Ordering::Release);
-        c::WakeByAddressSingle::PTR.store(wake_by_address_single.as_ptr(), Ordering::Release);
+        c::WaitOnAddress::PTR.store(wait_on_address.as_ptr(), Ordering::Relaxed);
+        c::WakeByAddressSingle::PTR.store(wake_by_address_single.as_ptr(), Ordering::Relaxed);
         Some(())
     }
 
-    // Try to load the module but skip loading if a previous attempt failed.
-    static LOAD_MODULE: AtomicBool = AtomicBool::new(true);
-    let module_loaded = LOAD_MODULE.load(Ordering::Acquire) && try_load().is_some();
-    LOAD_MODULE.store(module_loaded, Ordering::Release)
+    try_load();
 }