[Rust] FlatBufferBuilder::create_vector_direct reports undefined behavior when run through Miri

[shepmaster]

fn main() {
    let builder = &mut flatbuffers::FlatBufferBuilder::new_with_capacity(1024);
    builder.create_vector_direct(&[1,2,3]);
}

% cargo +nightly-2020-04-07-x86_64-apple-darwin miri
    Checking fb-repro v0.1.0 (/private/tmp/fb-repro)
error: Undefined Behavior: accessing memory with alignment 1, but alignment 4 is required
   --> /Users/shep/.cargo/registry/src/github.com-1ecc6299db9ec823/flatbuffers-0.6.1/src/endian_scalar.rs:157:9
    |
157 |         *mut_ptr = val;
    |         ^^^^^^^^^^^^^^ accessing memory with alignment 1, but alignment 4 is required
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: inside `flatbuffers::endian_scalar::emplace_scalar::<u32>` at /Users/shep/.cargo/registry/src/github.com-1ecc6299db9ec823/flatbuffers-0.6.1/src/endian_scalar.rs:157:9
    = note: inside `<u32 as flatbuffers::push::Push>::push` at /Users/shep/.cargo/registry/src/github.com-1ecc6299db9ec823/flatbuffers-0.6.1/src/push.rs:64:17
    = note: inside `flatbuffers::builder::FlatBufferBuilder::push::<u32>` at /Users/shep/.cargo/registry/src/github.com-1ecc6299db9ec823/flatbuffers-0.6.1/src/builder.rs:140:13
    = note: inside `flatbuffers::builder::FlatBufferBuilder::create_vector_direct::<i32>` at /Users/shep/.cargo/registry/src/github.com-1ecc6299db9ec823/flatbuffers-0.6.1/src/builder.rs:282:9
note: inside `main` at src/main.rs:3:5
   --> src/main.rs:3:5
    |
3   |     builder.create_vector_direct(&[1,2,3]);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: inside closure at /Users/shep/.rustup/toolchains/nightly-2020-04-07-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/rt.rs:67:34
    = note: inside closure at /Users/shep/.rustup/toolchains/nightly-2020-04-07-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/rt.rs:52:73
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<[closure@DefId(1:6093 ~ std[5a71]::rt[0]::lang_start_internal[0]::{{closure}}[0]::{{closure}}[0]) 0:&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /Users/shep/.rustup/toolchains/nightly-2020-04-07-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/sys_common/backtrace.rs:130:5
    = note: inside closure at /Users/shep/.rustup/toolchains/nightly-2020-04-07-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/rt.rs:52:13
    = note: inside `std::panicking::r#try::do_call::<[closure@DefId(1:6092 ~ std[5a71]::rt[0]::lang_start_internal[0]::{{closure}}[0]) 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /Users/shep/.rustup/toolchains/nightly-2020-04-07-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/panicking.rs:331:40
    = note: inside `std::panicking::r#try::<i32, [closure@DefId(1:6092 ~ std[5a71]::rt[0]::lang_start_internal[0]::{{closure}}[0]) 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe]>` at /Users/shep/.rustup/toolchains/nightly-2020-04-07-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/panicking.rs:274:15
    = note: inside `std::panic::catch_unwind::<[closure@DefId(1:6092 ~ std[5a71]::rt[0]::lang_start_internal[0]::{{closure}}[0]) 0:&&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /Users/shep/.rustup/toolchains/nightly-2020-04-07-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/panic.rs:394:14
    = note: inside `std::rt::lang_start_internal` at /Users/shep/.rustup/toolchains/nightly-2020-04-07-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/rt.rs:51:25
    = note: inside `std::rt::lang_start::<()>` at /Users/shep/.rustup/toolchains/nightly-2020-04-07-x86_64-apple-darwin/lib/rustlib/src/rust/src/libstd/rt.rs:67:5

[package]
name = "fb-repro"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
flatbuffers = "=0.6.1"

% rustc +nightly-2020-04-07-x86_64-apple-darwin --version --verbose
rustc 1.44.0-nightly (6dee5f112 2020-04-06)
binary: rustc
commit-hash: 6dee5f1126dfd5c9314ee5ae9d9eb010e35ef257
commit-date: 2020-04-06
host: x86_64-apple-darwin
release: 1.44.0-nightly
LLVM version: 9.0

[shepmaster]

There's a related error when reading the data:

   --> /Users/shep/.cargo/registry/src/github.com-1ecc6299db9ec823/flatbuffers-0.6.1/src/endian_scalar.rs:176:22
    |
176 |     let x = unsafe { *p };
    |                      ^^ accessing memory with alignment 1, but alignment 4 is required
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: inside `ingest::flatbuffers::read_scalar::<u32>` at /Users/shep/.cargo/registry/src/github.com-1ecc6299db9ec823/flatbuffers-0.6.1/src/endian_scalar.rs:176:22

[RalfJung]

Note that alignment errors in Miri can be false positives under some conditions, see rust-lang/miri#1074.

[github-actions]

This issue is stale because it has been open 6 months with no activity. Please comment or this will be closed in 14 days.

[CasperN]

Commenting to keep this open: I'm pretty sure this is correct. The current implementation assumes Vec<u8> is 16bit aligned which is only usually true for large vectors.

[RalfJung]

The Miri issue I mentioned above has been fixed, so re-running the tests in Miri should indicate if Miri found a true positive.

The current implementation assumes Vec is 16bit aligned which is only usually true for large vectors.

That would indeed definitely be UB.

[dvtomas]

What's the takeaway for me as a user? Should I avoid using create_vector_direct?

[CasperN]

No, its theoretically much worse than that. Users need to ensure the underlying buffer you're reading the buffer from is 8 byte aligned (which is usually true for buffers bigger than like 32 bytes) or your CPU can handle unaligned reads gracefully (which is also only usually true). This is a symptom of this other problem #5825. This problem is technically UB and its on our radar (we're building a verifier in #6161), but it doesn't seem to come up during normal use.

[RalfJung]

Users need to ensure the underlying buffer you're reading the buffer from is 8 byte aligned (which is usually true for buffers bigger than like 32 bytes) or your CPU can handle unaligned reads gracefully (which is also only usually true).

Correction: they need to ensure that either

• the buffer is sufficiently aligned, or
• the CPU can handle unaligned reads and the compiler did not introduce any code transformations that exploit well-alignment of these buffers.

It's not just CPUs that can cause ill-aligned buffers to break, it's also compiler transformations. One obvious example would be using vectored loads (that have alignment requirements even on x86); I am sure there are also more subtle examples such as optimizing ptr as usize & 0x1 to 0 because we know it is aligned.

[github-actions]

This issue is stale because it has been open 6 months with no activity. Please comment or this will be closed in 14 days.

[RalfJung]

FWIW, while the problem reproduces with flatbuffers 0.7...

error: Undefined Behavior: accessing memory with alignment 2, but alignment 4 is required
   --> /home/r/.cargo/registry/src/github.com-1ecc6299db9ec823/flatbuffers-0.7.0/src/endian_scalar.rs:157:9
    |
157 |         *mut_ptr = val;
    |         ^^^^^^^^^^^^^^ accessing memory with alignment 2, but alignment 4 is required
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

... at least the simple reproducer no longer works with flatbuffers 0.8.

[CasperN]

yea I think this is fixed at HEAD. I added Miri to the tests in #6393 (except for the fuzzing test)