1713400 - Generally use page lengths instead of byte lengths for linear memory

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, enhancement, P3)

Description

[Ryan Hunt [:rhunt]]

With large-buffers and memory32 the maximum specifiable page size is 2^16 which overflows a uint32_t when converted to a byte length. With memory64 the maximum page size will be 2^48, which will overflow a uint64_t.

We cannot whole-sale reject these values as an implementation limit as it's valid to specify a huge 'maximum' limit with the expectation that engines will just fail to grow to that limit if requested at runtime.

I've found that most logic for memory limits/objects is cleaner expressed in pages, and this works around the overflow issue.

Comment 1

[Lars T Hansen [:lth]]

(I think comment 0 has a couple of typos, talking about "maximum page size", it's really "maximum page count" or "maximum memory size".)

This seems reasonable. There's the lingering matter of perhaps one day allowing different page sizes, but at worst that means that we have to carry the specified page size along with the memory size.

Clearly bounds check limits used by jitted code must continue to be expressed in bytes.

Comment 2

[Ryan Hunt [:rhunt]]

Attachment: Bug 1713400 - wasm: Add MemoryDesc as analog to TableDesc. r?lth

This is a clean-up to add a first-class MemoryDesc object,
replacing the ad-hoc methods of describing a module's memory.

Temporarily, this is just a Limits (in pages) and the byte length converted
versions of Limits. The byte length fields will be dropped later.

asm.js used to rely heavily on mutating the ad-hoc representation on
ModuleEnvironment. This commit adds a ModuleEnvironmentShared::Memory
struct for simulating the old way of mutating ModuleEnvironment.

Comment 3

[Ryan Hunt [:rhunt]]

Attachment: Bug 1713400 - wasm: Reduce dependencies on MemoryDesc byte length's. r?lth

Add accessor methods and tweak code to reduce users
that depend on MemoryDesc exposing raw byte lengths for
the initial and maximum fields.

Additionally, MemoryKind is added to MemoryDesc so we can start
asserting in code that assumes Memory32.

Depends on D116650

Comment 4

[Ryan Hunt [:rhunt]]

Attachment: Bug 1713400 - wasm: Switch memory runtime code to use pages instead of byte lengths. r?lth

This commit updates the memory runtime code to generally
use pages instead of byte lengths. In terms of runtime
structures: limits are in pages, memory object max
size is in pages, memory object current size is in bytes,
and the TlsData bounds check limit is in bytes.

This code is tricky, so I've added a 'wasm::Pages' typed unit
to catch cases of type confusion and make intent clearer. I've
also tried to add asserts, comments, and rename unclear variables.

The array buffer's (shared and non-shared) still track their
length in bytes (this is best as the byte length is needed for
warm code in memory.copy/fill), but now the 'maximum' field is
stored in pages. This allows maximum values that would overflow
byte length's with memory64 to be represented.

The array buffer interface for wasm now generally only uses
pages, and most code internal operates on pages. There are
still a few cases where byte lengths are required and we add
code or assert that converting from pages to byte lengths is
valid.

Depends on D116651

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/89276eacc8d6
wasm: Add MemoryDesc as analog to TableDesc. r=lth
https://hg.mozilla.org/integration/autoland/rev/e3729fc28c4a
wasm: Reduce dependencies on MemoryDesc byte length's. r=lth
https://hg.mozilla.org/integration/autoland/rev/4bf424b6f46a
wasm: Switch memory runtime code to use pages instead of byte lengths. r=lth

Comment 6

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/89276eacc8d6
https://hg.mozilla.org/mozilla-central/rev/e3729fc28c4a
https://hg.mozilla.org/mozilla-central/rev/4bf424b6f46a