Read at the exact bigint position in fs.readSync (#2535)

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/fs-read-bigint-position.cjs.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/fs-read-bigint-position.cjs"
+ ],
+ "diags": []
+}

packages/compiler/test/ts7/baselines/order-parity/tests/corpus/fs-read-sync-bigint-position.cjs.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/corpus/fs-read-sync-bigint-position.cjs"
+ ],
+ "diags": []
+}

packages/runtime/src/scr_bytes_io.c

@@ -166,6 +166,9 @@ static ScrDyn *scr_fs_cb_arg(ScrDyn *const *args, size_t argc, size_t index) {
   return index < argc ? args[index] : scr_dyn_undefined();
 }
 
+static bool scr_fs_bigint_position_chk(ScrBigInt *position, double length);
+static double scr_fs_int32(double n);
+
 static ScrDyn *scr_fs_cb_option(const ScrDyn *options, const char *name) {
   if (scr_fs_dyn_absent(options)) return scr_dyn_retain(scr_dyn_undefined());
   ScrDyn *object = options->kind == SCR_DYN_TYPED_REF ? scr_dyn_typed_ref_materialize(options) : scr_dyn_retain((ScrDyn *)options);
@@ -1129,8 +1132,28 @@ static ScrDyn *scr_fs_cb_invoke(ScrStr *member, ScrDyn *const *args, size_t argc
       if (ownedBuffer) length = scr_fs_cb_option_number(options, "length", length);
       else if (readOp && count >= 4) length = readLength;
       else length = scr_fs_cb_number(ARG(3), "length", length);
-      double position = ownedBuffer ? scr_fs_cb_option_number(options, "position", -1) : scr_fs_cb_number(ARG(4), "position", -1);
-      if (!scr_exc_pending()) result = scr_dyn_new_num(readOp ? scr_fs_read_sync(fd, bytes, offset, length, position) : scr_fs_write_sync(fd, bytes, offset, length, position));
+      /* The position option is read once, so a getter runs once. A bigint
+       * read position takes readSync's range check against `length |= 0`
+       * and reads at its exact value: a double would round it past 2^53. */
+      ScrDyn *positionValue = ownedBuffer ? scr_fs_cb_option(options, "position")
+                                          : scr_dyn_retain(ARG(4));
+      bool bigPosition = readOp && positionValue && positionValue->kind == SCR_DYN_BIGINT;
+      double position = bigPosition || !positionValue
+                            ? -1
+                            : scr_fs_cb_number(positionValue, "position", -1);
+      if (!scr_exc_pending()) {
+        if (bigPosition) {
+          double count32 = scr_fs_int32(length);
+          if (scr_fs_read_offset_chk(offset) &&
+              scr_fs_bigint_position_chk(positionValue->v.bigint, count32))
+            result = scr_dyn_new_num(scr_fs_read_sync_at(fd, bytes, offset, count32,
+                                                         scr_bigint_to_i64(positionValue->v.bigint)));
+        } else {
+          result = scr_dyn_new_num(readOp ? scr_fs_read_sync(fd, bytes, offset, length, position)
+                                          : scr_fs_write_sync(fd, bytes, offset, length, position));
+        }
+      }
+      scr_dyn_release(positionValue);
     }
     scr_bytes_release(bytes);
     /* The completion retains the original buffer, including options.buffer. */
@@ -1914,14 +1937,16 @@ static bool scr_fs_length_int32(const ScrDyn *length, double *out) {
   } else if (!scr_dyn_number_coerce_js(length, &n)) {
     return false;
   }
-  if (!isfinite(n)) {
-    *out = 0;
-    return true;
-  }
+  *out = scr_fs_int32(n);
+  return true;
+}
+
+/* ToInt32 of a number: 0 for NaN and the infinities. */
+static double scr_fs_int32(double n) {
+  if (!isfinite(n)) return 0;
   double m = fmod(trunc(n), 4294967296.0);
   if (m < 0) m += 4294967296.0;
-  *out = m >= 2147483648.0 ? m - 4294967296.0 : m;
-  return true;
+  return m >= 2147483648.0 ? m - 4294967296.0 : m;
 }
 
 /* A bigint read position in [-1n, 2n ** 63n - 1n - BigInt(length)]
@@ -2101,12 +2126,15 @@ double scr_fs_read_sync_dyn(const ScrDyn *fd, const ScrDyn *buffer, const ScrDyn
   double count = 0;
   if (!scr_fs_read_validate(fd, buffer, offset, length, position, &count)) return 0;
   if (count == 0) return 0;
-  double at = scr_fs_dyn_absent(position)                ? -1
-              : position->kind == SCR_DYN_BIGINT ? scr_bigint_to_f64(position->v.bigint)
-                                                   : position->v.num;
+  double off = scr_fs_dyn_absent(offset) ? 0 : offset->v.num;
   ScrBytes *bytes = scr_dyn_bytes_unbox(buffer);
-  double read = scr_fs_read_sync(fd->v.num, bytes, scr_fs_dyn_absent(offset) ? 0 : offset->v.num,
-                                 count, at);
+  /* A bigint position passed its range check above and reads at its exact
+   * value: a double would round it past 2^53. */
+  double read =
+      !scr_fs_dyn_absent(position) && position->kind == SCR_DYN_BIGINT
+          ? scr_fs_read_sync_at(fd->v.num, bytes, off, count, scr_bigint_to_i64(position->v.bigint))
+          : scr_fs_read_sync(fd->v.num, bytes, off, count,
+                             scr_fs_dyn_absent(position) ? -1 : position->v.num);
   scr_bytes_release(bytes);
   return read;
 }

packages/runtime/src/scr_file_handle.h

@@ -60,6 +60,12 @@ ScrStats *scr_file_handle_stat(ScrFileHandle *h);
  * nonnegative positions leave that offset unchanged (pread/ReadFile seam). */
 double scr_fs_read_sync(double fd, ScrBytes *buf, double offset, double length,
                         double position);
+/* fs.readSync's offset check; false with ERR_OUT_OF_RANGE pending. */
+bool scr_fs_read_offset_chk(double offset);
+/* scr_fs_read_sync after its offset and position checks, with an exact
+ * position (-1 for the current offset). */
+double scr_fs_read_sync_at(double fd, ScrBytes *buf, double offset, double length,
+                           int64_t position);
 /* writeSync's classic Buffer window and utf8 string forms. position -1 (or
  * any other non-safe/nonnegative-integer value) writes at and advances the
  * current descriptor offset; safe nonnegative positions leave it unchanged. */

packages/runtime/src/scr_lib.c

@@ -3989,7 +3989,7 @@ void scr_fs_futimes(double fd, double atime, double mtime) {
  * supplies pread(2). Windows follows libuv's synchronous-handle recipe:
  * ReadFile with an OVERLAPPED byte offset, then restore the handle position
  * because synchronous ReadFile updates it even when OVERLAPPED is present. */
-static ssize_t scr_fs_pread(int fd, void *data, size_t length, double position) {
+static ssize_t scr_fs_pread(int fd, void *data, size_t length, int64_t position) {
 #ifdef _WIN32
   HANDLE handle = (HANDLE)_get_osfhandle(fd);
   if (handle == INVALID_HANDLE_VALUE) {
@@ -4365,14 +4365,11 @@ void scr_fs_write_file_fd(double fd, ScrStr *data) {
  * throw the RangeError shape. Position -1 means the fd's current offset;
  * nonnegative positions do not advance it. Returns the byte count the OS
  * reports; errors carry the errno name like the other fd operations. */
-double scr_fs_read_sync(double fd, ScrBytes *buf, double offset, double length,
-                        double position) {
-  size_t bytelen = buf->len; /* u8 buffers: elem count == byte count */
+/* fs.readSync's offset check: an integer in [0, 2^53 - 1], Node's exact
+ * texts (validateOffset); false with ERR_OUT_OF_RANGE pending otherwise. */
+bool scr_fs_read_offset_chk(double offset) {
   char msg[160];
   int mlen;
-  /* Node's exact texts: offset validates against MAX_SAFE_INTEGER's range
-   * form, length against the remaining window (validateOffset vs the
-   * buffer-bounds check in fs.readSync). */
   char numbuf[40];
   if (!(isfinite(offset) && trunc(offset) == offset)) {
     char recv[48];
@@ -4381,7 +4378,7 @@ double scr_fs_read_sync(double fd, ScrBytes *buf, double offset, double length,
                     "The value of \"offset\" is out of range. It must be an integer. Received %s",
                     recv);
     scr_throw_error_msg_code(SCR_ERR_RANGE, msg, (size_t)mlen, "ERR_OUT_OF_RANGE");
-    return 0;
+    return false;
   }
   if (offset < 0 || offset > 9007199254740991.0) {
     numbuf[scr_f64_to_str(offset, numbuf)] = 0;
@@ -4389,8 +4386,19 @@ double scr_fs_read_sync(double fd, ScrBytes *buf, double offset, double length,
                     "The value of \"offset\" is out of range. It must be >= 0 && <= 9007199254740991. Received %s",
                     numbuf);
     scr_throw_error_msg_code(SCR_ERR_RANGE, msg, (size_t)mlen, "ERR_OUT_OF_RANGE");
-    return 0;
+    return false;
   }
+  return true;
+}
+
+double scr_fs_read_sync(double fd, ScrBytes *buf, double offset, double length,
+                        double position) {
+  char msg[160];
+  int mlen;
+  /* Node's exact texts: offset validates against MAX_SAFE_INTEGER's range
+   * form, length against the remaining window (validateOffset vs the
+   * buffer-bounds check in fs.readSync). */
+  if (!scr_fs_read_offset_chk(offset)) return 0;
   if (!(isfinite(position) && trunc(position) == position)) {
     char recv[48];
     scr_num_received(position, recv);
@@ -4409,6 +4417,16 @@ double scr_fs_read_sync(double fd, ScrBytes *buf, double offset, double length,
     scr_throw_error_msg_code(SCR_ERR_RANGE, msg, (size_t)mlen, "ERR_OUT_OF_RANGE");
     return 0;
   }
+  return scr_fs_read_sync_at(fd, buf, offset, length, (int64_t)position);
+}
+
+/* fs.readSync after its offset and position checks: `position` is exact,
+ * so a bigint position past 2^53 reads where it says. */
+double scr_fs_read_sync_at(double fd, ScrBytes *buf, double offset, double length,
+                           int64_t position) {
+  size_t bytelen = buf->len; /* u8 buffers: elem count == byte count */
+  char msg[160];
+  int mlen;
   /* `length |= 0`, then Node's order: a zero length reads nothing after
    * the offset and position checks; otherwise the buffer must not be
    * empty and the window must fit it (validateOffsetLengthRead). */
@@ -4471,7 +4489,7 @@ double scr_fs_read_zero(double fd, double position) {
   if (!scr_fs_write_fd_valid(fd)) return 0;
   uint8_t dummy;
   ssize_t n = position < 0 ? read((int)fd, &dummy, 0)
-    : scr_fs_pread((int)fd, &dummy, 0, position);
+    : scr_fs_pread((int)fd, &dummy, 0, (int64_t)position);
   if (n < 0) scr_fs_throw_nopath(errno, "read");
   return 0;
 }
@@ -4552,7 +4570,8 @@ int scr_fs_raw_open(const char *path, int flags, int mode) {
 }
 
 int64_t scr_fs_raw_read(int fd, void *data, size_t length, double position) {
-  ssize_t n = position >= 0 ? scr_fs_pread(fd, data, length, position) : read(fd, data, length);
+  ssize_t n = position >= 0 ? scr_fs_pread(fd, data, length, (int64_t)position)
+                            : read(fd, data, length);
   return n < 0 ? -errno : n;
 }
 

tests/corpus/fs-read-bigint-position.cjs

@@ -0,0 +1,59 @@
+// fs.read's callback forms with bigint positions read at the exact
+// position, including past 2^53 (beyond the file's end, so nothing is
+// read), and -1n reads at the current offset. A fractional or infinite
+// length takes `length |= 0` before the position check, and a position
+// getter in either options form runs once.
+const fs = require("node:fs");
+const os = require("node:os");
+const path = require("node:path");
+
+const dir = fs.mkdtempSync(path.join(os.tmpdir(), "scriptc-read-cb-position-"));
+const file = path.join(dir, "data");
+fs.writeFileSync(file, "abc");
+const fd = fs.openSync(file, "r");
+const big = 9007199254740993n;
+
+function step(label, run, next) {
+  run((error, bytesRead, buffer) => {
+    console.log(label, error ? error.code : null, bytesRead, buffer ? buffer.toString("latin1", 0, bytesRead) : "");
+    next();
+  });
+}
+
+step("positional", (cb) => fs.read(fd, Buffer.alloc(1), 0, 1, big, cb), () =>
+  step("options", (cb) => fs.read(fd, { buffer: Buffer.alloc(1), position: big }, cb), () =>
+    step("buffer and options", (cb) => fs.read(fd, Buffer.alloc(2), { position: big }, cb), () =>
+      step("small", (cb) => fs.read(fd, Buffer.alloc(2), 0, 2, 1n, cb), () =>
+        step("current", (cb) => fs.read(fd, Buffer.alloc(1), 0, 1, -1n, cb), () => {
+          fractional();
+        })))));
+
+function counted(values) {
+  const options = { calls: 0 };
+  Object.defineProperty(options, "position", {
+    get() {
+      return values[options.calls++];
+    },
+  });
+  return options;
+}
+
+function fractional() {
+  step("fractional", (cb) => fs.read(fd, { buffer: Buffer.alloc(2), length: 0.5, position: 1n }, cb), () =>
+    step("infinite", (cb) => fs.read(fd, Buffer.alloc(2), { length: Infinity, position: 1n }, cb), () =>
+      step("truncated", (cb) => fs.read(fd, { buffer: Buffer.alloc(3), length: 2.7, position: 1n }, cb), () => {
+        const first = counted([0, 2]);
+        first.buffer = Buffer.alloc(1);
+        first.length = 1;
+        step("getter options", (cb) => fs.read(fd, first, cb), () => {
+          console.log("getter options calls", first.calls);
+          const second = counted([2, 0]);
+          second.length = 1;
+          step("getter buffer and options", (cb) => fs.read(fd, Buffer.alloc(1), second, cb), () => {
+            console.log("getter buffer and options calls", second.calls);
+            fs.closeSync(fd);
+            fs.rmSync(dir, { recursive: true });
+          });
+        });
+      })));
+}

tests/corpus/fs-read-sync-bigint-position.cjs

@@ -0,0 +1,32 @@
+// fs.readSync with bigint positions reads at the exact position, including
+// past 2^53 (beyond a one-byte file's end, so it reads nothing), leaves the
+// descriptor's offset unchanged, and -1n reads at the current offset.
+// Positions outside [-1n, 2n ** 63n - 1n - length] throw ERR_OUT_OF_RANGE.
+const fs = require("node:fs");
+const os = require("node:os");
+const path = require("node:path");
+
+const dir = fs.mkdtempSync(path.join(os.tmpdir(), "scriptc-read-position-"));
+const file = path.join(dir, "data");
+fs.writeFileSync(file, "xyz");
+const fd = fs.openSync(file, "r");
+const buffer = Buffer.alloc(2);
+try {
+  console.log(fs.readSync(fd, buffer, 0, 1, 0n), buffer.toString("latin1", 0, 1));
+  console.log(fs.readSync(fd, buffer, 0, 2, 1n), buffer.toString("latin1"));
+  console.log(fs.readSync(fd, buffer, 0, 1, 9007199254740993n));
+  console.log(fs.readSync(fd, buffer, 0, 1, 9223372036854775806n));
+  // The positioned reads above left the current offset at 0.
+  console.log(fs.readSync(fd, buffer, 0, 1, -1n), buffer.toString("latin1", 0, 1));
+  console.log(fs.readSync(fd, buffer, 0, 1, -1n), buffer.toString("latin1", 0, 1));
+  for (const position of [-2n, 9223372036854775807n]) {
+    try {
+      fs.readSync(fd, buffer, 0, 1, position);
+    } catch (error) {
+      console.log(error.code, error.message);
+    }
+  }
+} finally {
+  fs.closeSync(fd);
+  fs.rmSync(dir, { recursive: true });
+}

tests/harness/corpus-costs/fs-read-bigint-position.cjs.json

@@ -0,0 +1 @@
+{ "plain": 2700, "san": 4500 }

tests/harness/corpus-costs/fs-read-sync-bigint-position.cjs.json

@@ -0,0 +1 @@
+{ "plain": 2700, "san": 4500 }