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 <rw@users.noreply.github.com>

* Update tests/docker/languages/Dockerfile.testing.rust.big_endian.1_30_1
Co-Authored-By: rw <rw@users.noreply.github.com>

* Update tests/docker/languages/Dockerfile.testing.rust.big_endian.1_30_1
Co-Authored-By: rw <rw@users.noreply.github.com>

* 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

@@ -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)
         }
     }
 }

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<T>(buf: &[u8], loc: usize) -> &[T] {
     let sz = size_of::<T>();
     debug_assert!(sz > 0);

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

tests/docker/languages/Dockerfile.testing.rust.big_endian.1_30_1

+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

tests/rust_usage_test/tests/integration_test.rs

@@ -780,7 +780,13 @@ mod roundtrip_vectors {
 
         const N: u64 = 20;
 
-        fn prop<T: PartialEq + ::std::fmt::Debug + Copy + flatbuffers::EndianScalar + flatbuffers::Push>(xs: Vec<T>) {
+        fn prop<T>(xs: Vec<T>)
+        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 = <flatbuffers::ForwardsUOffset<&[T]>>::follow(buf, 0);
-            assert_eq!(got, &xs[..]);
+            let got = <flatbuffers::ForwardsUOffset<flatbuffers::Vector<T>>>::follow(&buf[..], 0);
+            let mut result_vec: Vec<T> = 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 = <flatbuffers::ForwardsUOffset<flatbuffers::Table>>::follow(buf, 0);
 
             for i in 0..xs.len() {
-                let v = tab.get::<flatbuffers::ForwardsUOffset<&[u8]>>(fi2fo(i as flatbuffers::VOffsetT), None);
-                assert_eq!(v, Some(&xs[i][..]));
+                let v = tab.get::<flatbuffers::ForwardsUOffset<flatbuffers::Vector<u8>>>(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<Vec<T>>) {
+        fn prop<T>(vecs: Vec<Vec<T>>)
+        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 = <flatbuffers::ForwardsUOffset<flatbuffers::Table>>::follow(buf, 0);
 
             for i in 0..vecs.len() {
-                let got = tab.get::<flatbuffers::ForwardsUOffset<&[T]>>(fi2fo(i as flatbuffers::VOffsetT), None);
+                let got = tab.get::<flatbuffers::ForwardsUOffset<flatbuffers::Vector<T>>>(fi2fo(i as flatbuffers::VOffsetT), None);
                 assert!(got.is_some());
                 let got2 = got.unwrap();
-                assert_eq!(&vecs[i][..], got2);
+                let mut got3: Vec<T> = 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::<FooStruct>(buf, loc)
+        }
+    }
+
     #[test]
     fn to_u8() {
         let vec: Vec<u8> = vec![255, 3];
@@ -1662,8 +1721,8 @@ mod follow_impls {
     #[test]
     fn to_byte_slice() {
         let vec: Vec<u8> = 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::Vector<u8>> = 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<u8> = 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::Vector<u8>> = 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::<FooStruct>(buf, loc)
-            }
-        }
-
         let vec: Vec<u8> = 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::<FooStruct>(buf, loc)
-            }
-        }
-
         let buf: Vec<u8> = vec![1, 0, 0, 0, /* struct data */ 1, 2, 3, 4];
         let fs: flatbuffers::FollowStart<flatbuffers::Vector<FooStruct>> = 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::<FooStruct>(buf, loc)
-            }
-        }
-
         let buf: Vec<u8> = vec![1, 0, 0, 0, /* struct data */ 1, 2, 3, 4];
         let fs: flatbuffers::FollowStart<flatbuffers::Vector<FooStruct>> = 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 = <flatbuffers::ForwardsUOffset<flatbuffers::Table>>::follow(&buf[..], 0);
         assert_eq!(tab.get::<flatbuffers::ForwardsUOffset<&str>>(fi2fo(0), None), Some("moo"));
-        assert_eq!(tab.get::<flatbuffers::ForwardsUOffset<&[u8]>>(fi2fo(0), None), Some(&vec![109, 111, 111][..]));
+        let byte_vec = tab.get::<flatbuffers::ForwardsUOffset<flatbuffers::Vector<u8>>>(fi2fo(0), None).unwrap().safe_slice();
+        assert_eq!(byte_vec, &vec![109, 111, 111][..]);
         let v = tab.get::<flatbuffers::ForwardsUOffset<flatbuffers::Vector<u8>>>(fi2fo(0), None).unwrap();
         assert_eq!(v.len(), 3);
         assert_eq!(v.get(0), 109);
@@ -1879,7 +1893,10 @@ mod follow_impls {
         ];
         let tab = <flatbuffers::ForwardsUOffset<flatbuffers::Table>>::follow(&buf[..], 0);
         assert_eq!(tab.get::<flatbuffers::ForwardsUOffset<&str>>(fi2fo(0), Some("abc")), Some("abc"));
+        #[cfg(target_endian = "little")]
+        {
             assert_eq!(tab.get::<flatbuffers::ForwardsUOffset<&[u8]>>(fi2fo(0), Some(&vec![70, 71, 72][..])), Some(&vec![70, 71, 72][..]));
+        }
 
         let default_vec_buf: Vec<u8> = 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 = <flatbuffers::ForwardsUOffset<flatbuffers::Table>>::follow(&buf[..], 0);
         assert_eq!(tab.get::<flatbuffers::ForwardsUOffset<&str>>(fi2fo(0), Some("abc")), Some("abc"));
+        #[cfg(target_endian = "little")]
+        {
             assert_eq!(tab.get::<flatbuffers::ForwardsUOffset<&[u8]>>(fi2fo(0), Some(&vec![70, 71, 72][..])), Some(&vec![70, 71, 72][..]));
+        }
 
         let default_vec_buf: Vec<u8> = vec![3, 0, 0, 0, 70, 71, 72, 0];
         let default_vec = flatbuffers::Vector::new(&default_vec_buf[..], 0);