No termination on out of memory

[denis-sh]

WasmAllocator.allocate does not terminate on out of memory

wasmMemoryGrow function returns -1 if allocation fails. But currently WasmAllocator.grow just ignores wasmMemoryGrow(pages) return value thus resulting in essentially undefined behavior of application, practically overwriting its own memory.

Instead onOutOfMemoryError() must be called.

[skoppe]

I think the wasm engine will stop you from accessing that memory. But I agree, better to call onOutOfMemoryError.

[denis-sh]

I think the wasm engine will stop you from accessing that memory

Do not forget about integer overflow. There are no overflow checks in WasmAllocator. Anyway, as a result practically application overwrites its own memory:

import spasm.rt.memory : allocator;

enum MB = 1024 * 1024;
void[] data1 = allocator.allocate(500 * MB);
void[] data2 = allocator.allocate(500 * MB);
void[] data3 = allocator.allocate(500 * MB);
void[] data4 = allocator.allocate(500 * MB);
void[] data5 = allocator.allocate(500 * MB);
void[] data6 = allocator.allocate(500 * MB); // same as data5 (at least in 32-bit mode)
assert(data1 !is data2);
assert(data2 !is data3);
assert(data3 !is data4);
assert(data4 !is data5);
assert(data5 is data6);

[skoppe]

Do not forget about integer overflow.

Fair point. I didn't think anybody would allocate so much as browsers are capped at 1gb.

Regardless, you already convinced me. We need some better checks there.

In master I do use the return value from wasmMemoryGrow https://github.com/skoppe/spasm/blob/a17a8c62599b66dbf6ff40477c82b23ded79281c/source/spasm/rt/allocator.d#L49

But there are still issues there. We need to check the result -1 and cap memory so it doesn't overflow or use long.