fix(runtime): a VTABLE_IC hit also requires the method name bytes; mysql2 no longer reads 'hi' as 617 (#11341)

[proggeramlug]

Fixes #11341

What was wrong

Under compiled mysql2 3.24.4, SELECT 1 + 1 AS two, 'hi' AS s intermittently returned s = 617 instead of 'hi'. 617 is what parseLengthCodedInt makes of the bytes hi: (0x68-48)*10 + (0x69-48). So the call packet.readLengthCodedString(...) was running parseLengthCodedInt.

mysql2 builds its row parser as source text and runs it through new Function, which Perry executes in its runtime interpreter (dyn_eval). The generated source is correct:

result["two"] = packet.parseLengthCodedInt(false);
result["s"] = packet.readLengthCodedString(fields[1].encoding);

The interpreter dispatches each member call by name. For each call it creates a new heap String for the name (i.sym.to_string() in dyn_eval/expr.rs) and frees it when the call returns. When the receiver is not on the class fast path, the call reaches handle_methods::dispatch_handle, which consults the per-thread VTABLE_IC. That cache chose and matched its entry on (class_id, name address) alone. Once parseLengthCodedInt's freed buffer was reused at the same address for readLengthCodedString, the lookup returned the cached parseLengthCodedInt entry.

To confirm the cause rather than infer it, I ran a debug build that reports an address-equal, bytes-different hit, together with a backtrace, against a live MySQL 8.0.46:

VTIC-MISMATCH cached="parseLengthCodedInt" asked="readLengthCodedString"
   0: …dispatch::vtable_ic_lookup::{closure#0}
   1: …native_call_method::handle_methods::dispatch_handle
   2: js_native_call_method
   3: perry_runtime::dyn_eval::expr::eval_expr
   …
  12: perry_method_node_modules_mysql2_lib_commands_query_js__Query__row

js_native_call_method_value also reaches this cache, with bytes from a GC string. The sibling OBJ_DISPATCH_IC (#7769) was written knowing that name addresses are not stable and has always compared the bytes. VTABLE_IC had not been updated to match.

The fix

A VTABLE_IC entry now stores the method name's bytes (up to 24; longer names are not cached, the same limit as OBJ_DISPATCH_IC). A hit requires the bytes to match as well as the class id and address. The slot is still chosen by address, so the lookup cost is one short compare on the hit. Both callers, handle_methods.rs and collection_methods.rs, pass method_name.as_bytes().

Evidence

Arm A is main at e6ad5f3 and arm B is this branch. Both were built the same way (cargo build --release -p perry -p perry-runtime-static -p perry-stdlib-static, codegen-units 16, PERRY_NO_AUTO_OPTIMIZE=1 programs). The databases were a live MySQL 8.0.46 and PostgreSQL 16.15, with mysql2 3.24.4. The oracle is Node 26.5.1.

program, 20 runs each	A (main)	B (this PR)
mysql2 program that reproduces it, mysql_native_password user, main-thread row	7/20 match Node (13 × 2 617)	20/20
same, caching_sha2_password user	20/20	20/20
probe covering connect, query, params, transactions, error code, Pool and a pool-connection transaction; both users	20/20	20/20

On B the debug build reports zero mismatches. How often it shows depends on how the allocator reuses the freed name buffer, so the bug is intermittent and appears only in some program shapes. The probe in the third row does not trigger it on either arm.

Unit test: vtable_ic_hit_requires_matching_name_bytes_not_only_the_address (in class_registry/dispatch.rs, next to the existing OBJ_DISPATCH_IC tests). It inserts an entry through one buffer, rewrites the buffer in place with a different name, and asserts the lookup misses. On main the same sequence would have hit.

No gap test. I tried to write a package-free gap test and could not make one fail on main: new Function bodies calling two methods with names of the same length, a receiver forced off the fast path, and computed keys with a GC between the two calls. Freed name buffers were not reused at the same address in those programs. So the regression coverage is the unit test above plus the live-server run. I would rather say that than add a gap test that passes on both arms.

Checks run

• RUST_TEST_THREADS=1 cargo test --release -p perry-runtime --lib: 4632 passed, and 2 event_pump::tests timing tests failed during that run (host load average about 48). Re-running event_pump::tests three times on its own gave 6/6 each time.
• RUSTFLAGS=-D warnings cargo check -p perry-runtime --all-targets: clean. cargo fmt --check: clean.
• SKIP_COMPILE_GATES=1 scripts/run_lint_gates.sh: 90 of 92 script gates pass. The two failures are Public benchmark evidence freshness (red on main) and cargo xwin (not installed on the host). The compile tier was not run. git status was clean afterwards.

Not run

• A full gap sweep.
• macOS.
• An instruction-count A/B. The hit path adds one length check and a compare of at most 24 bytes.

Related

• worker_threads: compiled pg / mysql2 misbehave on a worker agent (hang, setBuffer TypeError, wrong param value, cross-agent socket error) #11340: the same mysql2 program on a worker_threads worker. That is a separate cause (socket events racing between agents) with its own PR.
• mysql2 under a seeded GC schedule: TypeError: value is not a function in Command.execute (rooting-bug signature) #11400: under seeded GC schedules, mysql2 throws value is not a function in Command.execute.
• pg: parameterised query segfaults in js_buffer_write_len (Writer.addInt32PrefixedString $pshape clone) on current main #11430: a parameterised pg query segfaults in js_buffer_write_len on current main.

Summary by CodeRabbit

• Bug Fixes
  • Fixed incorrect method dispatch when different method names reused the same memory address. Cache hits now verify the method-name bytes, preventing calls from resolving to the wrong method.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 70b9206c-28ee-4144-9ab4-60cdd22f0880

📥 Commits

Reviewing files that changed from the base of the PR and between 0a740f2 and 01e30fb.

📒 Files selected for processing (4)

• changelog.d/11432-vtable-ic-name-bytes.md
• crates/perry-runtime/src/object/class_registry/dispatch.rs
• crates/perry-runtime/src/object/native_call_method/collection_methods.rs
• crates/perry-runtime/src/object/native_call_method/handle_methods.rs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The vtable inline cache now stores and compares method-name bytes in addition to the name pointer. Collection and handle method paths pass those bytes during cache lookup and insertion. A regression test checks that different names at the same buffer address produce a cache miss.

Changes

Vtable cache name validation

Layer / File(s)	Summary
Store and check method-name bytes crates/perry-runtime/src/object/class_registry/dispatch.rs, changelog.d/11432-vtable-ic-name-bytes.md	Cache entries store up to 24 method-name bytes. Lookup and insertion skip null pointers and names longer than 24 bytes. Lookup also checks the cached name length and bytes. A regression test checks that a different name at the same buffer address produces a miss. The changelog records the cache change and test.
Pass method-name bytes to the cache crates/perry-runtime/src/object/native_call_method/collection_methods.rs, crates/perry-runtime/src/object/native_call_method/handle_methods.rs	Collection and handle method paths pass method-name bytes to vtable cache lookup and insertion calls.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 01e30

No actionable merge-blocking risk is identified for the vtable cache fix; it is ready for normal merge checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 01e30

The change makes method matching stricter without evidently expanding who can invoke a method. No introduced security concern was identified in the reviewed call paths, though broader runtime coverage remains uncertain.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The affected outcome is which registered method a runtime call invokes. The inspected change does not establish a new service, tenant, or privilege boundary.

Trust Boundaries and Controls

• observed — Object-field matching precedes the raw-pointer cache lookup. In handle dispatch, a cache miss retains checks for prototype assignments and deletions before selecting a registered method.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the primary runtime fix and connects it to the mysql2 symptom. It is specific, concise enough, and directly related to the changes.
Description check	✅ Passed	The description provides detailed problem context, root cause, implementation changes, linked issue, regression testing, live-server evidence, and known test limitations. It does not use the template …
Linked Issues check	✅ Passed	The change directly addresses #11341. VTABLE_IC now stores and compares method-name bytes in addition to the method-name address and class ID. Both raw-pointer and handle dispatchers pass the method…
Out of Scope Changes check	✅ Passed	The changed runtime cache, dispatcher call sites, regression test, and changelog all support the #11341 fix. The pull request contains no demonstrated unrelated behavior or scope.

Full details: Docstring Coverage

Explanation

Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}