Backport #4078 and #4125 to 4.x: walk the prototype chain, and probe a wrapper's forward - #4170
Merged
Merged
Conversation
… instead of one native frame per link Backport of PR sebastienros#4078 (head b506e62, not yet merged on main) from main. A prototype chain is built by script, so its depth is an input. ObjectInstance's [[Get]], [[Set]] and [[HasProperty]] each resolved it by calling the same method on Prototype, one native frame per link, so let x = {}; for (let i = 0; i < 20000; i++) x = { __proto__: x }; x.missing ended the process with a native stack overflow no catch could see (sebastienros#4076). The write side was reached through `x.missing = 1`, the existence side through `'missing' in x`, and both through every identifier resolved inside `with (x) { ... }`. A chain of trapless proxies had the same defect one level up: `return target.Get(property, receiver)` with nothing in between. All four walks are loops now (GetFromPrototypeChain, the private receiver-threading TryGetValue, SetOnPrototypeChain, HasProperty inline), so an ordinary chain of any depth resolves. A walk hands the rest of the algorithm to the first link it may not walk -- on the read side a link carrying InternalTypes.ExoticGet | OwnValueHook, on the write and existence sides any link without the positive InternalTypes.PlainObject claim -- and probes the native stack at that hand-over, off the ordinary path. A trapless JsProxy forwards through ForwardToTarget(target), which probes; a trapped proxy's trap already probes as a callee. SharedShapeObject (what every JsObjectShape.Instantiate returns) takes PlainObject, which main's paired gate required after shaped host prototypes declined the walk. PrototypeChainWalkTests pins the flag's claim over every reachable object, in both directions. Adapted for 4.x: - JsProxy.cs, three conflicts, all context: 4.x's [[IsArray]] hook is IsArray() where main's is IsSpecArray(), and its [[IsExtensible]] is the virtual Extensible getter where main's is IsExtensible(). ForwardToTarget goes on the same forwards. The probe set otherwise matches main's: the 24 trapless forwards (11 internal methods x trapless arm and CLR-declined arm, plus IsArray and ToObject) take ForwardToTarget, and [[Call]]/[[Construct]] keep the entry probes sebastienros#4007 gave them, byte-identical. Their forwards (callable.Call, constructor.Construct) do not go through ForwardToTarget, so no route probes twice and none lost a probe. - ObjectInstance.cs: 4.x's SetUnlikely still inlines OrdinarySetWithOwnDescriptor, which main extracted in sebastienros#3944 (not on 4.x). SetOnPrototypeChain resolves a found link with that algorithm, so the same extraction is made here, private and with its body unchanged; SetUnlikely delegates to it as on main. - StackOverflowGuard is opt-in on 4.x, so every engine in the new depth cases asks for it (Guarded()), as sebastienros#4007's did. A default 4.x engine gets the loops -- an ordinary chain resolves at any depth either way -- but not the hand-over probes, which are gated on the guard. - Tests transcribed from NUnit to xUnit v3. Main's b506e62 hunk on the existing forwarding-chain rows is not taken: sebastienros#4007's backport already settled those rows for 4.x (256 KiB stack, proxy and bound call accepting either answer), and this change does not touch those routes. - The census allowlist is main's, unchanged: emptied on 4.x, the converse names exactly the same 19 types. Both directions were broken on purpose on 4.x (flag dropped from SharedShapeObject; flag added to ArrayInstance) and each named its offender. - Jint.Benchmark/PrototypeChainReadBenchmark.cs did not exist on 4.x (main added it with sebastienros#4048, which 4.x does not carry); it is added whole, rows unchanged, with its prose saying that on this branch the member cache only serves a direct-prototype holder. Not run. - Co-located AGENTS.md edits dropped (the files do not exist on 4.x); the two doc comments citing Jint/Constraints/AGENTS.md say it is main's. Evidence (Windows x64, Release): - Unfixed (the engine files as on 4.x, the ported tests), each depth row run in its own process: on net10.0, 17 of 23 rows end the test host (exit 0xC00000FD, "Stack overflow.", ObjectInstance.Get x3058 for the plain chain, x3047 shaped, JsProxy.Get x7119 for the proxy chain): all 9 plain rows, 5 shaped (read miss, write, in miss, with hit, with miss) and the 3 trapless proxy rows. The 6 that pass are the shaped hits (every level declares the name, so the first link answers) and the 3 trapped-proxy rows (the trap's callee already probed). On net472 ("Process is terminated due to StackOverflowException.") 10 rows die (plain read hit, read miss, inherited getter, write, inherited setter, with hit; shaped read miss, write, with hit; trapless write) and trapless has fails its assertion ("false" for the RangeError): the .NET Framework JIT turns the unfixed HasProperty and trapless-read forwards into tail calls, so plain in hit/in miss/with miss, shaped in miss/with miss and trapless read complete there even unfixed. The converse census fails naming SharedShapeObject on both. - Fixed: HostNativeRecursionGuardTests 34/34 and PrototypeChainWalkTests 5/5 on net10.0 and net472. - Depth: on a 1 MB thread the 10,000-proxy rows probe from ~6,330 hops (read, has) and ~2,720 (write) on net10.0, ~5,370 (has) and ~1,890 (write) on net472, where the read completes -- the carve-out main already has. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PanPJbBD7pQC9fRiTpHxvs
…ct wrappers from killing the process on a member miss Backport of PR sebastienros#4125 (head 02bed3e, not yet merged on main) from main. ObjectWrapper.Get answers a member miss by forwarding the read to its prototype and then inspecting the result for Options.Interop.ThrowOnUnresolvedMember. That post-check keeps the forward out of tail position, so the link cannot join the loop ObjectInstance now walks a chain with: a hop to an ordinary link re-enters that loop, but a hop to another wrapper is a native frame with no probe between the two. A wrapper does not override SetPrototypeOf, so script builds such a chain itself (`Object.setPrototypeOf(w[i - 1], w[i])`) and its depth is an input; twenty thousand links ended the process (sebastienros#4087). The forward now probes the native stack first, as every other hand-over does, which turns that into a catchable RangeError. It is the only site of that shape: Set and HasProperty end in base.<op>, which is the loop; GetOwnProperty and RemoveOwnProperty walk nothing; TypeReference and NamespaceReference forward nothing to a prototype. Adapted for 4.x: - The comment on the probe drops main's reference to the IL pin StackOverflowGuardTests.ExactlyTheInteropAndForwardingFunctionsProbeTheNativeStack, which 4.x does not have; the depth cases are what hold the probe in place. - The new tests are xUnit v3 (TheoryData/MemberData, Fact), at the top of the class as on main, and ask for StackOverflowGuard, which is opt-in here: on a default 4.x engine the probe is inert, as every sebastienros#4007 probe is. - Jint/Runtime/Interop/AGENTS.md does not exist on 4.x; that edit is dropped. Evidence (Windows x64, Release): - With sebastienros#4078 applied and ObjectWrapper.cs as on 4.x, both rows of AChainOfAdjacentHostWrappersRaisesACatchableErrorAndTheEngineRecovers end the test host, each run alone, on net10.0 ("Stack overflow.", ObjectWrapper.Get x2781, exit 0xC00000FD) and on net472 ("Process is terminated due to StackOverflowException."). The three-link AShortChainOfAdjacentWrappersAnswersExactlyAsItDid passes unfixed, which is what says the probe did not change an answer. - Fixed: HostNativeRecursionGuardTests 37/37 on net10.0 and net472. - Both commits, `dotnet test -c Release`: Jint.Tests 7655 + 7570, Jint.Tests.PublicInterface 1903 + 1895 (net10.0 + net472), CommonScripts 28 + 28, SourceGenerators 52, test262 102,509 passed / 0 failed / 175 skipped; 0 failures anywhere. JINT_HOST_CONTRACT_VERIFICATION=1: Jint.Tests 7655 + 7570, Jint.Tests.PublicInterface 1907 + 1899, 0 failures. Public API snapshots unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PanPJbBD7pQC9fRiTpHxvs
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.
Both are merged on main: #4078 as
c706f711and #4125 asb630bf42. Each merged engine patch is line-for-line the one gated and ported here (#4078's =fec5ade67's; #4125'sObjectWrapper.cs=02bed3ef1's, apart from a comment naming an IL-pin test 4.x does not have). Both main-side gates were clean — tables on #4078 and #4125.Two commits, ported from the open PR heads: #4078 at
b506e62d4and #4125 at02bed3ef1(both based onc3bb84209). Base:4.xata51a2b640.StackOverflowGuarddoes not cover the prototype-chain walk inObjectInstance.Get: a deep__proto__chain still ends the process #4076).ObjectInstance's[[Get]],[[Set]]and[[HasProperty]]recursed one native frame per prototype link, so a 20,000-deep{ __proto__: x }chain ended the process. They are loops now. Each loop hands the rest of the operation to the first link it may not walk, and probes the native stack only at that hand-over. A traplessJsProxyprobes each forward to its target throughForwardToTarget(target).SharedShapeObjecttakesInternalTypes.PlainObject, andPrototypeChainWalkTestschecks that flag in both directions.ObjectWrapper.Getforwards to its prototype (A chain of adjacent ObjectWrappers still recurses one native frame per link #4087). That forward is not a tail call, so a chain of adjacent host wrappers was still one native frame per link. This is the only forward of that shape.How the
JsProxy.csconflict was resolved, hunk by hunkWhere 4.x probed before this change. #4007 (the 4.x backport of #3877) probes at the entry of
[[Call]]and[[Construct]]only (JsProxy.cslines 85 and 134).main's basec3bb84209has the same two probes and nothing else inJsProxy. Before this change, 4.x had no probe on any trapless forward of[[Get]],[[Set]],[[HasProperty]]or the other internal methods.What #4078 adds.
ForwardToTarget(target)probes and then returns the target. Every trapless forward goes through it, as does every forward a CLRProxyHandlerdeclines. It is not used in[[Call]]or[[Construct]]. Those forward throughcallable.Callandconstructor.Construct, which the #4007 entry probes already cover, and main leaves both methods byte-identical. So no route gets a second probe, and no route loses one.The three conflicts are all context, not probes:
IsSpecArray()→ForwardToTarget(_target).IsSpecArray()[[IsArray]]hookIsArray()return ForwardToTarget(_target).IsArray();IsExtensible(), CLR-declined arm[[IsExtensible]]is the overriddenpublic override bool Extensible { get; }rather than main'sinternal override bool IsExtensible(), so the body is indented one level deeperreturn ForwardToTarget(target).Extensible;IsExtensible(), trapless armThe trap-result invariant read
var targetResult = target.Extensible;stays unprobed, as on main, because it follows a trap call rather than forwarding. The result has 24ForwardToTargetsites, the same as main: 11 internal methods × (trapless arm + CLR-declined arm), plusIsArrayandToObject. Two probes stay at method entry ([[Call]]and[[Construct]]).git diff pr/4078 -- Jint/Native/JsProxy.csshows only differences that already separated 4.x from main (HasCall/IsCallable,IsSpecArray/IsArray,IsExtensible()/Extensible,MustPropagateHostException).Other adaptations for 4.x
ObjectInstance.cs:SetOnPrototypeChaincallsOrdinarySetWithOwnDescriptor(property, value, receiver, ownDesc). Main extracted that method fromSetUnlikelyin Allow own properties on HTMLCollection-derived receivers #3944, which is not on 4.x. The same extraction is made here as aprivatemethod with the body unchanged, andSetUnlikelydelegates to it as on main. Apart from that, this file's changed lines match Walk the prototype chain in a loop instead of one native frame per link #4078's line for line.StackOverflowGuardis opt-in on 4.x. The new engines ask for it, as Backport #3877 to 4.x: Guard native recursion and forwarding paths #4007's did (Guarded()). A default 4.x engine gets the loops: an ordinary chain resolves at any depth, with or without the guard. It does not get the hand-over, proxy-forward or wrapper probes, because those are gated on the guard, as every Backport #3877 to 4.x: Guard native recursion and forwarding paths #4007 probe is.b506e62d4hunk on the existing forwarding-chain rows is not taken. Backport #3877 to 4.x: Guard native recursion and forwarding paths #4007's backport already settled those rows for 4.x (a 256 KiB stack, withproxy callandbound callaccepting either answer), and this change touches none of those routes. No bind chain is added. The existingbound callrow passes under a 1 GBDOTNET_GCHeapHardLimit(checked) and fails withOutOfMemoryExceptionunder 96 MB, which shows the cap was applied.Engine, so no browser or WebApi type is reachable. Both directions were broken on purpose on 4.x, and each failure named its offender:SharedShapeObject→but found "SharedShapeObject"ArrayInstance→but found "ArrayInstance.Get"Jint.Benchmark/PrototypeChainReadBenchmark.csdid not exist on 4.x. Main added it with The member-read inline cache can serve a holder deeper than the direct prototype #4048, which 4.x does not carry. It is added whole, rows unchanged, so the lead can measure it. Its prose now says that on this branch the member cache serves only a direct-prototype holder. It builds; it was not run.AGENTS.mdedits are dropped because those files do not exist on 4.x. Two doc comments that citeJint/Constraints/AGENTS.mdnow say it is main's. For Stop a chain of adjacent host object wrappers from killing the process on a member miss #4125, the comment's reference to the IL pinExactlyTheInteropAndForwardingFunctionsProbeTheNativeStackis dropped, because 4.x has no such test.Depths
The depths are main's: 20,000 links for ordinary, shaped and wrapper chains, and 10,000 for proxies. All deep rows run on a 1 MB
DedicatedThread. To check that 10,000 proxies still discriminate on 4.x, a temporary binary search (not committed) found the smallest chain that raises the probe'sRangeErroron a 1 MB thread (Windows x64):NETFRAMEWORKcarve-out, ported as-is)The whole
HostNativeRecursionGuardTestsclass takes 3–5 s per TFM. I could not measure Linux x64, Linux ARM64 or macOS here. The proxy rows keep main's strictRangeErrorexpectation off .NET Framework, which main's CI accepts on the same matrix. If a leg here answersundefinedinstead, it will show on this PR's CI.Evidence (Windows x64, Release)
Unfixed. The ported tests were run against the engine files as they are on 4.x. For #4125, #4078 was applied and
ObjectWrapper.cswas as on 4.x. Each depth row ran in its own VSTest process, filtered byDisplayName. On the fixed build, every one of those filters selects exactly one row and it passes.in hit,in missandwith misscomplete (the .NET Framework JIT tail-calls the unfixedHasPropertyforward)"false"where theRangeErrorwas expected); trapless read completes (carve-out); 3 trapped rows passAShortChainOfAdjacentWrappersAnswersExactlyAsItDidbut found "SharedShapeObject"The crash output, from running each method directly:
The four methods that already existed (traversals, forwarding chains, host callable, host constructor) pass unfixed on both TFMs.
Fixed (both commits):
HostNativeRecursionGuardTestsPrototypeChainWalkTests+StackOverflowGuardTests+ProxyTestsFull solution,
dotnet test -c Release(both commits, exit 0):The lead's control for test262 was 102,501 / 0 / 183. The total is the same (102,684). The shift of 8 comes from
4.xitself moving from6b39fa076toa51a2b640(#4168): #4163 removed fourgetWeekInfoexclusions, and each of those files runs in strict and sloppy mode. None of the four known load flakes appeared, so nothing was re-run.Host-contract leg (
JINT_HOST_CONTRACT_VERIFICATION=1): Jint.Tests 7655 + 7570, Jint.Tests.PublicInterface 1907 + 1899 (net10.0 + net472), 0 failures.The per-row unfixed runs above were made on
6b39fa076, before the rebase ontoa51a2b640. After the rebase, each method was re-run directly against the unfixed engine on both TFMs. Every depth method still ends the host, the census converse still namesSharedShapeObject, and the four methods that already existed still pass.The
Jint.Tests.PublicInterfacepublic-API Verify snapshots are byte-identical:PublicApiTestpasses 6/6, and no*.verified.txtdiffers from4.x.dotnet build -c Releasegives 0 errors and 1 warning, the MSB3277 inJint.Tests.CommonScripts(net472) that is already on4.x.Jint.Benchmarkbuilds, includingPrototypeChainReadBenchmark.No benchmarks were run. The paired gate (
HostPrototypeShapeBenchmark,PrototypeChainReadBenchmark,HostAccessorReadBenchmark, SunSpider/Dromaeo) is the lead's.🤖 Generated with Claude Code
https://claude.ai/code/session_01PanPJbBD7pQC9fRiTpHxvs