From d61f70f986ed6121ccaea994522ad7d217660ac6 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Fri, 9 Sep 2022 23:03:11 +0100 Subject: [PATCH 1/3] Change example to remove UB The example creates a new `AsciiString::from_raw_parts` using a shared reference casted to a mutable pointer. This changes the example to use an exclusive reference/pointer. --- src/ascii_string.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/ascii_string.rs b/src/ascii_string.rs index e85a512..7642221 100644 --- a/src/ascii_string.rs +++ b/src/ascii_string.rs @@ -65,6 +65,7 @@ impl AsciiString { /// /// * The memory at `buf` need to have been previously allocated by the same allocator this /// library uses. + /// * `buf` must be obtained from a valid `&mut` reference to guarentee exclusive ownership. /// * `length` needs to be less than or equal to `capacity`. /// * `capacity` needs to be the correct value. /// * `buf` must have `length` valid ascii elements and contain a total of `capacity` total, @@ -81,14 +82,14 @@ impl AsciiString { /// use std::mem; /// /// unsafe { - /// let s = AsciiString::from_ascii("hello").unwrap(); - /// let ptr = s.as_ptr(); + /// let mut s = AsciiString::from_ascii("hello").unwrap(); + /// let ptr = s.as_mut_ptr(); /// let len = s.len(); /// let capacity = s.capacity(); /// /// mem::forget(s); /// - /// let s = AsciiString::from_raw_parts(ptr as *mut _, len, capacity); + /// let s = AsciiString::from_raw_parts(ptr, len, capacity); /// /// assert_eq!(AsciiString::from_ascii("hello").unwrap(), s); /// } @@ -98,8 +99,8 @@ impl AsciiString { pub unsafe fn from_raw_parts(buf: *mut AsciiChar, length: usize, capacity: usize) -> Self { AsciiString { // SAFETY: Caller guarantees `buf` was previously allocated by this library, - // that `buf` contains `length` valid ascii elements and has a total - // capacity of `capacity` elements. + // is a unique pointer, `buf` contains `length` valid ascii elements, + // and has a total capacity of `capacity` elements. vec: unsafe { Vec::from_raw_parts(buf, length, capacity) }, } } From 19c6636cd60d2859513672f5e6b3840c39b437a3 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Fri, 9 Sep 2022 23:05:17 +0100 Subject: [PATCH 2/3] Add Miri to GitHub CI --- .github/workflows/ci.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c14eac8..e8cabbe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,3 +45,13 @@ jobs: with: rust-version: nightly - run: cargo test -Zminimal-versions --verbose --all-features + + miri: + name: Run tests under `miri` to check for UB + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: dtolnay/rust-toolchain@nightly + with: + components: miri + - run: cargo miri test --all-features From 6ecfc990da1949663f0bd1cf6042d45ead22afda Mon Sep 17 00:00:00 2001 From: tormol Date: Mon, 12 Sep 2022 00:41:04 +0200 Subject: [PATCH 3/3] Improve documentation of AsciiString::from_raw_parts() --- src/ascii_string.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ascii_string.rs b/src/ascii_string.rs index 7642221..32592e8 100644 --- a/src/ascii_string.rs +++ b/src/ascii_string.rs @@ -64,14 +64,14 @@ impl AsciiString { /// This is highly unsafe, due to the number of invariants that aren't checked: /// /// * The memory at `buf` need to have been previously allocated by the same allocator this - /// library uses. - /// * `buf` must be obtained from a valid `&mut` reference to guarentee exclusive ownership. + /// library uses, with an alignment of 1. /// * `length` needs to be less than or equal to `capacity`. /// * `capacity` needs to be the correct value. /// * `buf` must have `length` valid ascii elements and contain a total of `capacity` total, /// possibly, uninitialized, elements. + /// * Nothing else must be using the memory `buf` points to. /// - /// Violating these may cause problems like corrupting the allocator's internal datastructures. + /// Violating these may cause problems like corrupting the allocator's internal data structures. /// /// # Examples /// @@ -98,9 +98,9 @@ impl AsciiString { #[must_use] pub unsafe fn from_raw_parts(buf: *mut AsciiChar, length: usize, capacity: usize) -> Self { AsciiString { - // SAFETY: Caller guarantees `buf` was previously allocated by this library, - // is a unique pointer, `buf` contains `length` valid ascii elements, - // and has a total capacity of `capacity` elements. + // SAFETY: Caller guarantees that `buf` was previously allocated by this library, + // that `buf` contains `length` valid ascii elements and has a total capacity + // of `capacity` elements, and that nothing else is using the momory. vec: unsafe { Vec::from_raw_parts(buf, length, capacity) }, } }