Visitar URL original
fix: reject @locals values that overflow word-alignment by Acelogic · Pull Request #2838 · 0xMiden/miden-vm · GitHub
Skip to content

fix: reject @locals values that overflow word-alignment - #2838

Merged
bobbinth merged 2 commits into
0xMiden:nextfrom
Acelogic:fix/locals-overflow-bounds-check
Mar 15, 2026
Merged

bobbinth merged 2 commits into
0xMiden:nextfrom
Acelogic:fix/locals-overflow-bounds-check

Conversation

@Acelogic

Copy link
Copy Markdown
Contributor

Summary

  • @locals(65535) (and 65533, 65534) causes a panic because num_locals.next_multiple_of(4) overflows u16
  • Adds a bounds check in the parser grammar that rejects @locals values above 65532 with a descriptive error
  • 65532 is the largest u16 value that can be safely word-aligned (rounded up to a multiple of 4)

Details

The overflow occurs in fmp_start_frame_sequence and fmp_end_frame_sequence (crates/assembly/src/fmp.rs) at:

let locals_frame = Felt::from_u16(num_locals.next_multiple_of(WORD_SIZE as u16));

For num_locals = 65535, next_multiple_of(4) = 65536, which exceeds u16::MAX. In debug builds this panics; in release builds it wraps to 0.

The fix validates at parse time using the existing InvalidLocalsAttr error variant, so the user gets a clear diagnostic:

error: invalid procedure @locals attribute
  --> test:2:5
  |
2 |     @locals(65535)
  |     ^^^^^^^^^^^^^^ number of locals exceeds the maximum of 65532

Test plan

  • Added locals_overflow_rejected test: verifies @locals(65535) produces the expected error
  • Added locals_max_valid_accepted test: verifies @locals(65532) is accepted
  • All 86 existing miden-assembly-syntax tests pass

Fixes #2829

When @Locals(65535) (or 65533/65534) is specified, the word-alignment
rounding `num_locals.next_multiple_of(4)` overflows u16, causing a
panic in debug builds or wrapping to 0 in release builds.

Add a bounds check in the parser that rejects @Locals values above
65532 (the largest u16 value that can be safely rounded up to a
multiple of 4) with a descriptive error message.

Fixes 0xMiden#2829
@bitwalker

Copy link
Copy Markdown
Contributor

You'll need to sign your commits and rebase on next, but otherwise should be good! Be sure to read our contributing guidelines for information on what we expect from PRs from external contributors though, it saves everyone a lot of time if little things like commit signing are already sorted.

@bobbinth bobbinth added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Mar 15, 2026

@bobbinth bobbinth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! Thank you!

@bobbinth
bobbinth merged commit 9d50a65 into 0xMiden:next Mar 15, 2026
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog This PR does not require an entry in the `CHANGELOG.md` file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

@locals(65535) alignment overflow crashes assembler

3 participants