Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix compiler producing incorrect LOADNIL byte code #496

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func (cd *codeStore) PropagateMV(top int, save *int, reg *int, inc int) {

func (cd *codeStore) AddLoadNil(a, b, line int) {
last := cd.Last()
if opGetOpCode(last) == OP_LOADNIL && (opGetArgA(last)+opGetArgB(last)) == a {
if opGetOpCode(last) == OP_LOADNIL && (opGetArgB(last)+1) == a {
cd.SetB(cd.LastPC(), b)
} else {
cd.AddABC(OP_LOADNIL, a, b, 0, line)
Expand Down
84 changes: 84 additions & 0 deletions script_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"os"
"runtime"
"strings"
"sync/atomic"
"testing"
"time"
Expand Down Expand Up @@ -146,3 +147,86 @@ func TestGlua(t *testing.T) {
func TestLua(t *testing.T) {
testScriptDir(t, luaTests, "_lua5.1-tests")
}

func TestMergingLoadNilBug(t *testing.T) {
// there was a bug where a multiple load nils were being incorrectly merged, and the following code exposed it
s := `
function test()
local a = 0
local b = 1
local c = 2
local d = 3
local e = 4 -- reg 4
local f = 5
local g = 6
local h = 7

if e == 4 then
e = nil -- should clear reg 4, but clears regs 4-8 by mistake
end
if f == nil then
error("bad f")
end
if g == nil then
error("bad g")
end
if h == nil then
error("bad h")
end
end

test()
`

L := NewState()
defer L.Close()
if err := L.DoString(s); err != nil {
t.Error(err)
}
}

func TestMergingLoadNil(t *testing.T) {
// multiple nil assignments to consecutive registers should be merged
s := `
function test()
local a = 0
local b = 1
local c = 2

-- this should generate just one LOADNIL byte code instruction
a = nil
b = nil
c = nil

print(a,b,c)
end

test()`

chunk, err := parse.Parse(strings.NewReader(s), "test")
if err != nil {
t.Fatal(err)
}

compiled, err := Compile(chunk, "test")
if err != nil {
t.Fatal(err)
}

if len(compiled.FunctionPrototypes) != 1 {
t.Fatal("expected 1 function prototype")
}

// there should be exactly 1 LOADNIL instruction in the byte code generated for the above
// anymore, and the LOADNIL merging is not working correctly
count := 0
for _, instr := range compiled.FunctionPrototypes[0].Code {
if opGetOpCode(instr) == OP_LOADNIL {
count++
}
}

if count != 1 {
t.Fatalf("expected 1 LOADNIL instruction, found %d", count)
}
}
Loading