Summary
While adding the implicit_this/new_target catch_savepoints! members in
#10564 (exception-safety for the displaced-this/new.target save/restore
sites), I tried to build a compiled .ts gap-test reproduction for the most
tractable of the four named sites: dispatch_handle's prototype-walk
accessor dispatch in
crates/perry-runtime/src/object/native_call_method/handle_methods.rs
(around the IMPLICIT_THIS.with(|c| c.replace(receiver_f64.to_bits())) /
accessor_receiver_override_begin / js_object_get_field_by_name /
accessor_receiver_override_end sequence, roughly lines 1148-1173 on
PR #10564's branch).
What I did
class Base {}
Object.defineProperty(Base.prototype, "boom", {
get(): any { throw new Error("boom getter threw"); },
});
class Inner extends Base { tag = "INNER"; }
const inner: any = new Inner(); // laundered through a fn returning `any`
// so Perry can't statically narrow it back
class Outer {
tag: string;
constructor(tag: string) { this.tag = tag; }
run(this: any): string {
try { inner.boom(); } catch (e) {}
return this.tag;
}
}
inner.boom() is method-call syntax on a value whose static type doesn't
declare boom, and boom is a lazily-installed accessor on the declared
class prototype (not an individual/setPrototypeOf'd one, so it should miss
the already-fixed #9247/#10564 ImplicitThisScope early path and fall
through to the class-id-ascending vtable-miss walk).
I confirmed via --trace llvm that the call compiles to
js_native_call_method_by_id -> js_native_call_method_str_key ->
js_native_call_method, which does call handle_methods::dispatch_handle
later in its body. I then placed a temporary eprintln! directly at the
target statement inside the accessor-walk branch and rebuilt.
The eprintln! never fired for this test case, on either the baseline
build or the fixed build -- both print bad=0 / the correct value, i.e. no
observable defect either way. Something else resolves inner.boom() before
reaching this code, or this specific JS shape doesn't reach it at all.
What this means
The review finding that produced #10564's exception-safety follow-up
describes this site (and three others -- the Temporal/Intl subclass super()
bridges in fetch_globals.rs, and the stdlib listener/getter dispatchers) as
reachable with a bare save/call/restore defect. #10564 fixes the shape of
the defect at the exception.rs savepoint-mechanism level (proven with a
unit test that reproduces the bare-save/call/restore pattern directly using
the same primitives), which covers these sites regardless of how they're
reached -- so the fix is not contingent on this issue. But the specific claim
"site X is reachable this way" was never independently confirmed for any of
the four, and for this one site, one honest attempt to confirm it came back
negative.
Ask
Someone with more time on this code path should either:
- find the actual JS shape (if any) that reaches
dispatch_handle's accessor-walk branch through js_native_call_method,
and confirm whether the described defect is reachable there, or
- determine the branch is effectively dead for realistic receivers (e.g.
always intercepted earlier by IC/vtable resolution) and consider whether
it's worth simplifying/removing.
The other three named sites (the two fetch_globals.rs/intl/subclass.rs
Temporal/Intl super() bridges, and the stdlib listener/getter dispatchers)
were not probed at all and carry the same open question.
Not a regression, not urgent -- filed so the reachability question doesn't
get silently assumed answered by #10564.
Summary
While adding the
implicit_this/new_targetcatch_savepoints!members in#10564 (exception-safety for the displaced-
this/new.targetsave/restoresites), I tried to build a compiled
.tsgap-test reproduction for the mosttractable of the four named sites:
dispatch_handle's prototype-walkaccessor dispatch in
crates/perry-runtime/src/object/native_call_method/handle_methods.rs(around the
IMPLICIT_THIS.with(|c| c.replace(receiver_f64.to_bits()))/accessor_receiver_override_begin/js_object_get_field_by_name/accessor_receiver_override_endsequence, roughly lines 1148-1173 onPR #10564's branch).
What I did
inner.boom()is method-call syntax on a value whose static type doesn'tdeclare
boom, andboomis a lazily-installed accessor on the declaredclass prototype (not an individual/
setPrototypeOf'd one, so it should missthe already-fixed #9247/#10564
ImplicitThisScopeearly path and fallthrough to the class-id-ascending vtable-miss walk).
I confirmed via
--trace llvmthat the call compiles tojs_native_call_method_by_id->js_native_call_method_str_key->js_native_call_method, which does callhandle_methods::dispatch_handlelater in its body. I then placed a temporary
eprintln!directly at thetarget statement inside the accessor-walk branch and rebuilt.
The
eprintln!never fired for this test case, on either the baselinebuild or the fixed build -- both print
bad=0/ the correct value, i.e. noobservable defect either way. Something else resolves
inner.boom()beforereaching this code, or this specific JS shape doesn't reach it at all.
What this means
The review finding that produced #10564's exception-safety follow-up
describes this site (and three others -- the Temporal/Intl subclass
super()bridges in
fetch_globals.rs, and the stdlib listener/getter dispatchers) asreachable with a bare save/call/restore defect. #10564 fixes the shape of
the defect at the
exception.rssavepoint-mechanism level (proven with aunit test that reproduces the bare-save/call/restore pattern directly using
the same primitives), which covers these sites regardless of how they're
reached -- so the fix is not contingent on this issue. But the specific claim
"site X is reachable this way" was never independently confirmed for any of
the four, and for this one site, one honest attempt to confirm it came back
negative.
Ask
Someone with more time on this code path should either:
dispatch_handle's accessor-walk branch throughjs_native_call_method,and confirm whether the described defect is reachable there, or
always intercepted earlier by IC/vtable resolution) and consider whether
it's worth simplifying/removing.
The other three named sites (the two
fetch_globals.rs/intl/subclass.rsTemporal/Intl
super()bridges, and the stdlib listener/getter dispatchers)were not probed at all and carry the same open question.
Not a regression, not urgent -- filed so the reachability question doesn't
get silently assumed answered by #10564.