search

kyren / piccolo

An experimental stackless Lua VM implemented in pure Rust
Creative Commons Zero v1.0 Universal
1.62k stars 59 forks source link

TailCail OpCode bug #53

Closed Jengamon closed 4 months ago

Jengamon commented 4 months ago

So I tried testing https://github.com/kikito/middleclass/blob/master/middleclass.lua (which actually works when you patch for this and allow type to accept nil) and was bumping into a mysterious error.

I did some print-debugging and managed to get this minimal example to reproduce the problem.

local b = {}
function b.inner(a, bk)
  print(a, bk, b == bk)
end
setmetatable(b, {
  __call = function(_, ...)
    b.inner(...)
    return b.inner(...)
  end
})

local a = b("a")

PUC-Lua Output

a       nil     false
a       nil     false

Piccolo Output

a       nil     false
a       <table [ADDR]>  true

Piccolo Compiler Dump

===FunctionProto(0x16b156288)===
fixed_params: 0, has_varargs: true, stack_size: 5
---constants---
0: Integer(0)
1: String("inner")
2: String("setmetatable")
3: String("__call")
4: String("a")
---opcodes---
<line 1>
0: OpCode(NewTable { dest: RegisterIndex(0), array_size: 0, map_size: 0 })
1: OpCode(LoadConstant { dest: RegisterIndex(1), constant: ConstantIndex16(0) })
<line 2>
2: OpCode(Closure { dest: RegisterIndex(1), proto: PrototypeIndex(0) })
3: OpCode(SetTableCR { table: RegisterIndex(0), key: ConstantIndex8(1), value: Reg
<line 5>
4: OpCode(GetUpTableC { dest: RegisterIndex(1), table: UpValueIndex(0), key: Const
5: OpCode(Move { dest: RegisterIndex(2), source: RegisterIndex(0) })
6: OpCode(NewTable { dest: RegisterIndex(3), array_size: 0, map_size: 1 })
7: OpCode(Closure { dest: RegisterIndex(4), proto: PrototypeIndex(1) })
8: OpCode(SetTableCR { table: RegisterIndex(3), key: ConstantIndex8(3), value: Reg
9: OpCode(LoadConstant { dest: RegisterIndex(4), constant: ConstantIndex16(0) })
10: OpCode(Call { func: RegisterIndex(1), args: VarCount(Opt254(Some(2))), returns
<line 12>
11: OpCode(Move { dest: RegisterIndex(1), source: RegisterIndex(0) })
12: OpCode(LoadConstant { dest: RegisterIndex(2), constant: ConstantIndex16(4) })
13: OpCode(Call { func: RegisterIndex(1), args: VarCount(Opt254(Some(1))), returns
14: OpCode(Jump { offset: 0, close_upvalues: Opt254(Some(0)) })
<line 13>
15: OpCode(Return { start: RegisterIndex(0), count: VarCount(Opt254(Some(0))) })
---upvalues---
0: Environment
---prototypes---
  ===FunctionProto(0x6000025ac3c0)===
  fixed_params: 2, has_varargs: false, stack_size: 6
  ---constants---
  0: String("print")
  ---opcodes---
  <line 3>
  0: OpCode(GetUpTableC { dest: RegisterIndex(2), table: UpValueIndex(1), key: Con
  1: OpCode(Move { dest: RegisterIndex(3), source: RegisterIndex(0) })
  2: OpCode(Move { dest: RegisterIndex(4), source: RegisterIndex(1) })
  3: OpCode(GetUpValue { dest: RegisterIndex(5), source: UpValueIndex(0) })
  4: OpCode(EqRR { skip_if: false, left: RegisterIndex(5), right: RegisterIndex(1)
  5: OpCode(Jump { offset: 1, close_upvalues: Opt254(None) })
  6: OpCode(LoadBool { dest: RegisterIndex(5), value: false, skip_next: true })
  7: OpCode(LoadBool { dest: RegisterIndex(5), value: true, skip_next: false })
  8: OpCode(Call { func: RegisterIndex(2), args: VarCount(Opt254(Some(3))), return
  <line 4>
  9: OpCode(Return { start: RegisterIndex(0), count: VarCount(Opt254(Some(0))) })
  ---upvalues---
  0: ParentLocal(RegisterIndex(0))
  1: Outer(UpValueIndex(0))
  ===FunctionProto(0x6000025ac460)===
  fixed_params: 1, has_varargs: true, stack_size: 2
  ---constants---
  0: String("inner")
  ---opcodes---
  <line 7>
  0: OpCode(GetUpTableC { dest: RegisterIndex(1), table: UpValueIndex(0), key: ConstantIndex8(0) })
  1: OpCode(VarArgs { dest: RegisterIndex(2), count: VarCount(Opt254(None)) })
  2: OpCode(Call { func: RegisterIndex(1), args: VarCount(Opt254(None)), returns: VarCount(Opt254(Some(0))) })
  <line 8>
  3: OpCode(GetUpTableC { dest: RegisterIndex(1), table: UpValueIndex(0), key: ConstantIndex8(0) })
  4: OpCode(VarArgs { dest: RegisterIndex(2), count: VarCount(Opt254(None)) })
  5: OpCode(TailCall { func: RegisterIndex(1), args: VarCount(Opt254(None)) })
  <line 9>
  6: OpCode(Return { start: RegisterIndex(0), count: VarCount(Opt254(Some(0))) })
  ---upvalues---
  0: ParentLocal(RegisterIndex(0))

The only difference I see between the correct call and the incorrect one is the use of TailCall for Call so something about that implementation is causing this.

Jengamon commented 4 months ago

In more silly antics with Lua

local b = {}
function b.inner(a, bk)
  print(a, bk, b == bk)
end
setmetatable(b, {
  __call = function(_, ...)
    b.inner(...)
    return b.inner(...)
  end
})

local a = b("a")

function a(b, c)
  print(b, c, b==a)
end
function d(_, ...)
  a(...)
  return a(...)
end
d("s")

PUC-Lua Output

a       nil     false
a       nil     false
nil     nil     false
nil     nil     false

Piccolo Output

a       nil     false
a       <table [ADDR]>  true
Error: Runtime(RuntimeError(type error, expected function, found nil))

Piccolo Compiler Dump

===FunctionProto(0x16d7a6378)===
fixed_params: 0, has_varargs: true, stack_size: 5
---constants---
0: Integer(0)
1: String("inner")
2: String("setmetatable")
3: String("__call")
4: String("a")
5: String("d")
6: String("s")
---opcodes---
<line 1>
0: OpCode(NewTable { dest: RegisterIndex(0), array_size: 0, map_size: 0 })
1: OpCode(LoadConstant { dest: RegisterIndex(1), constant: ConstantIndex16(0) })
<line 2>
2: OpCode(Closure { dest: RegisterIndex(1), proto: PrototypeIndex(0) })
3: OpCode(SetTableCR { table: RegisterIndex(0), key: ConstantIndex8(1), value: RegisterIndex(1) })
<line 6>
4: OpCode(GetUpTableC { dest: RegisterIndex(1), table: UpValueIndex(0), key: ConstantIndex8(2) })
5: OpCode(Move { dest: RegisterIndex(2), source: RegisterIndex(0) })
6: OpCode(NewTable { dest: RegisterIndex(3), array_size: 0, map_size: 1 })
7: OpCode(Closure { dest: RegisterIndex(4), proto: PrototypeIndex(1) })
8: OpCode(SetTableCR { table: RegisterIndex(3), key: ConstantIndex8(3), value: RegisterIndex(4) })
9: OpCode(LoadConstant { dest: RegisterIndex(4), constant: ConstantIndex16(0) })
10: OpCode(Call { func: RegisterIndex(1), args: VarCount(Opt254(Some(2))), returns: VarCount(Opt254(Some(0))) })
<line 13>
11: OpCode(Move { dest: RegisterIndex(1), source: RegisterIndex(0) })
12: OpCode(LoadConstant { dest: RegisterIndex(2), constant: ConstantIndex16(4) })
13: OpCode(Call { func: RegisterIndex(1), args: VarCount(Opt254(Some(1))), returns: VarCount(Opt254(Some(1))) })
<line 15>
14: OpCode(Closure { dest: RegisterIndex(2), proto: PrototypeIndex(2) })
15: OpCode(SetUpTableCR { table: UpValueIndex(0), key: ConstantIndex8(4), value: RegisterIndex(2) })
<line 19>
16: OpCode(Closure { dest: RegisterIndex(2), proto: PrototypeIndex(3) })
17: OpCode(SetUpTableCR { table: UpValueIndex(0), key: ConstantIndex8(5), value: RegisterIndex(2) })
<line 24>
18: OpCode(GetUpTableC { dest: RegisterIndex(2), table: UpValueIndex(0), key: ConstantIndex8(5) })
19: OpCode(LoadConstant { dest: RegisterIndex(3), constant: ConstantIndex16(6) })
20: OpCode(Call { func: RegisterIndex(2), args: VarCount(Opt254(Some(1))), returns: VarCount(Opt254(Some(0))) })
21: OpCode(Jump { offset: 0, close_upvalues: Opt254(Some(0)) })
<line 25>
22: OpCode(Return { start: RegisterIndex(0), count: VarCount(Opt254(Some(0))) })
---upvalues---
0: Environment
---prototypes---
  ===FunctionProto(0x6000016706e0)===
  fixed_params: 2, has_varargs: false, stack_size: 6
  ---constants---
  0: String("print")
  ---opcodes---
  <line 3>
  0: OpCode(GetUpTableC { dest: RegisterIndex(2), table: UpValueIndex(1), key: ConstantIndex8(0) })
  1: OpCode(Move { dest: RegisterIndex(3), source: RegisterIndex(0) })
  2: OpCode(Move { dest: RegisterIndex(4), source: RegisterIndex(1) })
  3: OpCode(GetUpValue { dest: RegisterIndex(5), source: UpValueIndex(0) })
  4: OpCode(EqRR { skip_if: false, left: RegisterIndex(5), right: RegisterIndex(1) })
  5: OpCode(Jump { offset: 1, close_upvalues: Opt254(None) })
  6: OpCode(LoadBool { dest: RegisterIndex(5), value: false, skip_next: true })
  7: OpCode(LoadBool { dest: RegisterIndex(5), value: true, skip_next: false })
  8: OpCode(Call { func: RegisterIndex(2), args: VarCount(Opt254(Some(3))), returns: VarCount(Opt254(Some(0))) })
  <line 4>
  9: OpCode(Return { start: RegisterIndex(0), count: VarCount(Opt254(Some(0))) })
  ---upvalues---
  0: ParentLocal(RegisterIndex(0))
  1: Outer(UpValueIndex(0))
  ===FunctionProto(0x600001670780)===
  fixed_params: 1, has_varargs: true, stack_size: 2
  ---constants---
  0: String("inner")
  ---opcodes---
  <line 8>
  0: OpCode(GetUpTableC { dest: RegisterIndex(1), table: UpValueIndex(0), key: ConstantIndex8(0) })
  1: OpCode(VarArgs { dest: RegisterIndex(2), count: VarCount(Opt254(None)) })
  2: OpCode(Call { func: RegisterIndex(1), args: VarCount(Opt254(None)), returns: VarCount(Opt254(Some(0))) })
  <line 9>
  3: OpCode(GetUpTableC { dest: RegisterIndex(1), table: UpValueIndex(0), key: ConstantIndex8(0) })
  4: OpCode(VarArgs { dest: RegisterIndex(2), count: VarCount(Opt254(None)) })
  5: OpCode(TailCall { func: RegisterIndex(1), args: VarCount(Opt254(None)) })
  <line 10>
  6: OpCode(Return { start: RegisterIndex(0), count: VarCount(Opt254(Some(0))) })
  ---upvalues---
  0: ParentLocal(RegisterIndex(0))
  ===FunctionProto(0x600001670820)===
  fixed_params: 2, has_varargs: false, stack_size: 6
  ---constants---
  0: String("print")
  ---opcodes---
  <line 16>
  0: OpCode(GetUpTableC { dest: RegisterIndex(2), table: UpValueIndex(1), key: ConstantIndex8(0) })
  1: OpCode(Move { dest: RegisterIndex(3), source: RegisterIndex(0) })
  2: OpCode(Move { dest: RegisterIndex(4), source: RegisterIndex(1) })
  3: OpCode(GetUpValue { dest: RegisterIndex(5), source: UpValueIndex(0) })
  4: OpCode(EqRR { skip_if: false, left: RegisterIndex(0), right: RegisterIndex(5) })
  5: OpCode(Jump { offset: 1, close_upvalues: Opt254(None) })
  6: OpCode(LoadBool { dest: RegisterIndex(5), value: false, skip_next: true })
  7: OpCode(LoadBool { dest: RegisterIndex(5), value: true, skip_next: false })
  8: OpCode(Call { func: RegisterIndex(2), args: VarCount(Opt254(Some(3))), returns: VarCount(Opt254(Some(0))) })
  <line 17>
  9: OpCode(Return { start: RegisterIndex(0), count: VarCount(Opt254(Some(0))) })
  ---upvalues---
  0: ParentLocal(RegisterIndex(1))
  1: Outer(UpValueIndex(0))
  ===FunctionProto(0x6000016708c0)===
  fixed_params: 1, has_varargs: true, stack_size: 2
  ---opcodes---
  <line 20>
  0: OpCode(GetUpValue { dest: RegisterIndex(1), source: UpValueIndex(0) })
  1: OpCode(VarArgs { dest: RegisterIndex(2), count: VarCount(Opt254(None)) })
  2: OpCode(Call { func: RegisterIndex(1), args: VarCount(Opt254(None)), returns: VarCount(Opt254(Some(0))) })
  <line 21>
  3: OpCode(GetUpValue { dest: RegisterIndex(1), source: UpValueIndex(0) })
  4: OpCode(VarArgs { dest: RegisterIndex(2), count: VarCount(Opt254(None)) })
  5: OpCode(TailCall { func: RegisterIndex(1), args: VarCount(Opt254(None)) })
  <line 22>
  6: OpCode(Return { start: RegisterIndex(0), count: VarCount(Opt254(Some(0))) })
  ---upvalues---
  0: ParentLocal(RegisterIndex(1))
Jengamon commented 4 months ago

Calling just the bottom part of the example above gives this output in Piccolo:

nil     nil     false
s       <function [ADDR]>       false
kyren commented 4 months ago

The second example is an entirely separate bug to do with local variables and function a() function declaration syntax (edit: now fixed by 3c3118ff47ea7d9c637a4fdf790240b68330f731)