Skip to content

codegen: a non-numeric key on a Uint8Array/Buffer-typed local reads a byte instead of a property (u8[Symbol.iterator] is a number) #7700

Description

@proggeramlug

Summary

A non-numeric key on a Uint8Array/Buffer-typed local reads a byte instead of a property, so every such read is wrong:

node 26.5.1 perry
typeof u8[Symbol.iterator] function number
const k: any = "byteLength"; u8[k] 4 0
const k: any = "subarray"; typeof u8[k] function number
u8.tag = {…}; const k: any = "tag"; u8[k] {"kind":"buffer"} 0

Reproducer (matches node exactly, diverges on perry at every line):

const u8 = new Uint8Array([1, 2, 3, 4]);
const it = u8[Symbol.iterator];
console.log(typeof it);                      // node: function   perry: number
const name: any = "subarray";
console.log(typeof u8[name]);                // node: function   perry: number
const lenKey: any = "byteLength";
console.log(u8[lenKey]);                     // node: 4          perry: 0
const anyU8: any = u8;
anyU8.tag = { kind: "buffer" };
const tagKey: any = "tag";
console.log(JSON.stringify(u8[tagKey]));     // node: {"kind":"buffer"}  perry: 0

Where

lower/expr_member/member_tail.rs folds every non-STRING key on a
Uint8Array/Buffer-typed local onto Expr::Uint8ArrayGet — a symbol key is
not a string, and neither is a LocalGet of an any-typed local that happens
to hold a string at runtime. Confirmed on --print-hir:

Let { id: 1, name: "it", ty: Any, init: Some(Uint8ArrayGet { array: LocalGet(0),
      index: SymbolFor(String("@@__perry_wk_iterator")) }) }
Let { id: 5, name: "v",  ty: Any, init: Some(Uint8ArrayGet { array: LocalGet(0),
      index: LocalGet(4) }) }

expr/arrays_finds.rs's JS-value lowering of that node handles all three
correctly — a SymbolFor key goes to js_object_get_symbol_property, an
unproven key to js_typed_array_index_get_dynamic. The value never reaches
it
, because the local is classified numeric and the read is lowered in i32
context (lower_uint8array_get_i32) instead.

Six places make the same unconditional "a Uint8ArrayGet is a number"
assumption with no regard for the key kind:

  • collectors/integer_locals.rs:736, :1048
  • collectors/i32_locals.rs:120
  • collectors/int_valued_ta_locals.rs:153, :220, :268, :1153
  • collectors/not_bigint_locals.rs:125
  • type_analysis/numeric.rs:134, :617, :726

collectors/pointer_locals.rs made it too and was fixed in #6998 / #7699 — that
one is the GC-soundness half (a heap value in a local with no shadow slot), and
it is currently masked by this bug: no heap value reaches such a local while
every read returns a byte. It becomes load-bearing the moment this is repaired,
which is the order the two should be fixed in.

Why it was not folded into #6998

The listed collectors decide the buf[i] i32 fast path, which is the hottest
buffer code in the compiler (#6996's regression was one temp root per iteration
in exactly these loops). Adding a key-kind condition to them is a measured
change, not a soundness patch. expr/shadow_slot.rs's
expr_is_known_non_pointer_shadow_value already states the correct three-part
condition and is the model to copy:

Expr::Uint8ArrayGet { index, .. } => {
    !matches!(index.as_ref(), Expr::SymbolFor(_))
        && crate::type_analysis::is_numeric_expr(ctx, index)
        && super::index_get::numeric_index_has_integer_array_index_proof(ctx, index)
}

Acceptance

A test-files/test_gap_*.ts with the reproducer above, matching node 26.5.1
byte-for-byte, plus a before/after on churn_alloc / a buf[i] loop showing the
proven-index path still takes the inline byte load.

Found while fixing #6998 (PR #7699).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions