Rust Remove SafeSliceAccess for Arrays, and fix miri. (#6592)

* Fix Miri flag passing and bump Rust version.

* Fix Miri problems from Arrays PR.

SafeSliceAccess was removed for Arrays. It's kind of unsound.
It has two properties:
1. EndianSafe
2. Alignment 1

We only need 1. in create_vector_direct to memcpy data.
We both 1. and 2. for accessing things with slices as buffers are built on &[u8]
which is unaligned. Conditional compilation implements
SafeSliceAccess for >1byte scalars (like f32) on LittleEndian machines
which is wrong since they don't satisfy 2.

This UB is still accessible for Vectors (though not exercised our
tests) as it implements SafeSliceAccess. I'll fix this later by
splitting SafeSliceAccess into its 2 properties.

Co-authored-by: Casper Neo <cneo@google.com>
This commit is contained in:
Casper
2021-04-26 19:28:25 -04:00
committed by GitHub
parent c24031c36b
commit c87179e73e
5 changed files with 17 additions and 21 deletions

View File

@@ -1,6 +1,6 @@
[package] [package]
name = "flatbuffers" name = "flatbuffers"
version = "0.8.4" version = "0.8.5"
edition = "2018" edition = "2018"
authors = ["Robert Winslow <hello@rwinslow.com>", "FlatBuffers Maintainers"] authors = ["Robert Winslow <hello@rwinslow.com>", "FlatBuffers Maintainers"]
license = "Apache-2.0" license = "Apache-2.0"

View File

@@ -54,6 +54,9 @@ impl<'a, T: 'a, const N: usize> Array<'a, T, N> {
pub const fn len(&self) -> usize { pub const fn len(&self) -> usize {
N N
} }
pub fn as_ptr(&self) -> *const u8 {
self.0.as_ptr()
}
} }
impl<'a, T: Follow<'a> + 'a, const N: usize> Array<'a, T, N> { impl<'a, T: Follow<'a> + 'a, const N: usize> Array<'a, T, N> {
@@ -77,14 +80,7 @@ impl<'a, T: Follow<'a> + Debug, const N: usize> Into<[T::Inner; N]> for Array<'a
} }
} }
impl<'a, T: SafeSliceAccess + 'a, const N: usize> Array<'a, T, N> { // TODO(caspern): Implement some future safe version of SafeSliceAccess.
pub fn safe_slice(self) -> &'a [T] {
let sz = size_of::<T>();
debug_assert!(sz > 0);
let ptr = self.0.as_ptr() as *const T;
unsafe { from_raw_parts(ptr, N) }
}
}
/// Implement Follow for all possible Arrays that have Follow-able elements. /// Implement Follow for all possible Arrays that have Follow-able elements.
impl<'a, T: Follow<'a> + 'a, const N: usize> Follow<'a> for Array<'a, T, N> { impl<'a, T: Follow<'a> + 'a, const N: usize> Follow<'a> for Array<'a, T, N> {
@@ -100,12 +96,16 @@ pub fn emplace_scalar_array<T: EndianScalar, const N: usize>(
loc: usize, loc: usize,
src: &[T; N], src: &[T; N],
) { ) {
let mut buf_ptr = buf[loc..].as_mut_ptr() as *mut T; let mut buf_ptr = buf[loc..].as_mut_ptr();
for item in src.iter() { for item in src.iter() {
let item_le = item.to_little_endian(); let item_le = item.to_little_endian();
unsafe { unsafe {
buf_ptr.write(item_le); core::ptr::copy_nonoverlapping(
buf_ptr = buf_ptr.add(1); &item_le as *const T as *const u8,
buf_ptr,
size_of::<T>(),
);
buf_ptr = buf_ptr.add(size_of::<T>());
} }
} }
} }

View File

@@ -63,5 +63,5 @@ fi
# RUST_NIGHTLY environment variable set in dockerfile. # RUST_NIGHTLY environment variable set in dockerfile.
if [[ $RUST_NIGHTLY == 1 ]]; then if [[ $RUST_NIGHTLY == 1 ]]; then
rustup +nightly component add miri rustup +nightly component add miri
cargo +nightly miri test -- -Zmiri-disable-isolation MIRIFLAGS="-Zmiri-disable-isolation" cargo +nightly miri test
fi fi

View File

@@ -225,8 +225,8 @@ fn verify_struct_array_alignment() {
let array_table = root_as_array_table(buf).unwrap(); let array_table = root_as_array_table(buf).unwrap();
let array_struct = array_table.a().unwrap(); let array_struct = array_table.a().unwrap();
let struct_start_ptr = array_struct.0.as_ptr() as usize; let struct_start_ptr = array_struct.0.as_ptr() as usize;
let b_start_ptr = array_struct.b().safe_slice().as_ptr() as usize; let b_start_ptr = array_struct.b().as_ptr() as usize;
let d_start_ptr = array_struct.d().safe_slice().as_ptr() as usize; let d_start_ptr = array_struct.d().as_ptr() as usize;
// The T type of b // The T type of b
let b_aln = ::std::mem::align_of::<i32>(); let b_aln = ::std::mem::align_of::<i32>();
assert_eq!((b_start_ptr - struct_start_ptr) % b_aln, 0); assert_eq!((b_start_ptr - struct_start_ptr) % b_aln, 0);
@@ -272,8 +272,6 @@ mod array_fuzz {
let arr: flatbuffers::Array<$ty, ARRAY_SIZE> = flatbuffers::Array::follow(&test_buf, 0); let arr: flatbuffers::Array<$ty, ARRAY_SIZE> = flatbuffers::Array::follow(&test_buf, 0);
let got: [$ty; ARRAY_SIZE] = arr.into(); let got: [$ty; ARRAY_SIZE] = arr.into();
assert_eq!(got, xs.0); assert_eq!(got, xs.0);
#[cfg(target_endian = "little")]
assert_eq!(arr.safe_slice(), xs.0);
} }
#[test] #[test]
fn $test_name() { fn $test_name() {
@@ -317,13 +315,10 @@ mod array_fuzz {
let arr: flatbuffers::Array<NestedStruct, ARRAY_SIZE> = flatbuffers::Array::follow(&test_buf, 0); let arr: flatbuffers::Array<NestedStruct, ARRAY_SIZE> = flatbuffers::Array::follow(&test_buf, 0);
let got: [&NestedStruct; ARRAY_SIZE] = arr.into(); let got: [&NestedStruct; ARRAY_SIZE] = arr.into();
assert_eq!(got, native_struct_array); assert_eq!(got, native_struct_array);
let arr_slice = arr.safe_slice();
for i in 0..ARRAY_SIZE {
assert_eq!(arr_slice[i], *native_struct_array[i]);
}
} }
#[test] #[test]
#[cfg(not(miri))] // slow.
fn test_struct() { fn test_struct() {
quickcheck::QuickCheck::new().max_tests(MAX_TESTS).quickcheck(prop_struct as fn(FakeArray<NestedStructWrapper, ARRAY_SIZE>)); quickcheck::QuickCheck::new().max_tests(MAX_TESTS).quickcheck(prop_struct as fn(FakeArray<NestedStructWrapper, ARRAY_SIZE>));
} }

View File

@@ -1115,6 +1115,7 @@ mod roundtrip_byteswap {
// fn fuzz_f64() { quickcheck::QuickCheck::new().max_tests(N).quickcheck(prop_f64 as fn(f64)); } // fn fuzz_f64() { quickcheck::QuickCheck::new().max_tests(N).quickcheck(prop_f64 as fn(f64)); }
} }
#[cfg(not(miri))]
quickcheck! { quickcheck! {
fn struct_of_structs( fn struct_of_structs(
a_id: u32, a_id: u32,