Skip to content

Commit e9f917a

Browse files
committed
Make makeSetValue rounding consistent with HEAPI64.set()
This change does two things: 1. Require 8 byte alignment for makeSetValue of i64. Natural alignment is already required for all other types in `makeSetValue` and I don't know of good reason not to require this also for i64. The comment here implied that perhaps i64 was not always 8 bytes aligned, perhaps in the fastcomp days. But with wasm32 and wasm64 i64 should always be 8 bytes aligned. Folks with unaligned data already cannot use makeGetValue/makeSetValue. 2. Make splitI64 consistent with assignment to HEAPI64 in the way it handles numbers that are larger than the maximum i64 value.
1 parent 4f4b0e6 commit e9f917a

15 files changed

+28
-44
lines changed

src/parseTools.js

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ function splitI64(value) {
211211
//
212212
// $1$0 = ~~$d >>> 0;
213213
// $1$1 = Math.abs($d) >= 1 ? (
214-
// $d > 0 ? Math.min(Math.floor(($d)/ 4294967296.0), 4294967295.0)
214+
// $d > 0 ? Math.floor(($d)/ 4294967296.0) >>> 0,
215215
// : Math.ceil(Math.min(-4294967296.0, $d - $1$0)/ 4294967296.0)
216216
// ) : 0;
217217
//
@@ -227,9 +227,8 @@ function splitI64(value) {
227227
const high = makeInlineCalculation(
228228
asmCoercion('Math.abs(VALUE)', 'double') + ' >= ' + asmEnsureFloat('1', 'double') + ' ? ' +
229229
'(VALUE > ' + asmEnsureFloat('0', 'double') + ' ? ' +
230-
asmCoercion('Math.min(' + asmCoercion('Math.floor((VALUE)/' +
231-
asmEnsureFloat(4294967296, 'double') + ')', 'double') + ', ' +
232-
asmEnsureFloat(4294967295, 'double') + ')', 'i32') + '>>>0' +
230+
asmCoercion('Math.floor((VALUE)/' +
231+
asmEnsureFloat(4294967296, 'double') + ')', 'double') + '>>>0' +
233232
' : ' +
234233
asmFloatToInt(asmCoercion('Math.ceil((VALUE - +((' + asmFloatToInt('VALUE') + ')>>>0))/' +
235234
asmEnsureFloat(4294967296, 'double') + ')', 'double')) + '>>>0' +
@@ -363,12 +362,10 @@ function makeGetValue(ptr, pos, type, noNeedFirst, unsigned, ignore, align) {
363362
function makeSetValue(ptr, pos, value, type, noNeedFirst, ignore, align, sep = ';') {
364363
assert(typeof align === 'undefined', 'makeSetValue no longer supports align parameter');
365364
assert(typeof noNeedFirst === 'undefined', 'makeSetValue no longer supports noNeedFirst parameter');
366-
if (type == 'i64' && (!WASM_BIGINT || !MEMORY64)) {
367-
// If we lack either BigInt support or Memory64 then we must fall back to an
368-
// unaligned read of a 64-bit value: without BigInt we do not have HEAP64,
369-
// and without Memory64 i64 fields are not guaranteed to be aligned to 64
370-
// bits, so HEAP64[ptr>>3] might be broken.
371-
return '(tempI64 = [' + splitI64(value) + '],' +
365+
if (type == 'i64' && !WASM_BIGINT) {
366+
// If we lack either BigInt we must fall back to an reading a pair of I32
367+
// values.
368+
return '(tempI64 = [' + splitI64(value) + '], ' +
372369
makeSetValue(ptr, pos, 'tempI64[0]', 'i32', noNeedFirst, ignore, align, ',') + ',' +
373370
makeSetValue(ptr, getFastValue(pos, '+', getNativeTypeSize('i32')), 'tempI64[1]', 'i32', noNeedFirst, ignore, align, ',') + ')';
374371
}

src/preamble.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,9 +1156,11 @@ function createWasm() {
11561156
#endif
11571157
}
11581158

1159+
#if !WASM_BIGINT
11591160
// Globals used by JS i64 conversions (see makeSetValue)
11601161
var tempDouble;
11611162
var tempI64;
1163+
#endif
11621164

11631165
#include "runtime_debug.js"
11641166

src/preamble_minimal.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ function abort(what) {
3939
throw {{{ ASSERTIONS ? 'new Error(what)' : 'what' }}};
4040
}
4141

42-
#if SAFE_HEAP
42+
#if SAFE_HEAP && !WASM_BIGINT
4343
// Globals used by JS i64 conversions (see makeSetValue)
4444
var tempDouble;
4545
var tempI64;
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
25939
1+
25914
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
25903
1+
25878
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
30453
1+
30428
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
25748
1+
25723
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
30457
1+
30432
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
25939
1+
25914

test/other/test_parseTools.c

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ void test_makeGetValue(int64_t* ptr);
77
void test_makeSetValue(int64_t* ptr);
88
int test_receiveI64ParamAsI53(int64_t arg1, int64_t arg2);
99
int test_receiveI64ParamAsDouble(int64_t arg1, int64_t arg2);
10-
void test_makeSetValue_unaligned(int64_t* ptr);
1110

1211
#define MAX_SAFE_INTEGER (1ll << 53)
1312
#define MIN_SAFE_INTEGER (-MAX_SAFE_INTEGER)
@@ -55,18 +54,6 @@ int main() {
5554
rtn = test_receiveI64ParamAsDouble(MIN_SAFE_INTEGER - 1, 0);
5655
printf("rtn = %d\n", rtn);
5756

58-
printf("\ntest_makeSetValue_unaligned\n");
59-
// Test an unaligned read of an i64 in JS. To do that, get an unaligned
60-
// pointer. i64s are only 32-bit aligned, but we can't rely on the address to
61-
// happen to be unaligned here, so actually force an unaligned address (one
62-
// of the iterations will be unaligned).
63-
char buffer[16];
64-
for (size_t i = 0; i < 8; i += 4) {
65-
int64_t* unaligned_i64 = (int64_t*)(buffer + i);
66-
test_makeSetValue_unaligned(unaligned_i64);
67-
printf("i64 = 0x%llx\n", *unaligned_i64);
68-
}
69-
7057
printf("\ndone\n");
7158
return 0;
7259
}

0 commit comments

Comments
 (0)