Skip to content

Open-upvalue cells leak across do/block boundaries when registers are reused #276

@davydog187

Description

@davydog187

Problem

state.open_upvalues is keyed by register and is never closed when a do/if/while/for/repeat block ends. If a later block reuses the same register for a new captured local, the executor's closure handler at lib/lua/vm/executor.ex:707 reuses the existing cell ref — so reads of the new local through get_open_upvalue (or via any inner closure that captures it) return the previous block's value.

Repro

do
  local res = 1
  local function fact (n)
    if n == 0 then return res
    else return n * fact(n - 1) end
  end
  assert(fact(5) == 120)
end

do
  local a = {x = 100}
  local function read_a()
    return a.x
  end
  assert(read_a() == 100, "expected 100, got stale-cell value")
end

Expected: assertion passes (read_a() returns 100).
Actual: attempt to index a number valuea is resolved through the stale cell that previously held fact (or res), so the captured value is an integer, not the table.

The cell from the first block's local function fact self-capture stays in state.open_upvalues after the do ends. When the second block's function read_a captures a, the closure handler finds an existing cell at that register and reuses it.

Why it shows up now

Pre-existing latent bug. Surfaced by PR #274, which fixed the multi-name FuncDecl head resolution. With that fix, the head of function a:add(...) inside a do block now correctly resolves to the captured local (rather than spuriously falling back to _ENV.a), so get_open_upvalue is now emitted where it should have been all along — and this is the first time the staleness manifests on the suite.

Blocks calls.lua:65–69 (see the narrowed skip in test/lua53_skips.exs).

Suggested approach

Two options, in rough order of preference:

  1. Close cells at block end. Emit a :close_open_upvalues instruction at the end of each block scope, parametrised by the registers that had cells opened during the block. The executor removes those entries from state.open_upvalues. The cell refs remain valid for any closure that captured them; state.upvalue_cells is untouched.

  2. Track open ranges in scope analysis. Have the codegen know, per block, which registers may have had cells opened, and emit close instructions automatically. This is the same machinery PUC-Lua uses (the OP_CLOSE instruction in lopcodes.h).

The codegen-side tracking exists in spirit already — scope.ex knows captured_locals per function — but block-boundary granularity is not currently tracked.

Acceptance

  • Repro above passes.
  • calls.lua:65..69 removed from test/lua53_skips.exs and the file's range list narrows further (the next downstream blocker is documented in the same file).
  • Regression test in test/lua/vm/upvalue_test.exs covering the pattern: two sibling do blocks where both declare a captured local that lands on the same register.
  • mix test passes; no regressions in test/lua/vm/upvalue_test.exs, closure-style tests, or the lua53 suite.

Risk

  • Closing cells at block end is a subtle change to executor invariants. Existing closures hold cell refs and resolve through state.upvalue_cells, which we must not touch — only the open_upvalues index map.
  • Loops (especially for/while) re-enter the block body across iterations; cells must persist across iterations and only close when the loop exits.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingkind:suiteLua 5.3 official test suite work

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions