feat: support custom-page-size proposal

[HerrCai0907]

custom page size proposal

Fixed: #6873
Original PR: #7694

[kripken]

Comments on the first 1/4 or so. Nice work!

[HerrCai0907]

test/spec/testsuite/proposals/custom-page-sizes/memory_max.wast will use 4GB of RAM for memory. which may cause the tsan failed. It can work locally (in my 64GiB linux). @kripken @stevenfontanella what do you think for those cases, Do you think we can disable them in CI environment?

;; i32 (pagesize 1)
(assert_unlinkable
  (module
    (import "test" "unknown" (func))
    (memory 0xFFFF_FFFF (pagesize 1)))
  "unknown import")

;; i32 (default pagesize)
(assert_unlinkable
  (module
    (import "test" "unknown" (func))
    (memory 65536 (pagesize 65536)))
  "unknown import")

[HerrCai0907]

5c7061e can resolve the RAM usage issue, should I split it to another PR?

[kripken]

Hmm, why does that commit fix it?

I think the fix might be to use a limit on allocation size. Here is where we do it for GC allocations:

binaryen/src/wasm-interpreter.h

Lines 2246 to 2250 in 204c3f8

	// Arbitrary deterministic limit on size. If we need to allocate a Literals
	// vector that takes around 1-2GB of memory then we are likely to hit memory
	// limits on 32-bit machines, and in particular on wasm32 VMs that do not
	// have 4GB support, so give up there.
	static const Index DataLimit = (1 << 30) / sizeof(Literal);

Something similar for memories and tables makes sense to me?

[HerrCai0907]

In spec test, they do trick for it. It declare a non-existed import to make the module initialization failure. But in binaryen, we init the memory before analyzing import and export. It will allocate lots of RAM and slow down the whole test. That commit swap the order to let the initialization fast failed.

;;
;; These modules are valid, but instantiating them is unnecessary
;; and would only allocate very large memories and slow down running
;; the spec tests. Therefore, add an invalid import so that it cannot
;; be instantiated and use `assert_unlinkable`. This approach
;; enforces that the module itself is still valid, but that its
;; instantiation fails early (hopefully before any memories are
;; actually allocated).

;; Helper module that exports a non-matching value.
(module
  (memory (export "unknown") 0))

(register "test")

;; i32 (pagesize 1)
(assert_unlinkable
  (module
    (import "test" "unknown" (func))
    (memory 0xFFFF_FFFF (pagesize 1)))
  "unknown import")

[kripken]

I see, thanks. I still think my suggested fix is better, though - it is consistent with how we handle possible OOMs in other parts of the code, and will automatically work for any new tests added later in the spec?

[stevenfontanella]

Both changes sound good to me FWIW. Based on the test and the spec here:

, we are free to check imports before initializing the memory (which is all that externalInterface->init() does link). AFAICT the other way is valid too since nothing 'observable' happens yet, but we might as well check imports first.

[HerrCai0907]

I see, thanks. I still think my suggested fix is better, though - it is consistent with how we handle possible OOMs in other parts of the code, and will automatically work for any new tests added later in the spec?

I think that imports should always be checked before initializing memory, regardless of this PR. Limiting the max ram is also reasonable.

But maybe we could focus on custom page size proposal in this PR firstly. Personally I prefer to create a new PR for the latest commit (order of import checking and memory initializing), WDYT?