Skip to content

compile: fix OP_SETLIST extended form for >25,550-element array literals#537

Open
efiguti wants to merge 1 commit into
yuin:masterfrom
efiguti:fix-setlist-extended-form
Open

compile: fix OP_SETLIST extended form for >25,550-element array literals#537
efiguti wants to merge 1 commit into
yuin:masterfrom
efiguti:fix-setlist-extended-form

Conversation

@efiguti
Copy link
Copy Markdown

@efiguti efiguti commented May 19, 2026

Fix OP_SETLIST extended-form emission silently dropping table assignments

Summary

compileTableExpr in compile.go emits broken bytecode for any Lua
table-array literal with more than 25,550 elements
(MAXARG_C(511) × FieldsPerFlush(50)). Two compounding bugs:

  1. The page-index word that follows an extended (C=0) OP_SETLIST is
    emitted as uint32(c) after c has been clamped to 0. The VM
    then reads 0 as the page index and computes offset = (0-1)*50 = -50, silently losing every element in the final batch.
  2. The page-index word, as a uint32, decodes as OP_MOVE A=0 B=0
    (because OP_MOVE = 0). When the array literal is nested inside
    another expression, the caller's PropagateKMV pass inspects
    code.Last(), mistakes the page-word for a real OP_MOVE, and
    pops it from the bytecode. The next emitted instruction
    overwrites that slot. The VM then reads whatever ended up there as
    the page index (often a SETTABLE0x30040101 → ~805 M), writes
    into oblivion, AND the parent's key-assignment SETTABLE is gone
    because it was eaten as the page-word. The whole parent[key] = bigArray assignment silently disappears from the result.

No error is raised. The VM runs without panic. The output table just
appears to be missing keys. This affects every entry point — DoFile,
DoString, LoadString+PCall, parse.Parse + lua.Compile +
NewFunctionFromProto — and reproduces in the stock glua CLI built
from master.

Minimal reproducer

package main

import (
	"fmt"
	"strings"

	lua "github.com/yuin/gopher-lua"
)

func main() {
	const n = 25551 // one past 511 * FieldsPerFlush
	var b strings.Builder
	b.WriteString(`return { ["outer"] = { ["arr"] = {`)
	for i := 0; i < n; i++ {
		fmt.Fprintf(&b, "%d,", i+1)
	}
	b.WriteString(`} } }`)

	L := lua.NewState()
	defer L.Close()
	if err := L.DoString(b.String()); err != nil {
		panic(err)
	}
	outer := L.Get(-1).(*lua.LTable).RawGetString("outer").(*lua.LTable)
	fmt.Printf("outer mid-keys = %d (want 1: \"arr\")\n", outer.Len())
	outer.ForEach(func(k, v lua.LValue) {
		fmt.Printf("  [%s] = %s\n", k.String(), v.Type())
	})
}

Before the fix: outer mid-keys = 0 — the arr key is missing.
After the fix: outer mid-keys = 0 for Len() (string-keyed, so the
array part is 0), but ForEach enumerates [arr] = table and
outer.arr has all 25,551 elements correctly.

The exact threshold is n = 511 × FieldsPerFlush + 1 = 25,551.

Fix

In compileTableExpr (compile.go):

  1. Preserve the real page index before clamping c to 0:

    realC := (arraycount-1)/FieldsPerFlush + 1
    c := realC
    ...
    if c > 511 { c = 0 }
    code.AddABC(OP_SETLIST, tablereg, b, c, sline(line))
    if c == 0 {
        code.Add(uint32(realC), sline(line))   // was: uint32(c) = always 0
    }
  2. When an extended OP_SETLIST was emitted, follow up with a no-op
    OP_MOVE tablereg ← tablereg (only if shouldmove didn't already
    emit a MOVE) so the caller's PropagateKMV has a real instruction to
    pop and the raw page-word survives underneath:

    if shouldmove(ec, tablereg) {
        code.AddABC(OP_MOVE, ec.reg, tablereg, 0, sline(ex))
    } else if extendedSetlistEmitted {
        code.AddABC(OP_MOVE, tablereg, tablereg, 0, sline(ex))
    }

The injected OP_MOVE r r is a runtime no-op, so the only cost is one
extra instruction word per table literal large enough to need extended
OP_SETLIST — typically zero outside of generated/data-table use
cases. The VM's OP_SETLIST handler at vm.go:2061-2064 already reads
the next code word when C == 0; no VM change needed.

Test

compile_test.go (added) covers:

  • n = FieldsPerFlush * 511 — exactly the last single-instruction page,
    pre-fix passing.
  • n = FieldsPerFlush * 511 + 1 — first extended (C=0) page,
    pre-fix failing.
  • n = FieldsPerFlush * 600, n = FieldsPerFlush * 511 + 50*50
    well past the threshold.
  • A top-level (non-nested) variant that exercises only Bug 'whitespace' become possibly overflow on 32bit architecture. #1.

Each test asserts the array length, the parent key is present, and
the first/middle/last element values are correct (catches zero-offset
corruption).

Full table_test/state_test suite still passes locally with the patch
applied.

…rals

   Two compounding bugs in compileTableExpr's extended-form OP_SETLIST
   emission:

   1. The page-index word was emitted as uint32(c) after c had been
      clamped to 0 to signal the extended form, so the VM always read
      page=0 and silently lost every element past index 25,550.
   2. The raw page-index word decodes as OP_MOVE A=0 B=0, which the
      caller's PropagateKMV pass mistakes for a real instruction and
      pops, corrupting the bytecode and dropping the parent SETTABLE.

   Captures the real page index before clamping; appends a safety
   OP_MOVE tablereg<-tablereg after extended SETLIST when shouldmove
   wouldn't emit one, so PropagateKMV pops the safety MOVE instead of
   the page-word.

   Adds compile_test.go covering the threshold at 511*FieldsPerFlush.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant