From 6ae5c3c1782cdc78267df90a51202cdc83870e8f Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Thu, 24 Sep 2026 23:18:36 +0300 Subject: [PATCH 1/2] Backport #4078 to 4.x: Walk the prototype chain in a loop instead of one native frame per link Backport of PR #4078 (head b506e62d4, 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 (#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 #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 #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 #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 b506e62d4 hunk on the existing forwarding-chain rows is not taken: #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 #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) Claude-Session: https://claude.ai/code/session_01PanPJbBD7pQC9fRiTpHxvs --- Jint.Benchmark/PrototypeChainReadBenchmark.cs | 183 ++++++++ .../HostNativeRecursionGuardTests.cs | 192 +++++++++ Jint.Tests/Runtime/PrototypeChainWalkTests.cs | 391 ++++++++++++++++++ Jint/Native/JsProxy.cs | 87 ++-- Jint/Native/Object/ObjectInstance.cs | 178 ++++++-- Jint/Native/Object/SharedShapeObject.cs | 13 +- Jint/Runtime/InternalTypes.cs | 15 +- 7 files changed, 1007 insertions(+), 52 deletions(-) create mode 100644 Jint.Benchmark/PrototypeChainReadBenchmark.cs create mode 100644 Jint.Tests/Runtime/PrototypeChainWalkTests.cs diff --git a/Jint.Benchmark/PrototypeChainReadBenchmark.cs b/Jint.Benchmark/PrototypeChainReadBenchmark.cs new file mode 100644 index 0000000000..b020331148 --- /dev/null +++ b/Jint.Benchmark/PrototypeChainReadBenchmark.cs @@ -0,0 +1,183 @@ +using BenchmarkDotNet.Attributes; +using Jint.Native; + +namespace Jint.Benchmark; + +/// +/// What a member read costs as a function of how far up the prototype chain the member lives, with no +/// host object anywhere: four plain class declarations, one new, and a loop. +/// +/// +/// On this branch the member-read inline cache on JintMemberExpression remembers a holder only when it +/// is the receiver's direct prototype; anything deeper re-walks the chain and pays a +/// GetOwnProperty at every level, on every read (main lifted that in sebastienros/jint#4048, +/// which 4.x does not carry). That is not a niche shape — it is what +/// class Leaf extends Derived extends Middle extends Base does with every member Base declares. +/// This class is the engine-only statement of that cost, so the walk is measurable without a host object, a +/// wrapper or a binding generator in the picture. The rows are main's, unchanged, so the two +/// branches' tables read side by side. +/// +/// +/// The rows +/// +/// +/// — the baseline floor: the loop, the accumulate, and a read that resolves in +/// the receiver's own shape slot. Every other row is this plus one prototype resolution, so the baseline ratio +/// is the honest way to read the table. A control: nothing in this area may move it. +/// +/// +/// — a getter declared on the receiver's own class prototype, one link +/// up. The comparison, and the second control: this is the shape the member cache serves, so this row +/// moving is a regression. +/// +/// +/// — the same getter, declared on the root of a four-level +/// hierarchy and read off a leaf instance, so it sits three links behind the holder. On this branch the +/// member cache does not serve it, so it is resolved by ObjectInstance.Get's prototype walk on every +/// read. +/// +/// +/// — a name declared nowhere on the chain, so the read walks every link +/// including Object.prototype and ends in undefined. It is the only row that exercises the walk +/// to its end, which is the lane ObjectInstance.Get resolves in a loop rather than by recursing into +/// each prototype (sebastienros/jint#4076), and nothing absent can be cached on any branch. It reads +/// NaN into the accumulator, which costs the same add as every other row. +/// +/// +/// / — the same pair for a method +/// call, which resolves its callee through a different entry point on the same node +/// (GetCalleeForCall) and reads a data property rather than invoking an accessor. Both lanes share the +/// cache fields, so the pair is what shows the call lane behaves as the read lane does. +/// +/// +/// +/// +/// Why a class of its own rather than more rows on HostPrototypeShapeBenchmark. That class is +/// parameterised on HostPrototypeKind, and these rows contain no host object, so every one of them +/// would be reported twice with identical work under two labels that mean nothing to it. Its chain rows are +/// also not comparable to these: they drive ('name' in obj), which resolves through +/// ObjectInstance.HasProperty and never reaches the member-read lane at all. +/// +/// +/// +/// Engine isolation. One per row, built and warmed in [GlobalSetup] with +/// that row's script and nothing else (). These rows measure warm reads — +/// a per-site inline cache is the whole subject, and a site that never warms measures nothing about it — so +/// engine construction and the first evaluation stay outside the measurement. Sharing one engine would be +/// actively wrong here: the rows read the same property names off the same hierarchy, so one row's warm-up +/// would populate the caches of the next. +/// +/// +[MemoryDiagnoser] +public class PrototypeChainReadBenchmark +{ + private const int LoopIterations = 20000; + + /// + /// Four levels, so a member declared on Base sits three links behind the leaf instance's direct + /// prototype: instance → Leaf.prototype → Derived.prototype → Middle.prototype → + /// Base.prototype. The two intermediate classes declare nothing, which is exactly what an + /// interface hierarchy looks like from the point of view of one inherited member: levels that have to be + /// asked and have nothing to say. + /// + /// The leaf's own value is assigned in the constructor so the receiver is an ordinary shaped + /// object with one own property — the floor row's target, and the own-property miss every other row's + /// read has to establish before it may look at a prototype. + /// + /// + private const string Hierarchy = """ + class Base { + constructor() { this.value = 1; } + get rootAttribute() { return this.value; } + rootOperation() { return this.value; } + } + class Middle extends Base { } + class Derived extends Middle { } + class Leaf extends Derived { + get leafAttribute() { return this.value; } + leafOperation() { return this.value; } + } + var leaf = new Leaf(); + """; + + private IsolatedScript _ownRead; + private IsolatedScript _directGetterRead; + private IsolatedScript _rootGetterRead; + private IsolatedScript _absentRead; + private IsolatedScript _directMethodCall; + private IsolatedScript _rootMethodCall; + + [GlobalSetup] + public void GlobalSetup() + { + _ownRead = Loop("obj.value"); + _directGetterRead = Loop("obj.leafAttribute"); + _rootGetterRead = Loop("obj.rootAttribute"); + _absentRead = Loop("obj.absentAttribute"); + _directMethodCall = Loop("obj.leafOperation()"); + _rootMethodCall = Loop("obj.rootOperation()"); + } + + /// + /// One row's script and its private engine: the shared hierarchy, then a loop whose body is nothing but + /// the read under test. The read happens inside a function so the member node is a handler-tree node that + /// warms like one in real code, and the accumulation keeps the result observable so nothing can be + /// optimised away. + /// + private static IsolatedScript Loop(string read) => IsolatedScript.Warm( + Engine.PrepareScript($$""" + (function (obj) { + var total = 0; + for (var i = 0; i < {{LoopIterations}}; i++) { + total += {{read}}; + } + return total; + })(leaf) + """), + static () => + { + var engine = new Engine(); + engine.Execute(Hierarchy); + return engine; + }); + + /// + /// Each row owns an , and an engine is . BenchmarkDotNet runs + /// every case in its own process, so this changes nothing about the measurement — it is here so the class + /// does not model a lifecycle an embedder should not copy. + /// + [GlobalCleanup] + public void GlobalCleanup() + { + _ownRead.Engine.Dispose(); + _directGetterRead.Engine.Dispose(); + _rootGetterRead.Engine.Dispose(); + _absentRead.Engine.Dispose(); + _directMethodCall.Engine.Dispose(); + _rootMethodCall.Engine.Dispose(); + } + + /// The floor: an own-property read, resolved in the receiver's own shape slot. + [Benchmark(Baseline = true)] + public JsValue OwnPropertyRead() => _ownRead.Run(); + + /// The comparison: a getter one link up, the shape the member cache serves. + [Benchmark] + public JsValue DirectPrototypeGetterRead() => _directGetterRead.Run(); + + /// The same getter, four links up: a prototype walk on every read on this branch. + [Benchmark] + public JsValue RootPrototypeGetterRead() => _rootGetterRead.Run(); + + /// The walk to its end: a name nowhere on the chain, so every link is asked and none answers. + [Benchmark] + public JsValue AbsentNameRead() => _absentRead.Run(); + + /// The comparison for the call lane: a method one link up. + [Benchmark] + public JsValue DirectPrototypeMethodCall() => _directMethodCall.Run(); + + /// The subject for the call lane: the same method, four links up. + [Benchmark] + public JsValue RootPrototypeMethodCall() => _rootMethodCall.Run(); +} diff --git a/Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs b/Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs index 517d861918..5f276272a8 100644 --- a/Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs +++ b/Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs @@ -126,6 +126,198 @@ public void HostConstructorRecursionRaisesACatchableErrorAndTheEngineRecovers() }, maxStackSize: SmallStack); } + /// + /// A prototype chain twenty thousand links deep, and the property operations that have to walk it: + /// [[Get]], [[Set]], [[HasProperty]], and the [[HasProperty]] a with + /// statement reaches through its object environment. Every one of them used to descend one native frame + /// per link and end the host process with a stack overflow no catch could see (#4076). + /// + /// What each asserts is the answer, not a RangeError: a chain of ordinary objects is + /// walked iteratively now, so its depth costs no stack at all and the read, the write and the lookup all + /// resolve. The hits are there to prove the chain really is twenty thousand links long — a flat object + /// would answer every miss just as happily. + /// + /// + public static TheoryData DeepPrototypeChains => new() + { + { "read hit", "outcome = x.deep;", "found" }, + { "read miss", "outcome = String(x.missing);", "undefined" }, + { "inherited getter", "outcome = x.computed;", "found!" }, + { "write", "x.missing = 1; outcome = x.missing + ':' + x.hasOwnProperty('missing');", "1:true" }, + { "inherited setter", "x.written = 2; outcome = String(sink);", "2" }, + { "in hit", "outcome = String('deep' in x);", "true" }, + { "in miss", "outcome = String('missing' in x);", "false" }, + { "with hit", "with (x) { outcome = deep; }", "found" }, + { "with miss", "with (x) { outcome = typeof missing; }", "undefined" }, + }; + + [Theory] + [MemberData(nameof(DeepPrototypeChains))] + public void ADeepPrototypeChainIsWalkedWithoutExhaustingTheNativeStack(string route, string operation, string expected) + { + _ = route; + DedicatedThread.Run(() => + { + using var engine = Guarded(); + var outcome = engine.Evaluate(""" + var sink; + var x = { + deep: 'found', + get computed() { return this.deep + '!'; }, + set written(value) { sink = value; } + }; + for (var i = 0; i < 20000; i++) { x = { __proto__: x }; } + var outcome; + try { + """ + operation + """ + } catch (error) { outcome = error.name + ':' + error.message; } + String(outcome); + """).AsString(); + outcome.Should().Be(expected); + + engine.Evaluate("({ a: 1 }).a").AsNumber().Should().Be(1); + engine.Evaluate("'a' in { a: 1 }").AsBoolean().Should().BeTrue(); + }, maxStackSize: SmallStack); + } + + /// + /// The same chain, built out of the shaped prototypes a host declares through + /// — what a Web IDL binding generator emits. Every level overrides nothing and + /// runs the ordinary algorithm, so the walk must resolve it in its loop rather than hand it the rest of the + /// chain: a hand-over is a native frame per level, which at this depth is the probe's RangeError + /// instead of the answer. + /// + /// That is what makes this an assertion about the walk and not merely about the answers: a + /// shaped chain shallow enough to recurse through would answer identically either way. One shape + /// instantiated at every level is deliberate — a shape is process-shared and a host declares it once + /// per interface, so this is also the allocation shape a real binding has. + /// + /// + public static TheoryData DeepShapedPrototypeChains => new() + { + { "read hit", "outcome = x.tag;", "shaped" }, + { "read miss", "outcome = String(x.missing);", "undefined" }, + { "inherited getter", "outcome = x.computed;", "shaped!" }, + { "write", "x.missing = 1; outcome = x.missing + ':' + x.hasOwnProperty('missing');", "1:true" }, + { "in hit", "outcome = String('tag' in x);", "true" }, + { "in miss", "outcome = String('missing' in x);", "false" }, + { "with hit", "with (x) { outcome = tag; }", "shaped" }, + { "with miss", "with (x) { outcome = typeof missing; }", "undefined" }, + }; + + [Theory] + [MemberData(nameof(DeepShapedPrototypeChains))] + public void ADeepShapedPrototypeChainIsWalkedWithoutExhaustingTheNativeStack(string route, string operation, string expected) + { + _ = route; + DedicatedThread.Run(() => + { + using var engine = Guarded(); + + var shape = new JsObjectShape.Builder() + .Constant("tag", new JsString("shaped")) + .Accessor("computed", static (thisObject, _) => new JsString(((ObjectInstance) thisObject).Get("tag").AsString() + "!")) + .Build(); + + ObjectInstance level = shape.Instantiate(engine); + for (var i = 0; i < 20000; i++) + { + level = shape.Instantiate(engine, level); + } + + engine.SetValue("chainLeaf", level); + + var outcome = engine.Evaluate(""" + var x = Object.create(chainLeaf); + var outcome; + try { + """ + operation + """ + } catch (error) { outcome = error.name + ':' + error.message; } + String(outcome); + """).AsString(); + outcome.Should().Be(expected); + + engine.Evaluate("({ a: 1 }).a").AsNumber().Should().Be(1); + }, maxStackSize: SmallStack); + } + + /// + /// The same operations over a chain of proxies. A proxy runs its own algorithm rather than the + /// ordinary one, so it cannot be walked: each hop is a native frame, and the chain is bounded the way + /// every other forwarding hop in the engine is — by a probe, which turns the overflow into a catchable + /// RangeError. + /// + /// Both handlers are here because only one of them was ever broken, and the pair is what says so. A + /// trapless proxy forwards the whole algorithm itself (return target.Get(property, receiver)) + /// with nothing in between, and that is the shape that ended the host process. A trapped one + /// reaches its trap through ICallable.Call, where the callee probes for itself, so it already + /// raised this RangeError before any of this — which is precisely why the fix probes the forward + /// and not the entry of each operation. Trapped proxies are the shape real code ships (every reactivity + /// library puts a get trap on every object it proxies), so a probe at the entry would be a third + /// probe on the common path buying nothing. + /// + /// + public static TheoryData DeepProxyChains => new() + { + { "trapless read", TraplessHandler, "outcome = String(x.missing);" }, + { "trapless write", TraplessHandler, "x.missing = 1; outcome = 'written';" }, + { "trapless has", TraplessHandler, "outcome = String('missing' in x);" }, + { "trapped read", ForwardingHandler, "outcome = String(x.missing);" }, + { "trapped write", ForwardingHandler, "x.missing = 1; outcome = 'written';" }, + { "trapped has", ForwardingHandler, "outcome = String('missing' in x);" }, + }; + + private const string TraplessHandler = "{}"; + + private const string ForwardingHandler = """ + { + get: function (target, key) { return target[key]; }, + set: function (target, key, value) { target[key] = value; return true; }, + has: function (target, key) { return key in target; } + } + """; + + [Theory] + [MemberData(nameof(DeepProxyChains))] + public void ADeepProxyChainRaisesACatchableErrorAndTheEngineRecovers(string route, string handler, string operation) + { + DedicatedThread.Run(() => + { + using var engine = Guarded(); + var outcome = engine.Evaluate(""" + var handler = + """ + handler + """ + ; + var x = {}; + for (var i = 0; i < 10000; i++) { x = new Proxy(x, handler); } + var outcome; + try { + """ + operation + """ + } catch (error) { outcome = error.name + ':' + error.message; } + String(outcome); + """).AsString(); +#if NETFRAMEWORK + // Same carve-out as the trapless proxy *call* above, and for the same reason: the .NET Framework + // JIT turns `return target.Get(property, receiver)` into a tail call, so this one route can + // consume no stack and legitimately answer the read. Modern runtimes keep the forwarding frames, + // and a trap in the way is a real call on every target framework. + if (route == "trapless read") + { + outcome.Should().BeOneOf("undefined", "RangeError:Maximum call stack size exceeded"); + } + else + { + outcome.Should().Be("RangeError:Maximum call stack size exceeded"); + } +#else + _ = route; + outcome.Should().Be("RangeError:Maximum call stack size exceeded"); +#endif + + engine.Evaluate("6 * 7").AsNumber().Should().Be(42); + }, maxStackSize: SmallStack); + } + private const int SmallStack = 1024 * 1024; /// diff --git a/Jint.Tests/Runtime/PrototypeChainWalkTests.cs b/Jint.Tests/Runtime/PrototypeChainWalkTests.cs new file mode 100644 index 0000000000..38ca0c89c7 --- /dev/null +++ b/Jint.Tests/Runtime/PrototypeChainWalkTests.cs @@ -0,0 +1,391 @@ +#nullable enable + +using System.Globalization; +using System.Reflection; +using System.Runtime.CompilerServices; +using Jint.Native; +using Jint.Native.Object; +using Jint.Runtime; +using Jint.Runtime.Descriptors; + +namespace Jint.Tests.Runtime; + +/// +/// [[Get]], [[Set]] and [[HasProperty]] walk the prototype chain in a loop rather than +/// by recursing into each link, because a chain is built by script and its depth is an input +/// (sebastienros/jint#4076, and ObjectInstance.GetFromPrototypeChain for the mechanism). A loop in the +/// base class can only run the ordinary algorithm, so the whole correctness of it is in when it +/// declines to walk a link and hands the rest of the operation over instead. These are the two halves of +/// that: the classification the walks key on, and the behaviour it buys. +/// +public class PrototypeChainWalkTests +{ + /// + /// The [[Set]] and [[HasProperty]] walks read InternalTypes.PlainObject as + /// "this object does not override a property internal method" — the flag's own documented meaning — and + /// walk such a link themselves instead of calling its virtual. So an object that carried the flag + /// and overrode one of the three would have that override silently skipped by every other + /// object's walk: a wrong answer, not a slow one. + /// + /// Nothing in the type system says that, so it is asserted over the objects a built engine can actually + /// reach — every own property value and every prototype, transitively, from the global object and from a + /// sample of every exotic shape script can construct. The direction that is safe needs no check: a type + /// that does not carry the flag is merely handed over to, exactly as before. + /// + /// + [Fact] + public void NoObjectCarryingPlainObjectOverridesAPropertyInternalMethod() + { + using var engine = new Engine(options => options.AllowClr(typeof(List<>).Assembly)); + + var offenders = new SortedSet(StringComparer.Ordinal); + foreach (var reached in Census(engine)) + { + if ((reached._type & InternalTypes.PlainObject) == InternalTypes.Empty) + { + continue; + } + + var overridden = FirstOverridden(reached.GetType(), WalkMethods); + if (overridden is not null) + { + offenders.Add(overridden); + } + } + + string.Join(", ", offenders).Should().BeEmpty( + "a type carrying InternalTypes.PlainObject promises it does not override Get, Set or HasProperty — " + + "another object's chain walk resolves such a link itself and would skip the override"); + } + + /// + /// The converse, and the direction that was missing when a shaped host prototype + /// (SharedShapeObject) shipped without the flag: it overrides nothing at all, so every + /// [[Set]] and 'x' in o reaching it declined to walk it, handed the rest of the chain back + /// to the recursive path and paid the new flag test at every level — the old recursion plus the + /// new overhead, which a paired benchmark caught as a regression on shaped prototypes alone. + /// + /// A missing flag is only ever a slow answer, never a wrong one, which is why it cannot be caught by + /// any correctness test and needs a census instead. The set of methods is wider than the walk's three, + /// because the flag is a storage claim as well: the lanes in ObjectInstance.Get, + /// Set and CreateDataProperty read _properties (or the shape) without asking + /// GetOwnProperty, so a type that projects its own properties from anywhere else must not take + /// the flag however ordinary its [[Get]] is. Overriding SetOwnProperty or + /// DefineOwnProperty is compatible with it and deliberately absent from the list — + /// ObjectPrototype carries the flag and overrides both. + /// + /// + [Fact] + public void NoObjectOverridingNoPropertyInternalMethodLacksPlainObject() + { + using var engine = new Engine(options => options.AllowClr(typeof(List<>).Assembly)); + + var offenders = new SortedSet(StringComparer.Ordinal); + foreach (var reached in Census(engine)) + { + var type = reached.GetType(); + if ((reached._type & InternalTypes.PlainObject) != InternalTypes.Empty + || FirstOverridden(type, StorageMethods) is not null + || EligibleButUnflagged.Contains(type.Name)) + { + continue; + } + + offenders.Add(type.Name); + } + + string.Join(", ", offenders).Should().BeEmpty( + "a type that overrides none of the property internal methods is entitled to be walked, and every " + + "[[Set]] and [[HasProperty]] that meets it without InternalTypes.PlainObject hands the rest of " + + "the chain back to the recursive path instead — give it the flag, or add it to " + + nameof(EligibleButUnflagged) + " with the reason"); + } + + /// + /// The exceptions: in-box types that override none of the named methods, would be walked correctly with + /// InternalTypes.PlainObject, and do not carry it. Each is a deliberate omission rather than a + /// defect, for one reason that applies to all of them. + /// + /// The flag is two claims in one. "Overrides no property internal method" is what the walks read. "Own + /// string-keyed properties live in the base storage" is what ObjectInstance.Get, Set and + /// CreateDataProperty read, and taking the flag switches those three fast lanes on as well — + /// which is a change to measure on its own terms, not part of the shaped-prototype regression this + /// census was written for (sebastienros/jint#4076). A SharedShapeObject is flagged and absent + /// from this list precisely because for it the second half is free: all three lanes spell their + /// test == PlainObject against PlainObject | BuiltinShapeMode, so a shaped object is + /// excluded from them by construction while its shape is installed, and once a deopt has moved every + /// slot into _properties it really is the dictionary they assume. + /// + /// + /// A type belongs here only while that is the whole story. Anything whose own properties come from + /// somewhere else overrides one of the storage methods above and never reaches this list. + /// + /// + private static readonly HashSet EligibleButUnflagged = new(StringComparer.Ordinal) + { + // Built-ins whose string-keyed own properties live in a shared BuiltinShape, like a shaped host + // prototype — but built through the public ObjectInstance(Engine) constructor, so they derive + // their access semantics instead of declaring them. + "AtomicsInstance", + "GeneratorPrototype", + "JsonInstance", + "MathInstance", + "ReflectInstance", + "TemporalNow", + + // Ordinary in-box objects: internal slots in fields, own properties in the base storage. + "BooleanInstance", + "BooleanPrototype", + "ErrorPrototype", + "GeneratorInstance", + "IntlInstance", + "JsDataView", + "JsMap", + "JsPromise", + "JsSet", + "JsWeakMap", + "JsWeakSet", + "NumberInstance", + "TemporalInstance", + }; + + /// + /// The other half: a link the walk declines really does get to run its own algorithm. A Proxy is the + /// sharpest statement of it, because its answer is observable — a trap that does not fire produced no + /// log entry, and the value the read, the write and the in settle on could only have come from + /// the trap. Two ordinary links sit above it so the operation is genuinely mid-walk when it hands over. + /// + [Fact] + public void AnOverrideMidChainStillRunsItsOwnAlgorithm() + { + using var engine = new Engine(); + + var outcome = engine.Evaluate(""" + var log = []; + var mid = new Proxy({}, { + get: function (target, key, receiver) { log.push('get:' + key); return key === 'viaTrap' ? 'trapped' : undefined; }, + set: function (target, key, value, receiver) { log.push('set:' + key); return true; }, + has: function (target, key) { log.push('has:' + key); return key === 'present'; } + }); + var leaf = Object.create(Object.create(mid)); + + var read = leaf.viaTrap; + leaf.written = 1; + var has = ('present' in leaf) + '/' + ('absent' in leaf); + + // read through Object.prototype, not through leaf: a get trap answers for every name on the + // chain, hasOwnProperty included, and asking leaf for it would only log the trap again + read + '|' + has + '|' + Object.prototype.hasOwnProperty.call(leaf, 'written') + '|' + log.join(','); + """).AsString(); + + outcome.Should().Be("trapped|true/false|false|get:viaTrap,set:written,has:present,has:absent"); + } + + /// + /// The same thing for an in-box exotic that is not a Proxy: an array mid-chain answers length from + /// its own [[Get]] and an index from its own [[HasProperty]], neither of which an ordinary + /// probe of its property storage would have produced on its own. + /// + [Fact] + public void AnArrayMidChainStillAnswersForItsOwnIndices() + { + using var engine = new Engine(); + + engine.Evaluate(""" + var arr = [10, 20, 30]; + var leaf = Object.create(Object.create(arr)); + leaf.length + '|' + ('2' in leaf) + '|' + ('3' in leaf) + '|' + leaf[1]; + """).AsString().Should().Be("3|true|false|20"); + } + + /// + /// The shaped-prototype regression itself, stated as behaviour. A chain of + /// instances is what a Web IDL binding generator hands a host, and every + /// link of it overrides nothing — so the walk must resolve it rather than hand it over, and must get + /// the ordinary answers while doing so: an inherited method found at the deepest level, a name nothing + /// declares refused, a write landing on the receiver and not on the prototype that was walked past, an + /// inherited setter invoked with the receiver, and an inherited get-only accessor refusing a write. + /// + [Fact] + public void AShapedHostPrototypeChainIsWalkedAndAnswersCorrectly() + { + using var engine = new Engine(); + engine.SetValue("chainLeaf", ShapedChain(engine, levels: 3)); + + engine.Evaluate(""" + var obj = Object.create(chainLeaf); + + var inHit = ('l0_op' in obj) + '/' + ('l2_CONST' in obj); + var inMiss = String('__absent__' in obj); + var ownNone = String(obj.hasOwnProperty('l0_op')); + + obj.l0_op = 'shadowed'; + var write = obj.l0_op + '/' + obj.hasOwnProperty('l0_op') + '/' + (typeof chainLeaf.l0_op); + + obj.l1_sink = 42; + var setter = obj.seenBySetter + '/' + obj.hasOwnProperty('seenBySetter') + '/' + chainLeaf.hasOwnProperty('seenBySetter'); + + var strictWrite; + try { + (function () { 'use strict'; obj.l2_attr = 1; })(); + strictWrite = 'no error'; + } catch (error) { strictWrite = error.name; } + + [inHit, inMiss, ownNone, write, setter, strictWrite].join('|'); + """).AsString().Should().Be("true/true|false|false|shadowed/true/function|42/true/false|TypeError"); + } + + /// + /// Every object reachable from by own property values and prototypes. Accessors + /// are not invoked — a census must not run script — but a lazily-materialized data property is read, + /// which is what makes the walk see the built-ins rather than their unmaterialized slots. + /// + private static IEnumerable Reachable(params ObjectInstance[] roots) + { + var seen = new HashSet(ReferenceComparer.Instance); + var pending = new Stack(roots); + + while (pending.Count > 0) + { + var current = pending.Pop(); + if (!seen.Add(current)) + { + continue; + } + + yield return current; + + if (current.Prototype is { } prototype) + { + pending.Push(prototype); + } + + foreach (var key in current.GetOwnPropertyKeys()) + { + var descriptor = current.GetOwnProperty(key); + if (descriptor == PropertyDescriptor.Undefined || descriptor.IsAccessorDescriptor()) + { + continue; + } + + if (descriptor.Value is ObjectInstance value) + { + pending.Push(value); + } + } + } + } + + /// + /// The three the walks run in place of a link's own virtual: overriding one of these while carrying + /// InternalTypes.PlainObject is the wrong answer the first census rules out. + /// + private static readonly (string Name, Type[] Parameters)[] WalkMethods = + [ + (nameof(ObjectInstance.Get), [typeof(JsValue), typeof(JsValue)]), + (nameof(ObjectInstance.Set), [typeof(JsValue), typeof(JsValue), typeof(JsValue)]), + (nameof(ObjectInstance.HasProperty), [typeof(JsValue)]), + ]; + + /// + /// The whole contract: the three above plus the five that answer what this object's own properties + /// are. A type overriding one of those five projects its own properties from somewhere the + /// flag's storage lanes do not look, so the second census does not ask it to carry the flag. + /// + private static readonly (string Name, Type[] Parameters)[] StorageMethods = + [ + .. WalkMethods, + (nameof(ObjectInstance.GetOwnProperty), [typeof(JsValue)]), + (nameof(ObjectInstance.GetOwnPropertyKeys), [typeof(Types)]), + ("ProbeOwnProperty", [typeof(JsValue)]), + ("TryGetOwnPropertyValue", [typeof(JsValue), typeof(JsValue), typeof(JsValue).MakeByRefType()]), + ("GetInitialOwnStringPropertyKeys", []), + ]; + + /// + /// The first of that overrides below + /// , or if it overrides none. + /// + private static string? FirstOverridden(Type type, (string Name, Type[] Parameters)[] methods) + { + const BindingFlags Flags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; + + foreach (var (name, parameters) in methods) + { + var method = type.GetMethod(name, Flags, binder: null, types: parameters, modifiers: null); + + if (method?.DeclaringType is { } declaring && declaring != typeof(ObjectInstance) && declaring != typeof(JsValue)) + { + return declaring.Name + "." + name; + } + } + + return null; + } + + /// + /// The objects both censuses run over: everything reachable from a built engine's global object, from a + /// sample of every exotic shape script can construct, and from a shaped host prototype chain — the shape + /// a Web IDL binding generator emits, which a host reaches through and which + /// no engine builds on its own. + /// + private static IEnumerable Census(Engine engine) + { + var samples = engine.Evaluate(""" + [ + {}, Object.create(null), [1, 2], new Date(), new Error('e'), /r/g, new Map(), new Set(), + new WeakMap(), new WeakSet(), new Int8Array(2), new DataView(new ArrayBuffer(2)), + new Proxy({}, {}), (function () { return arguments; })(1), function f() {}, (class C {}), + new String('s'), new Number(1), new Boolean(true), Promise.resolve(1), + (function* g() { yield 1; })(), new (class D extends Array {})(), + Object.getOwnPropertyDescriptor(Map.prototype, 'size'), System.String, new System.Text.StringBuilder() + ] + """).AsObject(); + + return Reachable(engine.Realm.GlobalObject, samples, ShapedChain(engine, levels: 3)); + } + + /// + /// A shaped host prototype chain of links, built the way a binding generator + /// would: one process-shared per level, instantiated into this engine on top + /// of the level below it. Each level is touched once, because the storage representation settles on the + /// first property access rather than at construction. + /// + private static ObjectInstance ShapedChain(Engine engine, int levels) + { + ObjectInstance? previous = null; + for (var level = 0; level < levels; level++) + { + var prefix = "l" + level.ToString(CultureInfo.InvariantCulture) + "_"; + var shape = new JsObjectShape.Builder() + .Method(prefix + "op", static (_, _) => JsValue.Undefined) + .Accessor(prefix + "attr", static (_, _) => new JsString("attr")) + .Accessor( + prefix + "sink", + static (_, _) => JsValue.Undefined, + static (thisObject, arguments) => + { + // Proves the receiver semantics through the walk: an inherited setter runs with the + // object the write STARTED on, not with the prototype it was found on. + _ = ((ObjectInstance) thisObject).Set("seenBySetter", arguments[0]); + return JsValue.Undefined; + }) + .Constant(prefix + "CONST", new JsString(prefix)) + .Build(); + + previous = previous is null ? shape.Instantiate(engine) : shape.Instantiate(engine, previous); + previous.Get(prefix + "CONST"); + } + + return previous!; + } + + private sealed class ReferenceComparer : IEqualityComparer + { + public static readonly ReferenceComparer Instance = new(); + + public bool Equals(ObjectInstance? x, ObjectInstance? y) => ReferenceEquals(x, y); + + public int GetHashCode(ObjectInstance obj) => RuntimeHelpers.GetHashCode(obj); + } +} diff --git a/Jint/Native/JsProxy.cs b/Jint/Native/JsProxy.cs index 6df112eb0f..1a4f99c701 100644 --- a/Jint/Native/JsProxy.cs +++ b/Jint/Native/JsProxy.cs @@ -1,4 +1,5 @@ using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; using System.Runtime.ExceptionServices; using Jint.Native.Function; using Jint.Native.Object; @@ -185,13 +186,13 @@ ObjectInstance IConstructor.Construct(JsCallArguments arguments, JsValue newTarg internal override bool IsArray() { AssertNotRevoked(KeyIsArray); - return _target.IsArray(); + return ForwardToTarget(_target).IsArray(); } public override object ToObject() { AssertNotRevoked(KeyToObject); - return _target.ToObject(); + return ForwardToTarget(_target).ToObject(); } internal override bool IsConstructor => _isConstructor; @@ -211,7 +212,7 @@ public override JsValue Get(JsValue property, JsValue receiver) var clrResult = InvokeClrGet(clrHandler, target, TypeConverter.ToPropertyKey(property), receiver); if (clrResult is null) { - return target.Get(property, receiver); + return ForwardToTarget(target).Get(property, receiver); } result = clrResult; @@ -222,7 +223,7 @@ public override JsValue Get(JsValue property, JsValue receiver) var trap = GetTrap(handler, TrapGet); if (trap is null) { - return target.Get(property, receiver); + return ForwardToTarget(target).Get(property, receiver); } result = CallTrap(handler, trap, target, TypeConverter.ToPropertyKey(property), receiver); @@ -271,7 +272,7 @@ public override List GetOwnPropertyKeys(Types types = Types.Empty | Typ var clrResult = InvokeClrOwnKeys(clrHandler, target); if (clrResult is null) { - return target.GetOwnPropertyKeys(types); + return ForwardToTarget(target).GetOwnPropertyKeys(types); } result = CreateOwnKeysArray(clrResult); @@ -282,7 +283,7 @@ public override List GetOwnPropertyKeys(Types types = Types.Empty | Typ var trap = GetTrap(handler, TrapOwnKeys); if (trap is null) { - return target.GetOwnPropertyKeys(types); + return ForwardToTarget(target).GetOwnPropertyKeys(types); } result = CallTrap(handler, trap, target); @@ -399,7 +400,7 @@ public override PropertyDescriptor GetOwnProperty(JsValue property) var clrResult = InvokeClrGetOwnPropertyDescriptor(clrHandler, target, TypeConverter.ToPropertyKey(property)); if (clrResult is null) { - return target.GetOwnProperty(property); + return ForwardToTarget(target).GetOwnProperty(property); } // PropertyDescriptor.Undefined round-trips as JsValue.Undefined ("no such property") @@ -411,7 +412,7 @@ public override PropertyDescriptor GetOwnProperty(JsValue property) var trap = GetTrap(handler, TrapGetOwnPropertyDescriptor); if (trap is null) { - return target.GetOwnProperty(property); + return ForwardToTarget(target).GetOwnProperty(property); } trapResultObj = CallTrap(handler, trap, target, TypeConverter.ToPropertyKey(property)); @@ -522,7 +523,7 @@ public override bool Set(JsValue property, JsValue value, JsValue receiver) var clrResult = InvokeClrSet(clrHandler, target, TypeConverter.ToPropertyKey(property), value, receiver); if (clrResult is null) { - return target.Set(property, value, receiver); + return ForwardToTarget(target).Set(property, value, receiver); } if (!clrResult.Value) @@ -536,7 +537,7 @@ public override bool Set(JsValue property, JsValue value, JsValue receiver) var trap = GetTrap(handler, TrapSet); if (trap is null) { - return target.Set(property, value, receiver); + return ForwardToTarget(target).Set(property, value, receiver); } var trapResult = CallTrap(handler, trap, target, TypeConverter.ToPropertyKey(property), value, receiver); @@ -588,7 +589,7 @@ public override bool DefineOwnProperty(JsValue property, PropertyDescriptor desc var clrResult = InvokeClrDefineProperty(clrHandler, target, TypeConverter.ToPropertyKey(property), desc); if (clrResult is null) { - return target.DefineOwnProperty(property, desc); + return ForwardToTarget(target).DefineOwnProperty(property, desc); } if (!clrResult.Value) @@ -602,7 +603,7 @@ public override bool DefineOwnProperty(JsValue property, PropertyDescriptor desc var trap = GetTrap(handler, TrapDefineProperty); if (trap is null) { - return target.DefineOwnProperty(property, desc); + return ForwardToTarget(target).DefineOwnProperty(property, desc); } var descObj = PropertyDescriptor.FromPropertyDescriptor(_engine, desc, strictUndefined: true); @@ -675,7 +676,7 @@ public override bool HasProperty(JsValue property) var clrResult = InvokeClrHas(clrHandler, target, TypeConverter.ToPropertyKey(property)); if (clrResult is null) { - return target.HasProperty(property); + return ForwardToTarget(target).HasProperty(property); } trapResult = clrResult.Value; @@ -686,7 +687,7 @@ public override bool HasProperty(JsValue property) var trap = GetTrap(handler, TrapHas); if (trap is null) { - return target.HasProperty(property); + return ForwardToTarget(target).HasProperty(property); } trapResult = TypeConverter.ToBoolean(CallTrap(handler, trap, target, TypeConverter.ToPropertyKey(property))); @@ -731,7 +732,7 @@ public override bool Delete(JsValue property) var clrResult = InvokeClrDeleteProperty(clrHandler, target, TypeConverter.ToPropertyKey(property)); if (clrResult is null) { - return target.Delete(property); + return ForwardToTarget(target).Delete(property); } if (!clrResult.Value) @@ -745,7 +746,7 @@ public override bool Delete(JsValue property) var trap = GetTrap(handler, TrapDeleteProperty); if (trap is null) { - return target.Delete(property); + return ForwardToTarget(target).Delete(property); } if (!TypeConverter.ToBoolean(CallTrap(handler, trap, target, TypeConverter.ToPropertyKey(property)))) @@ -793,7 +794,7 @@ public override bool PreventExtensions() var clrResult = InvokeClrPreventExtensions(clrHandler, target); if (clrResult is null) { - return target.PreventExtensions(); + return ForwardToTarget(target).PreventExtensions(); } success = clrResult.Value; @@ -804,7 +805,7 @@ public override bool PreventExtensions() var trap = GetTrap(handler, TrapPreventExtensions); if (trap is null) { - return target.PreventExtensions(); + return ForwardToTarget(target).PreventExtensions(); } success = TypeConverter.ToBoolean(CallTrap(handler, trap, target)); @@ -835,7 +836,7 @@ public override bool Extensible var clrResult = InvokeClrIsExtensible(clrHandler, target); if (clrResult is null) { - return target.Extensible; + return ForwardToTarget(target).Extensible; } booleanTrapResult = clrResult.Value; @@ -846,7 +847,7 @@ public override bool Extensible var trap = GetTrap(handler, TrapIsExtensible); if (trap is null) { - return target.Extensible; + return ForwardToTarget(target).Extensible; } booleanTrapResult = TypeConverter.ToBoolean(CallTrap(handler, trap, target)); @@ -877,7 +878,7 @@ public override bool Extensible var clrResult = InvokeClrGetPrototypeOf(clrHandler, target); if (clrResult is null) { - return target.Prototype; + return ForwardToTarget(target).Prototype; } handlerProto = clrResult; @@ -888,7 +889,7 @@ public override bool Extensible var trap = GetTrap(handler, TrapGetProtoTypeOf); if (trap is null) { - return target.Prototype; + return ForwardToTarget(target).Prototype; } handlerProto = CallTrap(handler, trap, target); @@ -929,7 +930,7 @@ internal override bool SetPrototypeOf(JsValue value) var clrResult = InvokeClrSetPrototypeOf(clrHandler, target, value); if (clrResult is null) { - return target.SetPrototypeOf(value); + return ForwardToTarget(target).SetPrototypeOf(value); } success = clrResult.Value; @@ -940,7 +941,7 @@ internal override bool SetPrototypeOf(JsValue value) var trap = GetTrap(handler, TrapSetProtoTypeOf); if (trap is null) { - return target.SetPrototypeOf(value); + return ForwardToTarget(target).SetPrototypeOf(value); } success = TypeConverter.ToBoolean(CallTrap(handler, trap, target, value)); @@ -1046,6 +1047,9 @@ private JsValue CallTrap(ObjectInstance handler, ICallable trap, JsValue arg0, J return result; } + /// + /// The gate every proxy internal method passes through first: the proxy has not been revoked. + /// private void AssertNotRevoked(JsValue key) { if (_target is null) @@ -1054,6 +1058,41 @@ private void AssertNotRevoked(JsValue key) } } + /// + /// The target an internal method is about to forward its whole algorithm to, having found no + /// trap for what it was asked (or a CLR that declined it), with a native-stack + /// probe on the way past. It returns the target rather than merely probing so that every such site reads + /// as the one thing it is — return ForwardToTarget(target).Get(property, receiver) — and so that a + /// forward added later without a probe stands out beside its siblings. + /// + /// A trapless proxy forwards to its target, so new Proxy(new Proxy(new Proxy(…))) is a native + /// recursion as deep as script cares to build it, and it used to end the process with a stack overflow no + /// catch could see (sebastienros/jint#4076). Unlike an ordinary prototype chain this one cannot be + /// flattened into a loop — each hop runs trap lookup and the result invariants on the way back out — so it + /// is probed. Every internal method here recurses on its trapless path, the three that forward through a + /// property included: Prototype is GetPrototypeOf() and Extensible is + /// IsExtensible(), both virtual and both overridden here. + /// + /// + /// The probe is here and not at the entry of each operation, deliberately. A trapped proxy + /// reaches its trap through CallTrap → ICallable.Call, and the callee probes for itself: + /// 's four entry points carry the backstop, the interop and + /// forwarding leaves carry theirs, and a callable proxy carries [[Call]]'s. So a trapped chain + /// already raised a catchable RangeError before any of this, and an entry probe would only add a + /// redundant third probe to the shape real code actually ships — a reactivity library puts a get + /// trap on every object it proxies. That is the redundancy main's Jint/Constraints/AGENTS.md records as a + /// measured NO-GO for the dispatcher probes, and ProbeStackHeadroom is not free. [[Call]] + /// and [[Construct]] keep their own entry probes, unchanged, because they are the dispatcher for + /// everything reached through them. + /// + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private ObjectInstance ForwardToTarget(ObjectInstance target) + { + _engine._stackGuard.EnsureNativeStackHeadroom(); + return target; + } + internal bool IsRevoked => _target is null; /// diff --git a/Jint/Native/Object/ObjectInstance.cs b/Jint/Native/Object/ObjectInstance.cs index 6fd70ea062..18093578df 100644 --- a/Jint/Native/Object/ObjectInstance.cs +++ b/Jint/Native/Object/ObjectInstance.cs @@ -944,7 +944,7 @@ public override JsValue Get(JsValue property, JsValue receiver) return jo.GetSlotForRead(slot); } - return Prototype?.Get(property, receiver) ?? Undefined; + return GetFromPrototypeChain(Prototype, property, receiver); } if (_properties?.TryGetValue(property.ToString(), out var ownDesc) == true) @@ -952,7 +952,7 @@ public override JsValue Get(JsValue property, JsValue receiver) return UnwrapJsValue(ownDesc, receiver); } - return Prototype?.Get(property, receiver) ?? Undefined; + return GetFromPrototypeChain(Prototype, property, receiver); } // slow path — a host that overrides TryGetOwnPropertyValue answers the own-property question from its @@ -980,7 +980,7 @@ public override JsValue Get(JsValue property, JsValue receiver) AssertOwnValueAgreesWithDescriptor(this, property, receiver, answered: false, Undefined); } - return Prototype?.Get(property, receiver) ?? Undefined; + return GetFromPrototypeChain(Prototype, property, receiver); } var desc = GetOwnProperty(property); @@ -989,7 +989,52 @@ public override JsValue Get(JsValue property, JsValue receiver) return UnwrapJsValue(desc, receiver); } - return Prototype?.Get(property, receiver) ?? Undefined; + return GetFromPrototypeChain(Prototype, property, receiver); + } + + /// + /// The continuation of an ordinary [[Get]] once the receiver's own property has been ruled out: + /// the prototype chain, walked in a loop instead of one native frame per link. + /// + /// It used to be Prototype?.Get(property, receiver), which is the same algorithm written as a + /// recursion — and a chain is built by script, so its depth is an input. Twenty thousand links of + /// x = { __proto__: x } ended the process with a native stack overflow no catch could see + /// (sebastienros/jint#4076), where the same depth through JSON.stringify or flat(Infinity) + /// already raised a catchable RangeError. The loop removes the frames rather than bounding them, so + /// an ordinary chain of any depth now resolves instead of failing politely. + /// + /// + /// The walk is only entitled to run the ordinary algorithm, so it hands the rest of the read to + /// the first link carrying one of its own — an exotic [[Get]], or a host's own-value hook — exactly + /// as JintMemberExpression's member lane does, and for the same reason: probing such a link with + /// would run the wrong algorithm for the first and materialize the descriptor + /// the second exists to avoid. That hand-over is a native frame again, so it is the one place here that + /// probes the stack — off the ordinary path, and paid only by a chain that really does forward. The loop + /// itself deliberately does not probe: this is the hottest generic read lane in the engine, and a second + /// native-stack probe on a read has already been measured as unresolvable but not free (the double-probe + /// NO-GO recorded in main's Jint/Constraints/AGENTS.md). + /// + /// + private JsValue GetFromPrototypeChain(ObjectInstance? link, JsValue property, JsValue receiver) + { + while (link is not null) + { + if ((link._type & (InternalTypes.ExoticGet | InternalTypes.OwnValueHook)) != InternalTypes.Empty) + { + _engine._stackGuard.EnsureNativeStackHeadroom(); + return link.Get(property, receiver); + } + + var descriptor = link.GetOwnProperty(property); + if (!ReferenceEquals(descriptor, PropertyDescriptor.Undefined)) + { + return UnwrapJsValue(descriptor, receiver); + } + + link = link.Prototype; + } + + return Undefined; } /// @@ -1686,33 +1731,48 @@ public bool TryGetValue(JsValue property, out JsValue value) // most visibly through IsArrayLike, which probes 'length' before every array // destructuring, so `const [a, b] = someInstanceOfSuchAClass;` threw instead of // falling through to the iterator protocol. + // + // The chain is walked in a loop rather than by recursing into the prototype's own + // TryGetValue: this overload is private, so the recursion never dispatched to an + // override and the loop is the same walk with no frame per link. See + // GetFromPrototypeChain for why that matters (sebastienros/jint#4076). private bool TryGetValue(JsValue property, JsValue receiver, out JsValue value) { value = Undefined; - var desc = GetOwnProperty(property); - if (desc != PropertyDescriptor.Undefined) + var link = this; + while (true) { - var descValue = desc.Value; - if (desc.WritableSet && descValue is not null) + var desc = link.GetOwnProperty(property); + if (desc != PropertyDescriptor.Undefined) { - value = descValue; + var descValue = desc.Value; + if (desc.WritableSet && descValue is not null) + { + value = descValue; + return true; + } + + var getter = desc.Get ?? Undefined; + if (getter.IsUndefined()) + { + value = Undefined; + return false; + } + + // if getter is not undefined it must be ICallable + var callable = (ICallable) getter; + value = callable.Call(receiver, Arguments.Empty); return true; } - var getter = desc.Get ?? Undefined; - if (getter.IsUndefined()) + var parent = link.Prototype; + if (parent is null) { - value = Undefined; return false; } - // if getter is not undefined it must be ICallable - var callable = (ICallable) getter; - value = callable.Call(receiver, Arguments.Empty); - return true; + link = parent; } - - return Prototype?.TryGetValue(property, receiver, out value) == true; } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -1775,7 +1835,7 @@ public override bool Set(JsValue property, JsValue value, JsValue receiver) var shapeParent = GetPrototypeOf(); if (shapeParent is not null) { - return shapeParent.Set(property, value, receiver); + return SetOnPrototypeChain(shapeParent, property, value, receiver); } } else if (_properties?.TryGetValue(key, out var ownDesc) == true) @@ -1791,7 +1851,7 @@ public override bool Set(JsValue property, JsValue value, JsValue receiver) var parent = GetPrototypeOf(); if (parent is not null) { - return parent.Set(property, value, receiver); + return SetOnPrototypeChain(parent, property, value, receiver); } } } @@ -1799,17 +1859,64 @@ public override bool Set(JsValue property, JsValue value, JsValue receiver) return SetUnlikely(property, value, receiver); } + /// + /// The continuation of OrdinarySetWithOwnDescriptor once the receiver has no own property of that + /// name: the prototype chain, walked in a loop instead of one native frame per link. The read side's + /// says why (sebastienros/jint#4076); this is the write side of the + /// same defect, reached by x.missing = 1 on a chain script built. + /// + /// The classification differs from the read side's because the question does. [[Set]] has no + /// derived exotic/ordinary flag, so the walk asks for a positive claim instead and walks only a + /// link carrying — which, as that flag's own documentation says, + /// is exactly an object that does not override the property internal methods. Everything else runs its + /// own [[Set]], which is a hand-over and not a skip, and the hand-over probes: a chain of arrays, + /// wrappers or proxies still recurses, and the probe is what makes that a catchable RangeError. + /// Conservative in the safe direction — a link the walk declines is merely resolved the way it always was. + /// + /// + private bool SetOnPrototypeChain(ObjectInstance link, JsValue property, JsValue value, JsValue receiver) + { + while (true) + { + if ((link._type & InternalTypes.PlainObject) == InternalTypes.Empty) + { + _engine._stackGuard.EnsureNativeStackHeadroom(); + return link.Set(property, value, receiver); + } + + var ownDesc = link.GetOwnProperty(property); + if (!ReferenceEquals(ownDesc, PropertyDescriptor.Undefined)) + { + return link.OrdinarySetWithOwnDescriptor(property, value, receiver, ownDesc); + } + + var parent = link.GetPrototypeOf(); + if (parent is null) + { + // Step 2.a.ii of OrdinarySetWithOwnDescriptor: nothing on the chain owns the name, so the + // write is a fresh data property on the receiver. + return link.OrdinarySetWithOwnDescriptor(property, value, receiver, _marker); + } + + link = parent; + } + } + [MethodImpl(MethodImplOptions.NoInlining)] private bool SetUnlikely(JsValue property, JsValue value, JsValue receiver) - { - var ownDesc = GetOwnProperty(property); + => OrdinarySetWithOwnDescriptor(property, value, receiver, GetOwnProperty(property)); + /// + /// https://tc39.es/ecma262/#sec-ordinarysetwithowndescriptor + /// + private bool OrdinarySetWithOwnDescriptor(JsValue property, JsValue value, JsValue receiver, PropertyDescriptor ownDesc) + { if (ownDesc == PropertyDescriptor.Undefined) { var parent = GetPrototypeOf(); if (parent is not null) { - return parent.Set(property, value, receiver); + return SetOnPrototypeChain(parent, property, value, receiver); } ownDesc = _marker; @@ -1921,6 +2028,14 @@ internal bool CanPut(JsValue property) /// /// https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-hasproperty-p /// + /// + /// The chain is walked in a loop rather than by recursing into each prototype's own + /// HasProperty, on the same terms as : only a link carrying + /// — an object that does not override the property internal + /// methods — may be walked, and anything else is handed the rest of the question after a native-stack + /// probe. Reached by 'x' in obj and, less obviously, by every identifier resolved inside a + /// with statement, whose object environment asks this (sebastienros/jint#4076). + /// public virtual bool HasProperty(JsValue property) { var key = TypeConverter.ToPropertyKey(property); @@ -1929,10 +2044,21 @@ public virtual bool HasProperty(JsValue property) return true; } - var parent = GetPrototypeOf(); - if (parent is not null) + var link = GetPrototypeOf(); + while (link is not null) { - return parent.HasProperty(key); + if ((link._type & InternalTypes.PlainObject) == InternalTypes.Empty) + { + _engine._stackGuard.EnsureNativeStackHeadroom(); + return link.HasProperty(key); + } + + if (link.ProbeOwnPropertyChecked(key) != OwnPropertyProbe.Missing) + { + return true; + } + + link = link.GetPrototypeOf(); } return false; diff --git a/Jint/Native/Object/SharedShapeObject.cs b/Jint/Native/Object/SharedShapeObject.cs index a470fa40e9..0d4ca9cda5 100644 --- a/Jint/Native/Object/SharedShapeObject.cs +++ b/Jint/Native/Object/SharedShapeObject.cs @@ -51,8 +51,19 @@ internal sealed class SharedShapeObject : ObjectInstance, IBuiltinShaped /// internal object? HostState; + // PlainObject, because this type overrides no property internal method and so is a link another + // object's [[Set]] / [[HasProperty]] walk may resolve itself rather than hand the rest of the chain + // to. Without it a host's shaped prototype declined the walk AND paid its flag test at every level — + // the old recursion plus the new overhead (sebastienros/jint#4076). It is the flag's storage claim + // too, and that costs nothing here: the three lanes reading it for storage + // (ObjectInstance.Get / Set / CreateDataProperty) spell the test `== PlainObject` against + // `PlainObject | BuiltinShapeMode` and so exclude this object for as long as its shape is installed, + // while after a deopt DeoptBuiltinShape has moved every slot into _properties and the object really is + // the ordinary dictionary they assume. Object.prototype is the same pair (a Prototype, hence + // PlainObject, declared [JsObject(UseShape = true)], hence BuiltinShapeMode) and has been walked that + // way all along. internal SharedShapeObject(Engine engine, JsObjectShape shape, ObjectInstance? prototype) - : base(engine, ObjectClass.Object, InternalTypes.Object) + : base(engine, ObjectClass.Object, InternalTypes.Object | InternalTypes.PlainObject) { _shape = shape; _realm = engine.Realm; diff --git a/Jint/Runtime/InternalTypes.cs b/Jint/Runtime/InternalTypes.cs index d6aa620240..557a7d559b 100644 --- a/Jint/Runtime/InternalTypes.cs +++ b/Jint/Runtime/InternalTypes.cs @@ -27,7 +27,20 @@ internal enum InternalTypes RequiresCloning = 2048, Module = 4096, - // the object doesn't override important GetOwnProperty etc which change behavior + // the object doesn't override important GetOwnProperty etc which change behavior. Set only through the + // internal constructor, by JsObject, JsDate, GlobalObject, NumberPrototype, SharedShapeObject (what a + // host's JsObjectShape instantiates) and Prototype (the base of every built-in prototype) — none of + // which overrides a property internal method. It is orthogonal to BuiltinShapeMode rather than + // exclusive with it: the three lanes that read it as a *storage* claim spell the test + // `(_type & (PlainObject | BuiltinShapeMode)) == PlainObject` precisely so that a shaped object is + // excluded while its shape is installed and served again once a deopt has made it a dictionary. + // + // ObjectInstance's [[Set]] and [[HasProperty]] chain walks read it as exactly that claim: a LINK + // carrying it is resolved by the walk itself, anything else is handed the rest of the algorithm. So a + // type that takes this flag must not override Get, Set or HasProperty — declaring it while overriding + // one would make another object's walk skip that override. The direction is safe: a type that does not + // take the flag is merely handed over to, which is what always happened. Pinned by + // Jint.Tests/Runtime/PrototypeChainWalkTests over every object a built engine can reach. PlainObject = 8192, // our native array Array = 16384, From 9560235c319ccb0a01a0b7fad009ad7754541b81 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Thu, 24 Sep 2026 23:46:16 +0300 Subject: [PATCH 2/2] Backport #4125 to 4.x: Stop a chain of adjacent host object wrappers from killing the process on a member miss Backport of PR #4125 (head 02bed3ef1, 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 (#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., 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 #4007 probe is. - Jint/Runtime/Interop/AGENTS.md does not exist on 4.x; that edit is dropped. Evidence (Windows x64, Release): - With #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) Claude-Session: https://claude.ai/code/session_01PanPJbBD7pQC9fRiTpHxvs --- .../HostNativeRecursionGuardTests.cs | 118 ++++++++++++++++++ Jint/Runtime/Interop/ObjectWrapper.cs | 22 +++- 2 files changed, 139 insertions(+), 1 deletion(-) diff --git a/Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs b/Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs index 5f276272a8..6813f6470b 100644 --- a/Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs +++ b/Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs @@ -9,6 +9,124 @@ namespace Jint.Tests.PublicInterface; public class HostNativeRecursionGuardTests { + /// + /// A prototype chain whose links are adjacent host objects — one ObjectWrapper after + /// another, with no ordinary object between any two of them. It is the one chain shape the iterative + /// walk cannot flatten: a wrapper answers a member miss by forwarding the read to its prototype and + /// then inspecting the answer for Options.Interop.ThrowOnUnresolvedMember, so the forward is not + /// a tail call and the link cannot join the loop. A hop to an ordinary link re-enters that loop + /// and costs nothing further; a hop to another wrapper is a native frame, and a wrapper does not + /// override SetPrototypeOf, so script builds the chain itself and its depth is an input. Twenty + /// thousand links ended the host process with a native stack overflow no catch could see + /// (sebastienros/jint#4087, the shape left over from #4076). + /// + /// Both settings of ThrowOnUnresolvedMember are rows because the post-check is the whole reason + /// the forward cannot be flattened, so the fix has to hold with it on and off alike. What either may + /// answer is the read's own result or a catchable RangeError; what neither may do is end the + /// process. is what says the probe did + /// not buy that by changing the answer. + /// + /// + public static TheoryData AdjacentWrapperChains => new() + { + { "lenient", false }, + { "strict", true }, + }; + + [Theory] + [MemberData(nameof(AdjacentWrapperChains))] + public void AChainOfAdjacentHostWrappersRaisesACatchableErrorAndTheEngineRecovers(string route, bool throwOnUnresolvedMember) + { + _ = route; + DedicatedThread.Run(() => + { + using var engine = new Engine(options => + { + options.Constraints.StackOverflowGuard = true; + options.AllowClr(); + options.Interop.ThrowOnUnresolvedMember = throwOnUnresolvedMember; + }); + + engine.SetValue("w", WrapperChain(engine, 20000)); + + var outcome = engine.Evaluate(""" + for (var i = 1; i < w.length; i++) { Object.setPrototypeOf(w[i - 1], w[i]); } + var outcome; + try { outcome = String(w[0].missing); } + catch (error) { outcome = error.name + ':' + error.message; } + outcome; + """).AsString(); + + // Resolving the miss outright would be just as correct — the probe bounds the chain, it does + // not shorten it — but no thread this test may run on holds twenty thousand of these frames, + // so what is actually seen is the error the probe raises in place of the dead process. + outcome.Should().BeOneOf("undefined", "RangeError:Maximum call stack size exceeded"); + + engine.Evaluate("6 * 7").AsNumber().Should().Be(42); + }, maxStackSize: SmallStack); + } + + /// + /// The same chain three links long, which every stack holds: a probe on the forward must not change + /// what a wrapper answers. Reading tail walks all three links and resolves on the last, which is + /// what says the chain is a chain; reading a name no link carries is undefined when the option is + /// off and the host-facing the option exists for when it is on. + /// + [Fact] + public void AShortChainOfAdjacentWrappersAnswersExactlyAsItDid() + { + const string Link = "for (var i = 1; i < w.length; i++) { Object.setPrototypeOf(w[i - 1], w[i]); }"; + + using var lenient = new Engine(options => + { + options.Constraints.StackOverflowGuard = true; + options.AllowClr(); + }); + lenient.SetValue("w", WrapperChain(lenient, 3)); + lenient.Evaluate(Link); + lenient.Evaluate("w[0].Tail").AsString().Should().Be("reached"); + lenient.Evaluate("String(w[0].missing)").AsString().Should().Be("undefined"); + + using var strict = new Engine(options => + { + options.Constraints.StackOverflowGuard = true; + options.AllowClr(); + options.Interop.ThrowOnUnresolvedMember = true; + }); + strict.SetValue("w", WrapperChain(strict, 3)); + strict.Evaluate(Link); + strict.Evaluate("w[0].Tail").AsString().Should().Be("reached"); + strict.Invoking(e => e.Evaluate("w[0].missing")).Should().Throw(); + } + + /// + /// host objects, each wrapped once and held by a JavaScript array so the + /// wrappers keep their identity across reads — w[i] has to be the same object every time or the + /// script would be setting the prototype of a wrapper it then throws away. The last link is a different + /// type, so a read that resolves on it can only have walked the whole chain. + /// + private static JsValue WrapperChain(Engine engine, int length) + { + var links = new JsValue[length]; + for (var i = 0; i < length - 1; i++) + { + links[i] = JsValue.FromObject(engine, new HostChainLink()); + } + + links[length - 1] = JsValue.FromObject(engine, new HostChainTail()); + return new JsArray(engine, links); + } + + private sealed class HostChainLink + { + public string Kind => "link"; + } + + private sealed class HostChainTail + { + public string Tail => "reached"; + } + public static TheoryData NativeTraversals => new() { { "flat dense", "var a = [1]; for (var i = 0; i < 10000; i++) a = [a]; a.flat(Infinity);" }, diff --git a/Jint/Runtime/Interop/ObjectWrapper.cs b/Jint/Runtime/Interop/ObjectWrapper.cs index e9303815b1..582eac5396 100644 --- a/Jint/Runtime/Interop/ObjectWrapper.cs +++ b/Jint/Runtime/Interop/ObjectWrapper.cs @@ -957,7 +957,27 @@ public override JsValue Get(JsValue property, JsValue receiver) return UnwrapJsValue(desc, receiver); } - var protoResult = Prototype?.Get(property, receiver) ?? Undefined; + var protoResult = Undefined; + if (Prototype is { } prototype) + { + // A hand-over, and the one forward in this type that cannot be flattened: the result is + // post-processed for ThrowOnUnresolvedMember below, so this is not a tail call and the link + // cannot join the loop ObjectInstance's own [[Get]] walks the chain with. A hop to an + // *ordinary* link re-enters that loop and costs nothing further, but a hop to another wrapper + // is a native frame with no probe between the two, and a chain of wrappers is buildable from + // script (ObjectWrapper does not override SetPrototypeOf), so its depth is an input. The probe + // turns twenty thousand links from a native stack overflow no catch sees into a catchable + // RangeError, on the same terms as every other hand-over (sebastienros/jint#4087, after #4076). + // + // It is the only site of that shape in this directory: Set and HasProperty end in base., + // which *is* that loop; Delete and GetOwnProperty ask own-property questions and walk nothing; + // TypeReference forwards no operation to its prototype and NamespaceReference.Get resolves a + // path rather than a chain. What holds the probe in place is the depth cases in + // Jint.Tests.PublicInterface/HostNativeRecursionGuardTests, which end the host without it. + _engine._stackGuard.EnsureNativeStackHeadroom(); + protoResult = prototype.Get(property, receiver); + } + if (protoResult.IsUndefined() && property is JsString && !_typeDescriptor.IsDictionary