fix(argv): use inline(always) for get_args to preserve RSP #76
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The get_args() function reads argc from [rsp], but if it's not inlined, the call instruction pushes a return address onto the stack first. This causes [rsp] to contain the return address instead of argc, resulting in crashes when programs like cat try to parse arguments. Also adds Claude Code configuration files for project-specific settings. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Two fixes for shell command execution: 1. Shell argv optimization fix: Use core::hint::black_box() to prevent the compiler from optimizing away the stack-allocated arg_buf before the execv syscall reads from it. Without this, the compiler may reuse the stack memory since arg_buf is only used to obtain a pointer. 2. Stack allocation bounds check fix: Check bounds BEFORE allocating to prevent creating VirtAddr at the non-canonical boundary (0x8000_0000_0000). Changed from > to >= to ensure strict boundary enforcement and use saturating_add to prevent overflow in the calculation itself. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixed a critical bug where argc/argv was being written to the wrong physical address when setting up the stack for new processes. The bug was in write_byte_to_stack(): translate_page() returns the FULL physical address (frame base + page offset), but we were then adding page_offset again, causing the write to go to frame + 2*offset instead of frame + offset. When the process ran and read from the correct virtual address, the MMU translated to frame + offset, which was all zeros because we never wrote there. Changes: - kernel/src/process/manager.rs: Remove duplicate page_offset addition - userspace/tests/cat.rs: Use naked _start to properly capture RSP - xtask/src/main.rs: Add cat /hello.txt test to interactive tests - kernel/src/memory/stack.rs: Add stack bounds tests with proper >= check - Add exec_stack_argv_test for regression testing Fixes `cat /hello.txt` showing "missing file operand" when argc was correctly set to 2 by the kernel. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add 17 tests verifying stack allocation bounds checking logic: - User stack boundary conditions (at boundary, before, past) - Kernel stack boundary conditions - Integer overflow protection via saturating_add - State consistency on allocation failure - Consecutive allocation until exhaustion - Edge cases (zero size, single page, max valid) These tests mirror the logic in kernel/src/memory/stack.rs but run on the host machine for faster iteration during development. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The syscall() stub function requires this signature for C ABI compatibility but the current implementation doesn't use the arguments (returns fixed values for FUTEX and GETRANDOM). Prefix with underscore to silence warnings. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
fork() and execv() in libbreenix are safe functions - the unsafe blocks were unnecessary and caused compiler warnings. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
get_args()was marked#[inline(never)]which caused the function call to push a return address onto the stack before reading[rsp]. This meant programs reading argv (likecat) would read the return address as argc instead of the actual argument count, causing crashes.Changes
#[inline(never)]to#[inline(always)]inlibs/libbreenix/src/argv.rs.claude/settings.json,.claude/skills/os-research/)Test plan
cat /hello.txtshould now work in the interactive shell🤖 Generated with Claude Code