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

segmentation fault when allocating #23

Closed
Geal opened this issue Nov 21, 2018 · 7 comments
Closed

segmentation fault when allocating #23

Geal opened this issue Nov 21, 2018 · 7 comments

Comments

@Geal
Copy link
Contributor

Geal commented Nov 21, 2018

(posting this here instead of only IRC)
I was trying some wasm code with allocations and got a segmentation fault while running it.
To reproduce:

#![feature(link_args)]
#![link_args = "--import-memory --initial-memory=1114112 --max-memory=65536000"]

#[no_mangle]
pub extern "C" fn hello(ctx: *const *mut u8) {
  let v = "hello".to_string();
}

(note: I tried using std::alloc directly, but could not reproduce)

I get the following output with lldb:

$ lldb ./target/debug/wasmtime 
(lldb) target create "./target/debug/wasmtime"
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/copy.py", line 52, in <module>
    import weakref
  File "/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/weakref.py", line 14, in <module>
    from _weakref import (
ImportError: cannot import name _remove_dead_weakref
Current executable set to './target/debug/wasmtime' (x86_64).
(lldb) r ./testalloc.wasm --function=hello
Process 55272 launched: './target/debug/wasmtime' (x86_64)
Process 55272 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x2029ffff0)
    frame #0: 0x000000010179ecb9
->  0x10179ecb9: movl   %eax, (%rdx,%rcx)
    0x10179ecbd: movl   $0x5, %eax
    0x10179ecc3: movl   0x14(%rsp), %ecx
    0x10179eccb: movl   %ecx, %ecx
Target 0: (wasmtime) stopped.
(lldb) register read
General Purpose Registers:
       rax = 0x0000000000110008
       rbx = 0x0000000102821000
       rcx = 0x00000000fffffff0
       rdx = 0x0000000102a00000
       rdi = 0x0000000102a00000
       rsi = 0x0000000102a00000
       rbp = 0x00007ffeefbfd660
       rsp = 0x00007ffeefbfd640
        r8 = 0x000000000000006f
        r9 = 0x0000000000000005
       r10 = 0x000000010160d008
       r11 = 0x00007ffdee465e60
       r12 = 0x0000000000000000
       r13 = 0x0000000000000000
       r14 = 0x00007ffeefbfed00
       r15 = 0x00007ffeefbfecc0
       rip = 0x000000010179ecb9
    rflags = 0x0000000000010206
        cs = 0x000000000000002b
        fs = 0x0000000000000000
        gs = 0x0000000000000000

This happens right after grow_memory is called. It adds a page and returns 17 (17 pages specified by the --initial-memory link arg).

From the debugger output, I see in the movl %eax, (%rdx,%rcx) instructions that rdx = 0x0000000102a00000, which is the base address for the LinearMemory, to which rcx = 0x00000000fffffff0 is added.
What I suspect here is that it's not trying to write rax = 0x0000000000110008 (which is 1114120 ie the initial memory size + 8) at the address 0x2029ffff0, but at the LinearMemory's base address minus 16 bytes, using an addition with overflow. Wasm32 assumes 32bit addresses and offsets, but here we're compiling to x86_64, so it won't ovrflow.

@sunfishcode
Copy link
Member

Thanks for debugging this to this point! I'll take a closer look next week.

@Geal
Copy link
Contributor Author

Geal commented Nov 29, 2018

BTW, I was able to reproduce with a much smaller example:

#![feature(link_args)]
#![allow(unused_attributes)] // link_args actually is used
#![link_args = "--import-memory --initial-memory=1114112 --max-memory=65536000"]

fn address(v: *mut u8) {
  unsafe {
    let v2 = (v as usize - 16) as *mut u8;
    *v2 = 0x32;
  };
}
#[no_mangle]
pub extern "C" fn load() {
  unsafe {
    let v: *mut u8 = 2_147_483_647 as *mut u8;

    address(v);
  }
}
(module
  (type $t0 (func))
  (type $t1 (func (param i32)))
  (import "env" "memory" (memory $env.memory 17 1000))
  (func $testload::address::hfcb094981e27477e (type $t1) (param $p0 i32)
    (local $l0 i32) (local $l1 i32) (local $l2 i32)
    (set_local $l0
      (i32.const 50))
    (set_local $l1
      (i32.const 16))
    (set_local $l2
      (i32.sub
        (get_local $p0)
        (get_local $l1)))
    (i32.store8
      (get_local $l2)
      (get_local $l0))
    (return))
  (func $load (export "load") (type $t0)
    (local $l0 i32)
    (set_local $l0
      (i32.const 2147483647))
    (call $testload::address::hfcb094981e27477e
      (get_local $l0))
    (return))
  (table $__indirect_function_table (export "__indirect_function_table") 1 1 anyfunc)
  (global $__heap_base (export "__heap_base") i32 (i32.const 1048576))
  (global $__data_end (export "__data_end") i32 (i32.const 1048576)))

LLDB output:

(lldb) r
Process 49228 launched: './target/debug/wasmtime' (x86_64)
make_heap offset=0, offset32=0
Process 49228 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x182afffef)
    frame #0: 0x0000000101796599
->  0x101796599: movb   %al, (%rdx,%rcx)
    0x10179659d: popq   %rbp
    0x10179659f: retq
    0x1017965a0: addl   %eax, (%rax)
Target 0: (wasmtime) stopped.
(lldb) dis -s 0x101796590
    0x101796590: movl   %edi, %ecx
    0x101796592: movq   0x8(%rsi), %rdx
    0x101796596: movq   (%rdx), %rdx
->  0x101796599: movb   %al, (%rdx,%rcx)
    0x10179659d: popq   %rbp
    0x10179659f: retq
    0x1017965a0: addl   %eax, (%rax)
    0x1017965a2: addb   %al, (%rax)
    0x1017965a4: addb   %al, (%rax)
    0x1017965a6: addb   %al, (%rax)
    0x1017965a8: addl   %eax, (%rax)
    0x1017965aa: movl   $0x7ffeef, %edi           ; imm = 0x7FFEEF
(lldb) register read
General Purpose Registers:
       rax = 0x0000000000000032
       rbx = 0x00007ffeefbfeca8
       rcx = 0x000000007fffffef
       rdx = 0x0000000102b00000
       rdi = 0x000000007fffffef
       rsi = 0x000000010164b000
       rbp = 0x00007ffeefbfd640
       rsp = 0x00007ffeefbfd640
        r8 = 0x0000000000000064
        r9 = 0x0000000000000004
       r10 = 0x0000000000000004
       r11 = 0x00007ffdee465e50
       r12 = 0x0000000000000000
       r13 = 0x0000000000000000
       r14 = 0x00007ffeefbfece0
       r15 = 0x00007ffeefbfeca0
       rip = 0x0000000101796599
    rflags = 0x0000000000010203
        cs = 0x000000000000002b
        fs = 0x0000000000000000
        gs = 0x0000000000000000

@Geal
Copy link
Contributor Author

Geal commented Nov 29, 2018

here is the wat without memory imports:

(module
  (type $t0 (func))
  (type $t1 (func (param i32)))
  (func $testload::address::hfcb094981e27477e (type $t1) (param $p0 i32)
    (local $l0 i32) (local $l1 i32) (local $l2 i32)
    (set_local $l0
      (i32.const 50))
    (set_local $l1
      (i32.const 16))
    (set_local $l2
      (i32.sub
        (get_local $p0)
        (get_local $l1)))
    (i32.store8
      (get_local $l2)
      (get_local $l0))
    (return))
  (func $load (export "load") (type $t0)
    (local $l0 i32)
    (set_local $l0
      (i32.const 2147483647))
    (call $testload::address::hfcb094981e27477e
      (get_local $l0))
    (return))
  (table $__indirect_function_table (export "__indirect_function_table") 1 1 anyfunc)
  (memory $memory (export "memory") 16)
  (global $__heap_base (export "__heap_base") i32 (i32.const 1048576))
  (global $__data_end (export "__data_end") i32 (i32.const 1048576)))

@sunfishcode
Copy link
Member

This wat file is doing a store to an arbitrary memory address which is beyond the end of the linear memory, so the expected behavior is to trap. On master, it now reports the trap.

@Geal
Copy link
Contributor Author

Geal commented Dec 8, 2018

this last file was an example to demonstrate the issue of going from 32 bit wasm to x86_64 pointers. I have not tested yet if the first example would now work, since memory imports changed recently

@sunfishcode
Copy link
Member

There have now been a bunch more changes, and lots of bugs fixed -- Wasmtime now passes the entire spec testsuite! So it'd be interesting to retry your testcase here now.

@sunfishcode
Copy link
Member

Closing; if there are any further issues, please report them!

sunfishcode pushed a commit to sunfishcode/wasmtime that referenced this issue Oct 2, 2019
pchickey pushed a commit to pchickey/wasmtime that referenced this issue May 12, 2023
* Implement the `fd_readdir` function

This roughly matches the implementation in `wasi-common` today where it
uses the directory entry stream as a form of iterator and the `cookie`
represents the `n`th iteration. Not exactly efficient if the `buf`
provided to the hostcall is too small to hold many of the entries but
there's not a whole lot else that can be done at this time.

This also updates the WIT for `read-dir-entry` to return
`option<dir-entry>` instead of `dir-entry` to indicate EOF.

* Fix host compile

* Add a specific method to close a dir-entry-stream

* Add a cache to avoid quadratic dirent behavior

When a readdir call is truncated due to the output buffer not being
large enough save the final state of the readdir iterator into `State`
to possibly get reused on the next call to `readdir`. Some limits are
put in place such as:

* The next call to readdir must be for the same fd and the required
  cookie.
* The dirent that didn't fit must have its full name fit within the path
  cache.
* If `fd_close` is called after a readdir it clears the cache so a
  future `fd_readdir` doesn't actually resume now-stale state.

There's a fair bit of trickiness here so I've attempted to structure
things as "simply" as possible to hopefully reduce the chance there's an
issue, but this is all untested so there's still likely an off-by-one or
similar bug.
pchickey pushed a commit to pchickey/wasmtime that referenced this issue May 16, 2023
* Implement the `fd_readdir` function

This roughly matches the implementation in `wasi-common` today where it
uses the directory entry stream as a form of iterator and the `cookie`
represents the `n`th iteration. Not exactly efficient if the `buf`
provided to the hostcall is too small to hold many of the entries but
there's not a whole lot else that can be done at this time.

This also updates the WIT for `read-dir-entry` to return
`option<dir-entry>` instead of `dir-entry` to indicate EOF.

* Fix host compile

* Add a specific method to close a dir-entry-stream

* Add a cache to avoid quadratic dirent behavior

When a readdir call is truncated due to the output buffer not being
large enough save the final state of the readdir iterator into `State`
to possibly get reused on the next call to `readdir`. Some limits are
put in place such as:

* The next call to readdir must be for the same fd and the required
  cookie.
* The dirent that didn't fit must have its full name fit within the path
  cache.
* If `fd_close` is called after a readdir it clears the cache so a
  future `fd_readdir` doesn't actually resume now-stale state.

There's a fair bit of trickiness here so I've attempted to structure
things as "simply" as possible to hopefully reduce the chance there's an
issue, but this is all untested so there's still likely an off-by-one or
similar bug.
dhil added a commit to dhil/wasmtime that referenced this issue Oct 16, 2023
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

2 participants