Rework `std::sys::windows::alloc` #83065

CDirkx · 2021-03-12T23:44:20Z

I came across #76676 (comment), which points out that there was unsound code in the Windows alloc code, creating a &mut to possibly uninitialized memory. I reworked the code so that that particular issue does not occur anymore, and started adding more documentation and safety comments.

Full list of changes:

moved and documented the relevant Windows Heap API functions
refactor allocate_with_flags to allocate (and remove the other helper functions), which now takes just a bool if the memory should be zeroed
add checks for if GetProcessHeap returned null
add a test that checks if the size and alignment of a Header are indeed <= MIN_ALIGN
add #![deny(unsafe_op_in_unsafe_fn)] and the necessary unsafe blocks with safety comments

I feel like I may have overdone the documenting, the unsoundness fix is the most important part; I could spit this PR up in separate parts.

rust-highfive · 2021-03-12T23:44:23Z

r? @dtolnay

(rust-highfive has picked a reviewer for you, use r? to override)

dtolnay

Thanks, this is great.

library/std/src/sys/windows/alloc.rs

dtolnay · 2021-03-22T02:52:11Z

library/std/src/sys/windows/alloc.rs

+ // If the block was successfully freed, pointers pointing to the freed memory, such as `lpMem`,
+ // must not be dereferenced ever again.
+ //
+ // Note that both `hHeap` is allowed to be any value, and `lpMem` is allowed to be null,


Regarding "hHeap is allowed to be any value" -- I didn't get that from the link. What's the specific text that indicates this? Under the "Parameters" section hHeap is documented exactly the same as it is for HeapAlloc, where you've put a more restrictive safety condition: "hHeap must be a non-null handle returned by GetProcessHeap."

In the testing I did HeapAlloc and HeapRealloc throw an exception if given a wrong hHeap value, but HeapFree just returns true indicating success. But you're right that the documentation is the same so this maybe shouldn't be relied on.

I added the safety condition.

dtolnay · 2021-03-22T02:53:05Z

library/std/src/sys/windows/alloc.rs

+
+ // SAFETY: `block` is a pointer to the start of an allocated block.
+ unsafe {
+ let err = HeapFree(GetProcessHeap(), 0, block as c::LPVOID);


Relevant to my comment on HeapFree above -- does this need to check hHeap for null?

dealloc gets to make the same assumption as realloc, since ptr has been successfully allocated with this allocator, the default process heap must be available.

dtolnay · 2021-03-22T02:58:48Z

library/std/src/sys/windows/alloc.rs

 debug_assert!(err != 0, "Failed to free heap memory: {}", c::GetLastError());
 }
 }

 #[inline]
 unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
 if layout.align() <= MIN_ALIGN {
- c::HeapReAlloc(c::GetProcessHeap(), 0, ptr as c::LPVOID, new_size) as *mut u8


It's not documented but I could believe this gets to assume GetProcessHeap returns nonnull without a check, because ptr is guaranteed to have been previously allocated from the default process heap, so the process has a default heap. Do you believe we need to add a null check? Is it possible for a process to go from having a heap to not having a heap?

I tried looking into the conditions under which GetProcessHeap would fail, but couldn't find any extra information. It does sound like a reasonable assumption, and even if it is wrong it is legal for implemenations of GlobalAlloc to abort (not unwind), which is what HeapRealloc does in practice when given an invalid hHeap.

I don't think the return value need a null check. As we don't specify HEAP_GENERATE_EXCEPTIONS flag, the functions will fail when specified heap is null, and return a null pointer, which is just what we want.

HEAP_GENERATE_EXCEPTIONS may be enabled for the default process heap, at least on my machine the following code produces an exception, even with flags set to 0:

unsafe { let ptr = HeapAlloc(GetProcessHeap(), 0, 64); println!("{:?}", HeapReAlloc(std::ptr::null_mut(), 0, ptr, 128)) // exit code: 0xc0000005, STATUS_ACCESS_VIOLATION }

I added the following to the GetProcessHeap documentation:

// SAFETY: Successful calls to this function within the same process are assumed to // always return the same handle, which remains valid for the entire lifetime of the process.

Which means we can cache the value, and assume we have a valid handle in dealloc and realloc.

Berrysoft · 2021-03-23T15:48:11Z

library/std/src/sys/windows/alloc.rs

- if layout.align() <= MIN_ALIGN {
- return c::HeapAlloc(c::GetProcessHeap(), flags, layout.size()) as *mut u8;
+unsafe fn allocate(layout: Layout, zeroed: bool) -> *mut u8 {
+ let heap = unsafe { GetProcessHeap() };


Could we consider cache the return value of GetProcessHeap() in GlobalAlloc? It seems that ucrt is doing that.

We can't add a field to the System struct, but we could store it in a static I think.

rust/library/std/src/alloc.rs

Lines 130 to 132 in 673d0db

#[stable(feature = "alloc_system_type", since = "1.28.0")]

#[derive(Debug, Default, Copy, Clone)]

pub struct System;

I think it would be better to cache it somewhere.

Add documentation to the system functions and `SAFETY` comments. Refactored helper functions, fixing the correctness of `get_header`.

Co-authored-by: David Tolnay <[email protected]>

CDirkx · 2021-03-26T14:00:20Z

I added caching of GetProcessHeap: HEAP, which in my isolated testing provided a nice little performance gain. Because we cache the handle for the heap and assume it stays valid for the entire duration of the program, we can indeed remove the null checks in dealloc and realloc.

For now the initialization of HEAP is performed in alloc, but this could be moved to the global runtime initialization in sys::init.

CDirkx · 2021-03-26T14:09:17Z

library/std/src/sys/windows/alloc.rs

+// Cached handle to the default heap of the current process.
+// Either a non-null handle returned by `GetProcessHeap`, or null when not yet initialized or `GetProcessHeap` failed.
+static HEAP: AtomicPtr<c_void> = AtomicPtr::new(ptr::null_mut());
+
+// Get a handle to the default heap of the current process, or null if the operation fails.
+// SAFETY: If this operation is successful, `HEAP` will be successfully initialized and contain
+// a non-null handle returned by `GetProcessHeap`.
+#[inline]
+unsafe fn init_or_get_process_heap() -> c::HANDLE {
+ let heap = HEAP.load(Ordering::Relaxed);
+ if heap.is_null() {
+ // `HEAP` has not yet been successfully initialized
+ let heap = unsafe { GetProcessHeap() };
+ if !heap.is_null() {
+ // SAFETY: No locking is needed because within the same process,
+ // successful calls to `GetProcessHeap` will always return the same value, even on different threads.
+ HEAP.store(heap, Ordering::Relaxed);
+
+ // SAFETY: `HEAP` contains a non-null handle returned by `GetProcessHeap`
+ heap
+ } else {
+ // Could not get the current process heap.
+ ptr::null_mut()
+ }
+ } else {
+ // SAFETY: `HEAP` contains a non-null handle returned by `GetProcessHeap`
+ heap
+ }
+}


I believe this implementation to be sound, even when called simultaneously by different threads, but would like a double check.

Just OnceCell is not enough to implement this because it isn't Sync. I could use Once, SyncOnceCell or SyncLazy, but those are all provided by std, and in general it is preferred to not use std types in sys. Is there any other way? @dtolnay what are your thoughts on this?

I looked into the source of OnceCell and found that it is only a UnsafeCell<Option<T>>. Therefore I think it is OK to use AtomicPtr here. As for the possible multiple assignment, I suggest using fetch_update.

For example,

#[inline] unsafe fn init_or_get_process_heap() -> c::HANDLE { HEAP.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |heap| { if heap.is_null() { let heap = unsafe { GetProcessHeap() }; if heap.is_null() { None } else { Some(heap) } } else { Some(heap) } }).unwrap_or(ptr::null_mut()) }

GetProcessHeap may be called multiple times, but assignment will be only once.

One thing I'm concerned about is the performance. At least from the benchmarks I did, using a mutex or fetch_update is slower than just calling GetProcessHeap every time:

Benchmarks

#![feature(test)] #![feature(atomic_fetch_update)] extern crate test; use std::sync::atomic::{AtomicPtr, Ordering}; use std::ffi::c_void; use core::ptr; use std::sync::{Mutex, Once}; pub type HANDLE = LPVOID; pub type LPVOID = *mut std::ffi::c_void; extern "system" { pub fn GetProcessHeap() -> HANDLE; } #[inline] fn bench<F: Fn() -> HANDLE>(bencher: &mut test::bench::Bencher, f: F) { bencher.iter(|| { for _ in 0..1000 { let heap = f(); assert!(!heap.is_null()); test::black_box(heap); } }) } #[bench] fn syscall(bencher: &mut test::bench::Bencher) { bench(bencher, || { unsafe { GetProcessHeap() } }) } #[bench] fn once(bencher: &mut test::bench::Bencher) { static INIT: Once = Once::new(); static HEAP: AtomicPtr<c_void> = AtomicPtr::new(ptr::null_mut()); bench(bencher, || { INIT.call_once(|| { HEAP.store(unsafe { GetProcessHeap() }, Ordering::Relaxed) }); HEAP.load(Ordering::Relaxed) }) } #[bench] fn load_store(bencher: &mut test::bench::Bencher) { static HEAP: AtomicPtr<c_void> = AtomicPtr::new(ptr::null_mut()); bench(bencher, || { let heap = HEAP.load(Ordering::Relaxed); if heap.is_null() { let heap = unsafe { GetProcessHeap() }; if !heap.is_null() { HEAP.store(heap, Ordering::Relaxed); heap } else { ptr::null_mut() } } else { heap } }) } #[bench] fn fetch_update(bencher: &mut test::bench::Bencher) { static HEAP: AtomicPtr<c_void> = AtomicPtr::new(ptr::null_mut()); bench(bencher, || { let _old = HEAP.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |heap| { if heap.is_null() { let heap = unsafe { GetProcessHeap() }; if heap.is_null() { None } else { Some(heap) } } else { Some(heap) } }); HEAP.load(Ordering::Acquire) }) } #[bench] fn mutex(bencher: &mut test::bench::Bencher) { let mutex: Mutex<()> = Mutex::new(()); // would use a static sys_common::mutex::StaticMutex static mut HEAP: HANDLE = ptr::null_mut(); bench(bencher, || { unsafe { let _lock = mutex.lock(); if HEAP.is_null() { HEAP = GetProcessHeap(); } HEAP } }) }

Results:

test fetch_update ... bench: 10,205 ns/iter ( /- 930) test load_store ... bench: 270 ns/iter ( /- 27) test mutex ... bench: 12,145 ns/iter ( /- 788) test once ... bench: 495 ns/iter ( /- 24) test syscall ... bench: 2,018 ns/iter ( /- 221)

OK, that's fair.

CDirkx · 2021-03-26T14:13:26Z

library/std/src/sys/windows/alloc.rs

 //
 // See https://docs.microsoft.com/windows/win32/api/heapapi/nf-heapapi-heapfree
 fn HeapFree(hHeap: c::HANDLE, dwFlags: c::DWORD, lpMem: c::LPVOID) -> c::BOOL;
 }

+// Cached handle to the default heap of the current process.
+// Either a non-null handle returned by `GetProcessHeap`, or null when not yet initialized or `GetProcessHeap` failed.
+static HEAP: AtomicPtr<c_void> = AtomicPtr::new(ptr::null_mut());


I believe std is already guaranteed to have AtomicPtr available.

Berrysoft · 2021-03-26T14:17:40Z

Well, I think it would be better to use OnceCell, but it is a nightly API, and I don't know if the std lib allows using that.

CDirkx · 2021-03-26T14:22:17Z

Oh that might be better. I initially thought OnceCell was provided by std, and sys shouldn't depend on the rest of std, but now I see it is provided by core, so it should be fine to use it, even if it is unstable.

dtolnay · 2021-03-30T01:28:07Z

library/std/src/sys/windows/alloc.rs

+// SAFETY: If this operation is successful, `HEAP` will be successfully initialized and contain
+// a non-null handle returned by `GetProcessHeap`.
+#[inline]
+unsafe fn init_or_get_process_heap() -> c::HANDLE {


I'm not clear why this is unsafe to call.

I was thinking the unsafety of GetProcessHeap and the role it plays in correctly initializing HEAP which the realloc and dealloc code relies on.

That isn't what unsafe to call means; unsafe to call means the caller is responsible for establishing some precondition which is not enforced by the type system. What is the precondition the caller needs in order for a call to init_or_get_process_heap to be correct? That would need to be documented.

I added get_process_heap and updated some safety comments: get_process_heap is unsafe because it assumes HEAP has already been initialized, init_or_get_process_heap is not unsafe as there are no preconditions.

dtolnay · 2021-03-30T01:34:17Z

library/std/src/sys/windows/alloc.rs

+ // SAFETY: because `ptr` has been successfully allocated with this allocator,
+ // `HEAP` must have been successfully initialized and contain a non-null handle
+ // returned by `GetProcessHeap`.
+ let heap = HEAP.load(Ordering::Relaxed);
+
+ // SAFETY: `block` is a pointer to the start of an allocated block.
+ unsafe {
+ let err = HeapFree(heap, 0, block as c::LPVOID);


This looks unsound to me. If an allocation is performed on one thread and shortly deallocated on a different thread, why would the relaxed store in the first thread be guaranteed to be visible to the relaxed load in the second thread? It seems like the second thread could read null, which would violate HeapFree's documented contract.

That's true, would making the store Release and the loads in realloc and dealloc Aquire be enough?

CDirkx · 2021-04-02T15:56:13Z

library/std/src/sys/windows/alloc.rs

- } else {
- let header = get_header(ptr);
- let err = c::HeapFree(c::GetProcessHeap(), 0, header.0 as c::LPVOID);
- debug_assert!(err != 0, "Failed to free heap memory: {}", c::GetLastError());


The documentation of GlobalAlloc has the point:

It's undefined behavior if global allocators unwind. This restriction may be lifted in the future, but currently a panic from any of these functions may lead to memory unsafety.

So I removed the debug_assert

dtolnay

Nice! Looks good to me.

dtolnay · 2021-04-02T16:53:48Z

@bors r

bors · 2021-04-02T16:53:49Z

📌 Commit db1d003 has been approved by dtolnay

Berrysoft · 2021-04-02T17:03:51Z

library/std/src/sys/windows/alloc.rs

+// a non-null handle returned by `GetProcessHeap`.
+#[inline]
+fn init_or_get_process_heap() -> c::HANDLE {
+ let heap = HEAP.load(Ordering::Relaxed);


Should the ordering here be Acquire?

Rework `std::sys::windows::alloc` I came across rust-lang#76676 (comment), which points out that there was unsound code in the Windows alloc code, creating a &mut to possibly uninitialized memory. I reworked the code so that that particular issue does not occur anymore, and started adding more documentation and safety comments. Full list of changes: - moved and documented the relevant Windows Heap API functions - refactor `allocate_with_flags` to `allocate` (and remove the other helper functions), which now takes just a `bool` if the memory should be zeroed - add checks for if `GetProcessHeap` returned null - add a test that checks if the size and alignment of a `Header` are indeed <= `MIN_ALIGN` - add `#![deny(unsafe_op_in_unsafe_fn)]` and the necessary unsafe blocks with safety comments I feel like I may have overdone the documenting, the unsoundness fix is the most important part; I could spit this PR up in separate parts.

Rollup of 7 pull requests Successful merges: - rust-lang#83065 (Rework `std::sys::windows::alloc`) - rust-lang#83478 (rustdoc: Add unstable option to only emit shared/crate-specific files) - rust-lang#83629 (Fix double-drop in `Vec::from_iter(vec.into_iter())` specialization when items drop during panic) - rust-lang#83673 (give full path of constraint in suggest_constraining_type_param) - rust-lang#83755 (Simplify coverage tests) - rust-lang#83757 (2229: Support migration via rustfix) - rust-lang#83771 (Fix stack overflow detection on FreeBSD 11.1 ) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

Pkgsrc changes: * Bump bootstrap requirements to 1.52.1 * Adjust patches, adapt to upstream changes, adjust cargo checksums * If using an external llvm, require >= 10.0 Upsteream changes: Version 1.53.0 (2021-06-17) ============================ Language ----------------------- - [You can now use unicode for identifiers.][83799] This allows multilingual identifiers but still doesn't allow glyphs that are not considered characters such as `#` (diamond) or `<U 1F980>` (crab). More specifically you can now use any identifier that matches the UAX #31 "Unicode Identifier and Pattern Syntax" standard. This is the same standard as languages like Python, however Rust uses NFC normalization which may be different from other languages. - [You can now specify "or patterns" inside pattern matches.][79278] Previously you could only use `|` (OR) on complete patterns. E.g. ```rust let x = Some(2u8); // Before matches!(x, Some(1) | Some(2)); // Now matches!(x, Some(1 | 2)); ``` - [Added the `:pat_param` `macro_rules!` matcher.][83386] This matcher has the same semantics as the `:pat` matcher. This is to allow `:pat` to change semantics to being a pattern fragment in a future edition. Compiler ----------------------- - [Updated the minimum external LLVM version to LLVM 10.][83387] - [Added Tier 3\* support for the `wasm64-unknown-unknown` target.][80525] - [Improved debuginfo for closures and async functions on Windows MSVC.][83941] \* Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. Libraries ----------------------- - [Abort messages will now forward to `android_set_abort_message` on Android platforms when available.][81469] - [`slice::IterMut<'_, T>` now implements `AsRef<[T]>`][82771] - [Arrays of any length now implement `IntoIterator`.][84147] Currently calling `.into_iter()` as a method on an array will return `impl Iterator<Item=&T>`, but this may change in a future edition to change `Item` to `T`. Calling `IntoIterator::into_iter` directly on arrays will provide `impl Iterator<Item=T>` as expected. - [`leading_zeros`, and `trailing_zeros` are now available on all `NonZero` integer types.][84082] - [`{f32, f64}::from_str` now parse and print special values (`NaN`, `-0`) according to IEEE RFC 754.][78618] - [You can now index into slices using `(Bound<usize>, Bound<usize>)`.][77704] - [Add the `BITS` associated constant to all numeric types.][82565] Stabilised APIs --------------- - [`AtomicBool::fetch_update`] - [`AtomicPtr::fetch_update`] - [`BTreeMap::retain`] - [`BTreeSet::retain`] - [`BufReader::seek_relative`] - [`DebugStruct::non_exhaustive`] - [`Duration::MAX`] - [`Duration::ZERO`] - [`Duration::is_zero`] - [`Duration::saturating_add`] - [`Duration::saturating_mul`] - [`Duration::saturating_sub`] - [`ErrorKind::Unsupported`] - [`Option::insert`] - [`Ordering::is_eq`] - [`Ordering::is_ge`] - [`Ordering::is_gt`] - [`Ordering::is_le`] - [`Ordering::is_lt`] - [`Ordering::is_ne`] - [`OsStr::is_ascii`] - [`OsStr::make_ascii_lowercase`] - [`OsStr::make_ascii_uppercase`] - [`OsStr::to_ascii_lowercase`] - [`OsStr::to_ascii_uppercase`] - [`Peekable::peek_mut`] - [`Rc::decrement_strong_count`] - [`Rc::increment_strong_count`] - [`Vec::extend_from_within`] - [`array::from_mut`] - [`array::from_ref`] - [`char::MAX`] - [`char::REPLACEMENT_CHARACTER`] - [`char::UNICODE_VERSION`] - [`char::decode_utf16`] - [`char::from_digit`] - [`char::from_u32_unchecked`] - [`char::from_u32`] - [`cmp::max_by_key`] - [`cmp::max_by`] - [`cmp::min_by_key`] - [`cmp::min_by`] - [`f32::is_subnormal`] - [`f64::is_subnormal`] Cargo ----------------------- - [Cargo now supports git repositories where the default `HEAD` branch is not "master".][cargo/9392] This also includes a switch to the version 3 `Cargo.lock` format which can handle default branches correctly. - [macOS targets now default to `unpacked` split-debuginfo.][cargo/9298] - [The `authors` field is no longer included in `Cargo.toml` for new projects.][cargo/9282] Rustdoc ----------------------- - [Added the `rustdoc::bare_urls` lint that warns when you have URLs without hyperlinks.][81764] Compatibility Notes ------------------- - [Implement token-based handling of attributes during expansion][82608] - [`Ipv4::from_str` will now reject octal format IP addresses in addition to rejecting hexadecimal IP addresses.][83652] The octal format can lead to confusion and potential security vulnerabilities and [is no longer recommended][ietf6943]. Internal Only ------------- These changes provide no direct user facing benefits, but represent significant improvements to the internals and overall performance of rustc and related tools. - [Rework the `std::sys::windows::alloc` implementation.][83065] - [rustdoc: Don't enter an infer_ctxt in get_blanket_impls for impls that aren't blanket impls.][82864] - [rustdoc: Only look at blanket impls in `get_blanket_impls`][83681] - [Rework rustdoc const type][82873] [83386]: rust-lang/rust#83386 [82771]: rust-lang/rust#82771 [84147]: rust-lang/rust#84147 [84082]: rust-lang/rust#84082 [83799]: rust-lang/rust#83799 [83681]: rust-lang/rust#83681 [83652]: rust-lang/rust#83652 [83387]: rust-lang/rust#83387 [82873]: rust-lang/rust#82873 [82864]: rust-lang/rust#82864 [82608]: rust-lang/rust#82608 [82565]: rust-lang/rust#82565 [80525]: rust-lang/rust#80525 [79278]: rust-lang/rust#79278 [78618]: rust-lang/rust#78618 [77704]: rust-lang/rust#77704 [83941]: rust-lang/rust#83941 [83065]: rust-lang/rust#83065 [81764]: rust-lang/rust#81764 [81469]: rust-lang/rust#81469 [cargo/9298]: rust-lang/cargo#9298 [cargo/9282]: rust-lang/cargo#9282 [cargo/9392]: rust-lang/cargo#9392 [`char::MAX`]: https://doc.rust-lang.org/std/primitive.char.html#associatedconstant.MAX [`char::REPLACEMENT_CHARACTER`]: https://doc.rust-lang.org/std/primitive.char.html#associatedconstant.REPLACEMENT_CHARACTER [`char::UNICODE_VERSION`]: https://doc.rust-lang.org/std/primitive.char.html#associatedconstant.UNICODE_VERSION [`char::decode_utf16`]: https://doc.rust-lang.org/std/primitive.char.html#method.decode_utf16 [`char::from_u32`]: https://doc.rust-lang.org/std/primitive.char.html#method.from_u32 [`char::from_u32_unchecked`]: https://doc.rust-lang.org/std/primitive.char.html#method.from_u32_unchecked [`char::from_digit`]: https://doc.rust-lang.org/std/primitive.char.html#method.from_digit [`AtomicBool::fetch_update`]: https://doc.rust-lang.org/std/sync/atomic/struct.AtomicBool.html#method.fetch_update [`AtomicPtr::fetch_update`]: https://doc.rust-lang.org/std/sync/atomic/struct.AtomicPtr.html#method.fetch_update [`BTreeMap::retain`]: https://doc.rust-lang.org/std/collections/struct.BTreeMap.html#method.retain [`BTreeSet::retain`]: https://doc.rust-lang.org/std/collections/struct.BTreeSet.html#method.retain [`BufReader::seek_relative`]: https://doc.rust-lang.org/std/io/struct.BufReader.html#method.seek_relative [`DebugStruct::non_exhaustive`]: https://doc.rust-lang.org/std/fmt/struct.DebugStruct.html#method.finish_non_exhaustive [`Duration::MAX`]: https://doc.rust-lang.org/std/time/struct.Duration.html#associatedconstant.MAX [`Duration::ZERO`]: https://doc.rust-lang.org/std/time/struct.Duration.html#associatedconstant.ZERO [`Duration::is_zero`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.is_zero [`Duration::saturating_add`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.saturating_add [`Duration::saturating_mul`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.saturating_mul [`Duration::saturating_sub`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.saturating_sub [`ErrorKind::Unsupported`]: https://doc.rust-lang.org/std/io/enum.ErrorKind.html#variant.Unsupported [`Option::insert`]: https://doc.rust-lang.org/std/option/enum.Option.html#method.insert [`Ordering::is_eq`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.is_eq [`Ordering::is_ge`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.is_ge [`Ordering::is_gt`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.is_gt [`Ordering::is_le`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.is_le [`Ordering::is_lt`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.is_lt [`Ordering::is_ne`]: https://doc.rust-lang.org/std/cmp/enum.Ordering.html#method.is_ne [`OsStr::eq_ignore_ascii_case`]: https://doc.rust-lang.org/std/ffi/struct.OsStr.html#method.eq_ignore_ascii_case [`OsStr::is_ascii`]: https://doc.rust-lang.org/std/ffi/struct.OsStr.html#method.is_ascii [`OsStr::make_ascii_lowercase`]: https://doc.rust-lang.org/std/ffi/struct.OsStr.html#method.make_ascii_lowercase [`OsStr::make_ascii_uppercase`]: https://doc.rust-lang.org/std/ffi/struct.OsStr.html#method.make_ascii_uppercase [`OsStr::to_ascii_lowercase`]: https://doc.rust-lang.org/std/ffi/struct.OsStr.html#method.to_ascii_lowercase [`OsStr::to_ascii_uppercase`]: https://doc.rust-lang.org/std/ffi/struct.OsStr.html#method.to_ascii_uppercase [`Peekable::peek_mut`]: https://doc.rust-lang.org/std/iter/struct.Peekable.html#method.peek_mut [`Rc::decrement_strong_count`]: https://doc.rust-lang.org/std/rc/struct.Rc.html#method.increment_strong_count [`Rc::increment_strong_count`]: https://doc.rust-lang.org/std/rc/struct.Rc.html#method.increment_strong_count [`Vec::extend_from_within`]: https://doc.rust-lang.org/beta/std/vec/struct.Vec.html#method.extend_from_within [`array::from_mut`]: https://doc.rust-lang.org/beta/std/array/fn.from_mut.html [`array::from_ref`]: https://doc.rust-lang.org/beta/std/array/fn.from_ref.html [`cmp::max_by_key`]: https://doc.rust-lang.org/beta/std/cmp/fn.max_by_key.html [`cmp::max_by`]: https://doc.rust-lang.org/beta/std/cmp/fn.max_by.html [`cmp::min_by_key`]: https://doc.rust-lang.org/beta/std/cmp/fn.min_by_key.html [`cmp::min_by`]: https://doc.rust-lang.org/beta/std/cmp/fn.min_by.html [`f32::is_subnormal`]: https://doc.rust-lang.org/std/primitive.f64.html#method.is_subnormal [`f64::is_subnormal`]: https://doc.rust-lang.org/std/primitive.f64.html#method.is_subnormal [ietf6943]: https://datatracker.ietf.org/doc/html/rfc6943#section-3.1.1

rust-highfive assigned dtolnay Mar 12, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 12, 2021

dtolnay reviewed Mar 22, 2021

View reviewed changes

Berrysoft reviewed Mar 23, 2021

View reviewed changes

CDirkx and others added 3 commits March 26, 2021 12:38

Rework std::sys::windows::alloc

0dbed61

Add documentation to the system functions and `SAFETY` comments. Refactored helper functions, fixing the correctness of `get_header`.

Apply suggestions from code review

b01bf0e

Co-authored-by: David Tolnay <[email protected]>

Cache GetProcessHeap

4cce9e3

CDirkx force-pushed the win-alloc branch from 5af5b0e to 4cce9e3 Compare March 26, 2021 13:48

CDirkx commented Mar 26, 2021

View reviewed changes

dtolnay requested changes Mar 30, 2021

View reviewed changes

dtolnay added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 30, 2021

CDirkx added 2 commits April 2, 2021 17:37

Introduce get_process_heap and fix atomic ordering.

c86e098

Remove debug_assert

db1d003

CDirkx commented Apr 2, 2021

View reviewed changes

dtolnay approved these changes Apr 2, 2021

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 2, 2021

Berrysoft reviewed Apr 2, 2021

View reviewed changes

This was referenced Apr 2, 2021

Rollup of 7 pull requests #83789

Closed

Rollup of 7 pull requests #83790

Merged

bors merged commit 48ebad5 into rust-lang:master Apr 2, 2021

rustbot added this to the 1.53.0 milestone Apr 2, 2021

CDirkx deleted the win-alloc branch April 3, 2021 07:56

	#[stable(feature = "alloc_system_type", since = "1.28.0")]
	#[derive(Debug, Default, Copy, Clone)]
	pub struct System;

Rework std::sys::windows::alloc #83065

Rework std::sys::windows::alloc #83065

Conversation

CDirkx commented Mar 12, 2021

rust-highfive commented Mar 12, 2021

dtolnay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CDirkx Mar 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CDirkx commented Mar 26, 2021

Choose a reason for hiding this comment

CDirkx Mar 26, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Berrysoft Mar 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Berrysoft commented Mar 26, 2021

CDirkx commented Mar 26, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CDirkx Apr 2, 2021 • edited Loading

Choose a reason for hiding this comment

dtolnay left a comment

Choose a reason for hiding this comment

dtolnay commented Apr 2, 2021

bors commented Apr 2, 2021

Choose a reason for hiding this comment

Rework `std::sys::windows::alloc` #83065

Rework `std::sys::windows::alloc` #83065

CDirkx Mar 24, 2021 •

edited

Loading

CDirkx Mar 26, 2021 •

edited

Loading

Berrysoft Mar 28, 2021 •

edited

Loading

CDirkx commented Mar 26, 2021 •

edited

Loading

CDirkx Apr 2, 2021 •

edited

Loading