Skip to content

Commit

Permalink
Merge pull request #383 from capnproto/miri-debug
Browse files Browse the repository at this point in the history 
fix miri 'stacked borrow' errors
  • Loading branch information
@dwrensha
dwrensha committed Mar 13, 2023
2 parents be0c63b + 3665562 commit 28978ac
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 23 deletions.
49 changes: 29 additions & 20 deletions capnp/src/private/layout.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,23 +164,32 @@ impl WirePointer {
unsafe { this_addr.offset(8 * (1 + ((self.offset_and_kind.get() as i32) >> 2)) as isize) }
}

// At one point, we had `&self` here instead of `ptr: *const Self`, but miri
// flagged that as running afoul of "stacked borrow" rules.
#[inline]
pub fn target_from_segment(
&self,
fn target_from_segment(
ptr: *const Self,
arena: &dyn ReaderArena,
segment_id: u32,
) -> Result<*const u8> {
let this_addr: *const u8 = self as *const _ as *const _;
let offset = 1 + ((self.offset_and_kind.get() as i32) >> 2);
unsafe { arena.check_offset(segment_id, this_addr, offset) }
let this_addr: *const u8 = ptr as *const _;
unsafe {
let offset = 1 + (((*ptr).offset_and_kind.get() as i32) >> 2);
arena.check_offset(segment_id, this_addr, offset)
}
}

// At one point, we had `&mut self` here instead of `ptr: *mut Self`, but miri
// flagged that as running afoul of "stacked borrow" rules.
#[inline]
pub fn mut_target(&mut self) -> *mut u8 {
let this_addr: *mut u8 = self as *mut _ as *mut _;
this_addr.wrapping_offset(
BYTES_PER_WORD as isize * (1 + ((self.offset_and_kind.get() as i32) >> 2)) as isize,
)
fn mut_target(ptr: *mut Self) -> *mut u8 {
let this_addr: *mut u8 = ptr as *mut _;
unsafe {
this_addr.wrapping_offset(
BYTES_PER_WORD as isize
* (1 + (((*ptr).offset_and_kind.get() as i32) >> 2)) as isize,
)
}
}

#[inline]
Expand Down Expand Up @@ -470,7 +479,7 @@ mod wire_helpers {
let pad: *mut WirePointer =
(seg_start as *mut WirePointer).offset((*reff).far_position_in_segment() as isize);
if !(*reff).is_double_far() {
Ok(((*pad).mut_target(), pad, segment_id))
Ok((WirePointer::mut_target(pad), pad, segment_id))
} else {
//# Landing pad is another far pointer. It is followed by a
//# tag describing the pointed-to object.
Expand Down Expand Up @@ -511,7 +520,7 @@ mod wire_helpers {

if !(*reff).is_double_far() {
Ok((
(*pad).target_from_segment(arena, far_segment_id)?,
WirePointer::target_from_segment(pad, arena, far_segment_id)?,
pad,
far_segment_id,
))
Expand All @@ -528,7 +537,7 @@ mod wire_helpers {
}
} else {
Ok((
(*reff).target_from_segment(arena, segment_id)?,
WirePointer::target_from_segment(reff, arena, segment_id)?,
reff,
segment_id,
))
Expand All @@ -546,7 +555,7 @@ mod wire_helpers {

match (*reff).kind() {
WirePointerKind::Struct | WirePointerKind::List | WirePointerKind::Other => {
zero_object_helper(arena, segment_id, reff, (*reff).mut_target())
zero_object_helper(arena, segment_id, reff, WirePointer::mut_target(reff))
}
WirePointerKind::Far => {
let segment_id = (*reff).far_segment_id();
Expand Down Expand Up @@ -1016,7 +1025,7 @@ mod wire_helpers {
dst,
src_segment_id,
src,
(*src).mut_target(),
WirePointer::mut_target(src),
);
} else {
ptr::copy_nonoverlapping(src, dst, 1);
Expand Down Expand Up @@ -1133,7 +1142,7 @@ mod wire_helpers {
size: StructSize,
default: Option<&'a [crate::Word]>,
) -> Result<StructBuilder<'a>> {
let mut ref_target = (*reff).mut_target();
let mut ref_target = WirePointer::mut_target(reff);

if (*reff).is_null() {
match default {
Expand Down Expand Up @@ -1335,7 +1344,7 @@ mod wire_helpers {
"Use get_writable_struct_list_pointer() for struct lists"
);

let mut orig_ref_target = (*orig_ref).mut_target();
let mut orig_ref_target = WirePointer::mut_target(orig_ref);

if (*orig_ref).is_null() {
if default_value.is_null() || (*(default_value as *const WirePointer)).is_null() {
Expand Down Expand Up @@ -1468,7 +1477,7 @@ mod wire_helpers {
element_size: StructSize,
default_value: *const u8,
) -> Result<ListBuilder<'_>> {
let mut orig_ref_target = (*orig_ref).mut_target();
let mut orig_ref_target = WirePointer::mut_target(orig_ref);

if (*orig_ref).is_null() {
if default_value.is_null() || (*(default_value as *const WirePointer)).is_null() {
Expand Down Expand Up @@ -1772,7 +1781,7 @@ mod wire_helpers {
}
}
} else {
(*reff).mut_target()
WirePointer::mut_target(reff)
};

let (ptr, reff, _segment_id) = follow_builder_fars(arena, reff, ref_target, segment_id)?;
Expand Down Expand Up @@ -1865,7 +1874,7 @@ mod wire_helpers {
}
}
} else {
(*reff).mut_target()
WirePointer::mut_target(reff)
};

let (ptr, reff, _segment_id) = follow_builder_fars(arena, reff, ref_target, segment_id)?;
Expand Down
5 changes: 5 additions & 0 deletions capnp/src/serialize/no_alloc_slice_segments.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ mod tests {
}

quickcheck! {
#[cfg_attr(miri, ignore)] // miri takes a long time with quickcheck
fn test_no_alloc_buffer_segments_single_segment_optimization(
segment_0 : Vec<Word>) -> TestResult
{
Expand All @@ -420,6 +421,7 @@ mod tests {
TestResult::from_bool(true)
}

#[cfg_attr(miri, ignore)] // miri takes a long time with quickcheck
fn test_no_alloc_buffer_segments_multiple_segments(segments_vec: Vec<Vec<Word>>) -> TestResult {
if segments_vec.is_empty() { return TestResult::discard() };

Expand Down Expand Up @@ -486,6 +488,7 @@ mod tests {
}

quickcheck! {
#[cfg_attr(miri, ignore)] // miri takes a long time with quickcheck
fn test_no_alloc_buffer_segments_message_truncated(segments_vec: Vec<Vec<Word>>) -> TestResult {
if segments_vec.is_empty() { return TestResult::discard() }

Expand All @@ -508,6 +511,7 @@ mod tests {
TestResult::from_bool(true)
}

#[cfg_attr(miri, ignore)] // miri takes a long time with quickcheck
fn test_no_alloc_buffer_segments_message_options_limit(
segments_vec: Vec<Vec<Word>>) -> TestResult
{
Expand Down Expand Up @@ -541,6 +545,7 @@ mod tests {
TestResult::from_bool(true)
}

#[cfg_attr(miri, ignore)] // miri takes a long time with quickcheck
fn test_no_alloc_buffer_segments_bad_alignment(segment_0: Vec<Word>) -> TestResult {
if segment_0.is_empty() { return TestResult::discard(); }
let output_segments = OutputSegments::SingleSegment([Word::words_to_bytes(&segment_0)]);
Expand Down
8 changes: 5 additions & 3 deletions capnp/src/serialize_packed.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ where
assert!(len % 8 == 0, "PackedRead reads must be word-aligned.");

unsafe {
let mut out = out_buf.as_mut_ptr();
let out_end: *mut u8 = out_buf.as_mut_ptr().wrapping_add(len);
let out_buf_start = out_buf.as_mut_ptr();
let mut out = out_buf_start;
let out_end: *mut u8 = out.wrapping_add(len);

let (mut in_ptr, mut in_end) = self.get_read_buffer()?;
let mut buffer_begin = in_ptr;
Expand All @@ -101,7 +102,7 @@ where
let tag: u8;

assert_eq!(
ptr_sub(out, out_buf.as_mut_ptr()) % 8,
ptr_sub(out, out_buf_start) % 8,
0,
"Output pointer should always be aligned here."
);
Expand Down Expand Up @@ -538,6 +539,7 @@ mod tests {
}))
}

#[cfg_attr(miri, ignore)] // miri takes a long time with quickcheck
fn test_unpack(packed: Vec<u8>) -> TestResult {
let len = packed.len();
let mut packed_read = PackedRead { inner: &packed[..] };
Expand Down

0 comments on commit 28978ac

Please sign in to comment.