Panic when returning from a lua function directly, but not if I store the results in a variable first #46

Closed
mediocregopher opened this Issue Nov 10, 2015 · 3 comments

Projects

None yet

2 participants

@mediocregopher
package main

import (
    "bytes"
    "log"

    "github.com/Shopify/go-lua"
)

func main() {
    l := lua.NewState()
    lua.OpenLibraries(l)

    loadInline := func(code string) {
        if err := l.Load(bytes.NewBufferString(code), "", "bt"); err != nil {
            log.Fatal(err)
        }
    }

    // Load in a global function
    loadInline(`
        function doingStuff(a, b)
            return a + b
        end
    `)
    l.Call(0, 0)

    // Call the function, first storing the result in a local variable
    loadInline(`
        local res = doingStuff(1, 2)
        return res
    `)
    l.Call(0, 1)
    i, _ := l.ToInteger(-1)
    log.Printf("i: %d", i)
    l.Remove(-1)

    // Call the function, directly returning the result from it
    loadInline(`
        return doingStuff(1, 2)
    `)
    l.Call(0, 1)
    i, _ = l.ToInteger(-1)
    log.Printf("i: %d", i)
    l.Remove(-1)
}

So first I'm loading a more or less useless function into the global namespace. Then I go to call it. If I Store the return value as a local variable It works fine and prints out 3 like it should. However, if I just directly return the result without first storing it I get a panic:

panic: runtime error: index out of range

goroutine 1 [running]:
github.com/Shopify/go-lua.func·125(0xc2080485f0, 0xc200400006, 0x5d6ca8, 0xc200400006)
        /home/mediocregopher/src/go/src/github.com/Shopify/go-lua/vm.go:358 +0x3fc
github.com/Shopify/go-lua.(*State).executeFunctionTable(0xc208038000)
        /home/mediocregopher/src/go/src/github.com/Shopify/go-lua/vm.go:942 +0x22e
github.com/Shopify/go-lua.(*State).execute(0xc208038000)
        /home/mediocregopher/src/go/src/github.com/Shopify/go-lua/vm.go:928 +0x28
github.com/Shopify/go-lua.(*State).call(0xc208038000, 0x1, 0x1, 0xc208010000)
        /home/mediocregopher/src/go/src/github.com/Shopify/go-lua/stack.go:381 +0xa6
github.com/Shopify/go-lua.(*State).CallWithContinuation(0xc208038000, 0x0, 0x1, 0x0, 0x0)
        /home/mediocregopher/src/go/src/github.com/Shopify/go-lua/lua.go:349 +0x126
github.com/Shopify/go-lua.(*State).Call(0xc208038000, 0x0, 0x1)
        /home/mediocregopher/src/go/src/github.com/Shopify/go-lua/lua.go:1345 +0x4e
main.main()
        /tmp/wat.go:42 +0x240

Could this be a bug with the shopify implementation, or is what I'm doing fundamentally incorrect in some way? Thanks!

@mediocregopher

Bump, I've confirmed this is still happening on master.

@fbogsany
Member
fbogsany commented Mar 1, 2016

This is a known bug, but I haven't had time to get to the bottom of it (busy with a new kid). The workaround is to add parentheses to the return statement:

return (doingStuff(1, 2))

This seems to be a problem with tail calls:

return doingStuff(1, 2)

compiles as:

  0: GETTABUP 0 0 constant 0
  1: LOADK 1 1
  2: LOADK 2 2
  3: TAILCALL 0 3 0
  4: RETURN 0 0

whereas

return (doingStuff(1, 2))

and

local res = doingStuff(1, 2)
return res

both compile as:

  0: GETTABUP 0 0 constant 0
  1: LOADK 1 1
  2: LOADK 2 2
  3: CALL 0 3 2
  4: RETURN1 0 2
@mediocregopher

Ah alright, thanks for responding, mostly wanted to make sure this wasn't
me misunderstanding something on my end.

On Mon, Feb 29, 2016, 6:34 PM Francis Bogsanyi notifications@github.com
wrote:

This is a known bug, but I haven't had time to get to the bottom of it
(busy with a new kid). The workaround is to add parentheses to the return
statement:

return (doingStuff(1, 2))

This seems to be a problem with tail calls:

return doingStuff(1, 2)

compiles as:

0: GETTABUP 0 0 constant 0
1: LOADK 1 1
2: LOADK 2 2
3: TAILCALL 0 3 0
4: RETURN 0 0

whereas

return (doingStuff(1, 2))

and

local res = doingStuff(1, 2)return res

both compile as:

0: GETTABUP 0 0 constant 0
1: LOADK 1 1
2: LOADK 2 2
3: CALL 0 3 2
4: RETURN1 0 2


Reply to this email directly or view it on GitHub
#46 (comment).

@nilium nilium added a commit to Kochava/go-lua that referenced this issue Aug 9, 2016
@nilium nilium Add test to catch tail call error
No fix for this yet. Still looking through code and poking things.

Related to issue #46.
0a26862
@nilium nilium added a commit to Kochava/go-lua that referenced this issue Aug 29, 2016
@nilium nilium Add additional tests to narrow down tailcall issue
These should help identify the specific cases where opTailCall fails.
In general, failures only occur if oci and nci refer to different
functions/codepaths (i.e., the PC would different in C Lua 5.2).

Related to issue #46.
f667b63
@nilium nilium added a commit to Kochava/go-lua that referenced this issue Aug 29, 2016
@nilium nilium Copy code slice from nci to oci
This appears to be a basic error in converting Lua 5.2 from C to Go.
Since C allows pointer arithmetic, it made sense to just have savedpc
be a pointer into the instruction table and increment it as needed. So,
when Lua in C copies savedpc from nci to oci, that's equivalent to
taking copying both nci.savedPC and nci.code in Go.

Closes #46.
7d859e3
@nilium nilium added a commit to Kochava/go-lua that referenced this issue Aug 29, 2016
@nilium nilium Copy code slice from nci to oci
This appears to be a basic error in converting Lua 5.2 from C to Go.
Since C allows pointer arithmetic, it made sense to just have savedpc
be a pointer into the instruction table and increment it as needed. So,
when Lua in C copies savedpc from nci to oci, that's equivalent to
taking copying both nci.savedPC and nci.code in Go.

Closes #46.
63dee41
@nilium nilium added a commit to Kochava/go-lua that referenced this issue Aug 29, 2016
@nilium nilium Merge branch 'issue-46-fix-tailcall-split' into accum-fixes
Merge change to address opTailCall issue where codepaths code were
split between old and new callInfo objects. (#46)

* issue-46-fix-tailcall-split:
  Copy code slice from nci to oci
  Add additional tests to narrow down tailcall issue
  Add test to catch tail call error
19d007e
@fbogsany fbogsany pushed a commit that closed this issue Sep 22, 2016
@nilium nilium Copy code slice from nci to oci
This appears to be a basic error in converting Lua 5.2 from C to Go.
Since C allows pointer arithmetic, it made sense to just have savedpc
be a pointer into the instruction table and increment it as needed. So,
when Lua in C copies savedpc from nci to oci, that's equivalent to
taking copying both nci.savedPC and nci.code in Go.

Closes #46.
f3808c9
@fbogsany fbogsany closed this in f3808c9 Sep 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment