Skip to content

grow_memory takes page size #260

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 21, 2016
Merged

grow_memory takes page size #260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions ml-proto/spec/eval.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,17 +279,15 @@ and eval_hostop c hostop vs at =
| GrowMemory, [v] ->
let mem = memory c at in
let delta = address32 v at in
if I64.rem_u delta Memory.page_size <> 0L then
Trap.error at "growing memory by non-multiple of page size";
let old_size = (Memory.size mem) in
let new_size = Int64.add old_size delta in
let old_size = Memory.size mem in
let new_size = Int64.(add old_size (mul delta Memory.page_size)) in
if I64.lt_u new_size old_size then
Trap.error at "memory size overflow";
(* Test whether the new size overflows the memory type.
* Since we currently only support i32, just test that. *)
if I64.gt_u new_size (Int64.of_int32 Int32.max_int) then
Trap.error at "memory size exceeds implementation limit";
Memory.grow mem (Int64.div delta Memory.page_size);
Memory.grow mem delta;
Some (Int32 (Int64.to_int32 old_size))

| _, _ ->
Expand Down
12 changes: 6 additions & 6 deletions ml-proto/test/memory_trap.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
(memory 1)

(export "store" $store)
(func $store (param $i i32) (param $v i32) (result i32) (i32.store (i32.add (memory_size) (get_local $i)) (get_local $v)))
(func $store (param $i i32) (param $v i32) (result i32)
(i32.store (i32.add (memory_size) (get_local $i)) (get_local $v))
)

(export "load" $load)
(func $load (param $i i32) (result i32) (i32.load (i32.add (memory_size) (get_local $i))))

(export "grow_memory" $grow_memory)
(func $grow_memory (param $i i32) (grow_memory (get_local $i)))
(func $load (param $i i32) (result i32)
(i32.load (i32.add (memory_size) (get_local $i)))
)

(export "overflow_memory_size" $overflow_memory_size)
(func $overflow_memory_size
Expand All @@ -28,5 +29,4 @@
(assert_trap (invoke "load" (i32.const 0)) "out of bounds memory access")
(assert_trap (invoke "store" (i32.const 0x80000000) (i32.const 13)) "out of bounds memory access")
(assert_trap (invoke "load" (i32.const 0x80000000)) "out of bounds memory access")
(assert_trap (invoke "grow_memory" (i32.const 3)) "growing memory by non-multiple of page size")
(assert_trap (invoke "overflow_memory_size") "memory size exceeds implementation limit")
22 changes: 5 additions & 17 deletions ml-proto/test/resizing.wast
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
(module
(memory 0)

(export "round_up_to_page" $round_up_to_page)
(func $round_up_to_page (param i32) (result i32)
(i32.and (i32.add (get_local 0) (i32.const 0xFFFF))
(i32.const 0xFFFF0000))
)

(export "load_at_zero" $load_at_zero)
(func $load_at_zero (result i32) (i32.load (i32.const 0)))

Expand All @@ -20,32 +14,26 @@
(func $store_at_page_size (result i32) (i32.store (i32.const 0x10000) (i32.const 3)))

(export "grow" $grow)
(func $grow (param $sz i32) (result i32)
(grow_memory (call $round_up_to_page (get_local $sz)))
)

(export "size_at_least" $size_at_least)
(func $size_at_least (param i32) (result i32) (i32.ge_u (memory_size) (get_local 0)))
(func $grow (param $sz i32) (result i32) (grow_memory (get_local $sz)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AstSemantics says this should be a multiple of the page size, but this sends in values like 1 which are meant to be multiplied by it. Which is wrong?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kripken It looks like WebAssembly/design#598 has not been merged which updated AstSemantics. People seemed to support changing grow_memory to accept units of pages rather than bytes and there seems to be some support for also returning the new size in pages.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks.


(export "size" $size)
(func $size (result i32) (memory_size))
)

(assert_return (invoke "size") (i32.const 0))
(assert_return (invoke "size_at_least" (i32.const 0)) (i32.const 1))
(assert_trap (invoke "store_at_zero") "out of bounds memory access")
(assert_trap (invoke "load_at_zero") "out of bounds memory access")
(assert_trap (invoke "store_at_page_size") "out of bounds memory access")
(assert_trap (invoke "load_at_page_size") "out of bounds memory access")
(assert_return (invoke "grow" (i32.const 4)) (i32.const 0))
(assert_return (invoke "size_at_least" (i32.const 4)) (i32.const 1))
(assert_return (invoke "grow" (i32.const 1)) (i32.const 0))
(assert_return (invoke "size") (i32.const 0x10000))
(assert_return (invoke "load_at_zero") (i32.const 0))
(assert_return (invoke "store_at_zero") (i32.const 2))
(assert_return (invoke "load_at_zero") (i32.const 2))
(assert_trap (invoke "store_at_page_size") "out of bounds memory access")
(assert_trap (invoke "load_at_page_size") "out of bounds memory access")
(assert_return (invoke "grow" (i32.const 4)) (i32.const 65536))
(assert_return (invoke "size_at_least" (i32.const 8)) (i32.const 1))
(assert_return (invoke "grow" (i32.const 4)) (i32.const 0x10000))
(assert_return (invoke "size") (i32.const 0x50000))
(assert_return (invoke "load_at_zero") (i32.const 2))
(assert_return (invoke "store_at_zero") (i32.const 2))
(assert_return (invoke "load_at_page_size") (i32.const 0))
Expand Down