LongBits.fromBigInt drops the two's-complement carry: negative int64/sint64/sfixed64 values below −(2^53−1) with zero low 32 bits encode as value − 2^32 #214

Follow-up to the note in #212 ("INT64_MIN is mis-encoded on int64/sint64/sfixed64 … separate overflow in fromBigInt"). The same overflow is not limited to INT64_MIN. It affects every negative value below Number.MIN_SAFE_INTEGER whose low 32 bits are zero, for example -(2n ** 53n), -(2n ** 60n) and -(2n ** 63n - 2n ** 32n).

The output is valid protobuf, so nothing flags it. A protons round trip silently returns value - 2^32. For INT64_MIN the result wraps to a positive number.

• affected: int64, sint64, sfixed64 (encoding only)
• not affected: decoding (correct bytes decode to the correct value), and values inside the safe-integer range (they go through fromNumber)

Repro

node --input-type=module -e '
import { writer, reader } from "protons-runtime"
const v = -(2n ** 53n)
const w = writer(); w.int64(v); const bytes = w.finish()
console.log(Buffer.from(bytes).toString("hex"), reader(bytes).int64())'
# protons-runtime 8.0.1 and main (e6eb6bf):
# 80808080f0ffffefff01 -9007203549708288n
# expected:
# 80808080808080f0ff01 -9007199254740992n

Actual vs expected bytes (main @ e6eb6bf, and identical on 8.0.1):

What a protons round trip returns:

A self-contained node:test file (negative-carry.test.mjs, attached below) covers all three codecs, encode and decode, with 5 affected values and 4 controls, checked against a BigInt reference encoder written from the spec.

// protons-runtime: negative 64-bit values below -(2^53 - 1) whose low 32 bits
// are zero are mis-encoded by int64, sint64 and sfixed64.
// Run: node --test negative-carry.test.mjs
import { test } from 'node:test'
import assert from 'node:assert/strict'
import { writer, reader } from 'protons-runtime'

const TWO_64 = 1n << 64n

// reference encodings, straight from the protobuf spec
function varint (u) {
  const out = []
  do {
    let b = Number(u & 0x7fn)
    u >>= 7n
    if (u > 0n) b |= 0x80
    out.push(b)
  } while (u > 0n)
  return out
}
const expected = {
  int64: (v) => varint(v < 0n ? v + TWO_64 : v),
  sint64: (v) => varint(v >= 0n ? v << 1n : (-v << 1n) - 1n),
  sfixed64: (v) => {
    const b = Buffer.alloc(8)
    b.writeBigInt64LE(v)
    return [...b]
  }
}
const hex = (bytes) => Buffer.from(bytes).toString('hex')

const P = (n) => 1n << BigInt(n)
const cases = [
  // low 32 bits zero: affected
  [-P(53), true],
  [-(P(53) + P(32)), true],
  [-P(60), true],
  [-(P(63) - P(32)), true],
  [-P(63), true], // INT64_MIN
  // controls: low 32 bits non-zero, or within the safe-integer range
  [-(P(53) + 1n), false],
  [-(P(53) - 1n), false],
  [-P(32), false],
  [-(P(63) - 1n), false]
]

for (const method of ['int64', 'sint64', 'sfixed64']) {
  for (const [value, affected] of cases) {
    test(`${method} encode ${value}${affected ? '' : ' (control)'}`, () => {
      const w = writer()
      w[method](value)
      assert.equal(hex(w.finish()), hex(expected[method](value)))
    })

    test(`${method} decode ${value}${affected ? '' : ' (control)'}`, () => {
      assert.equal(reader(Uint8Array.from(expected[method](value)))[method](), value)
    })
  }
}

Cause

packages/protons-runtime/src/utils/longbits.ts, fromBigInt:

if (negative) {
  hi = ~hi | 0n
  lo = ~lo | 0n

  if (++lo > TWO_32) {      // L138
    lo = 0n
    if (++hi > TWO_32) { hi = 0n }
  }
}

These are BigInts, so nothing wraps at 32 bits. When the magnitude's low word is 0n, ~0n is -1n and ++lo is 0n. That is never > TWO_32, so the carry into hi is skipped and the high word comes out one too small. When lo != 0n, ~lo + 1n is simply -lo, which >>> 0 in the constructor maps to the right word, so those values are fine.

This came in with the TS port in d101804 (#60). protons-runtime 5.0.3 (protobufjs) passes all cases, and 5.0.4 is the first failing release. In 5.0.4, −2^32 also failed, presumably because it predates the fromNumber fast path for safe integers (ba35475).

Possible fix

Take the two's complement in one step:

const u = BigInt.asUintN(64, value)
return new LongBits(Number(u & 0xffffffffn), Number(u >> 32n))

This replaces everything after the safe-integer fast path. Checked locally against main @ e6eb6bf (patched build output only): the repro goes from 15 failures to 54/54, and packages/protons aegir test -t node passes (73 passing, including the #212 64-bit varints tests). I haven't run the browser targets. Happy to open a PR with a regression test if that's welcome.

Versions

Node v22.23.1, Linux x64.