From 9b9d72937b388f709cf2ee070b8cb015637e5c0c Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Wed, 17 Aug 2022 12:19:37 +0300 Subject: [PATCH] compiler: restrict return values count for manifest methods Exported functions from main package shouldn't have more than one return value. --- pkg/compiler/analysis.go | 14 +++- pkg/compiler/compiler_test.go | 145 ++++++++++++++++++++++++++++++++++ pkg/compiler/return_test.go | 24 +++--- 3 files changed, 166 insertions(+), 17 deletions(-) diff --git a/pkg/compiler/analysis.go b/pkg/compiler/analysis.go index e4211dcb2..c447cd5fe 100644 --- a/pkg/compiler/analysis.go +++ b/pkg/compiler/analysis.go @@ -13,8 +13,13 @@ import ( "golang.org/x/tools/go/packages" ) -// ErrMissingExportedParamName is returned when exported contract method has unnamed parameter. -var ErrMissingExportedParamName = errors.New("exported method is not allowed to have unnamed parameter") +// Various exported functions usage errors. +var ( + // ErrMissingExportedParamName is returned when exported contract method has unnamed parameter. + ErrMissingExportedParamName = errors.New("exported method is not allowed to have unnamed parameter") + // ErrInvalidExportedRetCount is returned when exported contract method has invalid return values count. + ErrInvalidExportedRetCount = errors.New("exported method is not allowed to have more than one return value") +) var ( // Go language builtin functions. @@ -289,7 +294,7 @@ func (c *codegen) analyzeFuncUsage() funcUsage { if isMain && n.Name.IsExported() || isInitFunc(n) || isDeployFunc(n) { diff[name] = true } - // exported functions are not allowed to have unnamed parameters + // exported functions are not allowed to have unnamed parameters or multiple return values if isMain && n.Name.IsExported() && n.Recv == nil { if n.Type.Params.List != nil { for i, param := range n.Type.Params.List { @@ -305,6 +310,9 @@ func (c *codegen) analyzeFuncUsage() funcUsage { } } } + if retCnt := n.Type.Results.NumFields(); retCnt > 1 { + c.prog.Err = fmt.Errorf("%w: %s/%d return values", ErrInvalidExportedRetCount, n.Name, retCnt) + } } nodeCache[name] = declPair{n, c.importMap, pkgPath} return false // will be processed in the next stage diff --git a/pkg/compiler/compiler_test.go b/pkg/compiler/compiler_test.go index a3c136a89..0e0756128 100644 --- a/pkg/compiler/compiler_test.go +++ b/pkg/compiler/compiler_test.go @@ -2,6 +2,7 @@ package compiler_test import ( "fmt" + "math/big" "os" "path/filepath" "strings" @@ -427,3 +428,147 @@ func TestUnnamedParameterCheck(t *testing.T) { require.NoError(t, err) // it's OK for exported method to have unnamed params as it won't be included into manifest }) } + +func TestReturnValuesCountCheck(t *testing.T) { + t.Run("void", func(t *testing.T) { + t.Run("exported", func(t *testing.T) { + t.Run("func", func(t *testing.T) { + src := `package testcase + var a int + func Main() { + a = 5 + }` + _, _, err := compiler.CompileWithOptions("test.go", strings.NewReader(src), nil) + require.NoError(t, err) + }) + t.Run("method", func(t *testing.T) { + src := `package testcase + type A int + var a int + func (rcv A) Main() { + a = 5 + }` + _, _, err := compiler.CompileWithOptions("test.go", strings.NewReader(src), nil) + require.NoError(t, err) + }) + }) + t.Run("unexported", func(t *testing.T) { + src := `package testcase + var a int + func main() { + a = 5 + }` + _, _, err := compiler.CompileWithOptions("test.go", strings.NewReader(src), nil) + require.NoError(t, err) + }) + }) + t.Run("single return", func(t *testing.T) { + t.Run("exported", func(t *testing.T) { + t.Run("func", func(t *testing.T) { + src := `package testcase + var a int + func Main() int { + a = 5 + return a + }` + eval(t, src, big.NewInt(5)) + }) + t.Run("method", func(t *testing.T) { + src := `package testcase + type A int + var a int + func (rcv A) Main() int { + a = 5 + return a + }` + _, _, err := compiler.CompileWithOptions("test.go", strings.NewReader(src), nil) + require.NoError(t, err) + }) + }) + t.Run("unexported", func(t *testing.T) { + src := `package testcase + var a int + func main() int { + a = 5 + return a + }` + _, _, err := compiler.CompileWithOptions("test.go", strings.NewReader(src), nil) + require.NoError(t, err) + }) + }) + t.Run("multiple unnamed return vals", func(t *testing.T) { + t.Run("exported", func(t *testing.T) { + t.Run("func", func(t *testing.T) { + src := `package testcase + var a int + func Main() (int, int) { + a = 5 + return a, a + }` + _, _, err := compiler.CompileWithOptions("test.go", strings.NewReader(src), nil) + require.Error(t, err) + require.ErrorIs(t, err, compiler.ErrInvalidExportedRetCount) + }) + t.Run("method", func(t *testing.T) { + src := `package testcase + type A int + var a int + func (rcv A) Main() (int, int) { + a = 5 + return a, a + }` + _, _, err := compiler.CompileWithOptions("test.go", strings.NewReader(src), nil) + require.NoError(t, err) // OK for method to have multiple return values as it won't be included into manifest + }) + }) + t.Run("unexported", func(t *testing.T) { + src := `package testcase + var a int + func main() (int, int) { + a = 5 + return a, a + }` + _, _, err := compiler.CompileWithOptions("test.go", strings.NewReader(src), nil) + require.NoError(t, err) // OK for unexported function to have multiple return values as it won't be included into manifest + }) + }) + t.Run("multiple named return vals", func(t *testing.T) { + t.Run("exported", func(t *testing.T) { + t.Run("func", func(t *testing.T) { + src := `package testcase + var a int + func Main() (a int, b int) { + a = 5 + b = 2 + return + }` + _, _, err := compiler.CompileWithOptions("test.go", strings.NewReader(src), nil) + require.Error(t, err) + require.ErrorIs(t, err, compiler.ErrInvalidExportedRetCount) + }) + t.Run("method", func(t *testing.T) { + src := `package testcase + type A int + var a int + func (rcv A) Main() (a int, b int) { + a = 5 + b = 2 + return + }` + _, _, err := compiler.CompileWithOptions("test.go", strings.NewReader(src), nil) + require.NoError(t, err) // OK for method to have multiple return values as it won't be included into manifest + }) + }) + t.Run("unexported", func(t *testing.T) { + src := `package testcase + var a int + func main() (a int, b int) { + a = 5 + b = 2 + return + }` + _, _, err := compiler.CompileWithOptions("test.go", strings.NewReader(src), nil) + require.NoError(t, err) // OK for unexported function to have multiple return values as it won't be included into manifest + }) + }) +} diff --git a/pkg/compiler/return_test.go b/pkg/compiler/return_test.go index 1a822c3f0..ead69d2fb 100644 --- a/pkg/compiler/return_test.go +++ b/pkg/compiler/return_test.go @@ -4,9 +4,6 @@ import ( "fmt" "math/big" "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestReturnInt64(t *testing.T) { @@ -99,7 +96,11 @@ func TestSingleReturn(t *testing.T) { func TestNamedReturn(t *testing.T) { src := `package foo - func Main() (a int, b int) { + func Main() int { + a, b := f() + return a + b + } + func f() (a int, b int) { a = 1 b = 2 c := 3 @@ -107,21 +108,16 @@ func TestNamedReturn(t *testing.T) { return %s }` - runCase := func(ret string, result ...interface{}) func(t *testing.T) { + runCase := func(ret string, result *big.Int) func(t *testing.T) { return func(t *testing.T) { src := fmt.Sprintf(src, ret) - v := vmAndCompile(t, src) - require.NoError(t, v.Run()) - require.Equal(t, len(result), v.Estack().Len()) - for i := range result { - assert.EqualValues(t, result[i], v.Estack().Pop().Value()) - } + eval(t, src, result) } } - t.Run("NormalReturn", runCase("a, b", big.NewInt(1), big.NewInt(2))) - t.Run("EmptyReturn", runCase("", big.NewInt(1), big.NewInt(2))) - t.Run("AnotherVariable", runCase("b, c", big.NewInt(2), big.NewInt(3))) + t.Run("NormalReturn", runCase("a, b", big.NewInt(3))) + t.Run("EmptyReturn", runCase("", big.NewInt(3))) + t.Run("AnotherVariable", runCase("b, c", big.NewInt(5))) } func TestTypeAssertReturn(t *testing.T) {