Skip to content

Commit 14ceab0

Browse files
authored
Fix miri bugs (#704)
1 parent 1a02960 commit 14ceab0

File tree

5 files changed

+66
-41
lines changed

5 files changed

+66
-41
lines changed

.github/workflows/miri.yml

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
name: Miri
2+
3+
on: [push, pull_request]
4+
5+
jobs:
6+
miri:
7+
runs-on: ubuntu-latest
8+
steps:
9+
- uses: actions/checkout@v4
10+
11+
- name: Install Rust toolchain
12+
uses: actions-rust-lang/setup-rust-toolchain@v1
13+
with:
14+
profile: minimal
15+
# Miri requires nightly Rust
16+
toolchain: nightly
17+
override: true
18+
components: miri
19+
20+
- name: Run Miri
21+
# Run all of your tests inside of Miri interpreter
22+
run: cargo miri test -p ntex-bytes

ntex-bytes/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "ntex-bytes"
3-
version = "1.0.0"
3+
version = "1.1.0"
44
authors = ["Nikolay Kim <fafhrd91@gmail.com>"]
55
description = "Types and traits for working with bytes (bytes crate fork)"
66
documentation = "https://docs.rs/ntex-bytes"

ntex-bytes/src/bvec.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -595,11 +595,8 @@ unsafe impl bytes::buf::BufMut for BytesVec {
595595
let len = self.len();
596596
unsafe {
597597
// This will never panic as `len` can never become invalid
598-
let ptr = &mut self.storage.as_raw()[len..];
599-
bytes::buf::UninitSlice::from_raw_parts_mut(
600-
ptr.as_mut_ptr(),
601-
self.capacity() - len,
602-
)
598+
let ptr = self.storage.as_ptr();
599+
bytes::buf::UninitSlice::from_raw_parts_mut(ptr.add(len), self.capacity() - len)
603600
}
604601
}
605602

ntex-bytes/src/storage.rs

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,13 @@ pub(crate) const INLINE_CAP: usize = 4 * 8 - 2;
224224
#[cfg(target_pointer_width = "32")]
225225
pub(crate) const INLINE_CAP: usize = 4 * 4 - 2;
226226

227+
// Inline storage
228+
const PTR_INLINE: NonNull<Shared> =
229+
unsafe { NonNull::new_unchecked(KIND_INLINE as *mut Shared) };
230+
// Inline storage
231+
const PTR_STATIC: NonNull<Shared> =
232+
unsafe { NonNull::new_unchecked(KIND_STATIC as *mut Shared) };
233+
227234
/*
228235
*
229236
* ===== Storage =====
@@ -234,7 +241,7 @@ impl Storage {
234241
#[inline]
235242
pub(crate) const fn empty() -> Storage {
236243
Storage {
237-
arc: unsafe { NonNull::new_unchecked(KIND_INLINE as *mut Shared) },
244+
arc: PTR_INLINE,
238245
ptr: ptr::null_mut::<u8>(),
239246
len: 0,
240247
cap: 0,
@@ -259,7 +266,7 @@ impl Storage {
259266
// `arc` won't ever store a pointer. Instead, use it to
260267
// track the fact that the `Bytes` handle is backed by a
261268
// static buffer.
262-
arc: unsafe { NonNull::new_unchecked(KIND_STATIC as *mut Shared) },
269+
arc: PTR_STATIC,
263270
ptr,
264271
len: bytes.len(),
265272
cap: bytes.len(),
@@ -309,14 +316,15 @@ impl Storage {
309316
fn from_slice_with_capacity(cap: usize, src: &[u8]) -> Storage {
310317
let ptr = SharedVec::create(cap, src);
311318
unsafe {
312-
let cap = (*ptr).cap - SHARED_VEC_SIZE;
313-
let arc = NonNull::new_unchecked((ptr as usize ^ KIND_VEC) as *mut Shared);
319+
let arc = NonNull::new_unchecked(
320+
ptr.as_ptr().map_addr(|addr| addr ^ KIND_VEC) as *mut Shared
321+
);
314322

315323
// Create new arc, so atomic operations can be avoided.
316324
Storage {
317325
cap,
318326
arc,
319-
ptr: ptr.add(1) as *mut u8,
327+
ptr: ptr.as_ptr().add(1) as *mut u8,
320328
len: src.len(),
321329
}
322330
}
@@ -325,7 +333,7 @@ impl Storage {
325333
#[inline]
326334
unsafe fn from_ptr_inline(src: *const u8, len: usize) -> Storage {
327335
let mut st = Storage {
328-
arc: NonNull::new_unchecked(KIND_INLINE as *mut Shared),
336+
arc: PTR_INLINE,
329337
ptr: ptr::null_mut(),
330338
len: 0,
331339
cap: 0,
@@ -441,8 +449,9 @@ impl Storage {
441449
debug_assert!(len <= INLINE_CAP);
442450
self.arc = unsafe {
443451
NonNull::new_unchecked(
444-
((self.arc.as_ptr() as usize & !INLINE_LEN_MASK)
445-
| (len << INLINE_LEN_OFFSET)) as _,
452+
self.arc
453+
.as_ptr()
454+
.map_addr(|addr| addr & !INLINE_LEN_MASK | (len << INLINE_LEN_OFFSET)),
446455
)
447456
};
448457
}
@@ -709,7 +718,7 @@ impl Storage {
709718
} else {
710719
assert!(kind == KIND_VEC);
711720

712-
let vec_arc = (arc as usize & KIND_UNMASK) as *mut SharedVec;
721+
let vec_arc = arc.map_addr(|addr| addr & KIND_UNMASK) as *mut SharedVec;
713722
let old_size = (*vec_arc).ref_count.fetch_add(1, Relaxed);
714723
if old_size == usize::MAX {
715724
abort();
@@ -815,7 +824,7 @@ impl Storage {
815824

816825
#[inline]
817826
fn shared_vec(&self) -> *mut SharedVec {
818-
((self.arc.as_ptr() as usize) & KIND_UNMASK) as *mut SharedVec
827+
self.arc.as_ptr().map_addr(|addr| addr & KIND_UNMASK) as *mut SharedVec
819828
}
820829

821830
#[inline]
@@ -943,16 +952,14 @@ fn release_shared(ptr: *mut Shared) {
943952
impl StorageVec {
944953
/// Create new empty storage with specified capacity
945954
pub(crate) fn with_capacity(capacity: usize) -> StorageVec {
946-
let shared_ptr = SharedVec::create(capacity, &[]);
947-
StorageVec(unsafe { NonNull::new_unchecked(shared_ptr) })
955+
StorageVec(SharedVec::create(capacity, &[]))
948956
}
949957

950958
/// Create new storage with capacity and copy slice
951959
///
952960
/// Caller must garantee cap is larger or eaqual to src length
953961
pub(crate) fn from_slice(src: &[u8]) -> StorageVec {
954-
let shared_ptr = SharedVec::create(src.len(), src);
955-
StorageVec(unsafe { NonNull::new_unchecked(shared_ptr) })
962+
StorageVec(SharedVec::create(src.len(), src))
956963
}
957964

958965
/// Return a slice for the handle's view into the shared buffer
@@ -972,7 +979,7 @@ impl StorageVec {
972979
}
973980

974981
/// Return a raw pointer to data
975-
unsafe fn as_ptr(&self) -> *mut u8 {
982+
pub(crate) unsafe fn as_ptr(&self) -> *mut u8 {
976983
(self.0.as_ptr() as *mut u8).add((*self.0.as_ptr()).offset as usize)
977984
}
978985

@@ -1009,7 +1016,7 @@ impl StorageVec {
10091016
len: self.len(),
10101017
cap: self.capacity(),
10111018
arc: NonNull::new_unchecked(
1012-
(self.0.as_ptr() as usize ^ KIND_VEC) as *mut Shared,
1019+
self.0.as_ptr().map_addr(|addr| addr ^ KIND_VEC) as *mut Shared,
10131020
),
10141021
};
10151022
mem::forget(self);
@@ -1042,7 +1049,7 @@ impl StorageVec {
10421049
len: self.len(),
10431050
cap: self.capacity(),
10441051
arc: NonNull::new_unchecked(
1045-
(self.0.as_ptr() as usize ^ KIND_VEC) as *mut Shared,
1052+
self.0.as_ptr().map_addr(|addr| addr ^ KIND_VEC) as *mut Shared,
10461053
),
10471054
},
10481055
};
@@ -1098,7 +1105,7 @@ impl StorageVec {
10981105
len: at,
10991106
cap: at,
11001107
arc: NonNull::new_unchecked(
1101-
(self.0.as_ptr() as usize ^ KIND_VEC) as *mut Shared,
1108+
self.0.as_ptr().map_addr(|addr| addr ^ KIND_VEC) as *mut Shared,
11021109
),
11031110
}
11041111
};
@@ -1178,10 +1185,7 @@ impl StorageVec {
11781185
}
11791186
} else {
11801187
// Create a new vector storage
1181-
*self = StorageVec(NonNull::new_unchecked(SharedVec::create(
1182-
new_cap,
1183-
self.as_ref(),
1184-
)));
1188+
*self = StorageVec(SharedVec::create(new_cap, self.as_ref()));
11851189
}
11861190
}
11871191
}
@@ -1244,15 +1248,16 @@ impl Shared {
12441248
}
12451249

12461250
impl SharedVec {
1247-
fn create(cap: usize, src: &[u8]) -> *mut SharedVec {
1251+
fn create(cap: usize, src: &[u8]) -> NonNull<SharedVec> {
12481252
let layout = shared_vec_layout(cap).unwrap();
12491253

12501254
// Alloc memory and store data
12511255
unsafe {
1252-
let shared_ptr = alloc::alloc(layout) as *mut SharedVec;
1253-
if shared_ptr.is_null() {
1256+
let ptr = alloc::alloc(layout);
1257+
if ptr.is_null() {
12541258
alloc::handle_alloc_error(layout);
12551259
}
1260+
let shared_ptr = ptr as *mut SharedVec;
12561261

12571262
ptr::write(
12581263
shared_ptr,
@@ -1266,11 +1271,11 @@ impl SharedVec {
12661271
);
12671272

12681273
if !src.is_empty() {
1269-
let data = &((&*shared_ptr).data) as *const u8 as *mut _;
1270-
ptr::copy_nonoverlapping(src.as_ptr(), data, src.len());
1274+
let ptr = ptr.add(SHARED_VEC_SIZE);
1275+
let sl = slice::from_raw_parts_mut(ptr, src.len());
1276+
sl.copy_from_slice(src);
12711277
}
1272-
1273-
shared_ptr
1278+
NonNull::new_unchecked(shared_ptr)
12741279
}
12751280
}
12761281

@@ -1309,7 +1314,8 @@ fn release_shared_vec(ptr: *mut SharedVec) {
13091314
// Drop the data
13101315
let cap = (*ptr).cap;
13111316
ptr::drop_in_place(ptr);
1312-
alloc::dealloc(ptr as *mut _, shared_vec_layout(cap).unwrap());
1317+
let layout = shared_vec_layout(cap - SHARED_VEC_SIZE).unwrap();
1318+
alloc::dealloc(ptr as *mut _, layout);
13131319
}
13141320
}
13151321

@@ -1320,7 +1326,7 @@ const fn shared_vec_layout(cap: usize) -> Result<Layout, LayoutError> {
13201326
Ok(l) => l,
13211327
Err(e) => return Err(e),
13221328
};
1323-
match Layout::new::<SharedVec>().extend(s_layout) {
1329+
match Layout::new::<SharedVec>().pad_to_align().extend(s_layout) {
13241330
Ok((l, _)) => Ok(l),
13251331
Err(err) => Err(err),
13261332
}

ntex-bytes/tests/test_buf.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,12 @@ fn test_bytes_mut_buf() {
7979
bytes::buf::BufMut::put_u8(&mut buf, b'3');
8080
bytes::buf::BufMut::put_i8(&mut buf, b'4' as i8);
8181
unsafe {
82-
bytes::buf::BufMut::advance_mut(&mut buf, 2);
8382
let c = bytes::buf::BufMut::chunk_mut(&mut buf);
8483
*c.as_mut_ptr() = b'5';
8584
assert!(c.as_mut_ptr().as_ref().unwrap() == &b'5');
85+
bytes::buf::BufMut::advance_mut(&mut buf, 1);
8686
}
87-
assert_eq!(&bytes::buf::Buf::chunk(&buf)[..4], b"1234");
87+
assert_eq!(&bytes::buf::Buf::chunk(&buf), b"12345");
8888
}
8989

9090
#[test]
@@ -127,12 +127,12 @@ fn test_bytes_vec_buf() {
127127
bytes::buf::BufMut::put_u8(&mut buf, b'3');
128128
bytes::buf::BufMut::put_i8(&mut buf, b'4' as i8);
129129
unsafe {
130-
bytes::buf::BufMut::advance_mut(&mut buf, 1);
131130
let c = bytes::buf::BufMut::chunk_mut(&mut buf);
132131
*c.as_mut_ptr() = b'5';
133132
assert!(c.as_mut_ptr().as_ref().unwrap() == &b'5');
133+
bytes::buf::BufMut::advance_mut(&mut buf, 1);
134134
}
135-
assert_eq!(&bytes::buf::Buf::chunk(&buf)[..4], b"1234");
135+
assert_eq!(&bytes::buf::Buf::chunk(&buf), b"12345");
136136
}
137137

138138
#[test]

0 commit comments

Comments
 (0)