Skip to content

fix(gnovm): track block item allocations in PrepareNewValues#5436

Open
omarsy wants to merge 6 commits intognolang:masterfrom
omarsy:fix/gc-alloc-mismatch-pr
Open

fix(gnovm): track block item allocations in PrepareNewValues#5436
omarsy wants to merge 6 commits intognolang:masterfrom
omarsy:fix/gc-alloc-mismatch-pr

Conversation

@omarsy
Copy link
Copy Markdown
Member

@omarsy omarsy commented Apr 6, 2026

Summary

  • Fix GC allocation/recount mismatch that causes "should not happen, allocation limit exceeded while gc." panic during infinite recursion
  • Root cause: PrepareNewValues appends block items without calling AllocateBlockItems, so GC recount exceeds alloc.bytes
  • Add AllocateBlockItems call before appending to block.Values in PrepareNewValues
  • Fix pre-existing nil pointer panic in GetLocalIndex debug logging

Test plan

  • New filetest alloc_12.gno verifies infinite recursion triggers correct "allocation limit exceeded" error
  • Updated gas test values for const.gno, nested_alloc.gno, slice_alloc.gno
  • Full go test ./gnovm/pkg/gnolang/ passes
  • ADR included at gnovm/adr/prxxxx_fix_gc_alloc_mismatch.md

Note: AI assisted PR — see ADR for detailed analysis.

@Gno2D2
Copy link
Copy Markdown
Collaborator

Gno2D2 commented Apr 6, 2026

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):
  • IGNORE the bot requirements for this PR (force green CI check)
Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ 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:
  1. Fix any issues flagged by automated checks.
  2. 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:
  1. 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: omarsy/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
    │       ├── 🟢 User davd-gzl already reviewed PR 5436 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

PrepareNewValues appends new block items to the package block's Values
slice without calling AllocateBlockItems. This causes GC recount to
exceed the allocator's tracked bytes, triggering a spurious
"should not happen, allocation limit exceeded while gc." panic
instead of the correct "allocation limit exceeded".

Add AllocateBlockItems call before appending to block.Values so the
allocator and GC recount stay consistent.
@omarsy omarsy force-pushed the fix/gc-alloc-mismatch-pr branch from b2d091b to 0057c3c Compare April 6, 2026 13:55
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

The added AllocateBlockItems call in PrepareNewValues increases gas
consumption by 40-80 bytes per affected code path. Update expected
gas values in integration tests and gas_test.go accordingly.
@github-actions github-actions bot added the 📦 ⛰️ gno.land Issues or PRs gno.land package related label Apr 6, 2026
@omarsy omarsy requested review from a team, ltzmaxwell and thehowl April 6, 2026 14:45
@omarsy omarsy marked this pull request as ready for review April 6, 2026 14:46
@Gno2D2 Gno2D2 added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Apr 6, 2026
@omarsy omarsy force-pushed the fix/gc-alloc-mismatch-pr branch from 910c2ac to d758618 Compare April 6, 2026 15:24
@davd-gzl davd-gzl self-requested a review April 6, 2026 19:02
Copy link
Copy Markdown
Member

@davd-gzl davd-gzl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any other similar vulnerabilities in the code-base?

@Gno2D2 Gno2D2 removed the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📦 ⛰️ gno.land Issues or PRs gno.land package related 📦 🤖 gnovm Issues or PRs gnovm related

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants