Skip to content

Commit c24031c

Browse files
CasperNCasper Neo
and
Casper Neo
authored
Mark endian_scalar as unsafe. (#6588)
* Mark endian_scalar as unsafe. Also - removed the deprecated flexbuffer slice from example - fixed some cargo warnings * Assertions and read_scalar made unsafe * Clippy lints * Add to Safety Co-authored-by: Casper Neo <[email protected]>
1 parent 4ccc52c commit c24031c

20 files changed

+134
-100
lines changed

rust/flatbuffers/src/array.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ where
3737
}
3838
}
3939

40+
#[allow(clippy::len_without_is_empty)]
41+
#[allow(clippy::from_over_into)] // TODO(caspern): Go from From to Into.
4042
impl<'a, T: 'a, const N: usize> Array<'a, T, N> {
4143
#[inline(always)]
4244
pub fn new(buf: &'a [u8]) -> Self {
@@ -49,7 +51,7 @@ impl<'a, T: 'a, const N: usize> Array<'a, T, N> {
4951
}
5052

5153
#[inline(always)]
52-
pub fn len(&self) -> usize {
54+
pub const fn len(&self) -> usize {
5355
N
5456
}
5557
}

rust/flatbuffers/src/builder.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -564,12 +564,14 @@ impl<'fbb> FlatBufferBuilder<'fbb> {
564564

565565
{
566566
let n = self.head + self.used_space() - object_revloc_to_vtable.value() as usize;
567-
let saw = read_scalar_at::<UOffsetT>(&self.owned_buf, n);
567+
let saw = unsafe { read_scalar_at::<UOffsetT>(&self.owned_buf, n) };
568568
debug_assert_eq!(saw, 0xF0F0_F0F0);
569-
emplace_scalar::<SOffsetT>(
570-
&mut self.owned_buf[n..n + SIZE_SOFFSET],
571-
vt_use as SOffsetT - object_revloc_to_vtable.value() as SOffsetT,
572-
);
569+
unsafe {
570+
emplace_scalar::<SOffsetT>(
571+
&mut self.owned_buf[n..n + SIZE_SOFFSET],
572+
vt_use as SOffsetT - object_revloc_to_vtable.value() as SOffsetT,
573+
);
574+
}
573575
}
574576

575577
self.field_locs.clear();

rust/flatbuffers/src/endian_scalar.rs

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16+
#![allow(clippy::wrong_self_convention)]
1617

1718
use std::mem::size_of;
1819

@@ -148,34 +149,36 @@ pub fn byte_swap_f64(x: f64) -> f64 {
148149

149150
/// Place an EndianScalar into the provided mutable byte slice. Performs
150151
/// endian conversion, if necessary.
152+
/// # Safety
153+
/// Caller must ensure `s.len() > size_of::<T>()`
154+
/// and `x` does not overlap with `s`.
151155
#[inline]
152-
pub fn emplace_scalar<T: EndianScalar>(s: &mut [u8], x: T) {
156+
pub unsafe fn emplace_scalar<T: EndianScalar>(s: &mut [u8], x: T) {
153157
let x_le = x.to_little_endian();
154-
unsafe {
155-
core::ptr::copy_nonoverlapping(
156-
&x_le as *const T as *const u8,
157-
s.as_mut_ptr() as *mut u8,
158-
size_of::<T>(),
159-
);
160-
}
158+
core::ptr::copy_nonoverlapping(
159+
&x_le as *const T as *const u8,
160+
s.as_mut_ptr() as *mut u8,
161+
size_of::<T>(),
162+
);
161163
}
162164

163165
/// Read an EndianScalar from the provided byte slice at the specified location.
164166
/// Performs endian conversion, if necessary.
167+
/// # Safety
168+
/// Caller must ensure `s.len() > loc + size_of::<T>()`.
165169
#[inline]
166-
pub fn read_scalar_at<T: EndianScalar>(s: &[u8], loc: usize) -> T {
170+
pub unsafe fn read_scalar_at<T: EndianScalar>(s: &[u8], loc: usize) -> T {
167171
read_scalar(&s[loc..])
168172
}
169173

170174
/// Read an EndianScalar from the provided byte slice. Performs endian
171175
/// conversion, if necessary.
176+
/// # Safety
177+
/// Caller must ensure `s.len() > size_of::<T>()`.
172178
#[inline]
173-
pub fn read_scalar<T: EndianScalar>(s: &[u8]) -> T {
179+
pub unsafe fn read_scalar<T: EndianScalar>(s: &[u8]) -> T {
174180
let mut mem = core::mem::MaybeUninit::<T>::uninit();
175181
// Since [u8] has alignment 1, we copy it into T which may have higher alignment.
176-
let x = unsafe {
177-
core::ptr::copy_nonoverlapping(s.as_ptr(), mem.as_mut_ptr() as *mut u8, size_of::<T>());
178-
mem.assume_init()
179-
};
180-
x.from_little_endian()
182+
core::ptr::copy_nonoverlapping(s.as_ptr(), mem.as_mut_ptr() as *mut u8, size_of::<T>());
183+
mem.assume_init().from_little_endian()
181184
}

rust/flatbuffers/src/primitives.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,9 @@ impl<T> Push for WIPOffset<T> {
137137
#[inline(always)]
138138
fn push(&self, dst: &mut [u8], rest: &[u8]) {
139139
let n = (SIZE_UOFFSET + rest.len() - self.value() as usize) as UOffsetT;
140-
emplace_scalar::<UOffsetT>(dst, n);
140+
unsafe {
141+
emplace_scalar::<UOffsetT>(dst, n);
142+
}
141143
}
142144
}
143145

@@ -179,7 +181,7 @@ impl<'a, T: Follow<'a>> Follow<'a> for ForwardsUOffset<T> {
179181
#[inline(always)]
180182
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
181183
let slice = &buf[loc..loc + SIZE_UOFFSET];
182-
let off = read_scalar::<u32>(slice) as usize;
184+
let off = unsafe { read_scalar::<u32>(slice) as usize };
183185
T::follow(buf, loc + off)
184186
}
185187
}
@@ -200,7 +202,7 @@ impl<'a, T: Follow<'a>> Follow<'a> for ForwardsVOffset<T> {
200202
#[inline(always)]
201203
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
202204
let slice = &buf[loc..loc + SIZE_VOFFSET];
203-
let off = read_scalar::<VOffsetT>(slice) as usize;
205+
let off = unsafe { read_scalar::<VOffsetT>(slice) as usize };
204206
T::follow(buf, loc + off)
205207
}
206208
}
@@ -230,7 +232,7 @@ impl<'a, T: Follow<'a>> Follow<'a> for BackwardsSOffset<T> {
230232
#[inline(always)]
231233
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
232234
let slice = &buf[loc..loc + SIZE_SOFFSET];
233-
let off = read_scalar::<SOffsetT>(slice);
235+
let off = unsafe { read_scalar::<SOffsetT>(slice) };
234236
T::follow(buf, (loc as SOffsetT - off) as usize)
235237
}
236238
}
@@ -293,7 +295,7 @@ impl<'a> Follow<'a> for bool {
293295
type Inner = bool;
294296
#[inline(always)]
295297
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
296-
read_scalar_at::<u8>(buf, loc) != 0
298+
unsafe { read_scalar_at::<u8>(buf, loc) != 0 }
297299
}
298300
}
299301

@@ -308,7 +310,7 @@ macro_rules! impl_follow_for_endian_scalar {
308310
type Inner = $ty;
309311
#[inline(always)]
310312
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
311-
read_scalar_at::<$ty>(buf, loc)
313+
unsafe { read_scalar_at::<$ty>(buf, loc) }
312314
}
313315
}
314316
};

rust/flatbuffers/src/push.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ macro_rules! impl_push_for_endian_scalar {
6161

6262
#[inline]
6363
fn push(&self, dst: &mut [u8], _rest: &[u8]) {
64-
emplace_scalar::<$ty>(dst, *self);
64+
unsafe {
65+
emplace_scalar::<$ty>(dst, *self);
66+
}
6567
}
6668
}
6769
};

rust/flatbuffers/src/vector.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ impl<'a, T: 'a> Vector<'a, T> {
7373

7474
#[inline(always)]
7575
pub fn len(&self) -> usize {
76-
read_scalar_at::<UOffsetT>(&self.0, self.1) as usize
76+
unsafe { read_scalar_at::<UOffsetT>(&self.0, self.1) as usize }
7777
}
7878
#[inline(always)]
7979
pub fn is_empty(&self) -> bool {
@@ -84,7 +84,7 @@ impl<'a, T: 'a> Vector<'a, T> {
8484
impl<'a, T: Follow<'a> + 'a> Vector<'a, T> {
8585
#[inline(always)]
8686
pub fn get(&self, idx: usize) -> T::Inner {
87-
debug_assert!(idx < read_scalar_at::<u32>(&self.0, self.1) as usize);
87+
debug_assert!(idx < self.len() as usize);
8888
let sz = size_of::<T>();
8989
debug_assert!(sz > 0);
9090
T::follow(self.0, self.1 as usize + SIZE_UOFFSET + sz * idx)
@@ -103,7 +103,7 @@ impl<'a, T: SafeSliceAccess + 'a> Vector<'a, T> {
103103
let loc = self.1;
104104
let sz = size_of::<T>();
105105
debug_assert!(sz > 0);
106-
let len = read_scalar_at::<UOffsetT>(&buf, loc) as usize;
106+
let len = unsafe { read_scalar_at::<UOffsetT>(&buf, loc) } as usize;
107107
let data_buf = &buf[loc + SIZE_UOFFSET..loc + SIZE_UOFFSET + len * sz];
108108
let ptr = data_buf.as_ptr() as *const T;
109109
let s: &'a [T] = unsafe { from_raw_parts(ptr, len) };
@@ -144,7 +144,7 @@ pub fn follow_cast_ref<'a, T: Sized + 'a>(buf: &'a [u8], loc: usize) -> &'a T {
144144
impl<'a> Follow<'a> for &'a str {
145145
type Inner = &'a str;
146146
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
147-
let len = read_scalar_at::<UOffsetT>(&buf, loc) as usize;
147+
let len = unsafe { read_scalar_at::<UOffsetT>(&buf, loc) } as usize;
148148
let slice = &buf[loc + SIZE_UOFFSET..loc + SIZE_UOFFSET + len];
149149
unsafe { from_utf8_unchecked(slice) }
150150
}
@@ -154,7 +154,7 @@ impl<'a> Follow<'a> for &'a str {
154154
fn follow_slice_helper<T>(buf: &[u8], loc: usize) -> &[T] {
155155
let sz = size_of::<T>();
156156
debug_assert!(sz > 0);
157-
let len = read_scalar_at::<UOffsetT>(&buf, loc) as usize;
157+
let len = unsafe { read_scalar_at::<UOffsetT>(&buf, loc) as usize };
158158
let data_buf = &buf[loc + SIZE_UOFFSET..loc + SIZE_UOFFSET + len * sz];
159159
let ptr = data_buf.as_ptr() as *const T;
160160
let s: &[T] = unsafe { from_raw_parts(ptr, len) };

rust/flatbuffers/src/vtable.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,28 +40,30 @@ impl<'a> VTable<'a> {
4040
(self.num_bytes() / SIZE_VOFFSET) - 2
4141
}
4242
pub fn num_bytes(&self) -> usize {
43-
read_scalar_at::<VOffsetT>(self.buf, self.loc) as usize
43+
unsafe { read_scalar_at::<VOffsetT>(self.buf, self.loc) as usize }
4444
}
4545
pub fn object_inline_num_bytes(&self) -> usize {
46-
let n = read_scalar_at::<VOffsetT>(self.buf, self.loc + SIZE_VOFFSET);
46+
let n = unsafe { read_scalar_at::<VOffsetT>(self.buf, self.loc + SIZE_VOFFSET) };
4747
n as usize
4848
}
4949
pub fn get_field(&self, idx: usize) -> VOffsetT {
5050
// TODO(rw): distinguish between None and 0?
5151
if idx > self.num_fields() {
5252
return 0;
5353
}
54-
read_scalar_at::<VOffsetT>(
55-
self.buf,
56-
self.loc + SIZE_VOFFSET + SIZE_VOFFSET + SIZE_VOFFSET * idx,
57-
)
54+
unsafe {
55+
read_scalar_at::<VOffsetT>(
56+
self.buf,
57+
self.loc + SIZE_VOFFSET + SIZE_VOFFSET + SIZE_VOFFSET * idx,
58+
)
59+
}
5860
}
5961
pub fn get(&self, byte_loc: VOffsetT) -> VOffsetT {
6062
// TODO(rw): distinguish between None and 0?
6163
if byte_loc as usize >= self.num_bytes() {
6264
return 0;
6365
}
64-
read_scalar_at::<VOffsetT>(self.buf, self.loc + byte_loc as usize)
66+
unsafe { read_scalar_at::<VOffsetT>(self.buf, self.loc + byte_loc as usize) }
6567
}
6668
pub fn as_bytes(&self) -> &[u8] {
6769
let len = self.num_bytes();

rust/flatbuffers/src/vtable_writer.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,18 @@ impl<'a> VTableWriter<'a> {
4040
/// to the provided value.
4141
#[inline(always)]
4242
pub fn write_vtable_byte_length(&mut self, n: VOffsetT) {
43-
emplace_scalar::<VOffsetT>(&mut self.buf[..SIZE_VOFFSET], n);
43+
unsafe {
44+
emplace_scalar::<VOffsetT>(&mut self.buf[..SIZE_VOFFSET], n);
45+
}
4446
debug_assert_eq!(n as usize, self.buf.len());
4547
}
4648

4749
/// Writes an object length (in bytes) into the vtable.
4850
#[inline(always)]
4951
pub fn write_object_inline_size(&mut self, n: VOffsetT) {
50-
emplace_scalar::<VOffsetT>(&mut self.buf[SIZE_VOFFSET..2 * SIZE_VOFFSET], n);
52+
unsafe {
53+
emplace_scalar::<VOffsetT>(&mut self.buf[SIZE_VOFFSET..2 * SIZE_VOFFSET], n);
54+
}
5155
}
5256

5357
/// Gets an object field offset from the vtable. Only used for debugging.
@@ -57,7 +61,7 @@ impl<'a> VTableWriter<'a> {
5761
#[inline(always)]
5862
pub fn get_field_offset(&self, vtable_offset: VOffsetT) -> VOffsetT {
5963
let idx = vtable_offset as usize;
60-
read_scalar_at::<VOffsetT>(&self.buf, idx)
64+
unsafe { read_scalar_at::<VOffsetT>(&self.buf, idx) }
6165
}
6266

6367
/// Writes an object field offset into the vtable.
@@ -67,7 +71,9 @@ impl<'a> VTableWriter<'a> {
6771
#[inline(always)]
6872
pub fn write_field_offset(&mut self, vtable_offset: VOffsetT, object_data_offset: VOffsetT) {
6973
let idx = vtable_offset as usize;
70-
emplace_scalar::<VOffsetT>(&mut self.buf[idx..idx + SIZE_VOFFSET], object_data_offset);
74+
unsafe {
75+
emplace_scalar::<VOffsetT>(&mut self.buf[idx..idx + SIZE_VOFFSET], object_data_offset);
76+
}
7177
}
7278

7379
/// Clears all data in this VTableWriter. Used to cleanly undo a

samples/monster_generated.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@ impl<'a> flatbuffers::Follow<'a> for Color {
7676
type Inner = Self;
7777
#[inline]
7878
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
79-
let b = flatbuffers::read_scalar_at::<i8>(buf, loc);
79+
let b = unsafe {
80+
flatbuffers::read_scalar_at::<i8>(buf, loc)
81+
};
8082
Self(b)
8183
}
8284
}
@@ -85,7 +87,7 @@ impl flatbuffers::Push for Color {
8587
type Output = Color;
8688
#[inline]
8789
fn push(&self, dst: &mut [u8], _rest: &[u8]) {
88-
flatbuffers::emplace_scalar::<i8>(dst, self.0);
90+
unsafe { flatbuffers::emplace_scalar::<i8>(dst, self.0); }
8991
}
9092
}
9193

@@ -161,7 +163,9 @@ impl<'a> flatbuffers::Follow<'a> for Equipment {
161163
type Inner = Self;
162164
#[inline]
163165
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
164-
let b = flatbuffers::read_scalar_at::<u8>(buf, loc);
166+
let b = unsafe {
167+
flatbuffers::read_scalar_at::<u8>(buf, loc)
168+
};
165169
Self(b)
166170
}
167171
}
@@ -170,7 +174,7 @@ impl flatbuffers::Push for Equipment {
170174
type Output = Equipment;
171175
#[inline]
172176
fn push(&self, dst: &mut [u8], _rest: &[u8]) {
173-
flatbuffers::emplace_scalar::<u8>(dst, self.0);
177+
unsafe { flatbuffers::emplace_scalar::<u8>(dst, self.0); }
174178
}
175179
}
176180

samples/sample_flexbuffers.rs

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -149,24 +149,6 @@ fn main() {
149149
.iter()
150150
.map(|r| r.as_u8())
151151
.eq(vec![5, 10, 25, 25, 25, 100].into_iter()));
152-
// For very speed sensitive applications, you can directly read the slice if all of the
153-
// following are true:
154-
//
155-
// * The provided data buffer contains a valid flexbuffer.
156-
// * You correctly specify the flexbuffer type and width.
157-
// * The host machine is little endian.
158-
// * The provided data buffer itself is aligned in memory to 8 bytes.
159-
//
160-
// Vec<u8> has alignment 1 so special care is needed to get your buffer's alignment to 8.
161-
#[cfg(target_endian = "little")]
162-
{
163-
if monster_coins.is_aligned() {
164-
assert_eq!(
165-
monster_coins.get_slice::<i8>().unwrap(),
166-
&[5, 10, 25, 25, 25, 100]
167-
);
168-
}
169-
}
170152

171153
// Build the answer to life the universe and everything. Reusing a builder resets it. The
172154
// reused internals won't need to reallocate leading to a potential 2x speedup.

src/idl_gen_rust.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -746,9 +746,9 @@ class RustGenerator : public BaseGenerator {
746746
code_ += " type Inner = Self;";
747747
code_ += " #[inline]";
748748
code_ += " fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {";
749-
code_ +=
750-
" let b = flatbuffers::read_scalar_at::<{{BASE_TYPE}}>(buf,"
751-
" loc);";
749+
code_ += " let b = unsafe {";
750+
code_ += " flatbuffers::read_scalar_at::<{{BASE_TYPE}}>(buf, loc)";
751+
code_ += " };";
752752
code_ += " {{FROM_BASE}}";
753753
code_ += " }";
754754
code_ += "}";
@@ -758,8 +758,8 @@ class RustGenerator : public BaseGenerator {
758758
code_ += " #[inline]";
759759
code_ += " fn push(&self, dst: &mut [u8], _rest: &[u8]) {";
760760
code_ +=
761-
" flatbuffers::emplace_scalar::<{{BASE_TYPE}}>"
762-
"(dst, {{INTO_BASE}});";
761+
" unsafe { flatbuffers::emplace_scalar::<{{BASE_TYPE}}>"
762+
"(dst, {{INTO_BASE}}); }";
763763
code_ += " }";
764764
code_ += "}";
765765
code_ += "";

tests/arrays_test_generated.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,9 @@ impl<'a> flatbuffers::Follow<'a> for TestEnum {
7676
type Inner = Self;
7777
#[inline]
7878
fn follow(buf: &'a [u8], loc: usize) -> Self::Inner {
79-
let b = flatbuffers::read_scalar_at::<i8>(buf, loc);
79+
let b = unsafe {
80+
flatbuffers::read_scalar_at::<i8>(buf, loc)
81+
};
8082
Self(b)
8183
}
8284
}
@@ -85,7 +87,7 @@ impl flatbuffers::Push for TestEnum {
8587
type Output = TestEnum;
8688
#[inline]
8789
fn push(&self, dst: &mut [u8], _rest: &[u8]) {
88-
flatbuffers::emplace_scalar::<i8>(dst, self.0);
90+
unsafe { flatbuffers::emplace_scalar::<i8>(dst, self.0); }
8991
}
9092
}
9193

0 commit comments

Comments
 (0)