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

Invalid behavior in some functions with return parameters #898

Open
zapateo opened this issue Oct 5, 2021 · 5 comments · May be fixed by #920
Open

Invalid behavior in some functions with return parameters #898

zapateo opened this issue Oct 5, 2021 · 5 comments · May be fixed by #920
Assignees
Labels
bug Bug: something already implemented does not work as it should InvalidBehavior A valid or not valid code has an invalid behavior at runtime needsInvestigation Need to investigate

Comments

@zapateo
Copy link
Member

zapateo commented Oct 5, 2021

Source code

package main

import (
        "fmt"
)

func f() (err error) {
        defer func() {
                _ = recover()
        }()
        var i int
        _ = 1 / i
        return nil
}

func main() {
        fmt.Println(f())
}

Scriggo output

runtime error: integer divide by zero

gc output

<nil>
@zapateo zapateo added bug Bug: something already implemented does not work as it should needsInvestigation Need to investigate labels Oct 5, 2021
@zapateo zapateo self-assigned this Oct 5, 2021
@zapateo zapateo changed the title Inconsistent behavior [investigate] Invalid behaviour with panic/recover Oct 5, 2021
@zapateo zapateo removed their assignment Oct 5, 2021
@zapateo zapateo added the InvalidBehavior A valid or not valid code has an invalid behavior at runtime label Oct 5, 2021
@gazerro gazerro added the emitter/builder Related to VM's emitter and builder label Oct 6, 2021
@gazerro gazerro changed the title Invalid behaviour with panic/recover emitter: invalid behavior recovering when the surrounding function has named result Oct 6, 2021
@gazerro gazerro changed the title emitter: invalid behavior recovering when the surrounding function has named result emitter: invalid behavior deferring when the surrounding function has named result Oct 6, 2021
@gazerro
Copy link
Member

gazerro commented Oct 6, 2021

The function f does not panic, it returns the value returned from the recover built-in instead of nil.

The disassembled code is

Package main

Func f() (g1 error)
	; regs(3,0,0,3)
	Move nil g2
	Move g2 g1
	Func g3 func()
		; regs(0,0,0,1)
		Recover g1   <-- ERROR: it writes into register g1 which is the named result
		Return
	Move g3 g2
	Defer g2 _ _ _ _	; func()
	Move 0 i1
	Load 1 i3
	Div i3 i1 i2
	Move nil g2
	Move g2 g1
	Return
	Return

Func main()
	; regs(0,0,0,2)
	Call main.f _ _ _ g1	; func() (g1 error)
	Move g1 g2
	Print g2
	Typify string "\n" g1
	Print g1
	Return

The error is not due to the recover built-in. Replacing it with the make built-in, the error is the same:

package main

func f() (err error) {
        defer func() {
                _ = make([]int, 1)
        }()
        return nil
}

func main() {
        println(f())
}

and disassembled:

Package main

Func f() (g1 error)
	; regs(0,0,0,3)
	Move nil g2
	Move g2 g1
	Func g3 func()
		; regs(0,0,0,1)
		MakeSlice int 1 1 g1   <-- ERROR: it writes into register g1 which is the named result
		Return
	Move g3 g2
	Defer g2 _ _ _ _	; func()
	Move nil g2
	Move g2 g1
	Return
	Return

Func main()
	; regs(0,0,0,2)
	Call main.f _ _ _ g1	; func() (g1 error)
	Move g1 g2
	Print g2
	Typify string "\n" g1
	Print g1
	Return

@zapateo
Copy link
Member Author

zapateo commented Oct 7, 2021

Another version of this issue:

package main

import (
        "fmt"
)

func s() []int { return []int{} }

func f() (err error) {
        defer func() {
                _ = recover()
                _ = s()
        }()
        var i int
        _ = 1 / i
        return nil
}

func main() {
        fmt.Println(f())
}

Yet another version of the issue:

package main

import (
	"fmt"
)

func f() interface{} {
	defer func() {
		_ = []int{}
		recover()
	}()
	panic(0)
}

func main() {
	fmt.Println(f())
}

May be the problem related to the finalizer?

@zapateo zapateo self-assigned this Oct 7, 2021
@zapateo zapateo removed the emitter/builder Related to VM's emitter and builder label Oct 7, 2021
@zapateo zapateo changed the title emitter: invalid behavior deferring when the surrounding function has named result Invalid behavior in some functions with return parameters Oct 7, 2021
@zapateo
Copy link
Member Author

zapateo commented Oct 7, 2021

A simplified version of the issue:

package main

import (
	"fmt"
)

func f() []int {

	defer func() {
		recover()
	}()
	panic("")

}

func main() {
	fmt.Println(f())
}

@zapateo
Copy link
Member Author

zapateo commented Nov 10, 2021

First problem

The problem of uninitialized registers may be solved by changing prepareFunctionBodyParameters (see the PR associated to this issue).

Second problem (just an optimization)

The code above should be executed only if the function has at least one defer statement in it; otherwise, it's useful. This information may be returned by the type checker.

Third problem

Even implementing the above changes, the problem continues to arise with some tests. This appears to be caused by something wrong in the emission of the statement defer; perhaps it is related to stack shift.

@zapateo zapateo linked a pull request Nov 10, 2021 that will close this issue
@zapateo
Copy link
Member Author

zapateo commented Nov 10, 2021

@gazerro

The linked PR fixes the problem of registers initialization, but still remains something related to the defer statement.

This code shows the invalid behaviour:

package main

import (
	"fmt"
)

func f() interface{} {
	defer func() {
		_ = []int{}
		recover()
	}()
	panic(0)
}

func main() {
	fmt.Println(f())
}

It should print <nil> but it prints [] instead; this is probably caused by the deferred function that writes the []int value on a register owned by the f function.

@zapateo zapateo assigned gazerro and unassigned zapateo Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug: something already implemented does not work as it should InvalidBehavior A valid or not valid code has an invalid behavior at runtime needsInvestigation Need to investigate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants