Skip to content

vm.c: make stack_extend_alloc cover the frame offset - #7305

Merged
matz merged 1 commit into
mruby:masterfrom
harukasan:fix-stack-extend-frame-offset
Aug 22, 2026
Merged

vm.c: make stack_extend_alloc cover the frame offset#7305
matz merged 1 commit into
mruby:masterfrom
harukasan:fix-stack-extend-frame-offset

Conversation

@harukasan

@harukasan harukasan commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Problem

In stack_extend_alloc(), commit 16baea0 changed the stack size floor calculation from the frame base offset to the remaining capacity:

// Current calculation
size_t off = mrb->c->ci->stack ? mrb->c->stend - mrb->c->ci->stack : 0;
if (off > size) size = off;

Impact

This reversal causes two issues depending on ci->stack:

  1. As long as ci->stack is within stbase..stend, the calculated value never exceeds the old size, rendering the floor calculation useless.
  2. If ci->stack sits past stend (e.g., when an embedder enters the VM with an undersized context, as previously seen in mruby-task: fix VM stack overflow when a larger proc is set on an existing task #7279), the unsigned subtraction wraps around to a huge value. This poisons the growth math, raising a NoMemoryError instead of properly growing the stack.

Fix

Restore the pre-refactor logic. The floor must be the frame base offset from the stack bottom (mrb->c->ci->stack - oldbase).

An assertion on ci->stack <= stend would not be a substitute. Asserts compile out in release builds and the wrap-around would remain there.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed stack growth in certain call-frame scenarios, improving runtime stability when additional stack space is required.

Commit 16baea0 changed the stack size floor from the frame base
offset to the remaining capacity, stend - ci->stack. While ci->stack
is within stbase..stend the floor never fires. When ci->stack sits
past stend the unsigned subtraction wraps around and the VM raises
NoMemoryError instead of growing the stack. Restore the frame base
offset from the stack bottom.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@harukasan
harukasan requested a review from matz as a code owner August 22, 2026 14:40
@github-actions github-actions Bot added the core label Aug 22, 2026
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d4786424-a27b-4bec-9781-229c38ce5257

📥 Commits

Reviewing files that changed from the base of the PR and between 06ed794 and 148b3c7.

📒 Files selected for processing (1)
  • src/vm.c

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

stack_extend_alloc now calculates the active call frame offset from the stack base. This allows stack growth to account for call frames located beyond stend.

Changes

Stack growth allocation

Layer / File(s) Summary
Active frame offset calculation
src/vm.c
stack_extend_alloc now measures the current call-info stack pointer from oldbase instead of measuring unused capacity from stend.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 148b3

This is a localized stack-growth correction, and no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: matz, takumin

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: updating stack_extend_alloc to cover the frame offset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@matz
matz merged commit 57a8e0b into mruby:master Aug 22, 2026
21 checks passed
@harukasan
harukasan deleted the fix-stack-extend-frame-offset branch August 23, 2026 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants