Skip to content

[interpreter/test] Fix boundary conditions on memory/table growth - #2290

Merged
rossberg merged 3 commits into
mainfrom
fix.2281
Oct 21, 2026
Merged

rossberg merged 3 commits into
mainfrom
fix.2281

Conversation

@rossberg

Copy link
Copy Markdown
Member

Fixes #2281 and fixes #2288.

@rossberg
rossberg requested a review from tlively October 21, 2026 11:06
Comment thread interpreter/runtime/table.ml Outdated
Comment on lines +28 to +29
with Out_of_memory | Invalid_argument _ -> raise OutOfMemory
with Out_of_memory -> raise OutOfMemory

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Gemini tells me this can still raise Invalid_argument when size exceeds Sys.max_array_length or Int64.of_int Stdlib.max_int.

| I64AT -> I64.le_u i 0x1_0000_0000_0000L

let create n =
if n > Int64.(div max_int page_size) then raise OutOfMemory else

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Gemini tells me that there will be an uncaught Invalid_argument for page counts in [2^46, 2^47 - 1] on 64-bit OCaml. This is the range where size > Int64.of_int Stdlib.max_int.

Comment thread test/core/memory_grow.wast Outdated
Comment on lines +440 to +442
;; (assert_return (invoke "size" (i32.const 0xFFFF))
;; (either (i32.const 1) (i32.const 0x1_0000))
;; )

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
;; (assert_return (invoke "size" (i32.const 0xFFFF))
;; (either (i32.const 1) (i32.const 0x1_0000))
;; )
;; (assert_return (invoke "size"))
;; (either (i32.const 1) (i32.const 0x1_0000))
;; )

Comment on lines +105 to +110
(assert_return (invoke "grow" (i64.const 0xFFFF_FFFF_FFFF))
(either (i64.const -1) (i64.const 1))
)
(assert_return (invoke "size")
(either (i64.const 1) (i64.const 0x1_0000_0000_0000))
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are these commented out as being potentially too slow for 32-bit tables and memories but not 64-bit tables and memories? Is it assumed that the 64-bit tests will always fail to grow quickly in practice?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, precisely. For the 64-bit ones the grow cannot succeed on any practical machine, since it would take up the entire hardware address space.

@rossberg

Copy link
Copy Markdown
Member Author

Comments addressed, PTAL.

@rossberg
rossberg merged commit ee7bdb5 into main Oct 21, 2026
13 checks passed
@rossberg
rossberg deleted the fix.2281 branch October 21, 2026 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants