You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
js_class_method_bind still binds a movable heap string's interior in four places #7747 missed (timer handles, TextDecoder/TextEncoder) #8133
#7747 fixed two callers that bound a movable GC heap string's interior as a bound-method name. Four more sites in the same neighbourhood still do it.
The contract
js_class_method_bind(instance, method_name_ptr, method_name_len) stores the name pointer in the bound closure (capture 1) and dispatch_bound_method (closure/dispatch/bound.rs:12) re-reads it at call time. Its doc requires the pointer to stay valid for the closure's lifetime, which codegen satisfies with rodata.
#7747's commit message states the failure mode exactly:
get_field_by_name_tail passed key + size_of::<StringHeader>() — the interior of a movable GC heap string that is unreachable once the read returns — so a copying minor could relocate or reclaim the bytes the closure names. […] Whether the stale bytes still spell the method is an allocator property, not a program property, which is why this passed locally and took a SIGSEGV on conformance-smoke shards 7 and 8.
It fixed the two Buffer arms (object/field_get_set/buffer_own_prop.rs and the computed-key arm in object/polymorphic_index.rs), both of which now bind a 'static literal via buffer_dispatch::buffer_method_name_static.
Still unfixed, same file, same pattern
Each of these computes let key_ptr = (key as *const u8).add(size_of::<crate::StringHeader>()) and hands that pointer to js_class_method_bind:
crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs:48 — timer-handle method key on a small-handle receiver (is_timer_handle_method_key + timer::is_known_timer_id).
crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs:121 — the same arm for an already-stripped handle-band receiver.
crates/perry-runtime/src/text.rs:649 and :670 (text_handle_property) — TextDecoder.prototype.decode and TextEncoder.prototype.encode/encodeInto read as VALUES. That function's own docstring says value reads are the reason it exists (K.decode.bind(K) — "the shape a minified SDK's cached decodeText helper takes"), so this is the intended hot path, not an edge. Reached from get_field_by_name_tail.rs:53 and :125, get_field_by_name.rs:875, and ic_miss.rs:540, all of which pass the same heap-string interior.
Not affected
crates/perry-runtime/src/symbol/get.rs:262 binds let mname = b"values" — a 'static byte-string literal. Fine as written.
Suggested shape
The same one #7747 used: a macro-generated *_method_name_static(name) -> Option<&'static str> for each surface (timer handles, TextDecoder, TextEncoder), so the bound closure names a literal rather than a borrow of its argument.
The inequality against the key string could pass with the bug present […] Comparing the BYTES only fails on a host where the freed memory has already been reused, which is the lucky-allocator problem these tests exist to avoid. Identity with the literal cannot be lucky.
So the test should assert pointer identity with the static literal (see gc/tests/buffer_bound_method_name.rs), not that the bytes still read correctly.
Provenance
Found while bisecting #8117's two pass -> crash gap regressions. Those two bisect to #7314 and are a different defect — these sites are not on their path (their receivers are small handles, not Buffers). Filing separately so the finding is not lost inside that thread.
Activity
docs(changelog): add the #8177 fragment for the #8133 bound-method-na…
fix(runtime): bind a 'static method name on the timer and TextDecoder…
feat(runtime): TextEncoder / TextDecoder instances are ordinary objec…