Skip to content

Commit

Permalink
fix(gnovm): fix max names in block check (#2645)
Browse files Browse the repository at this point in the history
I noticed this while reviewing #2429. The previous conditional could
never evaluate to true: `if (1<<16 - 1) < sb.NumNames {`. `sb.NumNames`
is a `uint16` and the value it is being compared to, `1<<16 - 1` is the
max uint16 value, so it is impossible the the number of names to be
greater than this value. Instead, we should check to see if the current
number of names is the maximum value and panic if this is the case.

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
- [x] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
  • Loading branch information
deelawn authored Jul 31, 2024
1 parent 5f28803 commit 14b9015
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 1 deletion.
3 changes: 2 additions & 1 deletion gnovm/pkg/gnolang/nodes.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"go/parser"
"go/token"
"math"
"os"
"path/filepath"
"reflect"
Expand Down Expand Up @@ -1802,7 +1803,7 @@ func (sb *StaticBlock) Define2(isConst bool, n Name, st Type, tv TypedValue) {
if int(sb.NumNames) != len(sb.Names) {
panic("StaticBlock.NumNames and len(.Names) mismatch")
}
if (1<<16 - 1) < sb.NumNames {
if sb.NumNames == math.MaxUint16 {
panic("too many variables in block")
}
if tv.T == nil && tv.V != nil {
Expand Down
44 changes: 44 additions & 0 deletions gnovm/pkg/gnolang/nodes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package gnolang_test

import (
"math"
"testing"

"github.com/gnolang/gno/gnovm/pkg/gnolang"
)

func TestStaticBlock_Define2_MaxNames(t *testing.T) {
defer func() {
if r := recover(); r != nil {
panicString, ok := r.(string)
if !ok {
t.Errorf("expected panic string, got %v", r)
}

if panicString != "too many variables in block" {
t.Errorf("expected panic string to be 'too many variables in block', got '%s'", panicString)
}

return
}

// If it didn't panic, fail.
t.Errorf("expected panic when exceeding maximum number of names")
}()

staticBlock := new(gnolang.StaticBlock)
staticBlock.NumNames = math.MaxUint16 - 1
staticBlock.Names = make([]gnolang.Name, staticBlock.NumNames)

// Adding one more is okay.
staticBlock.Define2(false, gnolang.Name("a"), gnolang.BoolType, gnolang.TypedValue{T: gnolang.BoolType})
if staticBlock.NumNames != math.MaxUint16 {
t.Errorf("expected NumNames to be %d, got %d", math.MaxUint16, staticBlock.NumNames)
}
if len(staticBlock.Names) != math.MaxUint16 {
t.Errorf("expected len(Names) to be %d, got %d", math.MaxUint16, len(staticBlock.Names))
}

// This one should panic because the maximum number of names has been reached.
staticBlock.Define2(false, gnolang.Name("a"), gnolang.BoolType, gnolang.TypedValue{T: gnolang.BoolType})
}

0 comments on commit 14b9015

Please sign in to comment.