compile: fix OP_SETLIST extended form for >25,550-element array literals#537
Open
efiguti wants to merge 1 commit into
Open
compile: fix OP_SETLIST extended form for >25,550-element array literals#537efiguti wants to merge 1 commit into
efiguti wants to merge 1 commit into
Conversation
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix OP_SETLIST extended-form emission silently dropping table assignments
Summary
compileTableExprincompile.goemits broken bytecode for any Luatable-array literal with more than 25,550 elements
(
MAXARG_C(511) × FieldsPerFlush(50)). Two compounding bugs:C=0)OP_SETLISTisemitted as
uint32(c)afterchas been clamped to0. The VMthen reads
0as the page index and computesoffset = (0-1)*50 = -50, silently losing every element in the final batch.uint32, decodes asOP_MOVE A=0 B=0(because
OP_MOVE = 0). When the array literal is nested insideanother expression, the caller's
PropagateKMVpass inspectscode.Last(), mistakes the page-word for a realOP_MOVE, andpops 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
SETTABLE≈0x30040101→ ~805 M), writesinto oblivion, AND the parent's key-assignment
SETTABLEis gonebecause it was eaten as the page-word. The whole
parent[key] = bigArrayassignment 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 stockgluaCLI builtfrom
master.Minimal reproducer
Before the fix:
outer mid-keys = 0— thearrkey is missing.After the fix:
outer mid-keys = 0forLen()(string-keyed, so thearray part is 0), but
ForEachenumerates[arr] = tableandouter.arrhas all 25,551 elements correctly.The exact threshold is
n = 511 × FieldsPerFlush + 1 = 25,551.Fix
In
compileTableExpr(compile.go):Preserve the real page index before clamping
cto 0:When an extended
OP_SETLISTwas emitted, follow up with a no-opOP_MOVE tablereg ← tablereg(only ifshouldmovedidn't alreadyemit a MOVE) so the caller's
PropagateKMVhas a real instruction topop and the raw page-word survives underneath:
The injected
OP_MOVE r ris a runtime no-op, so the only cost is oneextra instruction word per table literal large enough to need extended
OP_SETLIST— typically zero outside of generated/data-table usecases. The VM's
OP_SETLISThandler atvm.go:2061-2064already readsthe 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.
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.