Binaryen: Misuse of size_t vs uint64_t?

[juj]

The type size_t is not guaranteed to be 64-bit, but it looks like the code assumes in several places that it always is. We should replace size_t with uint64_t in all places where a strictly necessary 64-bit integer is required.

An example of where it looks like 64-bit integers are intended(?) is S2WasmBuilder in https://github.com/WebAssembly/binaryen/blob/master/src/s2wasm.h#L50, and the size_t members in https://github.com/WebAssembly/binaryen/blob/master/src/s2wasm.h#L90. These are explicitly parsed as unsigned long long from https://github.com/WebAssembly/binaryen/blob/master/src/s2wasm-main.cpp#L86, indicating that these should be 64-bit always? However e.g. when building on Visual Studio on Windows as a 32-bit executable, size_t is only 32-bit and truncation will occur.

In current executing form, in wasm runtime addresses are still 32-bit, but I presume the code intends to be forward compatible and operate on 64-bit integers for all addresses for the future when we might expand to a 64-bit address space?

[juj]

Another such location is in src\wasm-interpreter.h, where there is a function size_t getFinalAddress(LS* curr, Literal ptr), but the function body returns a uint64_t, which gets truncated to 32-bit size_t. The return value should probably be a uint64_t, but this in turn ripples to where the function gets called, so needs a bit larger change to coordinate.

[dschuff]

We can't really support running wasm64 code in a 32-bit process, so I don't think values that are supposed to hold linear address indices necessarily need to be 64-bit. Maybe what we really should have is some new type address_t or some thing like that to make it more clear.

Actually it's a bit more subtle; the interpreter can't support 64-on-32, but I don't see any reason s2wasm (and maybe asm2wasm?) can't. Either way though, a new type name would be good just for readability.

[juj]

"We can't really support running wasm64 code in a 32-bit process, so I don't think values that are supposed to hold linear address indices necessarily need to be 64-bit."

I was thinking about the case that the binaryen toolchain was compiled as a 32-bit executable (e.g. default CMake config does that), but that it was still used to target wasm64. I know wasm64 is not a thing yet, but I presume the size_ts were being used with the intent to be forward compatible? So the bitness of the native executables doesn't have a meaning here, but addresses are intended to always be 64-bit to be future compatible with wasm64?

Agreed that this is not a problem yet, but marked this down since this is generating a bunch of Visual Studio compiler warnings when doing truncating assignments. I'd be ok with using uint64_t to signal address or a size of a memory block, but having a specific typedefs like wasmptr_t and wasmsize_t (which equal to uint64_t) or something like that would be good as well.