diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index eb4c6c6e..5a590905 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -61,13 +61,38 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 - - run: cargo bench -p tendril --bench tendril -- --quick --significance-level 0.01 + - run: cargo bench -p tendril-bench --bench tendril -- --quick --significance-level 0.01 env: RUSTFLAGS: --cfg bench - - run: cargo bench -p tendril --bench futf -- --quick --significance-level 0.01 + - run: cargo bench -p tendril-bench --bench futf -- --quick --significance-level 0.01 env: RUSTFLAGS: --cfg bench + miri: + name: Tests with Miri + runs-on: ubuntu-latest + env: + # Miri is much slower than native, so running the entire test suite takes too much time + WHICH_TESTS: -p markup5ever -p tendril --features encoding_rs + steps: + - name: Which tests to run + run: echo "$WHICH_TESTS" + - uses: actions/checkout@v7 + - name: Setup Miri + run: | + rustup set profile minimal + rustup override set nightly + rustup component add miri + cargo miri setup + cargo miri setup --target i686-unknown-linux-gnu + cargo miri setup --target s390x-unknown-linux-gnu + - name: Run some tests with Miri + run: cargo miri test $WHICH_TESTS + - name: Run some tests with Miri 32-bit + run: cargo miri test $WHICH_TESTS --target i686-unknown-linux-gnu + - name: Run some tests with Miri big-endian + run: cargo miri test $WHICH_TESTS --target s390x-unknown-linux-gnu + msrv: name: MSRV runs-on: ubuntu-latest @@ -115,6 +140,7 @@ jobs: - msrv - bench-html5ever - bench-tendril + - miri steps: - name: Success diff --git a/Cargo.toml b/Cargo.toml index e69d8376..49730ebe 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,6 +7,7 @@ members = [ "rcdom", "xml5ever", "tendril", + "tendril-bench", ] [workspace.package] diff --git a/tendril-bench/Cargo.toml b/tendril-bench/Cargo.toml new file mode 100644 index 00000000..a3aabd94 --- /dev/null +++ b/tendril-bench/Cargo.toml @@ -0,0 +1,24 @@ +[package] +name = "tendril-bench" +publish = false +edition.workspace = true + +[dependencies] +tendril = {path = "../tendril"} + +[dev-dependencies] +criterion = { workspace = true } + +[lib] +test = false +doctest = false +bench = false +doc = false + +[[bench]] +name = "futf" +harness = false + +[[bench]] +name = "tendril" +harness = false diff --git a/tendril/benches/futf.rs b/tendril-bench/benches/futf.rs similarity index 100% rename from tendril/benches/futf.rs rename to tendril-bench/benches/futf.rs diff --git a/tendril/benches/tendril.rs b/tendril-bench/benches/tendril.rs similarity index 100% rename from tendril/benches/tendril.rs rename to tendril-bench/benches/tendril.rs diff --git a/tendril-bench/src/lib.rs b/tendril-bench/src/lib.rs new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/tendril-bench/src/lib.rs @@ -0,0 +1 @@ + diff --git a/tendril/Cargo.toml b/tendril/Cargo.toml index c85322ac..95938947 100644 --- a/tendril/Cargo.toml +++ b/tendril/Cargo.toml @@ -22,12 +22,3 @@ new_debug_unreachable = { workspace = true } [dev-dependencies] rand = { workspace = true } -criterion = { workspace = true } - -[[bench]] -name = "futf" -harness = false - -[[bench]] -name = "tendril" -harness = false diff --git a/tendril/src/tendril.rs b/tendril/src/tendril.rs index c1506f22..65696e1e 100644 --- a/tendril/src/tendril.rs +++ b/tendril/src/tendril.rs @@ -11,8 +11,8 @@ use std::default::Default; use std::fmt as strfmt; use std::iter::FromIterator; use std::marker::PhantomData; -use std::num::NonZeroUsize; use std::ops::{Deref, DerefMut}; +use std::ptr::NonNull; use std::sync::atomic::Ordering as AtomicOrdering; use std::sync::atomic::{self, AtomicUsize}; use std::{hash, io, mem, ptr, str}; @@ -30,9 +30,12 @@ const MAX_INLINE_TAG: usize = 0xF; const EMPTY_TAG: usize = 0xF; #[inline(always)] -fn inline_tag(len: u32) -> NonZeroUsize { +fn inline_tag(len: u32) -> NonNull { debug_assert!(len <= MAX_INLINE_LEN as u32); - unsafe { NonZeroUsize::new_unchecked(if len == 0 { EMPTY_TAG } else { len as usize }) } + const _: () = assert!(EMPTY_TAG != 0); + let address = if len == 0 { EMPTY_TAG } else { len as usize }; + // SAFETY: in either case, the address used is non-zero + unsafe { NonNull::new_unchecked(std::ptr::without_provenance_mut(address)) } } /// The multithreadedness of a tendril. @@ -185,7 +188,7 @@ where F: fmt::Format, A: Atomicity, { - ptr: Cell, + ptr: Cell>>, buf: UnsafeCell, marker: PhantomData<*mut F>, refcount_marker: PhantomData, @@ -225,7 +228,7 @@ where #[inline] fn clone(&self) -> Tendril { unsafe { - if self.ptr.get().get() > MAX_INLINE_TAG { + if self.addr() > MAX_INLINE_TAG { self.make_buf_shared(); self.incref(); } @@ -243,11 +246,10 @@ where #[inline] fn drop(&mut self) { unsafe { - let p = self.ptr.get().get(); + let p = self.addr(); if p <= MAX_INLINE_TAG { return; } - let (buf, shared, _) = self.assume_buf(); if shared { let header = self.header(); @@ -529,7 +531,7 @@ where { #[inline] fn fmt(&self, f: &mut strfmt::Formatter) -> strfmt::Result { - let kind = match self.ptr.get().get() { + let kind = match self.addr() { p if p <= MAX_INLINE_TAG => "inline", p if p & 1 == 1 => "shared", _ => "owned", @@ -605,7 +607,7 @@ where /// slice, if any. #[inline(always)] pub fn len32(&self) -> u32 { - match self.ptr.get().get() { + match self.addr() { EMPTY_TAG => 0, n if n <= MAX_INLINE_LEN => n as u32, _ => unsafe { self.raw_len() }, @@ -615,7 +617,7 @@ where /// Is the backing buffer shared? #[inline] pub fn is_shared(&self) -> bool { - let n = self.ptr.get().get(); + let n = self.addr(); (n > MAX_INLINE_TAG) && ((n & 1) == 1) } @@ -623,17 +625,20 @@ where /// Is the backing buffer shared with this other `Tendril`? #[inline] pub fn is_shared_with(&self, other: &Tendril) -> bool { - let n = self.ptr.get().get(); + let n = self.addr(); - (n > MAX_INLINE_TAG) && (n == other.ptr.get().get()) + (n > MAX_INLINE_TAG) && (n == other.addr()) } /// Truncate to length 0 without discarding any owned storage. #[inline] pub fn clear(&mut self) { - if self.ptr.get().get() <= MAX_INLINE_TAG { - self.ptr - .set(unsafe { NonZeroUsize::new_unchecked(EMPTY_TAG) }); + if self.addr() <= MAX_INLINE_TAG { + let ptr = std::ptr::without_provenance_mut(EMPTY_TAG); + const _: () = assert!(EMPTY_TAG != 0); + // SAFETY: the tag used as an address is non-zero + let ptr = unsafe { NonNull::new_unchecked(ptr) }; + self.ptr.set(ptr); } else { let (_, shared, _) = unsafe { self.assume_buf() }; if shared { @@ -773,7 +778,7 @@ where let new_len = self.len32().checked_add(other.len32()).expect(OFLOW); unsafe { - if (self.ptr.get().get() > MAX_INLINE_TAG) && (other.ptr.get().get() > MAX_INLINE_TAG) { + if (self.addr() > MAX_INLINE_TAG) && (other.addr() > MAX_INLINE_TAG) { let (self_buf, self_shared, _) = self.assume_buf(); let (other_buf, other_shared, _) = other.assume_buf(); @@ -1042,12 +1047,12 @@ where #[inline] unsafe fn make_buf_shared(&self) { - let p = self.ptr.get().get(); - if p & 1 == 0 { - let header = p as *mut Header; + let p = self.ptr.get(); + if p.addr().get() & 1 == 0 { + let header = p.as_ptr(); (*header).cap = self.aux(); - self.ptr.set(NonZeroUsize::new_unchecked(p | 1)); + self.ptr.set(p.map_addr(|p| p | 1)); self.set_aux(0); } } @@ -1058,7 +1063,7 @@ where #[inline] fn make_owned(&mut self) { unsafe { - let ptr = self.ptr.get().get(); + let ptr = self.addr(); if ptr <= MAX_INLINE_TAG || (ptr & 1) == 1 { *self = Tendril::owned_copy(self.as_byte_slice()); } @@ -1070,18 +1075,18 @@ where self.make_owned(); let mut buf = self.assume_buf().0; buf.grow(cap); - self.ptr.set(NonZeroUsize::new_unchecked(buf.ptr as usize)); + self.ptr.set(NonNull::new_unchecked(buf.ptr)); self.set_aux(buf.cap); } #[inline(always)] unsafe fn header(&self) -> *mut Header { - (self.ptr.get().get() & !1) as *mut Header + self.ptr.get().as_ptr().map_addr(|p| p & !1) } #[inline] unsafe fn assume_buf(&self) -> (Buf32>, bool, u32) { - let ptr = self.ptr.get().get(); + let ptr = self.addr(); let header = self.header(); let shared = (ptr & 1) == 1; let (cap, offset) = match shared { @@ -1116,7 +1121,7 @@ where #[inline] unsafe fn owned(x: Buf32>) -> Tendril { Tendril { - ptr: Cell::new(NonZeroUsize::new_unchecked(x.ptr as usize)), + ptr: Cell::new(NonNull::new_unchecked(x.ptr)), buf: UnsafeCell::new(Buffer { heap: Heap { len: x.len, @@ -1139,8 +1144,9 @@ where #[inline] unsafe fn shared(buf: Buf32>, off: u32, len: u32) -> Tendril { + let non_null = NonNull::new_unchecked(buf.ptr); Tendril { - ptr: Cell::new(NonZeroUsize::new_unchecked((buf.ptr as usize) | 1)), + ptr: Cell::new(non_null.map_addr(|p| p | 1)), buf: UnsafeCell::new(Buffer { heap: Heap { len, aux: off }, }), @@ -1152,7 +1158,7 @@ where #[inline] fn as_byte_slice(&self) -> &[u8] { unsafe { - match self.ptr.get().get() { + match self.addr() { EMPTY_TAG => &[], n if n <= MAX_INLINE_LEN => (*self.buf.get()).inline.get_unchecked(..n), _ => { @@ -1171,7 +1177,7 @@ where #[inline] fn as_mut_byte_slice(&mut self) -> &mut [u8] { unsafe { - match self.ptr.get().get() { + match self.addr() { EMPTY_TAG => &mut [], n if n <= MAX_INLINE_LEN => (*self.buf.get()).inline.get_unchecked_mut(..n), _ => { @@ -1199,6 +1205,10 @@ where unsafe fn set_aux(&self, aux: u32) { (*self.buf.get()).heap.aux = aux; } + + fn addr(&self) -> usize { + self.ptr.get().addr().get() + } } impl Tendril @@ -1468,7 +1478,7 @@ where #[inline] pub unsafe fn push_uninitialized(&mut self, n: u32) { let new_len = self.len32().checked_add(n).expect(OFLOW); - if new_len <= MAX_INLINE_LEN as u32 && self.ptr.get().get() <= MAX_INLINE_TAG { + if new_len <= MAX_INLINE_LEN as u32 && self.addr() <= MAX_INLINE_TAG { self.ptr.set(inline_tag(new_len)) } else { self.make_owned_with_capacity(new_len); @@ -2305,18 +2315,24 @@ mod test { assert_send::>(); let s: Tendril = Tendril::from_slice("this is a string"); assert!(!s.is_shared()); - let mut t = s.clone(); - assert!(s.is_shared()); - let sp = s.as_ptr() as usize; - thread::spawn(move || { - assert!(t.is_shared()); - t.push_slice(" extended"); - assert_eq!("this is a string extended", &*t); - assert!(t.as_ptr() as usize != sp); - assert!(!t.is_shared()); - }) - .join() - .unwrap(); + let threads: Vec<_> = (0..32) + .map(|_| { + let t = s.clone(); + assert!(s.is_shared()); + let sp = s.as_ptr() as usize; + thread::spawn(move || { + let mut t = t.clone(); // atomic refcount from multiple threads + assert!(t.is_shared()); + t.push_slice(" extended"); + assert_eq!("this is a string extended", &*t); + assert!(t.as_ptr() as usize != sp); + assert!(!t.is_shared()); + }) + }) + .collect(); + for thread in threads { + thread.join().unwrap(); + } assert!(s.is_shared()); assert_eq!("this is a string", &*s); }