fix(runtime): break Object.prototype.hasOwnProperty self-dispatch recursion (#4276)#4282
Closed
proggeramlug wants to merge 3 commits into
Closed
fix(runtime): break Object.prototype.hasOwnProperty self-dispatch recursion (#4276)#4282proggeramlug wants to merge 3 commits into
proggeramlug wants to merge 3 commits into
Conversation
Contributor
Author
|
Closing as redundant. #4276 was already fixed by #4279 (merged), which makes the proto thunks call their underlying ops directly — no re-dispatch, so the recursion this PR guards against can't occur on main. This PR's approach (re-introduce the js_native_call_method re-dispatch + add a skip guard) would revert main's simpler fix for no behavioral gain. The superior test coverage from this branch is salvaged in #4293 (test-only, byte-identical to node on main). |
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.
Summary
Fixes #4276. The "uncurry-this" idiom
Function.prototype.call.bind(Object.prototype.hasOwnProperty)returned an empty[object Object](typeof"object") instead of invoking the method — but onlywhen the eventual receiver was a builtin prototype object such as
Object.prototypeorError.prototype/ aNativeError.prototype. Plainobjects,
Array.prototype, andMathalready worked.This idiom is the backbone of test262's
propertyHelper.js(
__hasOwnProperty = Function.prototype.call.bind(Object.prototype.hasOwnProperty)),so it gated
verifyPropertyfor every descriptor test subjectingObject.prototype/Error.prototype.Root cause
Object.prototype(and every Error/NativeError prototype) carries its ownhasOwnPropertyas a thunk (object_prototype_has_own_property_thunk) thatreads
IMPLICIT_THISand re-dispatchesjs_native_call_method(this, "hasOwnProperty"). When the receiver is one of those prototypes, thedispatcher's own-field scan re-found the
hasOwnPropertyfield, called the thunkagain, and recursed until the call-depth guard (
MAX_CALL_METHOD_DEPTH = 512)bailed and returned the empty
NULL_OBJECT_BYTESsentinel — which stringifies as[object Object].Plain objects don't carry an own
hasOwnProperty, so the field scan missed andthe real
"hasOwnProperty"arm computed the answer. That's why onlybuiltin-prototype receivers regressed, and why the direct
.callform (folded bycodegen) was unaffected.
Fix
In the own-field scan of
js_native_call_method, skip a field whose storedclosure is the self-redispatching proto thunk and fall through to the real
"hasOwnProperty"arm. A small predicate(
is_self_redispatching_proto_thunk) inglobal_this.rsidentifies the thunk byfunc-ptr. Only
hasOwnProperty's thunk re-dispatches by name; the siblingObject.prototype thunks (
toString,valueOf,propertyIsEnumerable,isPrototypeOf,toLocaleString) delegate to dedicatedjs_object_*helpersand never recurse, so they're deliberately left untouched.
Verification
true/boolean, no[object Object]).test-parity/node-suite/globals/uncurry-this-builtin-proto.ts— byte-identical with
node --experimental-strip-types. CoversObject.prototype,Error.prototype,TypeError.prototype,RangeError.prototype, the anchor receivers (plain/Array.prototype/Math),and the other uncurried Object.prototype methods (guarding against
over-reach).
cargo test --release -p perry-runtime— 977 passed, 0 failed.test_gap_3716_uncurry_this's pre-existing line-8 gap(
typeof call.bindoff a reifiedFunction.prototype.callvalue — a separatereification gap) is byte-unchanged by this fix.
Note
The companion
fix-error-descriptorsbranch referenced in the issue (Errorinstance + prototype descriptor parity) becomes observable through test262 once
this dispatch fix lands.