Skip to content

Commit

Permalink
Rollup merge of #100984 - ChrisDenton:reinstate-init, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
matthiaskrgr authored Aug 31, 2022
2 parents ea9c370 + 7bb47a6 commit b2a8d9d
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 33 deletions.
3 changes: 0 additions & 3 deletions library/std/src/sys/windows/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,6 @@ pub const IPV6_ADD_MEMBERSHIP: c_int = 12;
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 {
Expand Down Expand Up @@ -1032,7 +1030,6 @@ extern "system" {
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);
Expand Down
82 changes: 52 additions & 30 deletions library/std/src/sys/windows/compat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {{
Expand Down Expand Up @@ -75,20 +118,6 @@ impl Module {
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:
Expand Down Expand Up @@ -182,14 +211,10 @@ macro_rules! compat_fn_optional {

#[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) })
}
}
)+
Expand All @@ -205,17 +230,14 @@ pub(super) fn load_synch_functions() {

// 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();
}

0 comments on commit b2a8d9d

Please sign in to comment.