builtins: fix BigInt prototype methods panicking on a primitive receiver - #45
Draft
mparrett wants to merge 2 commits into
Draft
builtins: fix BigInt prototype methods panicking on a primitive receiver#45mparrett wants to merge 2 commits into
mparrett wants to merge 2 commits into
Conversation
BigInt.prototype.toString/toLocaleString/valueOf and the BigInt()
constructor each inlined the same object-wrapper unwrap:
if v.Type() == vm.TypeBigInt {
if po := v.AsPlainObject(); po != nil { // panics
AsPlainObject panics unless the value is TypeObject, and a BigInt object
wrapper is TypeObject - never TypeBigInt. So the guard admitted exactly
the values the body could not handle: the wrapper branch was unreachable
and every primitive receiver panicked. `10n.toString()` printed nothing
and exited 0.
Replace all four copies with one thisBigIntValue helper implementing the
spec's ThisBigIntValue. The wrapper slot is spelled [[PrimitiveValue]],
which is what object_init.go's Object(bigint) case actually writes - the
spec calls it [[BigIntData]], and looking for that name is why the
wrapper arm never matched. Number/String/Boolean/Symbol wrappers share
the slot, so the stored value's type is checked before unwrapping.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The radix argument was coerced with a bare ToFloat and then clamped into 2..36, with a comment noting that real JS throws. Spec runs ToIntegerOrInfinity - which rejects a Symbol, and a ToPrimitive that returns a BigInt - and then throws RangeError outside 2..36. Uses the package's existing toIntegerOrInfinityWithVM. Separable from the panic fix, but included here because that fix is what makes these paths reachable at all: three test262 radix tests previously "passed" only because the panic ended the script before their assertions ran. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mparrett
force-pushed
the
fix/bigint-prototype-this-unwrap
branch
from
July 25, 2026 22:41
ed0009a to
935d954
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
10n.toString()prints nothing and exits 0. So dovalueOf(),toLocaleString(), andBigInt(10n)— every BigInt prototype method is unusable on a primitive receiver.Two commits: the panic fix, and a separable radix fix that the first one makes reachable.
1. The panic
All four sites inlined the same object-wrapper unwrap:
AsPlainObjectpanics unless the value isTypeObject(value.go:968-971), and a BigInt object wrapper isTypeObject— neverTypeBigInt. The guard admits exactly the values the body cannot handle: the wrapper branch was unreachable, and every primitive receiver panicked.It reads as a silent stop rather than a crash because
VM.run's recover discards the panic and returnsInterpretOK— filed separately as #44.Replaced all four copies with one
thisBigIntValuehelper implementing ThisBigIntValue. Four hand-inlined copies of a subtle unwrap is what produced four identical bugs.The slot name matters here. The spec calls it
[[BigIntData]], and that is what the old code looked up — but nothing in this repo ever writes that name.Object(bigint)stores[[PrimitiveValue]](object_init.go:944), so searching for the spec name would have left the wrapper arm dead in a new place. The helper uses[[PrimitiveValue]]and checks the stored value's type, since Number/String/Boolean/Symbol wrappers share that slot.2. The radix
toString's radix was coerced with a bareToFloatand clamped into 2..36, with a comment noting real JS throws. Now runs the package's existingtoIntegerOrInfinityWithVMand throwsRangeErroroutside 2..36.Separable, but included because commit 1 is what makes these paths reachable — see the test262 accounting below. Happy to split it out if you'd rather.
Verification
Primitive and wrapper receivers, all matching Node:
Non-BigInt receiver fallbacks are unchanged:
toString/toLocaleStringstringify,valueOfthrowsTypeError.tests/scripts/bigint_prototype_methods.tscovers all of the above. With commit 1 reverted it fails with"undefined"— the script produces nothing at all.go build/vet/gofmtclean;TestScripts,pkg/vm,pkg/builtins,pkg/compilerpass.test262
built-ins/BigInt: 41 → 41, andlanguage: 23222/23648 with zero per-test diffs.The count is flat but the composition changed, so the per-test diff is worth stating plainly:
prototype/valueOf/cross-realm.js— genuinely fixed by the wrapper unwrap.prototype/toString/radix-tointegerorinfinity-throws-toprimitive-or-bigint.js— this was a false pass. The panic ended the script before its assertion ran, and the harness scored the silence as green. It now fails for a real, pre-existing reason:toIntegerOrInfinityWithVMdoesn't reject a BigInt returned fromToPrimitive. That helper is shared with Array, so fixing it belongs in its own change where the Array impact can be measured — deliberately not touched here.Two other radix tests (
radix-err.js,radix-tointegerorinfinity-throws-symbol.js) were false passes for the same reason; commit 2 turns those into genuine passes, which is why the total lands back at 41 rather than 38.Note on the test
It calls through
any. The checker rejects property access on a bigint receiver outright ("property access is not supported on type bigint"), rejects unary-on bigint, and types a bigint literal as a number literal (const b: bigint = 10nfails). All pre-existing and unrelated to this PR — happy to file separately if you want them tracked.🤖 Generated with Claude Code