From a21c2d870470b07d559faeaf88e7d16c765fc6ef Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 23 Apr 2024 09:32:35 +0200 Subject: windows fill_utf16_buf: explain the expected return value --- library/std/src/sys/pal/windows/mod.rs | 17 ++++++++++++----- library/std/src/sys/pal/windows/os.rs | 2 ++ 2 files changed, 14 insertions(+), 5 deletions(-) (limited to 'library/std/src') diff --git a/library/std/src/sys/pal/windows/mod.rs b/library/std/src/sys/pal/windows/mod.rs index b49585599cb..ff41f6e77be 100644 --- a/library/std/src/sys/pal/windows/mod.rs +++ b/library/std/src/sys/pal/windows/mod.rs @@ -201,14 +201,21 @@ pub fn to_u16s>(s: S) -> crate::io::Result> { // currently reside in the buffer. This function is an abstraction over these // functions by making them easier to call. // -// The first callback, `f1`, is yielded a (pointer, len) pair which can be +// The first callback, `f1`, is passed a (pointer, len) pair which can be // passed to a syscall. The `ptr` is valid for `len` items (u16 in this case). -// The closure is expected to return what the syscall returns which will be -// interpreted by this function to determine if the syscall needs to be invoked -// again (with more buffer space). +// The closure is expected to: +// - On success, return the actual length of the written data *without* the null terminator. +// This can be 0. In this case the last_error must be left unchanged. +// - On insufficient buffer space, +// - either return the required length *with* the null terminator, +// - or set the last-error to ERROR_INSUFFICIENT_BUFFER and return `len`. +// - On other failure, return 0 and set last_error. +// +// This is how most but not all syscalls indicate the required buffer space. +// Other syscalls may need translation to match this protocol. // // Once the syscall has completed (errors bail out early) the second closure is -// yielded the data which has been read from the syscall. The return value +// passed the data which has been read from the syscall. The return value // from this closure is then the return value of the function. pub fn fill_utf16_buf(mut f1: F1, f2: F2) -> crate::io::Result where diff --git a/library/std/src/sys/pal/windows/os.rs b/library/std/src/sys/pal/windows/os.rs index 374c9845ea4..64d8b72aed2 100644 --- a/library/std/src/sys/pal/windows/os.rs +++ b/library/std/src/sys/pal/windows/os.rs @@ -326,6 +326,8 @@ fn home_dir_crt() -> Option { super::fill_utf16_buf( |buf, mut sz| { + // GetUserProfileDirectoryW does not quite use the usual protocol for + // negotiating the buffer size, so we have to translate. match c::GetUserProfileDirectoryW( ptr::without_provenance_mut(CURRENT_PROCESS_TOKEN), buf, -- cgit 1.4.1-3-g733a5 From d5d714bb34d1c257a6328bde8614d1a9f5e1914d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 23 Apr 2024 08:22:29 +0200 Subject: fix weak memory bug in TLS on Windows --- .../std/src/sys/pal/windows/thread_local_key.rs | 26 ++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) (limited to 'library/std/src') diff --git a/library/std/src/sys/pal/windows/thread_local_key.rs b/library/std/src/sys/pal/windows/thread_local_key.rs index 4c00860dae3..e5ba619fc6b 100644 --- a/library/std/src/sys/pal/windows/thread_local_key.rs +++ b/library/std/src/sys/pal/windows/thread_local_key.rs @@ -141,9 +141,15 @@ impl StaticKey { panic!("out of TLS indexes"); } - self.key.store(key + 1, Release); register_dtor(self); + // Release-storing the key needs to be the last thing we do. + // This is because in `fn key()`, other threads will do an acquire load of the key, + // and if that sees this write then it will entirely bypass the `InitOnce`. We thus + // need to establish synchronization through `key`. In particular that acquire load + // must happen-after the register_dtor above, to ensure the dtor actually runs! + self.key.store(key + 1, Release); + let r = c::InitOnceComplete(self.once.get(), 0, ptr::null_mut()); debug_assert_eq!(r, c::TRUE); @@ -313,8 +319,22 @@ unsafe fn run_dtors() { // Use acquire ordering to observe key initialization. let mut cur = DTORS.load(Acquire); while !cur.is_null() { - let key = (*cur).key.load(Relaxed) - 1; + let pre_key = (*cur).key.load(Acquire); let dtor = (*cur).dtor.unwrap(); + cur = (*cur).next.load(Relaxed); + + // In StaticKey::init, we register the dtor before setting `key`. + // So if one thread's `run_dtors` races with another thread executing `init` on the same + // `StaticKey`, we can encounter a key of 0 here. That means this key was never + // initialized in this thread so we can safely skip it. + if pre_key == 0 { + continue; + } + // If this is non-zero, then via the `Acquire` load above we synchronized with + // everything relevant for this key. (It's not clear that this is needed, since the + // release-acquire pair on DTORS also establishes synchronization, but better safe than + // sorry.) + let key = pre_key - 1; let ptr = c::TlsGetValue(key); if !ptr.is_null() { @@ -322,8 +342,6 @@ unsafe fn run_dtors() { dtor(ptr as *mut _); any_run = true; } - - cur = (*cur).next.load(Relaxed); } if !any_run { -- cgit 1.4.1-3-g733a5 From f56afa047748e2921f5ff4fe1f6217cd9a5aeeec Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Tue, 16 Apr 2024 12:51:57 +0000 Subject: Stabilize `std::path::absolute` --- compiler/rustc_fs_util/src/lib.rs | 2 -- library/std/src/path.rs | 4 +--- src/tools/miri/src/lib.rs | 1 - src/tools/miri/tests/pass/shims/path.rs | 1 - 4 files changed, 1 insertion(+), 7 deletions(-) (limited to 'library/std/src') diff --git a/compiler/rustc_fs_util/src/lib.rs b/compiler/rustc_fs_util/src/lib.rs index 3359fef8c1c..d376c24cb58 100644 --- a/compiler/rustc_fs_util/src/lib.rs +++ b/compiler/rustc_fs_util/src/lib.rs @@ -1,5 +1,3 @@ -#![feature(absolute_path)] - use std::ffi::CString; use std::fs; use std::io; diff --git a/library/std/src/path.rs b/library/std/src/path.rs index 0636f55771e..1c58671a0cf 100644 --- a/library/std/src/path.rs +++ b/library/std/src/path.rs @@ -3332,7 +3332,6 @@ impl Error for StripPrefixError { /// ## Posix paths /// /// ``` -/// #![feature(absolute_path)] /// # #[cfg(unix)] /// fn main() -> std::io::Result<()> { /// use std::path::{self, Path}; @@ -3357,7 +3356,6 @@ impl Error for StripPrefixError { /// ## Windows paths /// /// ``` -/// #![feature(absolute_path)] /// # #[cfg(windows)] /// fn main() -> std::io::Result<()> { /// use std::path::{self, Path}; @@ -3382,7 +3380,7 @@ impl Error for StripPrefixError { /// /// [posix-semantics]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_13 /// [windows-path]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfullpathnamew -#[unstable(feature = "absolute_path", issue = "92750")] +#[stable(feature = "absolute_path", since = "CURRENT_RUSTC_VERSION")] pub fn absolute>(path: P) -> io::Result { let path = path.as_ref(); if path.as_os_str().is_empty() { diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index e1c0da9118d..44727e01ea2 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -12,7 +12,6 @@ #![feature(let_chains)] #![feature(lint_reasons)] #![feature(trait_upcasting)] -#![feature(absolute_path)] // Configure clippy and other lints #![allow( clippy::collapsible_else_if, diff --git a/src/tools/miri/tests/pass/shims/path.rs b/src/tools/miri/tests/pass/shims/path.rs index 9fc6e7faefb..cadbeb476bd 100644 --- a/src/tools/miri/tests/pass/shims/path.rs +++ b/src/tools/miri/tests/pass/shims/path.rs @@ -1,5 +1,4 @@ //@compile-flags: -Zmiri-disable-isolation -#![feature(absolute_path)] use std::path::{absolute, Path}; #[track_caller] -- cgit 1.4.1-3-g733a5