Skip to content

#[repr(align(...))] structures cause an out of bounds memory access #1435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
marmistrz opened this issue Mar 30, 2020 · 5 comments
Closed

#[repr(align(...))] structures cause an out of bounds memory access #1435

marmistrz opened this issue Mar 30, 2020 · 5 comments
Labels
bug Incorrect behavior in the current implementation that needs fixing

Comments

@marmistrz
Copy link
Contributor

I'm not sure if I should report this issue here or in rust-lang/rust or even somewhere else.

Consider the following code:

#[repr(align(64))]
#[derive(Copy, Clone, Default, Debug)]
pub struct CacheBucket([CacheEntry; 3]);

#[repr(C)]
#[derive(Copy, Clone, Default, Debug)]
pub struct CacheEntry {
    pub alpha: bool
}

fn main() {
    let buckets = 4194304;
    let xd = vec![CacheBucket::default(); buckets];
    println!("{:?}", xd);
}

compiled to wasm using rustup run stable cargo build --target=wasm32-wasi with rustc 1.42.0 (b8cedc004 2020-03-09)

Then run the wasm binary using wasmtime:

$ cargo run --release -- run oob.wasm
    Finished release [optimized] target(s) in 0.09s
     Running `target/release/wasmtime run ../chess/oob/target/wasm32-wasi/debug/oob.wasm`
Error: failed to run main module `../chess/oob/target/wasm32-wasi/debug/oob.wasm`

Caused by:
    0: failed to invoke `_start`
    1: wasm trap: out of bounds memory access, source location: @56bf
       wasm backtrace:
         0: <unknown>!core::ptr::write::h5550856a4b897d6d
         1: <unknown>!alloc::vec::Vec<T>::extend_with::h42807d753bccf81d
         2: <unknown>!<T as alloc::vec::SpecFromElem>::from_elem::h3a932571fd38db0f
         3: <unknown>!alloc::vec::from_elem::ha55e830b1b839b10
         4: <unknown>!oob::main::h5b4df0d7d93a566d
         5: <unknown>!std::rt::lang_start::{{closure}}::h4b37f8cbafc64616
         6: <unknown>!std::sys_common::backtrace::__rust_begin_short_backtrace::h09ce0967d7b1e05d
         7: <unknown>!std::panicking::try::do_call::h9f56360d08e2b60c
         8: <unknown>!__rust_maybe_catch_panic
         9: <unknown>!std::rt::lang_start_internal::h1c0f367ef14e29b3
         10: <unknown>!std::rt::lang_start::h81110b05d9ba24fa
         11: <unknown>!__original_main
         12: <unknown>!_start

The code runs correctly if buckets is small (e.g. 4) or if #[repr(align(64))] is removed.
wasmtime master, 78772cf

@marmistrz marmistrz added the bug Incorrect behavior in the current implementation that needs fixing label Mar 30, 2020
@bjorn3
Copy link
Contributor

bjorn3 commented Mar 30, 2020

How much memory is the wasm module allowed to use? The version with #[repr(align(64))] would need 256MiB for buckets alone, while the version without would need just 16MiB.

@marmistrz
Copy link
Contributor Author

How should I check it? For reference, the original structure looks like this:

pub struct CacheEntry {
    pub alpha: bool,
    pub beta: bool,
    pub pv_node: bool,
    pub depth: i8,
    pub plies_played: u16,
    pub score: i16,
    pub upper_hash: u32,
    pub lower_hash: u32,
    pub mv: u16,
    pub static_evaluation: i16,
}

Switching to this defintion of CacheEntry doesn't change anything.

@pchickey
Copy link
Contributor

FWIW: Lucet gets a HeapOutOfBounds fault running this module (as I compiled it locally with 1.40, we're still stuck on snapshot 0).

@alexcrichton
Copy link
Member

alexcrichton commented Mar 30, 2020

Looks like this was a bug in the Rust standard library, fixed here.

As a local workaround while we wait for that to propagate you can depend directly on dlmalloc and use that as a global allocator:

# in Cargo.toml
[dependencies]
dlmalloc = { version = "0.1", features = ['global'] }
#[global_allocator]
static A: dlmalloc::GlobalDlmalloc = dlmalloc::GlobalDlmalloc;

#[repr(align(64))]
#[derive(Copy, Clone, Default, Debug)]
pub struct CacheBucket([CacheEntry; 3]);

#[repr(C)]
#[derive(Copy, Clone, Default, Debug)]
pub struct CacheEntry {
    pub alpha: bool,
}

fn main() {
    let buckets = 4194304;
    let xd = vec![CacheBucket::default(); buckets];
    let string = format!("{:?}", xd);
    println!("{}", string.len());
}

@marmistrz
Copy link
Contributor Author

This works for me on nightly, so I guess we can close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior in the current implementation that needs fixing
Projects
None yet
Development

No branches or pull requests

4 participants