diff --git a/Cargo.toml b/Cargo.toml index c3236592..5cd6091d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,6 +4,9 @@ version = "0.1.0" edition = "2021" license = "MIT OR Apache-2.0" +[dependencies] +sptr = "0.3.2" + [dependencies.tinyvec] version = "1.6.0" features = ["std", "rustc_1_57"] @@ -14,7 +17,7 @@ criterion = { version = "0.3.5", features = ["html_reports"] } dhat = "0.3.0" rustc-hash = "1.1.0" -[target.'cfg(target_arch = "x86")'.dependencies] +[target.'cfg(target_arch = "x86")'.dev-dependencies] criterion-perf-events = "0.1.4" perfcnt = "0.7.2" diff --git a/README.md b/README.md index 7dca4ba7..316a28a5 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,18 @@ - [Documentation for the `main` branch](https://declanvk.github.io/blart/) +## Testing + +### Miri + +Currently we're using some specific crates (`sptr` and in the future back to `core::ptr::*`) to ensure that we're compatible with [Strict Provenance][sp-issue]. The following `MIRIFLAGS` setup should enable checking to make sure that we're compatible. + +```bash +MIRIFLAGS="-Zmiri-strict-provenance -Zmiri-symbolic-alignment-check" cargo miri test +``` + +I think this is useful because we're doing some pointer times with our tagged pointers implementation, mutating the contents of the pointer to store bits of data. + ## Fuzzing To run the fuzzer I use the command: @@ -58,3 +70,5 @@ at your option. Unless you explicitly state otherwise, any contribution intentionally submitted for inclusion in the work by you, as defined in the Apache-2.0 license, shall be dual licensed as above, without any additional terms or conditions. + +[sp-issue]: https://github.com/rust-lang/rust/issues/95228 diff --git a/src/lib.rs b/src/lib.rs index c40e45b7..9967f26b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,6 @@ // TODO(#20): Use rust stable distribution, remove usage of nightly features -#![feature(slice_ptr_get, strict_provenance, hasher_prefixfree_extras)] +#![feature(slice_ptr_get, hasher_prefixfree_extras)] +#![allow(unstable_name_collisions)] #![deny( missing_docs, clippy::missing_safety_doc, diff --git a/src/nodes/representation/iterators.rs b/src/nodes/representation/iterators.rs index ef6f3707..59141bfc 100644 --- a/src/nodes/representation/iterators.rs +++ b/src/nodes/representation/iterators.rs @@ -1,3 +1,5 @@ +use sptr::Strict; + use super::*; use std::{ iter::FusedIterator, diff --git a/src/nodes/representation/tests.rs b/src/nodes/representation/tests.rs index dee1032f..7b2fdf48 100644 --- a/src/nodes/representation/tests.rs +++ b/src/nodes/representation/tests.rs @@ -1,3 +1,5 @@ +use sptr::Strict; + use super::*; use std::mem; diff --git a/src/tagged_pointer.rs b/src/tagged_pointer.rs index 0beb8880..986a9e38 100644 --- a/src/tagged_pointer.rs +++ b/src/tagged_pointer.rs @@ -8,7 +8,9 @@ //! pointed-to type, so that it can store several bits of information. For a //! type with alignment `A`, the number of available bits is `log_2(A)`. -use std::{fmt, mem::align_of, num::NonZeroUsize, ptr::NonNull}; +use std::{fmt, mem::align_of, ptr::NonNull}; + +use sptr::Strict; /// A non-null pointer type which carries several bits of metadata. #[repr(transparent)] @@ -16,6 +18,13 @@ pub struct TaggedPointer
(NonNull
); impl
TaggedPointer
{ /// The ABI-required minimum alignment of the `P` type. + /// + /// The maximum alignment of a rust type is 2^29 according to the + /// [reference][align-reference]. On 32-bit and 64-bit platforms that means + /// that it is impossible for the POINTER_MASK to be zero, there must be at + /// least 3 bit present. + /// + /// [align-reference]: https://doc.rust-lang.org/reference/type-layout.html#the-alignment-modifiers pub const ALIGNMENT: usize = align_of::
(); /// A mask for data-carrying bits of the pointer. pub const DATA_MASK: usize = !Self::POINTER_MASK; @@ -63,12 +72,12 @@ impl
TaggedPointer
{ // assumes that null is always a zero value. let unchecked_ptr = unsafe { NonNull::new_unchecked(pointer) }; - let ptr_addr = unchecked_ptr.addr(); + let ptr_addr = unchecked_ptr.as_ptr().addr(); // Double-check that there are no existing bits stored in the data-carrying // positions assert_eq!( - ptr_addr.get() & Self::DATA_MASK, + ptr_addr & Self::DATA_MASK, 0, "this pointer was not aligned" ); @@ -98,18 +107,14 @@ impl
TaggedPointer
{ /// This pointer is guaranteed to be non-null. pub fn to_ptr(self) -> *mut P { self.0 - .map_addr(|ptr_addr| { - // SAFETY: The result is guaranteed to be non-zero because this tagged pointer - // was created with the same non-zero value. - unsafe { NonZeroUsize::new_unchecked(ptr_addr.get() & Self::POINTER_MASK) } - }) .as_ptr() + .map_addr(|ptr_addr| ptr_addr & Self::POINTER_MASK) } /// Consume this tagged pointer and produce the data it carries. pub fn to_data(self) -> usize { - let ptr_addr = self.0.addr(); - ptr_addr.get() & Self::DATA_MASK + let ptr_addr = self.0.as_ptr().addr(); + ptr_addr & Self::DATA_MASK } /// Update the data this tagged pointer carries to a new value. @@ -125,15 +130,18 @@ impl
TaggedPointer
{
);
let data = data & Self::DATA_MASK;
- self.0 = self.0.map_addr(|ptr_addr| {
- let new_addr = (ptr_addr.get() & Self::POINTER_MASK) | data;
-
- // clear the data bits and then write the new data
- // SAFETY: This value will always be non-zero because the upper bits (from
- // POINTER_MASK) will always be non-zero. This a property of the type and its
- // construction.
- unsafe { NonZeroUsize::new_unchecked(new_addr) }
+ let ptr_with_new_data = self.0.as_ptr().map_addr(|ptr_addr| {
+ (ptr_addr & Self::POINTER_MASK) | data
});
+
+ // The `ptr_with_new_data` is guaranteed to be non-null because it's pointer
+ // address was derived from a non-null pointer using operations that would not
+ // have zeroed out the address.
+ //
+ // The bit-and operation combining the original pointer address (non-null)
+ // combined with the POINTER_MASK would not have produced a zero value because
+ // the POINTER_MASK value must have the high bits set.
+ self.0 = unsafe { NonNull::new_unchecked(ptr_with_new_data) };
}
/// Casts to a [`TaggedPointer`] of another type.
@@ -423,7 +431,7 @@ mod tests {
0b11111111_11111111_11111111_11111111_11111111_11111111_11111111_11111000usize
);
- // Something weird about the representation of u128 on x86 vs aarch64 platforms:
+ // Something weird about the representation of u128 on x86 vs aarch64 platforms:
// https://github.com/rust-lang/rust/issues/54341
if cfg!(target_arch = "x86") {
assert_eq!(TaggedPointer::