fix: Missing Type Check for Built-in Function Arguments (len, cap) Causes Runtime Panic #3454 #4033

KemalBekir · 2025-03-30T22:07:10Z

Invalid arguments provided to builtin functions len,cap and make would compile successfully but it was triggering a runtime panic when executed.

fixes #3454

Gno2D2 · 2025-03-30T22:08:22Z

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):

IGNORE the bot requirements for this PR (force green CI check)
The pull request description provides enough details

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info)
🟢 Pending initial approval by a review team member, or review from tech-staff

☑️ Contributor Actions:

Fix any issues flagged by automated checks.
Follow the Contributor Checklist to ensure your PR is ready for review.
- Add new tests, or document why they are unnecessary.
- Provide clear examples/screenshots, if necessary.
- Update documentation, if required.
- Ensure no breaking changes, or include BREAKING CHANGE notes.
- Link related issues/PRs, where applicable.

☑️ Reviewer Actions:

Complete manual checks for the PR, including the guidelines and additional checks if applicable.

📚 Resources:

Debug

Automated Checks

Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: KemalBekir/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Pending initial approval by a review team member, or review from tech-staff

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)

Then

🟢 Requirement satisfied
└── 🟢 If
    ├── 🟢 Condition
    │   └── 🟢 Or
    │       ├── 🔴 At least 1 user(s) of the organization reviewed the pull request (with state "APPROVED")
    │       ├── 🟢 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🟢 Then
        └── 🟢 Not (🔴 This label is applied to pull request: review/triage-pending)

Manual Checks

**IGNORE** the bot requirements for this PR (force green CI check)
If
🟢 Condition met
└── 🟢 On every pull request
Can be checked by

Any user with comment edit permission
The pull request description provides enough details
If
🟢 Condition met
└── 🟢 And
    ├── 🟢 Not (🔴 Pull request author is a member of the team: core-contributors)
    └── 🟢 Not (🔴 Pull request author is user: dependabot[bot])
Can be checked by

team core-contributors

thehowl · 2025-03-31T14:10:03Z

gnovm/pkg/gnolang/gno_test.go

These should be filetests

I will move them there.

thehowl

a first sanity review

thehowl · 2025-03-31T14:16:04Z

gnovm/pkg/gnolang/preprocess.go

+const (
+	BuiltinLen  = "len"
+	BuiltinCap  = "cap"
+	BuiltinMake = "make"
+)


We don't use constant names, so just use the raw strings in the comparison above.

thehowl · 2025-03-31T14:19:31Z

gnovm/pkg/gnolang/preprocess.go

@@ -1362,6 +1363,10 @@ func preprocess1(store Store, ctx BlockNode, n Node) Node {
 				ift := evalStaticTypeOf(store, last, n.Func)
 				switch cft := baseOf(ift).(type) {
 				case *FuncType:
+					fn := extractFunctionName(n)
+					if fn == BuiltinMake || fn == BuiltinCap || fn == BuiltinLen {
+						assertValidBuiltinArgType(store, last, n)


Just use evalStaticTypeOf on each argument.

thehowl · 2025-03-31T14:20:29Z

gnovm/pkg/gnolang/preprocess.go

+	case *ConstExpr:
+		assertValidBuiltinArgType(store, last, currExpr.Source)
+	case *NameExpr:
+		assertValidBuiltinArgType(store, last, currExpr)


This is an infinite loop

petar-dambovaliev

Looks okay. Just fix the requested changes.

petar-dambovaliev · 2025-04-01T01:46:38Z

gnovm/pkg/gnolang/preprocess.go

+	case *BasicLitExpr:
+		var num int64
+		if e.Kind == FLOAT {
+			f, err := hasFractionalPart(e.Value)


Can you change the name of this function? The name looks like it returns a boolean but actually returns a number and an error.

ltzmaxwell

a first round review.

ltzmaxwell · 2025-04-07T03:28:35Z

gnovm/pkg/gnolang/preprocess.go

+
+	switch at := unwrapPointerType(baseOf(at)).(type) {
+	case PrimitiveType:
+		if at.Kind() != StringKind && at != UntypedStringType {


seems at.Kind() != StringKind is already good.

ltzmaxwell · 2025-04-07T03:34:37Z

gnovm/pkg/gnolang/preprocess.go

+		if capacity < length {
+			capacity = length
+		}


shouldn't just panic here?

ltzmaxwell · 2025-04-07T03:37:48Z

gnovm/pkg/gnolang/preprocess.go

+
+// Check for invalid arguments in builtin functions
+// len, cap, make
+func assertValidBuiltinArgType(store Store, last BlockNode, currExpr Expr) {
+	switch currExpr := currExpr.(type) {
+	case *ConstExpr:
+		assertValidBuiltinArgType(store, last, currExpr.Source)
+	case *CallExpr:
+		ift := evalStaticTypeOf(store, last, currExpr.Func)
+		switch baseOf(ift).(type) {
+		case *FuncType:


it'll be better to put these checks in type_check.go.

ltzmaxwell · 2025-04-07T03:53:25Z

gnovm/tests/files/cap8.gno

@@ -6,4 +6,4 @@ func main() {
 }

 // Error:
-// unexpected type for cap(): *int
+// main/files/cap8.gno:5:17: unexpected type for cap(): int


looks not right, the type should be *int.

ltzmaxwell · 2025-04-07T04:02:15Z

gnovm/pkg/gnolang/preprocess.go

+	if f != intPart {
+		return f, fmt.Errorf("invalid argument: %s has a fractional part", s)
+	}


is there a test cover this?

KemalBekir added 3 commits March 3, 2025 00:55

gnolang#3454 - to be fixed not all tests passing

e23dfc3

To be fixed: Some Gnovm tests fail

1f456ee

fixed: tests passing

ea05c64

github-actions bot assigned KemalBekir Mar 30, 2025

github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label Mar 30, 2025

Gno2D2 requested a review from a team March 30, 2025 22:08

Gno2D2 added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Mar 30, 2025

KemalBekir changed the title ~~fix:Missing Type Check for Built-in Function Arguments (len, cap) Causes Runtime Panic #3454~~ fix: Missing Type Check for Built-in Function Arguments (len, cap) Causes Runtime Panic #3454 Mar 30, 2025

zivkovicmilos added this to the 🚀 Mainnet beta launch milestone Mar 31, 2025

zivkovicmilos requested review from thehowl, ltzmaxwell and petar-dambovaliev and removed request for a team March 31, 2025 08:43

thehowl reviewed Mar 31, 2025

View reviewed changes

petar-dambovaliev requested changes Apr 1, 2025

View reviewed changes

jefft0 removed the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Apr 1, 2025

KemalBekir added 2 commits April 4, 2025 21:18

PR review requested changes

f61cb2f

fixed issue with linter

b31fcab

ltzmaxwell requested changes Apr 7, 2025

View reviewed changes

ltzmaxwell reviewed Apr 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Missing Type Check for Built-in Function Arguments (len, cap) Causes Runtime Panic #3454 #4033

fix: Missing Type Check for Built-in Function Arguments (len, cap) Causes Runtime Panic #3454 #4033

KemalBekir commented Mar 30, 2025

Gno2D2 commented Mar 30, 2025 •

edited

Loading

✅ Automated Checks (for Contributors):

☑️ Contributor Actions:

☑️ Reviewer Actions:

📚 Resources:

If

Then

If

Then

If

Can be checked by

If

Can be checked by

thehowl Mar 31, 2025

KemalBekir Apr 2, 2025

thehowl left a comment

thehowl Mar 31, 2025

thehowl Mar 31, 2025

thehowl Mar 31, 2025

petar-dambovaliev left a comment

petar-dambovaliev Apr 1, 2025

ltzmaxwell left a comment

ltzmaxwell Apr 7, 2025

ltzmaxwell Apr 7, 2025

ltzmaxwell Apr 7, 2025

ltzmaxwell Apr 7, 2025

ltzmaxwell Apr 7, 2025

fix: Missing Type Check for Built-in Function Arguments (len, cap) Causes Runtime Panic #3454 #4033

Are you sure you want to change the base?

fix: Missing Type Check for Built-in Function Arguments (len, cap) Causes Runtime Panic #3454 #4033

Conversation

KemalBekir commented Mar 30, 2025

Gno2D2 commented Mar 30, 2025 • edited Loading

🛠 PR Checks Summary

Manual Checks (for Reviewers):

✅ Automated Checks (for Contributors):

☑️ Contributor Actions:

☑️ Reviewer Actions:

📚 Resources:

If

Then

If

Then

If

Can be checked by

If

Can be checked by

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thehowl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petar-dambovaliev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ltzmaxwell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gno2D2 commented Mar 30, 2025 •

edited

Loading