Skip to content
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

Inserting large structure in to HashMap causes "memory access out of bounds" #246

Closed
NathanFlurry opened this issue Jan 12, 2019 · 9 comments

Comments

@NathanFlurry
Copy link

When attempting to insert a large structure in to a HashMap with the wasm32-unknown-unknown target, I get this error:

wasm-00555126-2:79 Uncaught (in promise) RuntimeError: memory access out of bounds
    at _ZN66_$LT$std..collections..hash..table..RawTable$LT$K$C$$u20$V$GT$$GT$26new_uninitialized_internal17h68411f9faf813ecfE (wasm-function[2]:150)
    at _ZN72_$LT$std..collections..hash..map..HashMap$LT$K$C$$u20$V$C$$u20$S$GT$$GT$10try_resize17h6ab1bbea772cb23bE (wasm-function[5]:74)
    at _ZN72_$LT$std..collections..hash..map..HashMap$LT$K$C$$u20$V$C$$u20$S$GT$$GT$6insert17hfec45d344a00f122E (wasm-function[6]:269)
    at crash_wasm (wasm-function[18]:78)
    at fetch.then.then.then.results (http://localhost:8000/:13:26)

This code produces the error:
Rust:

use std::collections::HashMap;

const LARGE_STRUCT_SIZE: usize = 131072;  // Try setting this number to something like 65536 to make the demo work

struct LargeStruct {
    values: [u8; LARGE_STRUCT_SIZE]
}

#[no_mangle]
pub unsafe fn crash_wasm() -> i32 {
    let ls = LargeStruct { values: [0; LARGE_STRUCT_SIZE] };

    let mut hm = HashMap::new();
    hm.insert("test", ls);  // This panics

    // HashSet is broken too, of course, since it uses a `HashMap` under the hood

    // But this works:
//    let b = Box::new(ls);

    42
}

JavaScript:

fetch('main.wasm')
    .then(response => response.arrayBuffer())
    .then(bytes => WebAssembly.instantiate(bytes, {}))
    .then(results => {
        let { crash_wasm } = results.instance.exports
        let result = crash_wasm();
        console.log("result", result);
    });

Build script:

rustc +nightly --target wasm32-unknown-unknown -O --crate-type=cdylib src/lib.rs -o public/main.wasm

The LARGE_STRUCTURE_SIZE cutoff is somewhere between 65536 to 131072 bytes; I get different values with and without stdweb. I've tried this with wee_alloc also with no success.

@CryZe
Copy link

CryZe commented Jan 12, 2019

OP was also able to replicate this with Box, so this is likely unrelated to HashMap itself. They posted this earlier in Discord:

//const LARGE_STRUCT_SIZE: usize = 87333;  // This is the largest I can create the object without any issues
const LARGE_STRUCT_SIZE: usize = 87334;

struct LargeStruct {
    values: [u8; LARGE_STRUCT_SIZE]
}

fn main() {
    let ls = LargeStruct { values: [0; LARGE_STRUCT_SIZE] };
    let mut hm = Box::new(ls);
}

(May need stdweb)

@fitzgen
Copy link
Member

fitzgen commented Jan 14, 2019

This sounds like maybe a bug in the dlmalloc wasm port. Would be interested in knowing if this also reproduces with wee_alloc as the global allocator.

+cc @alexcrichton

@CryZe
Copy link

CryZe commented Jan 14, 2019

This also happens with wee_alloc.

@alexcrichton
Copy link
Contributor

This sounds like it's likely related to the stack size being too slow and we're probably seeing a stack overflow here. Large structs like this are copied around the stack a bunch of times, especially in debug mode.

Are these examples compiled in debug mode? Is it possible to see the faulting instruction and maybe see if the global stack pointer is less than zero at that point?

@pepyakin
Copy link
Member

pepyakin commented Jan 14, 2019

Can't reproduce this.

I took the code from this comment. Then I compiled it with this command
rustc -Copt-level=3 --target=wasm32-unknown-unknown main.rs

and launched with this js snippet from node:

const fs = require('fs');
const buffer = fs.readFileSync('main.wasm');

let m = new WebAssembly.Module(buffer);
let instance = new WebAssembly.Instance(m, {});
instance.exports.main();

I tried both stable and nightly and both with opts and without.

However, I can reproduce this with the original one. I have this stack trace:

RuntimeError: memory access out of bounds
    at _ZN66_$LT$std..collections..hash..table..RawTable$LT$K$C$$u20$V$GT$$GT$12new_internal17h09cc4625f0ee3597E.llvm.8592423435358234713 (wasm-function[7]:149)
    at _ZN72_$LT$std..collections..hash..map..HashMap$LT$K$C$$u20$V$C$$u20$S$GT$$GT$10try_resize17h65d502b1a88241dcE (wasm-function[2]:72)
    at _ZN72_$LT$std..collections..hash..map..HashMap$LT$K$C$$u20$V$C$$u20$S$GT$$GT$6insert17h24438fbe74e23baeE (wasm-function[3]:266)
    at crash_wasm (wasm-function[5]:84)
    at Object.<anonymous> (/Users/pepyakin/tmp/rustwasm-246/op.js:6:18)
    at Module._compile (internal/modules/cjs/loader.js:688:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:699:10)
    at Module.load (internal/modules/cjs/loader.js:598:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:537:12)
    at Function.Module._load (internal/modules/cjs/loader.js:529:3)

Quick inspection of a resulting wasm shows that new_internal doesn't open a new stack frame, so there are good chances that this is not a stackoverflow ...

Update:

Here is the listing of what I'm getting, and here is the line where it is crashing.

Update:

The load is happening with negative address: -84

@NathanFlurry
Copy link
Author

NathanFlurry commented Jan 14, 2019

@pepyakin I believe when I was testing the issue with Box, I was using cargo web start which watches for changes and automatically recompiles. However, the auto-compilation wasn't working when I thought it was, therefore giving me a false positive since the version I was running was still using the HashMap. As far as I can tell, the issue is contained to HashMap. My bad!

Edit: I only tested wee_aloc with stdweb and the issue still occurred.

@pepyakin
Copy link
Member

pepyakin commented Jan 14, 2019

No worries @NathanFlurry !

I can confirm that this is the regular stack overflow. -84 is value that is derived from the stack pointer. Here are stack frame sizes that I can see:

crash_wasm - 262192
insert - 655408
try_resize - 262192
new_internal - 0

which is totaling: 1179792 bytes
and we reserve 1048576 bytes for the stack. Case closed! : )

@NathanFlurry
Copy link
Author

Great! Before we close the issue, is there a way to see the default stack size for wasm targets? Right now I'm trying something like this with no success:

rustc -Copt-level=3 -Clink-args='-z stack-size=5000000' --target=wasm32-unknown-unknown src/lib.rs -o public/main.wasm

I saw this commit which shouldn't override whatever stack size I pass, am I missing something?

@alexcrichton
Copy link
Contributor

@NathanFlurry there's currently no API to see the default stack size, but a recent PR was targeted at allowing the exact invocation you had above to set the stack size. It was a mistake that doesn't work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants