Improve handling of incoming i64 syscall arguments. (#16787)

Add new `receiveI64ParamAsI53` to replace the old
`receiveI64ParamAsDouble`.  The new function checks the value fits
within I53 and requires an `onError` value to be returned in the case of
out-of-bounds.

`receiveI64ParamAsI53` makes use of the existing `convertI32PairToI53`
utility.

There is now some amount of duplication between `test_parseTools`
and `js_library_i64_params` which I would prefer to address as
a followup.

Author: sbc100

src/library_int53.js

@@ -93,9 +93,8 @@ mergeInto(LibraryManager.library, {
     return HEAPU32[ptr>>2] + HEAPU32[ptr+4>>2] * 4294967296;
   },
 
-  // Converts the given signed 32-bit low-high pair to a JavaScript Number that can
-  // represent 53 bits of precision.
-  // TODO: Add $convertI32PairToI53Signaling() variant.
+  // Converts the given signed 32-bit low-high pair to a JavaScript Number that
+  // can represent 53 bits of precision.
   $convertI32PairToI53: function(lo, hi) {
 #if ASSERTIONS
     // This function should not be getting called with too large unsigned numbers
@@ -119,8 +118,27 @@ mergeInto(LibraryManager.library, {
 
   // Converts the given unsigned 32-bit low-high pair to a JavaScript Number that can
   // represent 53 bits of precision.
-  // TODO: Add $convertU32PairToI53Signaling() variant.
+  // TODO: Add $convertU32PairToI53Checked() variant.
   $convertU32PairToI53: function(lo, hi) {
     return (lo >>> 0) + (hi >>> 0) * 4294967296;
-  }
+  },
+
+#if WASM_BIGINT
+  $MAX_INT53: '{{{ Math.pow(2, 53) }}}',
+  $MIN_INT53: '-{{{ Math.pow(2, 53) }}}',
+  // Counvert a bigint value (usually coming from Wasm->JS call) into an int53
+  // JS Number.  This is used when we have an incoming i64 that we know is a
+  // pointer or size_t and is expected to be withing the int53 range.
+  // Returns NaN if the incoming bigint is outside the range.
+  $bigintToI53Checked__deps: ['$MAX_INT53', '$MIN_INT53'],
+  $bigintToI53Checked: function(num) {
+    return (num < MIN_INT53 || num > MAX_INT53) ? NaN : Number(num);
+  },
+#endif
 });
+
+#if WASM_BIGINT
+global.i53ConversionDeps = ['$bigintToI53Checked'];
+#else
+global.i53ConversionDeps = ['$convertI32PairToI53Checked'];
+#endif

src/library_syscall.js

@@ -625,15 +625,17 @@ var SyscallsLibrary = {
     return cwdLengthInBytes;
   },
   __syscall_truncate64__sig: 'ipj',
+  __syscall_truncate64__deps: i53ConversionDeps,
   __syscall_truncate64: function(path, {{{ defineI64Param('length') }}}) {
-    {{{ receiveI64ParamAsDouble('length') }}}
+    {{{ receiveI64ParamAsI53('length', -cDefine('EOVERFLOW')) }}}
     path = SYSCALLS.getStr(path);
     FS.truncate(path, length);
     return 0;
   },
   __syscall_ftruncate64__sig: 'iij',
+  __syscall_ftruncate64__deps: i53ConversionDeps,
   __syscall_ftruncate64: function(fd, {{{ defineI64Param('length') }}}) {
-    {{{ receiveI64ParamAsDouble('length') }}}
+    {{{ receiveI64ParamAsI53('length', -cDefine('EOVERFLOW')) }}}
     FS.ftruncate(fd, length);
     return 0;
   },
@@ -967,9 +969,10 @@ var SyscallsLibrary = {
     FS.utime(path, atime, mtime);
     return 0;
   },
+  __syscall_fallocate__deps: i53ConversionDeps,
   __syscall_fallocate: function(fd, mode, {{{ defineI64Param('offset') }}}, {{{ defineI64Param('len') }}}) {
-    {{{ receiveI64ParamAsDouble('offset') }}}
-    {{{ receiveI64ParamAsDouble('len') }}}
+    {{{ receiveI64ParamAsI53('offset', -cDefine('EOVERFLOW')) }}}
+    {{{ receiveI64ParamAsI53('len', -cDefine('EOVERFLOW')) }}}
     var stream = SYSCALLS.getStreamFromFD(fd)
 #if ASSERTIONS
     assert(mode === 0);

src/library_wasi.js

@@ -138,8 +138,7 @@ var WasiLibrary = {
   clock_time_get__nothrow: true,
   clock_time_get__sig: 'iijp',
   clock_time_get__deps: ['emscripten_get_now', '$nowIsMonotonic', '$checkWasiClock'],
-  clock_time_get: function(clk_id, {{{ defineI64Param('precision') }}}, ptime) {
-    {{{ receiveI64ParamAsI32s('precision') }}}
+  clock_time_get: function(clk_id, {{{ defineI64Param('ignored_precision') }}}, ptime) {
     if (!checkWasiClock(clk_id)) {
       return {{{ cDefine('EINVAL') }}};
     }
@@ -270,17 +269,16 @@ var WasiLibrary = {
     return 0;
   },
 
+  fd_pwrite__deps: [
 #if SYSCALLS_REQUIRE_FILESYSTEM
-  fd_pwrite__deps: ['$doWritev'],
+    '$doWritev',
 #endif
+  ].concat(i53ConversionDeps),
   fd_pwrite: function(fd, iov, iovcnt, {{{ defineI64Param('offset') }}}, pnum) {
 #if SYSCALLS_REQUIRE_FILESYSTEM
-    {{{ receiveI64ParamAsI32s('offset') }}}
+    {{{ receiveI64ParamAsI53('offset', cDefine('EOVERFLOW')) }}}
     var stream = SYSCALLS.getStreamFromFD(fd)
-#if ASSERTIONS
-    assert(!offset_high, 'offsets over 2^32 not yet supported');
-#endif
-    var num = doWritev(stream, iov, iovcnt, offset_low);
+    var num = doWritev(stream, iov, iovcnt, offset);
     {{{ makeSetValue('pnum', 0, 'num', 'i32') }}};
     return 0;
 #elif ASSERTIONS
@@ -327,18 +325,17 @@ var WasiLibrary = {
 #endif // SYSCALLS_REQUIRE_FILESYSTEM
   },
 
+  fd_pread__deps: [
 #if SYSCALLS_REQUIRE_FILESYSTEM
-  fd_pread__deps: ['$doReadv'],
+    '$doReadv',
 #endif
+  ].concat(i53ConversionDeps),
   fd_pread__sig: 'iippjp',
   fd_pread: function(fd, iov, iovcnt, {{{ defineI64Param('offset') }}}, pnum) {
 #if SYSCALLS_REQUIRE_FILESYSTEM
-    {{{ receiveI64ParamAsI32s('offset') }}}
-#if ASSERTIONS
-    assert(!offset_high, 'offsets over 2^32 not yet supported');
-#endif
+    {{{ receiveI64ParamAsI53('offset', cDefine('EOVERFLOW')) }}}
     var stream = SYSCALLS.getStreamFromFD(fd)
-    var num = doReadv(stream, iov, iovcnt, offset_low);
+    var num = doReadv(stream, iov, iovcnt, offset);
     {{{ makeSetValue('pnum', 0, 'num', 'i32') }}};
     return 0;
 #elif ASSERTIONS
@@ -349,20 +346,11 @@ var WasiLibrary = {
   },
 
   fd_seek__sig: 'iijip',
+  fd_seek__deps: i53ConversionDeps,
   fd_seek: function(fd, {{{ defineI64Param('offset') }}}, whence, newOffset) {
 #if SYSCALLS_REQUIRE_FILESYSTEM
-    {{{ receiveI64ParamAsI32s('offset') }}}
+    {{{ receiveI64ParamAsI53('offset', cDefine('EOVERFLOW')) }}}
     var stream = SYSCALLS.getStreamFromFD(fd);
-    var HIGH_OFFSET = 0x100000000; // 2^32
-    // use an unsigned operator on low and shift high by 32-bits
-    var offset = offset_high * HIGH_OFFSET + (offset_low >>> 0);
-
-    var DOUBLE_LIMIT = 0x20000000000000; // 2^53
-    // we also check for equality since DOUBLE_LIMIT + 1 == DOUBLE_LIMIT
-    if (offset <= -DOUBLE_LIMIT || offset >= DOUBLE_LIMIT) {
-      return {{{ cDefine('EOVERFLOW') }}};
-    }
-
     FS.llseek(stream, offset, whence);
     {{{ makeSetValue('newOffset', '0', 'stream.position', 'i64') }}};
     if (stream.getdents && offset === 0 && whence === {{{ cDefine('SEEK_SET') }}}) stream.getdents = null; // reset readdir state

src/modules.js

@@ -36,6 +36,7 @@ global.LibraryManager = {
     // Core system libraries (always linked against)
     let libraries = [
       'library.js',
+      'library_int53.js',
       'library_formatString.js',
       'library_getvalue.js',
       'library_math.js',
@@ -44,7 +45,6 @@ global.LibraryManager = {
       'library_html5.js',
       'library_stack_trace.js',
       'library_wasi.js',
-      'library_int53.js',
       'library_dylink.js',
       'library_eventloop.js',
     ];

src/parseTools.js

@@ -689,6 +689,16 @@ function getHeapForType(type) {
   assert(false, 'bad heap type: ' + type);
 }
 
+function makeReturn64(value) {
+  if (WASM_BIGINT) {
+    return `BigInt(${value})`;
+  }
+  const pair = splitI64(value);
+  // `return (a, b, c)` in JavaScript will execute `a`, and `b` and return the final
+  // element `c`
+  return `(setTempRet0(${pair[1]}), ${pair[0]})`;
+}
+
 function makeThrow(what) {
   if (ASSERTIONS && DISABLE_EXCEPTION_CATCHING) {
     what += ' + " - Exception catching is disabled, this exception cannot be caught. Compile with -sNO_DISABLE_EXCEPTION_CATCHING or -sEXCEPTION_CATCHING_ALLOWED=[..] to catch."';
@@ -1100,17 +1110,14 @@ function receiveI64ParamAsI32s(name) {
   return '';
 }
 
-// TODO: use this in library_wasi.js and other places. but we need to add an
-//       error-handling hook here.
-function receiveI64ParamAsDouble(name) {
+function receiveI64ParamAsI53(name, onError) {
   if (WASM_BIGINT) {
     // Just convert the bigint into a double.
-    return `var ${name} = Number(${name}_bigint);`;
+    return `var ${name} = bigintToI53Checked(${name}_bigint); if (isNaN(${name})) return ${onError};`;
   }
-
-  // Combine the i32 params. Use an unsigned operator on low and shift high by
-  // 32 bits.
-  return `var ${name} = ${name}_high * 0x100000000 + (${name}_low >>> 0);`;
+  // Covert the high/low pair to a Number, checking for
+  // overflow of the I53 range and returning onError in that case.
+  return `var ${name} = convertI32PairToI53Checked(${name}_low, ${name}_high); if (isNaN(${name})) return ${onError};`;
 }
 
 function sendI64Argument(low, high) {

src/parseTools_legacy.js

@@ -11,3 +11,15 @@ function makeStructuralReturn(values) {
   assert(values.length == 2);
   return 'setTempRet0(' + values[1] + '); return ' + asmCoercion(values[0], 'i32');
 }
+
+// Replaced (at least internally) with receiveI64ParamAsI53 that does
+// bounds checking.
+function receiveI64ParamAsDouble(name) {
+  if (WASM_BIGINT) {
+    // Just convert the bigint into a double.
+    return `var ${name} = Number(${name}_bigint);`;
+  }
+  // Combine the i32 params. Use an unsigned operator on low and shift high by
+  // 32 bits.
+  return `var ${name} = ${name}_high * 0x100000000 + (${name}_low >>> 0);`;
+}

tests/core/js_library_i64_params.c

@@ -1,14 +1,37 @@
 #include <assert.h>
 #include <stdio.h>
 
-int jscall(uint64_t arg);
+#define MAX_SAFE_INTEGER (1ll<<53)
+#define MIN_SAFE_INTEGER (-MAX_SAFE_INTEGER)
 
-int main() {
-  int rtn = jscall(42);
-  printf("%d\n", rtn);
-  assert(rtn == 84);
+#define ERROR_VALUE 42
+
+int64_t jscall(int64_t arg);
+
+void check_ok(int64_t val) {
+  printf("checking: %lld\n", val);
+  int64_t rtn = jscall(val);
+  int64_t expected = val/2;
+  printf("got:      %lld\n", rtn);
+  printf("expected: %lld\n", expected);
+  assert(rtn == expected);
+}
 
-  // TODO(sbc): Test edge cases such as i64 values that overflow int32 and
-  // int53
+void check_invalid(int64_t val) {
+  printf("checking: %lld\n", val);
+  int64_t rtn = jscall(val);
+  printf("got:      %lld\n", rtn);
+  assert(rtn == ERROR_VALUE);
+}
+
+int main() {
+  check_ok(42);
+  check_ok(MAX_SAFE_INTEGER/2);
+  check_ok(MIN_SAFE_INTEGER/2);
+  check_ok(MAX_SAFE_INTEGER);
+  check_ok(MIN_SAFE_INTEGER);
+  check_invalid(MAX_SAFE_INTEGER + 1);
+  check_invalid(MIN_SAFE_INTEGER - 1);
+  printf("done\n");
   return 0;
 }

tests/core/js_library_i64_params.js

@@ -1,11 +1,15 @@
 TestLibrary = {
-#if !WASM_BIGINT
-  jscall__deps: ['$convertI32PairToI53' ],
-#endif
+  jscall__deps: i53ConversionDeps,
   jscall__sig: 'ij',
   jscall: function({{{ defineI64Param('foo') }}}) {
-    {{{ receiveI64ParamAsDouble('foo') }}}
-    return foo * 2;
+    {{{ receiveI64ParamAsI53('foo', `(err('overflow'), ${makeReturn64('42')})`) }}}
+    err('js:got:       ' + foo);
+    if (foo < 0)
+      var rtn = Math.ceil(foo / 2);
+    else
+      rtn = Math.floor(foo / 2);
+    err('js:returning: ' + rtn);
+    return {{{ makeReturn64('rtn') }}};
   },
 }
 

tests/core/js_library_i64_params.out

@@ -1 +1,32 @@
-84
+checking: 42
+js:got:       42
+js:returning: 21
+got:      21
+expected: 21
+checking: 4503599627370496
+js:got:       4503599627370496
+js:returning: 2251799813685248
+got:      2251799813685248
+expected: 2251799813685248
+checking: -4503599627370496
+js:got:       -4503599627370496
+js:returning: -2251799813685248
+got:      -2251799813685248
+expected: -2251799813685248
+checking: 9007199254740992
+js:got:       9007199254740992
+js:returning: 4503599627370496
+got:      4503599627370496
+expected: 4503599627370496
+checking: -9007199254740992
+js:got:       -9007199254740992
+js:returning: -4503599627370496
+got:      -4503599627370496
+expected: -4503599627370496
+checking: 9007199254740993
+overflow
+got:      42
+checking: -9007199254740993
+overflow
+got:      42
+done

tests/other/test_parseTools.c

@@ -3,11 +3,44 @@
 #include <inttypes.h>
 
 void test_makeGetValue(int32_t* ptr);
+int  test_receiveI64ParamAsI53(int64_t arg1, int64_t arg2);
+int  test_receiveI64ParamAsDouble(int64_t arg1, int64_t arg2);
+
+#define MAX_SAFE_INTEGER (1ll << 53)
+#define MIN_SAFE_INTEGER (-MAX_SAFE_INTEGER)
 
 int main() {
+  int rtn;
+  printf("MAX_SAFE_INTEGER: %lld\n", MAX_SAFE_INTEGER);
+  printf("MIN_SAFE_INTEGER: %lld\n", MIN_SAFE_INTEGER);
+
   int32_t num = -0x12345678;
   test_makeGetValue(&num);
 
+  rtn = test_receiveI64ParamAsI53(42, -42);
+  printf("rtn = %d\n", rtn);
+
+  rtn = test_receiveI64ParamAsI53(MAX_SAFE_INTEGER, MIN_SAFE_INTEGER);
+  printf("rtn = %d\n", rtn);
+
+  rtn = test_receiveI64ParamAsI53(MAX_SAFE_INTEGER + 1, 0);
+  printf("rtn = %d\n", rtn);
+
+  rtn = test_receiveI64ParamAsI53(MIN_SAFE_INTEGER - 1, 0);
+  printf("rtn = %d\n", rtn);
+
+  rtn = test_receiveI64ParamAsDouble(42, -42);
+  printf("rtn = %d\n", rtn);
+
+  rtn = test_receiveI64ParamAsDouble(MAX_SAFE_INTEGER, MIN_SAFE_INTEGER);
+  printf("rtn = %d\n", rtn);
+
+  rtn = test_receiveI64ParamAsDouble(MAX_SAFE_INTEGER + 1, 0);
+  printf("rtn = %d\n", rtn);
+
+  rtn = test_receiveI64ParamAsDouble(MIN_SAFE_INTEGER - 1, 0);
+  printf("rtn = %d\n", rtn);
+
   printf("done\n");
   return 0;
 }