Skip to content

Lazy array index probe ignores the front offset, so a materialized array misreads after shift() #10272

Description

@proggeramlug

js_lazy_array_index_probe in crates/perry-runtime/src/json_tape/cached_read.rs computes the element base of a materialized lazy array as the end of its header:

let elements =
    (cached as *const u8).add(std::mem::size_of::<crate::array::ArrayHeader>()) as *const u64;

shift() leaves a front offset: a dense queue keeps its live elements in a suffix of the allocation (array::storage, #10077), so logical element zero is not the header's end. The general read in the same file gets this right and uses array_elements_ptr (line ~65); the fix for it was 936b8bc, "make the JSON cached read honour the array front offset". The probe was added afterwards, by the lazy indexed-cache tier (#10114), and copied the pre-fix form.

Effect. After one shift(), the probe returns the element before the one asked for. A throwaway unit test over [10,20,30,40,50,60], materialized and shifted once, then read through both paths:

index general read probe
0 20 declines, so it falls back correctly
1 30 20
2 40 30
4 60 50

Index 0 escapes because shift_dense writes a hole into the vacated slot and the probe treats a hole as "ask the rooted accessor".

Not reproduced from TypeScript yet. I tried several shapes: read then shift then index, shift on a fresh parse, a hot loop after shifts, and object elements, each compared against Node, all identical. Two things stand between the defect and a compiled program:

  • the probe is called only from the miss exit of one dynamic index-read shape, through js_packed_arraylike_index_get; and
  • it declines while cached_length is stale, which shift() makes it, until a general read refreshes the mirror.

So it needs a program where a general read refreshes the mirror after the shift and a later read of the same array arrives through that one site. That is a narrow window, not a guarantee of safety.

Suggested fix. Use array_elements_ptr in the probe's materialized branch, the same call the general read makes, with a regression test in the file's existing test module driving materialize, shift, refresh, probe, and asserting the probe agrees with lazy_get at every index.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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