From f9a86b3531b372ffc4226987a266c82f41b74281 Mon Sep 17 00:00:00 2001 From: Noah Kennedy Date: Sat, 6 Nov 2021 15:40:16 -0500 Subject: [PATCH 01/12] Appease miri --- tests/test_bytes_vec_alloc.rs | 104 +++++++++++++++++++++------------- 1 file changed, 65 insertions(+), 39 deletions(-) diff --git a/tests/test_bytes_vec_alloc.rs b/tests/test_bytes_vec_alloc.rs index 418a9cd64..8a4d13263 100644 --- a/tests/test_bytes_vec_alloc.rs +++ b/tests/test_bytes_vec_alloc.rs @@ -1,61 +1,87 @@ use std::alloc::{GlobalAlloc, Layout, System}; -use std::{mem, ptr}; +use std::{mem}; +use std::ptr::null_mut; +use std::sync::atomic::{AtomicPtr, AtomicUsize, Ordering}; use bytes::{Buf, Bytes}; #[global_allocator] -static LEDGER: Ledger = Ledger; +static LEDGER: Ledger = Ledger::new(); -struct Ledger; +#[repr(C)] +struct Ledger { + alloc_table: [(AtomicPtr, AtomicUsize); 512], +} -const USIZE_SIZE: usize = mem::size_of::(); +impl Ledger { + const fn new() -> Self { + // equivalent to size of (AtomicPtr, AtomicUsize), hopefully + #[cfg(target_pointer_width = "64")] + let tricky_bits = 0u128; -unsafe impl GlobalAlloc for Ledger { - unsafe fn alloc(&self, layout: Layout) -> *mut u8 { - if layout.align() == 1 && layout.size() > 0 { - // Allocate extra space to stash a record of - // how much space there was. - let orig_size = layout.size(); - let size = orig_size + USIZE_SIZE; - let new_layout = match Layout::from_size_align(size, 1) { - Ok(layout) => layout, - Err(_err) => return ptr::null_mut(), - }; - let ptr = System.alloc(new_layout); - if !ptr.is_null() { - (ptr as *mut usize).write(orig_size); - let ptr = ptr.offset(USIZE_SIZE as isize); - ptr - } else { - ptr + #[cfg(target_pointer_width = "32")] + let tricky_bits = 0u64; + + let magic_table = [tricky_bits; 512]; + + // i know this looks bad but all the good ways to do this are unstable or not yet + // supported in const contexts (even though they should be!) + let alloc_table = unsafe { mem::transmute(magic_table) }; + + Self { alloc_table } + } + + /// Iterate over our table until we find an open entry, then insert into said entry + fn insert(&self, ptr: *mut u8, size: usize) { + for (entry_ptr, entry_size) in self.alloc_table.iter() { + // SeqCst is good enough here, we don't care about perf, i just want to be correct! + if entry_ptr.compare_exchange( + null_mut(), + ptr, + Ordering::SeqCst, + Ordering::SeqCst, + ).is_ok() + { + entry_size.store(size, Ordering::Relaxed); + break; } - } else { - System.alloc(layout) } } - unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { - if layout.align() == 1 && layout.size() > 0 { - let off_ptr = (ptr as *mut usize).offset(-1); - let orig_size = off_ptr.read(); - if orig_size != layout.size() { - panic!( - "bad dealloc: alloc size was {}, dealloc size is {}", - orig_size, - layout.size() - ); + fn lookup_size(&self, ptr: *mut u8) -> usize { + for (entry_ptr, entry_size) in self.alloc_table.iter() { + if entry_ptr.load(Ordering::Relaxed) == ptr { + return entry_size.load(Ordering::Relaxed); } + } - let new_layout = match Layout::from_size_align(layout.size() + USIZE_SIZE, 1) { - Ok(layout) => layout, - Err(_err) => std::process::abort(), - }; - System.dealloc(off_ptr as *mut u8, new_layout); + panic!("Couldn't find a matching entry for {:x?}", ptr); + } +} + +unsafe impl GlobalAlloc for Ledger { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + let size = layout.size(); + let ptr = System.alloc(layout); + self.insert(ptr, size); + ptr + } + + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + let orig_size = self.lookup_size(ptr); + + if orig_size != layout.size() { + panic!( + "bad dealloc: alloc size was {}, dealloc size is {}", + orig_size, + layout.size() + ); } else { System.dealloc(ptr, layout); } } } + #[test] fn test_bytes_advance() { let mut bytes = Bytes::from(vec![10, 20, 30]); From 366742489840f3d7886f734b6dcdbf4d6d4c60fd Mon Sep 17 00:00:00 2001 From: Noah Kennedy Date: Sat, 6 Nov 2021 15:47:06 -0500 Subject: [PATCH 02/12] fmt --- tests/test_bytes_vec_alloc.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/tests/test_bytes_vec_alloc.rs b/tests/test_bytes_vec_alloc.rs index 8a4d13263..d1d068f10 100644 --- a/tests/test_bytes_vec_alloc.rs +++ b/tests/test_bytes_vec_alloc.rs @@ -1,5 +1,5 @@ use std::alloc::{GlobalAlloc, Layout, System}; -use std::{mem}; +use std::mem; use std::ptr::null_mut; use std::sync::atomic::{AtomicPtr, AtomicUsize, Ordering}; @@ -17,10 +17,10 @@ impl Ledger { const fn new() -> Self { // equivalent to size of (AtomicPtr, AtomicUsize), hopefully #[cfg(target_pointer_width = "64")] - let tricky_bits = 0u128; + let tricky_bits = 0u128; #[cfg(target_pointer_width = "32")] - let tricky_bits = 0u64; + let tricky_bits = 0u64; let magic_table = [tricky_bits; 512]; @@ -35,12 +35,9 @@ impl Ledger { fn insert(&self, ptr: *mut u8, size: usize) { for (entry_ptr, entry_size) in self.alloc_table.iter() { // SeqCst is good enough here, we don't care about perf, i just want to be correct! - if entry_ptr.compare_exchange( - null_mut(), - ptr, - Ordering::SeqCst, - Ordering::SeqCst, - ).is_ok() + if entry_ptr + .compare_exchange(null_mut(), ptr, Ordering::SeqCst, Ordering::SeqCst) + .is_ok() { entry_size.store(size, Ordering::Relaxed); break; From b615c3f5bffd0d0e86480f75c5e9e2fe51732563 Mon Sep 17 00:00:00 2001 From: Noah Kennedy Date: Sat, 6 Nov 2021 15:57:55 -0500 Subject: [PATCH 03/12] update to more recent nightly --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 00a4414eb..88908ab5d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ on: env: RUSTFLAGS: -Dwarnings RUST_BACKTRACE: 1 - nightly: nightly-2021-04-13 + nightly: nightly-2021-11-1 defaults: run: From 50ecafe83c38153b873973cda445d93606ae8ff6 Mon Sep 17 00:00:00 2001 From: Noah Kennedy Date: Sat, 6 Nov 2021 16:00:56 -0500 Subject: [PATCH 04/12] try another nightly --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 88908ab5d..24b9cae9e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ on: env: RUSTFLAGS: -Dwarnings RUST_BACKTRACE: 1 - nightly: nightly-2021-11-1 + nightly: nightly-2021-11-5 defaults: run: From 16bb234f20339adafa91e20fe8803e5926a8dab6 Mon Sep 17 00:00:00 2001 From: Noah Kennedy Date: Sat, 6 Nov 2021 16:03:23 -0500 Subject: [PATCH 05/12] last resort --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 24b9cae9e..671636c61 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ on: env: RUSTFLAGS: -Dwarnings RUST_BACKTRACE: 1 - nightly: nightly-2021-11-5 + nightly: nightly defaults: run: From 298d4369b029676e5f1277bb75ced12430d89ba6 Mon Sep 17 00:00:00 2001 From: Noah Kennedy Date: Sat, 6 Nov 2021 16:04:30 -0500 Subject: [PATCH 06/12] follow suggestion --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 671636c61..d41dd0dc6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ on: env: RUSTFLAGS: -Dwarnings RUST_BACKTRACE: 1 - nightly: nightly + nightly: nightly-2021-11-05 defaults: run: From a3058ef7ce99a3a810b6706865ce919ad97836c9 Mon Sep 17 00:00:00 2001 From: Noah Kennedy Date: Sat, 6 Nov 2021 16:38:11 -0500 Subject: [PATCH 07/12] Fix test error --- tests/test_bytes_vec_alloc.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_bytes_vec_alloc.rs b/tests/test_bytes_vec_alloc.rs index d1d068f10..591da069b 100644 --- a/tests/test_bytes_vec_alloc.rs +++ b/tests/test_bytes_vec_alloc.rs @@ -45,10 +45,10 @@ impl Ledger { } } - fn lookup_size(&self, ptr: *mut u8) -> usize { + fn remove(&self, ptr: *mut u8) -> usize { for (entry_ptr, entry_size) in self.alloc_table.iter() { - if entry_ptr.load(Ordering::Relaxed) == ptr { - return entry_size.load(Ordering::Relaxed); + if entry_ptr.compare_exchange(ptr, null_mut(), Ordering::SeqCst, Ordering::SeqCst).is_ok() { + return entry_size.swap(0, Ordering::Relaxed); } } @@ -65,7 +65,7 @@ unsafe impl GlobalAlloc for Ledger { } unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { - let orig_size = self.lookup_size(ptr); + let orig_size = self.remove(ptr); if orig_size != layout.size() { panic!( From fe2de3b3a106643626a84652cacb4227d3125e19 Mon Sep 17 00:00:00 2001 From: Noah Kennedy Date: Sat, 6 Nov 2021 16:39:05 -0500 Subject: [PATCH 08/12] fix fmt --- tests/test_bytes_vec_alloc.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_bytes_vec_alloc.rs b/tests/test_bytes_vec_alloc.rs index 591da069b..f1e38c9da 100644 --- a/tests/test_bytes_vec_alloc.rs +++ b/tests/test_bytes_vec_alloc.rs @@ -47,7 +47,10 @@ impl Ledger { fn remove(&self, ptr: *mut u8) -> usize { for (entry_ptr, entry_size) in self.alloc_table.iter() { - if entry_ptr.compare_exchange(ptr, null_mut(), Ordering::SeqCst, Ordering::SeqCst).is_ok() { + if entry_ptr + .compare_exchange(ptr, null_mut(), Ordering::SeqCst, Ordering::SeqCst) + .is_ok() + { return entry_size.swap(0, Ordering::Relaxed); } } From ba987a83b4e0439835ebdcb4d5ed24c0ee566e7b Mon Sep 17 00:00:00 2001 From: Noah Kennedy Date: Sat, 6 Nov 2021 16:51:15 -0500 Subject: [PATCH 09/12] use suggestion from paolobarbolini --- tests/test_bytes_vec_alloc.rs | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/tests/test_bytes_vec_alloc.rs b/tests/test_bytes_vec_alloc.rs index f1e38c9da..6ad5da9df 100644 --- a/tests/test_bytes_vec_alloc.rs +++ b/tests/test_bytes_vec_alloc.rs @@ -1,5 +1,4 @@ use std::alloc::{GlobalAlloc, Layout, System}; -use std::mem; use std::ptr::null_mut; use std::sync::atomic::{AtomicPtr, AtomicUsize, Ordering}; @@ -15,18 +14,9 @@ struct Ledger { impl Ledger { const fn new() -> Self { - // equivalent to size of (AtomicPtr, AtomicUsize), hopefully - #[cfg(target_pointer_width = "64")] - let tricky_bits = 0u128; - - #[cfg(target_pointer_width = "32")] - let tricky_bits = 0u64; - - let magic_table = [tricky_bits; 512]; - - // i know this looks bad but all the good ways to do this are unstable or not yet - // supported in const contexts (even though they should be!) - let alloc_table = unsafe { mem::transmute(magic_table) }; + const ELEM: (AtomicPtr, AtomicUsize) = + (AtomicPtr::new(null_mut()), AtomicUsize::new(0)); + let alloc_table = [ELEM; 512]; Self { alloc_table } } @@ -39,7 +29,7 @@ impl Ledger { .compare_exchange(null_mut(), ptr, Ordering::SeqCst, Ordering::SeqCst) .is_ok() { - entry_size.store(size, Ordering::Relaxed); + entry_size.store(size, Ordering::SeqCst); break; } } @@ -51,7 +41,7 @@ impl Ledger { .compare_exchange(ptr, null_mut(), Ordering::SeqCst, Ordering::SeqCst) .is_ok() { - return entry_size.swap(0, Ordering::Relaxed); + return entry_size.swap(0, Ordering::SeqCst); } } From bf0d8ef01bb58eb543e09ca83d75a3d8bfe1eb72 Mon Sep 17 00:00:00 2001 From: Noah Kennedy Date: Sat, 6 Nov 2021 17:05:30 -0500 Subject: [PATCH 10/12] resolve possible race condition --- tests/test_bytes_vec_alloc.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/test_bytes_vec_alloc.rs b/tests/test_bytes_vec_alloc.rs index 6ad5da9df..e2a3ea1ed 100644 --- a/tests/test_bytes_vec_alloc.rs +++ b/tests/test_bytes_vec_alloc.rs @@ -7,16 +7,17 @@ use bytes::{Buf, Bytes}; #[global_allocator] static LEDGER: Ledger = Ledger::new(); -#[repr(C)] +const LEDGER_LENGTH: usize = 2048; + struct Ledger { - alloc_table: [(AtomicPtr, AtomicUsize); 512], + alloc_table: [(AtomicPtr, AtomicUsize); LEDGER_LENGTH], } impl Ledger { const fn new() -> Self { const ELEM: (AtomicPtr, AtomicUsize) = (AtomicPtr::new(null_mut()), AtomicUsize::new(0)); - let alloc_table = [ELEM; 512]; + let alloc_table = [ELEM; LEDGER_LENGTH]; Self { alloc_table } } @@ -37,11 +38,15 @@ impl Ledger { fn remove(&self, ptr: *mut u8) -> usize { for (entry_ptr, entry_size) in self.alloc_table.iter() { + // set the value to be something that will never try and be deallocated, so that we + // don't have any chance of a race condition + // + // dont worry, LEDGER_LENGTH is really long to compensate for us not reclaiming space if entry_ptr - .compare_exchange(ptr, null_mut(), Ordering::SeqCst, Ordering::SeqCst) + .compare_exchange(ptr, usize::MAX as *mut u8, Ordering::SeqCst, Ordering::SeqCst) .is_ok() { - return entry_size.swap(0, Ordering::SeqCst); + return entry_size.load(Ordering::Relaxed); } } From d67d8ccfb0ddeddcc786b1e73f88fc10780fc50d Mon Sep 17 00:00:00 2001 From: Noah Kennedy Date: Sat, 6 Nov 2021 17:06:30 -0500 Subject: [PATCH 11/12] fmt --- tests/test_bytes_vec_alloc.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_bytes_vec_alloc.rs b/tests/test_bytes_vec_alloc.rs index e2a3ea1ed..317cd08c5 100644 --- a/tests/test_bytes_vec_alloc.rs +++ b/tests/test_bytes_vec_alloc.rs @@ -43,7 +43,12 @@ impl Ledger { // // dont worry, LEDGER_LENGTH is really long to compensate for us not reclaiming space if entry_ptr - .compare_exchange(ptr, usize::MAX as *mut u8, Ordering::SeqCst, Ordering::SeqCst) + .compare_exchange( + ptr, + usize::MAX as *mut u8, + Ordering::SeqCst, + Ordering::SeqCst, + ) .is_ok() { return entry_size.load(Ordering::Relaxed); From 9a81da024adf1d66eb225aef899493ac3e943d45 Mon Sep 17 00:00:00 2001 From: Noah Kennedy Date: Sun, 7 Nov 2021 10:03:25 -0600 Subject: [PATCH 12/12] Update tests/test_bytes_vec_alloc.rs Co-authored-by: Alice Ryhl --- tests/test_bytes_vec_alloc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_bytes_vec_alloc.rs b/tests/test_bytes_vec_alloc.rs index 317cd08c5..9d9823284 100644 --- a/tests/test_bytes_vec_alloc.rs +++ b/tests/test_bytes_vec_alloc.rs @@ -51,7 +51,7 @@ impl Ledger { ) .is_ok() { - return entry_size.load(Ordering::Relaxed); + return entry_size.load(Ordering::SeqCst); } }