Skip to content

Commit f4c35e4

Browse files
yuinyuin
authored
Merge pull request yuin#316 from tul/nil_reg_entry_panic_fix
Nil reg entry panic fix
2 parents ee81675 + 4403b04 commit f4c35e4

File tree

4 files changed

+108
-53
lines changed

4 files changed

+108
-53
lines changed

‎_state.go

+9-5
Original file line numberDiff line numberDiff line change
@@ -936,18 +936,22 @@ func (ls *LState) initCallFrame(cf *callFrame) { // +inline-start
936936
proto := cf.Fn.Proto
937937
nargs := cf.NArgs
938938
np := int(proto.NumParameters)
939-
newSize := cf.LocalBase + np
940-
// +inline-call ls.reg.checkSize newSize
941-
for i := nargs; i < np; i++ {
942-
ls.reg.array[cf.LocalBase+i] = LNil
939+
if nargs < np {
940+
// default any missing arguments to nil
941+
newSize := cf.LocalBase + np
942+
// +inline-call ls.reg.checkSize newSize
943+
for i := nargs; i < np; i++ {
944+
ls.reg.array[cf.LocalBase+i] = LNil
945+
}
943946
nargs = np
947+
ls.reg.top = newSize
944948
}
945949

946950
if (proto.IsVarArg & VarArgIsVarArg) == 0 {
947951
if nargs < int(proto.NumUsedRegisters) {
948952
nargs = int(proto.NumUsedRegisters)
949953
}
950-
newSize = cf.LocalBase + nargs
954+
newSize := cf.LocalBase + nargs
951955
// +inline-call ls.reg.checkSize newSize
952956
for i := np; i < nargs; i++ {
953957
ls.reg.array[cf.LocalBase+i] = LNil

‎state.go

+32-24
Original file line numberDiff line numberDiff line change
@@ -982,26 +982,30 @@ func (ls *LState) initCallFrame(cf *callFrame) { // +inline-start
982982
proto := cf.Fn.Proto
983983
nargs := cf.NArgs
984984
np := int(proto.NumParameters)
985-
newSize := cf.LocalBase + np
986-
// this section is inlined by go-inline
987-
// source function is 'func (rg *registry) checkSize(requiredSize int) ' in '_state.go'
988-
{
989-
rg := ls.reg
990-
requiredSize := newSize
991-
if requiredSize > cap(rg.array) {
992-
rg.resize(requiredSize)
985+
if nargs < np {
986+
// default any missing arguments to nil
987+
newSize := cf.LocalBase + np
988+
// this section is inlined by go-inline
989+
// source function is 'func (rg *registry) checkSize(requiredSize int) ' in '_state.go'
990+
{
991+
rg := ls.reg
992+
requiredSize := newSize
993+
if requiredSize > cap(rg.array) {
994+
rg.resize(requiredSize)
995+
}
996+
}
997+
for i := nargs; i < np; i++ {
998+
ls.reg.array[cf.LocalBase+i] = LNil
993999
}
994-
}
995-
for i := nargs; i < np; i++ {
996-
ls.reg.array[cf.LocalBase+i] = LNil
9971000
nargs = np
1001+
ls.reg.top = newSize
9981002
}
9991003

10001004
if (proto.IsVarArg & VarArgIsVarArg) == 0 {
10011005
if nargs < int(proto.NumUsedRegisters) {
10021006
nargs = int(proto.NumUsedRegisters)
10031007
}
1004-
newSize = cf.LocalBase + nargs
1008+
newSize := cf.LocalBase + nargs
10051009
// this section is inlined by go-inline
10061010
// source function is 'func (rg *registry) checkSize(requiredSize int) ' in '_state.go'
10071011
{
@@ -1090,26 +1094,30 @@ func (ls *LState) pushCallFrame(cf callFrame, fn LValue, meta bool) { // +inline
10901094
proto := cf.Fn.Proto
10911095
nargs := cf.NArgs
10921096
np := int(proto.NumParameters)
1093-
newSize := cf.LocalBase + np
1094-
// this section is inlined by go-inline
1095-
// source function is 'func (rg *registry) checkSize(requiredSize int) ' in '_state.go'
1096-
{
1097-
rg := ls.reg
1098-
requiredSize := newSize
1099-
if requiredSize > cap(rg.array) {
1100-
rg.resize(requiredSize)
1097+
if nargs < np {
1098+
// default any missing arguments to nil
1099+
newSize := cf.LocalBase + np
1100+
// this section is inlined by go-inline
1101+
// source function is 'func (rg *registry) checkSize(requiredSize int) ' in '_state.go'
1102+
{
1103+
rg := ls.reg
1104+
requiredSize := newSize
1105+
if requiredSize > cap(rg.array) {
1106+
rg.resize(requiredSize)
1107+
}
1108+
}
1109+
for i := nargs; i < np; i++ {
1110+
ls.reg.array[cf.LocalBase+i] = LNil
11011111
}
1102-
}
1103-
for i := nargs; i < np; i++ {
1104-
ls.reg.array[cf.LocalBase+i] = LNil
11051112
nargs = np
1113+
ls.reg.top = newSize
11061114
}
11071115

11081116
if (proto.IsVarArg & VarArgIsVarArg) == 0 {
11091117
if nargs < int(proto.NumUsedRegisters) {
11101118
nargs = int(proto.NumUsedRegisters)
11111119
}
1112-
newSize = cf.LocalBase + nargs
1120+
newSize := cf.LocalBase + nargs
11131121
// this section is inlined by go-inline
11141122
// source function is 'func (rg *registry) checkSize(requiredSize int) ' in '_state.go'
11151123
{

‎state_test.go

+35
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,41 @@ func TestRegistryAutoGrow(t *testing.T) {
525525
reg.Push(test)
526526
}
527527

528+
// This test exposed a panic caused by accessing an unassigned var in the lua registry.
529+
// The panic was caused by initCallFrame. It was calling resize() on the registry after it had written some values
530+
// directly to the reg's array, but crucially, before it had updated "top". This meant when the resize occurred, the
531+
// values beyond top where not copied, and were lost, leading to a later uninitialised value being found in the registry.
532+
func TestUninitializedVarAccess(t *testing.T) {
533+
L := NewState(Options{
534+
RegistrySize: 128,
535+
RegistryMaxSize: 256,
536+
})
537+
defer L.Close()
538+
// This test needs to trigger a resize when the local vars are allocated, so we need it to
539+
// be 128 for the padding amount in the test function to work. If it's larger, we will need
540+
// more padding to force the error.
541+
errorIfNotEqual(t, cap(L.reg.array), 128)
542+
ctx, cancel := context.WithCancel(context.Background())
543+
L.SetContext(ctx)
544+
defer cancel()
545+
errorIfScriptFail(t, L, `
546+
local function test(arg1, arg2, arg3)
547+
-- padding to cause a registry resize when the local vars for this func are reserved
548+
local a0,b0,c0,d0,e0,f0,g0,h0,i0,j0,k0,l0,m0,n0,o0,p0,q0,r0,s0,t0,u0,v0,w0,x0,y0,z0
549+
local a1,b1,c1,d1,e1,f1,g1,h1,i1,j1,k1,l1,m1,n1,o1,p1,q1,r1,s1,t1,u1,v1,w1,x1,y1,z1
550+
local a2,b2,c2,d2,e2,f2,g2,h2,i2,j2,k2,l2,m2,n2,o2,p2,q2,r2,s2,t2,u2,v2,w2,x2,y2,z2
551+
local a3,b3,c3,d3,e3,f3,g3,h3,i3,j3,k3,l3,m3,n3,o3,p3,q3,r3,s3,t3,u3,v3,w3,x3,y3,z3
552+
local a4,b4,c4,d4,e4,f4,g4,h4,i4,j4,k4,l4,m4,n4,o4,p4,q4,r4,s4,t4,u4,v4,w4,x4,y4,z4
553+
if arg3 == nil then
554+
return 1
555+
end
556+
return 0
557+
end
558+
559+
test(1,2)
560+
`)
561+
}
562+
528563
func BenchmarkCallFrameStackPushPopAutoGrow(t *testing.B) {
529564
stack := newAutoGrowingCallFrameStack(256)
530565

‎vm.go

+32-24
Original file line numberDiff line numberDiff line change
@@ -728,26 +728,30 @@ func init() {
728728
proto := cf.Fn.Proto
729729
nargs := cf.NArgs
730730
np := int(proto.NumParameters)
731-
newSize := cf.LocalBase + np
732-
// this section is inlined by go-inline
733-
// source function is 'func (rg *registry) checkSize(requiredSize int) ' in '_state.go'
734-
{
735-
rg := ls.reg
736-
requiredSize := newSize
737-
if requiredSize > cap(rg.array) {
738-
rg.resize(requiredSize)
731+
if nargs < np {
732+
// default any missing arguments to nil
733+
newSize := cf.LocalBase + np
734+
// this section is inlined by go-inline
735+
// source function is 'func (rg *registry) checkSize(requiredSize int) ' in '_state.go'
736+
{
737+
rg := ls.reg
738+
requiredSize := newSize
739+
if requiredSize > cap(rg.array) {
740+
rg.resize(requiredSize)
741+
}
742+
}
743+
for i := nargs; i < np; i++ {
744+
ls.reg.array[cf.LocalBase+i] = LNil
739745
}
740-
}
741-
for i := nargs; i < np; i++ {
742-
ls.reg.array[cf.LocalBase+i] = LNil
743746
nargs = np
747+
ls.reg.top = newSize
744748
}
745749

746750
if (proto.IsVarArg & VarArgIsVarArg) == 0 {
747751
if nargs < int(proto.NumUsedRegisters) {
748752
nargs = int(proto.NumUsedRegisters)
749753
}
750-
newSize = cf.LocalBase + nargs
754+
newSize := cf.LocalBase + nargs
751755
// this section is inlined by go-inline
752756
// source function is 'func (rg *registry) checkSize(requiredSize int) ' in '_state.go'
753757
{
@@ -906,26 +910,30 @@ func init() {
906910
proto := cf.Fn.Proto
907911
nargs := cf.NArgs
908912
np := int(proto.NumParameters)
909-
newSize := cf.LocalBase + np
910-
// this section is inlined by go-inline
911-
// source function is 'func (rg *registry) checkSize(requiredSize int) ' in '_state.go'
912-
{
913-
rg := ls.reg
914-
requiredSize := newSize
915-
if requiredSize > cap(rg.array) {
916-
rg.resize(requiredSize)
913+
if nargs < np {
914+
// default any missing arguments to nil
915+
newSize := cf.LocalBase + np
916+
// this section is inlined by go-inline
917+
// source function is 'func (rg *registry) checkSize(requiredSize int) ' in '_state.go'
918+
{
919+
rg := ls.reg
920+
requiredSize := newSize
921+
if requiredSize > cap(rg.array) {
922+
rg.resize(requiredSize)
923+
}
924+
}
925+
for i := nargs; i < np; i++ {
926+
ls.reg.array[cf.LocalBase+i] = LNil
917927
}
918-
}
919-
for i := nargs; i < np; i++ {
920-
ls.reg.array[cf.LocalBase+i] = LNil
921928
nargs = np
929+
ls.reg.top = newSize
922930
}
923931

924932
if (proto.IsVarArg & VarArgIsVarArg) == 0 {
925933
if nargs < int(proto.NumUsedRegisters) {
926934
nargs = int(proto.NumUsedRegisters)
927935
}
928-
newSize = cf.LocalBase + nargs
936+
newSize := cf.LocalBase + nargs
929937
// this section is inlined by go-inline
930938
// source function is 'func (rg *registry) checkSize(requiredSize int) ' in '_state.go'
931939
{

0 commit comments

Comments
 (0)