From d680329eed21fe8f1c89b54a479a172d56befccf Mon Sep 17 00:00:00 2001 From: Wolfy-J Date: Fri, 21 Aug 2026 18:40:08 -0400 Subject: [PATCH] compile: fence the LOADNIL merge peephole at jump targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The AddLoadNil peephole folds a new LOADNIL into the previous instruction whenever the register spans are adjacent. When the previous LOADNIL ends a conditionally-skipped path — the nil arm of an 'or nil' expression — the fold moves the next statement's nil-register init onto that skippable path. The short-circuit path then reads an uninitialized stack slot and the VM dereferences an empty LValue: 'local v = t[key] or nil' inside a generic for loop panics with a Go nil pointer whenever t[key] is truthy. codeStore now records the highest label pc (MarkLabelPc, fed by SetLabelPc) and AddLoadNil merges only when the previous instruction lies strictly after every label position, mirroring PUC Lua's fs->lasttarget fence in luaK_nil. Claude-Session: https://claude.ai/code/session_0134uPSdqJwq5Sp8qyUseaq5 --- compile.go | 22 +++++-- compile_loadnil_fence_test.go | 112 ++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 4 deletions(-) create mode 100644 compile_loadnil_fence_test.go diff --git a/compile.go b/compile.go index e7f65d320..b8110eff5 100644 --- a/compile.go +++ b/compile.go @@ -210,6 +210,12 @@ type codeStore struct { // {{{ codes *[]uint32 lines *[]int pc int + // lastTarget is the highest pc recorded as a jump-label position. + // Instruction-merging peepholes must not fold across it: an instruction + // at or before a label boundary can be skipped or jumped past, so + // extending it changes behavior on the jumping path. Mirrors PUC Lua's + // fs->lasttarget. + lastTarget int } func newCodeStore() *codeStore { @@ -218,9 +224,10 @@ func newCodeStore() *codeStore { *codes = (*codes)[:0] *lines = (*lines)[:0] return &codeStore{ - codes: codes, - lines: lines, - pc: 0, + codes: codes, + lines: lines, + pc: 0, + lastTarget: -1, } } @@ -297,9 +304,15 @@ func (cd *codeStore) PropagateMV(top int, save *int, reg *int, inc int) { *reg = *reg + inc } +func (cd *codeStore) MarkLabelPc(pc int) { + if pc > cd.lastTarget { + cd.lastTarget = pc + } +} + func (cd *codeStore) AddLoadNil(a, b, line int) { last := cd.Last() - if opGetOpCode(last) == OP_LOADNIL && (opGetArgB(last)+1) == a { + if opGetOpCode(last) == OP_LOADNIL && (opGetArgB(last)+1) == a && cd.LastPC() > cd.lastTarget { cd.SetB(cd.LastPC(), b) } else { cd.AddABC(OP_LOADNIL, a, b, 0, line) @@ -592,6 +605,7 @@ func (fc *funcContext) NewLabel() int { func (fc *funcContext) SetLabelPc(label int, pc int) { fc.labelPc[label] = pc + fc.Code.MarkLabelPc(pc) } func (fc *funcContext) GetLabelPc(label int) int { diff --git a/compile_loadnil_fence_test.go b/compile_loadnil_fence_test.go new file mode 100644 index 000000000..878613d68 --- /dev/null +++ b/compile_loadnil_fence_test.go @@ -0,0 +1,112 @@ +// SPDX-License-Identifier: MPL-2.0 + +package lua + +import ( + "testing" +) + +// The LOADNIL merge peephole (codeStore.AddLoadNil) must not extend a LOADNIL +// that sits before a jump target: an `or nil` arm ends in a skippable LOADNIL, +// and folding the next statement's nil-register init into it leaves that +// register uninitialized on the short-circuit path. The VM then dereferences +// an empty stack slot. Mirrors PUC Lua's `fs->lasttarget` fence in luaK_nil. +func TestLoadNilMergeStopsAtJumpTarget(t *testing.T) { + tests := []struct { + name string + code string + expected string + }{ + { + // The or takes the truthy lhs: the skipped nil arm must not own + // the comparison's nil register. + name: "or nil over a table hit inside ipairs", + code: ` + local t = { k = { skip = true } } + local out = 0 + for _, key in ipairs({ "k" }) do + local v = t[key] or nil + if v ~= nil then out = out + 1 end + end + return out + `, + expected: "1", + }, + { + name: "or nil over a table miss inside ipairs", + code: ` + local t = { k = { skip = true } } + local out = 0 + for _, key in ipairs({ "absent" }) do + local v = t[key] or nil + if v == nil then out = out + 1 end + end + return out + `, + expected: "1", + }, + { + name: "scalar or nil inside ipairs", + code: ` + local out = 0 + for _, key in ipairs({ "k" }) do + local x = key or nil + if x ~= nil then out = out + 1 end + end + return out + `, + expected: "1", + }, + { + name: "or nil inside pairs", + code: ` + local t = { k = { skip = true } } + local out = 0 + for key in pairs({ k = 1 }) do + local v = t[key] or nil + if v ~= nil then out = out + 1 end + end + return out + `, + expected: "1", + }, + { + name: "guarded chain with mixed hit and miss keys", + code: ` + local t = { k = { skip = true } } + local out = 0 + for _, key in ipairs({ "a", "k" }) do + local v = key ~= nil and t[key] or nil + if v ~= nil and v.skip == true then out = out + 1 end + end + return out + `, + expected: "1", + }, + { + // Adjacent nil locals with no label in between keep merging. + name: "plain adjacent nil locals still fold", + code: ` + local a, b + local c = nil + if a == nil and b == nil and c == nil then return 1 end + return 0 + `, + expected: "1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + L := NewState() + defer L.Close() + if err := L.DoString(tt.code); err != nil { + t.Fatalf("runtime error: %v", err) + } + got := L.Get(-1) + if got.String() != tt.expected { + t.Fatalf("expected %s, got %s", tt.expected, got.String()) + } + }) + } +}