From 9e82ee25275e07e35f489f6feac543611f81fb83 Mon Sep 17 00:00:00 2001 From: tymcauley <16469394+tymcauley@users.noreply.github.com> Date: Fri, 8 Mar 2019 04:06:25 -0500 Subject: [PATCH] Fix rust crate for big-endian targets (#5229) Thanks for tackling this, @tymcauley ! * big endian docker test -- wip * tweaks * tweaks * tweaks * docker tweaks * fix conditional compilation issues * reactivate other docker tests * try some more cross-platform config (from tymcauley) * Update tests/docker/languages/Dockerfile.testing.rust.big_endian.1_30_1 Co-Authored-By: rw * Update tests/docker/languages/Dockerfile.testing.rust.big_endian.1_30_1 Co-Authored-By: rw * Update tests/docker/languages/Dockerfile.testing.rust.big_endian.1_30_1 Co-Authored-By: rw * Resolved Rust warnings during big-endian builds. * Unify Rust test suites for x86 and MIPS builds. Note that I had to add four extra packages to the MIPS `Dockerfile`: `libexpat1`, `libmagic1`, `libmpdec2`, and `libreadline7`. For a reason I couldn't identify, even the simplest Rust MIPS binaries run with `qemu-mips` would fail with a segfault when run through this `Dockerfile`. After installing the `gdb-multiarch` package to attempt to debug the issue, the binaries ran successfully. I pared down the packages installed by `gdb-multiarch`, and these four packages are the minimum subset necessary to get Rust MIPS binaries running under `qemu-mips`. * Changed Rust tests to use `Vector`s instead of direct-slice-access. The direct-slice-access method is not available on big-endian targets, but `flatbuffers::Vector`s provide an array interface that is available on all platforms. * Resolved FooStruct endianness issues using explicit struct constructor. This more closely resembles how FlatBuffers structs are constructed in generated Rust code. * Added explanation of how `FooStruct` parallels generated struct code. Also collected duplicate implementations of `FooStruct` into a common location. --- rust/flatbuffers/src/endian_scalar.rs | 8 +- rust/flatbuffers/src/vector.rs | 6 +- tests/RustTest.sh | 12 +- .../Dockerfile.testing.rust.big_endian.1_30_1 | 15 ++ .../rust_usage_test/tests/integration_test.rs | 148 ++++++++++-------- 5 files changed, 117 insertions(+), 72 deletions(-) create mode 100644 tests/docker/languages/Dockerfile.testing.rust.big_endian.1_30_1 diff --git a/rust/flatbuffers/src/endian_scalar.rs b/rust/flatbuffers/src/endian_scalar.rs index 00f2ebef4..3e7244e09 100644 --- a/rust/flatbuffers/src/endian_scalar.rs +++ b/rust/flatbuffers/src/endian_scalar.rs @@ -88,7 +88,7 @@ impl EndianScalar for f32 { } #[cfg(not(target_endian = "little"))] { - byte_swap_f32(&self) + byte_swap_f32(self) } } /// Convert f32 from little-endian to host endian-ness. @@ -100,7 +100,7 @@ impl EndianScalar for f32 { } #[cfg(not(target_endian = "little"))] { - byte_swap_f32(&self) + byte_swap_f32(self) } } } @@ -115,7 +115,7 @@ impl EndianScalar for f64 { } #[cfg(not(target_endian = "little"))] { - byte_swap_f64(&self) + byte_swap_f64(self) } } /// Convert f64 from little-endian to host endian-ness. @@ -127,7 +127,7 @@ impl EndianScalar for f64 { } #[cfg(not(target_endian = "little"))] { - byte_swap_f64(&self) + byte_swap_f64(self) } } } diff --git a/rust/flatbuffers/src/vector.rs b/rust/flatbuffers/src/vector.rs index 8c2d6d509..397089a66 100644 --- a/rust/flatbuffers/src/vector.rs +++ b/rust/flatbuffers/src/vector.rs @@ -19,7 +19,9 @@ use std::mem::size_of; use std::slice::from_raw_parts; use std::str::from_utf8_unchecked; -use endian_scalar::{EndianScalar, read_scalar}; +#[cfg(target_endian = "little")] +use endian_scalar::EndianScalar; +use endian_scalar::read_scalar; use follow::Follow; use primitives::*; @@ -85,6 +87,7 @@ mod le_safe_slice_impls { impl super::SafeSliceAccess for f64 {} } +#[cfg(target_endian = "little")] pub use self::le_safe_slice_impls::*; pub fn follow_cast_ref<'a, T: Sized + 'a>(buf: &'a [u8], loc: usize) -> &'a T { @@ -104,6 +107,7 @@ impl<'a> Follow<'a> for &'a str { } } +#[cfg(target_endian = "little")] fn follow_slice_helper(buf: &[u8], loc: usize) -> &[T] { let sz = size_of::(); debug_assert!(sz > 0); diff --git a/tests/RustTest.sh b/tests/RustTest.sh index ac050c42f..0a3974b91 100755 --- a/tests/RustTest.sh +++ b/tests/RustTest.sh @@ -15,8 +15,14 @@ set -e # See the License for the specific language governing permissions and # limitations under the License. +if [[ "$1" == "mips-unknown-linux-gnu" ]]; then + TARGET_FLAG="--target mips-unknown-linux-gnu" + export CARGO_TARGET_MIPS_UNKNOWN_LINUX_GNU_LINKER=mips-linux-gnu-gcc + export CARGO_TARGET_MIPS_UNKNOWN_LINUX_GNU_RUNNER="qemu-mips -L /usr/mips-linux-gnu" +fi + cd ./rust_usage_test -cargo test -- --quiet +cargo test $TARGET_FLAG -- --quiet TEST_RESULT=$? if [[ $TEST_RESULT == 0 ]]; then echo "OK: Rust tests passed." @@ -25,7 +31,7 @@ else exit 1 fi -cargo run --bin=alloc_check +cargo run $TARGET_FLAG --bin=alloc_check TEST_RESULT=$? if [[ $TEST_RESULT == 0 ]]; then echo "OK: Rust heap alloc test passed." @@ -34,4 +40,4 @@ else exit 1 fi -cargo bench +cargo bench $TARGET_FLAG diff --git a/tests/docker/languages/Dockerfile.testing.rust.big_endian.1_30_1 b/tests/docker/languages/Dockerfile.testing.rust.big_endian.1_30_1 new file mode 100644 index 000000000..f2e93f4e0 --- /dev/null +++ b/tests/docker/languages/Dockerfile.testing.rust.big_endian.1_30_1 @@ -0,0 +1,15 @@ +FROM rust:1.30.1-slim-stretch as base +RUN apt -qq update -y && apt -qq install -y \ + gcc-mips-linux-gnu \ + libexpat1 \ + libmagic1 \ + libmpdec2 \ + libreadline7 \ + qemu-user +RUN rustup target add mips-unknown-linux-gnu +WORKDIR /code +ADD . . +RUN cp flatc_debian_stretch flatc +WORKDIR /code/tests +RUN rustc --version +RUN ./RustTest.sh mips-unknown-linux-gnu diff --git a/tests/rust_usage_test/tests/integration_test.rs b/tests/rust_usage_test/tests/integration_test.rs index f4db4ff83..538df0349 100644 --- a/tests/rust_usage_test/tests/integration_test.rs +++ b/tests/rust_usage_test/tests/integration_test.rs @@ -780,7 +780,13 @@ mod roundtrip_vectors { const N: u64 = 20; - fn prop(xs: Vec) { + fn prop(xs: Vec) + where + T: for<'a> flatbuffers::Follow<'a, Inner = T> + + flatbuffers::EndianScalar + + flatbuffers::Push + + ::std::fmt::Debug, + { use flatbuffers::Follow; let mut b = flatbuffers::FlatBufferBuilder::new(); @@ -793,8 +799,12 @@ mod roundtrip_vectors { let buf = b.finished_data(); - let got = >::follow(buf, 0); - assert_eq!(got, &xs[..]); + let got = >>::follow(&buf[..], 0); + let mut result_vec: Vec = Vec::with_capacity(got.len()); + for i in 0..got.len() { + result_vec.push(got.get(i)); + } + assert_eq!(result_vec, xs); } #[test] @@ -1139,8 +1149,10 @@ mod roundtrip_table { let tab = >::follow(buf, 0); for i in 0..xs.len() { - let v = tab.get::>(fi2fo(i as flatbuffers::VOffsetT), None); - assert_eq!(v, Some(&xs[i][..])); + let v = tab.get::>>(fi2fo(i as flatbuffers::VOffsetT), None); + assert!(v.is_some()); + let v2 = v.unwrap().safe_slice(); + assert_eq!(v2, &xs[i][..]); } } prop(vec![vec![1,2,3]]); @@ -1187,7 +1199,13 @@ mod roundtrip_table { const N: u64 = 20; - fn prop<'a, T: flatbuffers::Follow<'a> + 'a + flatbuffers::EndianScalar + flatbuffers::Push + ::std::fmt::Debug>(vecs: Vec>) { + fn prop(vecs: Vec>) + where + T: for<'a> flatbuffers::Follow<'a, Inner = T> + + flatbuffers::EndianScalar + + flatbuffers::Push + + ::std::fmt::Debug, + { use flatbuffers::field_index_to_field_offset as fi2fo; use flatbuffers::Follow; @@ -1218,10 +1236,14 @@ mod roundtrip_table { let tab = >::follow(buf, 0); for i in 0..vecs.len() { - let got = tab.get::>(fi2fo(i as flatbuffers::VOffsetT), None); + let got = tab.get::>>(fi2fo(i as flatbuffers::VOffsetT), None); assert!(got.is_some()); let got2 = got.unwrap(); - assert_eq!(&vecs[i][..], got2); + let mut got3: Vec = Vec::with_capacity(got2.len()); + for i in 0..got2.len() { + got3.push(got2.get(i)); + } + assert_eq!(vecs[i], got3); } } @@ -1631,6 +1653,43 @@ mod follow_impls { use flatbuffers::Follow; use flatbuffers::field_index_to_field_offset as fi2fo; + // Define a test struct to use in a few tests. This replicates the work that the code generator + // would normally do when defining a FlatBuffer struct. For reference, compare the following + // `FooStruct` code with the code generated for the `Vec3` struct in + // `../../monster_test_generated.rs`. + use flatbuffers::EndianScalar; + #[derive(Copy, Clone, Debug, PartialEq)] + #[repr(C, packed)] + struct FooStruct { + a: i8, + b: u8, + c: i16, + } + impl FooStruct { + fn new(_a: i8, _b: u8, _c: i16) -> Self { + FooStruct { + a: _a.to_little_endian(), + b: _b.to_little_endian(), + c: _c.to_little_endian(), + } + } + } + impl flatbuffers::SafeSliceAccess for FooStruct {} + impl<'a> flatbuffers::Follow<'a> for FooStruct { + type Inner = &'a FooStruct; + #[inline(always)] + fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { + <&'a FooStruct>::follow(buf, loc) + } + } + impl<'a> flatbuffers::Follow<'a> for &'a FooStruct { + type Inner = &'a FooStruct; + #[inline(always)] + fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { + flatbuffers::follow_cast_ref::(buf, loc) + } + } + #[test] fn to_u8() { let vec: Vec = vec![255, 3]; @@ -1662,8 +1721,8 @@ mod follow_impls { #[test] fn to_byte_slice() { let vec: Vec = vec![255, 255, 255, 255, 4, 0, 0, 0, 1, 2, 3, 4]; - let off: flatbuffers::FollowStart<&[u8]> = flatbuffers::FollowStart::new(); - assert_eq!(off.self_follow(&vec[..], 4), &[1, 2, 3, 4][..]); + let off: flatbuffers::FollowStart> = flatbuffers::FollowStart::new(); + assert_eq!(off.self_follow(&vec[..], 4).safe_slice(), &[1, 2, 3, 4][..]); } #[test] @@ -1676,8 +1735,8 @@ mod follow_impls { #[test] fn to_byte_string_zero_teriminated() { let vec: Vec = vec![255, 255, 255, 255, 3, 0, 0, 0, 1, 2, 3, 0]; - let off: flatbuffers::FollowStart<&[u8]> = flatbuffers::FollowStart::new(); - assert_eq!(off.self_follow(&vec[..], 4), &[1, 2, 3][..]); + let off: flatbuffers::FollowStart> = flatbuffers::FollowStart::new(); + assert_eq!(off.self_follow(&vec[..], 4).safe_slice(), &[1, 2, 3][..]); } #[cfg(target_endian = "little")] @@ -1699,24 +1758,9 @@ mod follow_impls { #[test] fn to_struct() { - #[derive(Copy, Clone, Debug, PartialEq)] - #[repr(C, packed)] - struct FooStruct { - a: i8, - b: u8, - c: i16, - } - impl<'a> flatbuffers::Follow<'a> for &'a FooStruct { - type Inner = &'a FooStruct; - #[inline(always)] - fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { - flatbuffers::follow_cast_ref::(buf, loc) - } - } - let vec: Vec = vec![255, 255, 255, 255, 1, 2, 3, 4]; let off: flatbuffers::FollowStart<&FooStruct> = flatbuffers::FollowStart::new(); - assert_eq!(*off.self_follow(&vec[..], 4), FooStruct{a: 1, b: 2, c: 1027}); + assert_eq!(*off.self_follow(&vec[..], 4), FooStruct::new(1, 2, 1027)); } #[test] @@ -1729,48 +1773,17 @@ mod follow_impls { #[test] fn to_slice_of_struct_elements() { - #[derive(Copy, Clone, Debug, PartialEq)] - #[repr(C, packed)] - struct FooStruct { - a: i8, - b: u8, - c: i16, - } - impl flatbuffers::SafeSliceAccess for FooStruct {} - impl<'a> flatbuffers::Follow<'a> for FooStruct { - type Inner = &'a FooStruct; - #[inline(always)] - fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { - flatbuffers::follow_cast_ref::(buf, loc) - } - } - let buf: Vec = vec![1, 0, 0, 0, /* struct data */ 1, 2, 3, 4]; let fs: flatbuffers::FollowStart> = flatbuffers::FollowStart::new(); - assert_eq!(fs.self_follow(&buf[..], 0).safe_slice(), &vec![FooStruct{a: 1, b: 2, c: 1027}][..]); + assert_eq!(fs.self_follow(&buf[..], 0).safe_slice(), &vec![FooStruct::new(1, 2, 1027)][..]); } #[test] fn to_vector_of_struct_elements() { - #[derive(Copy, Clone, Debug, PartialEq)] - #[repr(C, packed)] - struct FooStruct { - a: i8, - b: u8, - c: i16, - } - impl<'a> flatbuffers::Follow<'a> for FooStruct { - type Inner = &'a FooStruct; - #[inline(always)] - fn follow(buf: &'a [u8], loc: usize) -> Self::Inner { - flatbuffers::follow_cast_ref::(buf, loc) - } - } - let buf: Vec = vec![1, 0, 0, 0, /* struct data */ 1, 2, 3, 4]; let fs: flatbuffers::FollowStart> = flatbuffers::FollowStart::new(); assert_eq!(fs.self_follow(&buf[..], 0).len(), 1); - assert_eq!(fs.self_follow(&buf[..], 0).get(0), &FooStruct{a: 1, b: 2, c: 1027}); + assert_eq!(fs.self_follow(&buf[..], 0).get(0), &FooStruct::new(1, 2, 1027)); } #[test] @@ -1858,7 +1871,8 @@ mod follow_impls { ]; let tab = >::follow(&buf[..], 0); assert_eq!(tab.get::>(fi2fo(0), None), Some("moo")); - assert_eq!(tab.get::>(fi2fo(0), None), Some(&vec![109, 111, 111][..])); + let byte_vec = tab.get::>>(fi2fo(0), None).unwrap().safe_slice(); + assert_eq!(byte_vec, &vec![109, 111, 111][..]); let v = tab.get::>>(fi2fo(0), None).unwrap(); assert_eq!(v.len(), 3); assert_eq!(v.get(0), 109); @@ -1879,7 +1893,10 @@ mod follow_impls { ]; let tab = >::follow(&buf[..], 0); assert_eq!(tab.get::>(fi2fo(0), Some("abc")), Some("abc")); - assert_eq!(tab.get::>(fi2fo(0), Some(&vec![70, 71, 72][..])), Some(&vec![70, 71, 72][..])); + #[cfg(target_endian = "little")] + { + assert_eq!(tab.get::>(fi2fo(0), Some(&vec![70, 71, 72][..])), Some(&vec![70, 71, 72][..])); + } let default_vec_buf: Vec = vec![3, 0, 0, 0, 70, 71, 72, 0]; let default_vec = flatbuffers::Vector::new(&default_vec_buf[..], 0); @@ -1904,7 +1921,10 @@ mod follow_impls { ]; let tab = >::follow(&buf[..], 0); assert_eq!(tab.get::>(fi2fo(0), Some("abc")), Some("abc")); - assert_eq!(tab.get::>(fi2fo(0), Some(&vec![70, 71, 72][..])), Some(&vec![70, 71, 72][..])); + #[cfg(target_endian = "little")] + { + assert_eq!(tab.get::>(fi2fo(0), Some(&vec![70, 71, 72][..])), Some(&vec![70, 71, 72][..])); + } let default_vec_buf: Vec = vec![3, 0, 0, 0, 70, 71, 72, 0]; let default_vec = flatbuffers::Vector::new(&default_vec_buf[..], 0);