fix: Buffer own-property reads via dynamic key (#6412) and shadowed folded read intrinsic (#6405)

[proggeramlug]

Summary

Two related Buffer own-property correctness bugs where a folded byte-access path failed to honor that a Node Buffer is an ordinary Uint8Array — user code can store own properties on one, and an own key shadows a same-named prototype method. Both bugs' write/dynamic halves already worked; these fix the folded read halves.

#6412 — a dynamic (any-typed) string key silently read a byte instead of the own property

const buf = Buffer.alloc(4);
const keys: any[] = ["dyn"];
const k: any = keys[0];        // a string at runtime, `any` statically
(buf as any)[k] = "D";
console.log((buf as any)[k]);  // node: "D"   perry (before): undefined

The write stored the own property fine (js_object_set_index_polymorphic → buffer_set_own_prop), but the read folds to Uint8ArrayGet → js_typed_array_index_get_dynamic → (for a BufferHeader) js_dyn_index_get, whose buffer arm returned undefined for any non-numeric key — it never consulted the own-prop table.

Fix (crates/perry-runtime/src/value/dyn_index.rs): the buffer arm now routes a string key to js_object_get_field_by_name_f64 — the same own-prop/method-aware getter (buffer_own_prop_or_method) the dotted buf.k and static-string buf["k"] paths already use. The numeric byte fast path is unchanged. This brings the arm in line with its already-correct sibling js_object_get_index_polymorphic.

#6405 — an own property did not shadow a prototype method when the call folded to the inline byte intrinsic

const b = Buffer.alloc(8);
b[0] = 0xab;
(b as any).readUInt8 = function () { return "shadowed"; };
b.readUInt8(0);        // node: "shadowed"   perry (before): 171
b["readUInt8"](0);     // node: "shadowed"   perry (before): 171

A statically-provable buf.readUInt8(0) folds to try_emit_buffer_read_intrinsic (an inline LLVM load) that never consults the property table. Every dynamic dispatch path already honors the shadow (dispatch_buffer_method checks own props first); only the static fold did not.

Fix (codegen): a whole-module pre-codegen scan (module_shadows_buffer_read_method) records whether the module assigns any Buffer numeric read-method name as a property. When it does, try_emit_buffer_read_intrinsic bails so the call routes through the own-prop-aware js_native_call_method → dispatch_buffer_method. This is the issue's "whole-program compile-time flag" option: zero runtime cost for the overwhelmingly common program that never shadows a Buffer method — the inline fast path is untouched there, so the native-region-proof compiler-output gate stays green.

A per-module scan is sufficient: the intrinsic only fires on a buffer that lower_buffer_access_proof proves non-escaping — a closure-captured or cross-module-shared/exported buffer is stamped hazardous and never folds (verified — an exported buffer shadowed in another module already dispatches correctly). Only literal property names are matched; a dynamic buf[computedName] = fn is out of scope (that shape already defeats the static buffer proof in practice).

Verification

• Both issue repros now match node --experimental-strip-types.
• Cross-module shadow still dispatches correctly.
• native-region-proof compiler-output gate passes — the buffer fast path (incl. width_aware_buffer_kernels's folded readInt16BE/readUInt32BE/… loops) is unchanged for non-shadowing programs.
• New gap tests: test_gap_buffer_dyn_key_own_prop_6412.ts, test_gap_buffer_own_prop_shadow_intrinsic_6405.ts.
• New codegen unit tests for the shadow scan.

No version bump or changelog entry (left for the maintainer to fold in at merge).

Fixes #6412. Fixes #6405.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Fixed Buffer/Uint8Array dynamic access for string keys so canonical numeric index strings behave like byte indices, while non-canonical keys resolve as own properties.
  • Prevented incorrect inline intrinsic folding when a Buffer instance defines own numeric read-method properties, ensuring shadowed prototype methods use the correct runtime dispatch.
• Tests
  • Added coverage for shadowed Buffer read methods across dot, literal bracket, and dynamic-key access.
  • Added coverage for dynamic string-key canonical index behavior and undefined on out-of-range reads.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 84f2a9ed-c563-43d5-8ebf-9d8782dc3b01

📥 Commits

Reviewing files that changed from the base of the PR and between 69c831b and f1cf56b.

📒 Files selected for processing (3)

• crates/perry-codegen/src/lower_call/buffer_intrinsic.rs
• crates/perry-runtime/src/value/dyn_index.rs
• test-files/test_gap_buffer_dyn_key_own_prop_6412.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• test-files/test_gap_buffer_dyn_key_own_prop_6412.ts
• crates/perry-runtime/src/value/dyn_index.rs
• crates/perry-codegen/src/lower_call/buffer_intrinsic.rs

---

📝 Walkthrough

Walkthrough

Changes

Buffer dispatch correctness

Layer / File(s)	Summary
Dynamic Buffer key dispatch crates/perry-runtime/src/value/dyn_index.rs, test-files/test_gap_buffer_dyn_key_own_prop_6412.ts	String keys in dynamic Buffer reads use by-name property lookup, while numeric keys retain byte access; regression coverage validates both paths.
Module shadow detection and context propagation crates/perry-codegen/src/lower_call/buffer_intrinsic.rs, crates/perry-codegen/src/codegen/*, crates/perry-codegen/src/expr/mod.rs	Module scans detect assignments to Buffer numeric read methods and propagate the result through CrossModuleCtx and each FnCtx construction path.
Shadow-aware intrinsic fallback crates/perry-codegen/src/lower_call/buffer_intrinsic.rs, test-files/test_gap_buffer_own_prop_shadow_intrinsic_6405.ts	Inline Buffer read folding is skipped for shadowing modules, with tests covering overridden methods and unaffected reads.

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

Possibly related PRs

• PerryTS/perry#6406: Implements related Buffer own-property and method dispatch behavior used by this shadowing fix.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly names both fixes and the affected Buffer behavior without extra noise.
Description check	✅ Passed	The description is detailed and covers the bugs, fixes, and verification, though it doesn't mirror the template's headings exactly.
Linked Issues check	✅ Passed	The changes address #6412 and #6405 by restoring own-property reads and disabling folded read intrinsics when Buffer methods are shadowed.
Out of Scope Changes check	✅ Passed	The added canonical numeric-index handling and broader shadow scan stay aligned with the linked Buffer correctness fixes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry-codegen/src/lower_call/buffer_intrinsic.rs`:
- Around line 297-319: Update module_shadows_buffer_read_method’s class
traversal to check constructor, method, getter, setter, and computed-member
names with is_buffer_numeric_read_method, setting found when any match. Also
scan class.fields, class.static_fields, extends_expr, and each computed member’s
key_expr and function body via scan_body so assignments in initializers and
class expressions are detected.

In `@crates/perry-runtime/src/value/dyn_index.rs`:
- Around line 180-190: Update the string-key handling in the dynamic index
lookup to detect canonical numeric strings before calling
js_object_get_field_by_name_f64. Route those keys through the existing Buffer
indexed-access path, such as js_buffer_get, so buf[k] with a string key like "2"
returns the byte value; preserve name lookup for non-numeric strings and the
existing undefined fallback.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 652e74c2-5958-4d3e-836c-90dc93634e90

📥 Commits

Reviewing files that changed from the base of the PR and between 520894f and 69c831b.

📒 Files selected for processing (12)

• crates/perry-codegen/src/codegen/closure.rs
• crates/perry-codegen/src/codegen/entry.rs
• crates/perry-codegen/src/codegen/function.rs
• crates/perry-codegen/src/codegen/method.rs
• crates/perry-codegen/src/codegen/mod.rs
• crates/perry-codegen/src/codegen/opts.rs
• crates/perry-codegen/src/expr/mod.rs
• crates/perry-codegen/src/lower_call/buffer_intrinsic.rs
• crates/perry-codegen/src/lower_call/mod.rs
• crates/perry-runtime/src/value/dyn_index.rs
• test-files/test_gap_buffer_dyn_key_own_prop_6412.ts
• test-files/test_gap_buffer_own_prop_shadow_intrinsic_6405.ts