Skip to content

Commit 654f45f

Browse files
authored
[Wasm64] Fix makeSetValue of i64s with WASM_BIGINT (#17619)
The code here assumed that we can write HEAP64[ptr>>3] to read an i64 value. That assumes the pointer has 64-bit alignment, which is not the case with wasm32+WASM_BIGINT. This fixes a WasmFS issue where we'd pass a pointer to a file size, a 64-bit number, and write it from JS in the fetch backend (specifically the async jsimpl code that it uses), but this is a much more general issue. Fixes #17614
1 parent 2b2e142 commit 654f45f

File tree

5 files changed

+32
-3
lines changed

5 files changed

+32
-3
lines changed

src/parseTools.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,11 @@ function makeSetValue(ptr, pos, value, type, noNeedFirst, ignore, align, sep = '
404404
return '(' + makeSetTempDouble(0, 'double', value) + ',' +
405405
makeSetValue(ptr, pos, makeGetTempDouble(0, 'i32'), 'i32', noNeedFirst, ignore, align, ',') + ',' +
406406
makeSetValue(ptr, getFastValue(pos, '+', Runtime.getNativeTypeSize('i32')), makeGetTempDouble(1, 'i32'), 'i32', noNeedFirst, ignore, align, ',') + ')';
407-
} else if (!WASM_BIGINT && type == 'i64') {
407+
} else if (type == 'i64' && (!WASM_BIGINT || !MEMORY64)) {
408+
// If we lack either BigInt support or Memory64 then we must fall back to an
409+
// unaligned read of a 64-bit value: without BigInt we do not have HEAP64,
410+
// and without Memory64 i64 fields are not guaranteed to be aligned to 64
411+
// bits, so HEAP64[ptr>>3] might be broken.
408412
return '(tempI64 = [' + splitI64(value) + '],' +
409413
makeSetValue(ptr, pos, 'tempI64[0]', 'i32', noNeedFirst, ignore, align, ',') + ',' +
410414
makeSetValue(ptr, getFastValue(pos, '+', Runtime.getNativeTypeSize('i32')), 'tempI64[1]', 'i32', noNeedFirst, ignore, align, ',') + ')';

test/other/test_parseTools.c

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
void test_makeGetValue(int32_t* ptr);
66
int test_receiveI64ParamAsI53(int64_t arg1, int64_t arg2);
77
int test_receiveI64ParamAsDouble(int64_t arg1, int64_t arg2);
8+
void test_makeSetValue_i64(int64_t* ptr);
89

910
#define MAX_SAFE_INTEGER (1ll << 53)
1011
#define MIN_SAFE_INTEGER (-MAX_SAFE_INTEGER)
@@ -41,6 +42,18 @@ int main() {
4142
rtn = test_receiveI64ParamAsDouble(MIN_SAFE_INTEGER - 1, 0);
4243
printf("rtn = %d\n", rtn);
4344

44-
printf("done\n");
45+
printf("\ntest_makeSetValue_i64\n");
46+
// Test an unaligned read of an i64 in JS. To do that, get an unaligned
47+
// pointer. i64s are only 32-bit aligned, but we can't rely on the address to
48+
// happen to be unaligned here, so actually force an unaligned address (one
49+
// of the iterations will be unaligned).
50+
char buffer[16];
51+
for (size_t i = 0; i < 8; i += 4) {
52+
int64_t* unaligned_i64 = (int64_t*)(buffer + i);
53+
test_makeSetValue_i64(unaligned_i64);
54+
printf("i64 = 0x%llx\n", *unaligned_i64);
55+
}
56+
57+
printf("\ndone\n");
4558
return 0;
4659
}

test/other/test_parseTools.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,5 +81,9 @@ mergeInto(LibraryManager.library, {
8181
val = {{{ makeGetValue('ptr', '0', 'i32*') }}};
8282
out('ptr: ' + val.toString(16))
8383
assert(val == 0xedcba988);
84-
}
84+
},
85+
86+
test_makeSetValue_i64: function(ptr) {
87+
{{{ makeSetValue('ptr', '0', 0x12345678AB, 'i64') }}};
88+
},
8589
});

test/other/test_parseTools.out

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,9 @@ test_receiveI64ParamAsDouble:
4949
arg1: -9007199254740992
5050
arg2: 0
5151
rtn = 0
52+
53+
test_makeSetValue_i64
54+
i64 = 0x12345678ab
55+
i64 = 0x12345678ab
56+
5257
done

test/test_browser.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5290,6 +5290,9 @@ def test(args):
52905290
'main_thread': (['-sPTHREAD_POOL_SIZE=5'],),
52915291
# using proxy_to_pthread also works, of course
52925292
'proxy_to_pthread': (['-sPROXY_TO_PTHREAD', '-sINITIAL_MEMORY=32MB', '-DPROXYING'],),
5293+
# using BigInt support affects the ABI, and should not break things. (this
5294+
# could be tested on either thread; do the main thread for simplicity)
5295+
'bigint': (['-sPTHREAD_POOL_SIZE=5', '-sWASM_BIGINT'],),
52935296
})
52945297
@requires_threads
52955298
def test_wasmfs_fetch_backend(self, args):

0 commit comments

Comments
 (0)