From e513e4d489971be7a5c73a8cfe5408d004b6e136 Mon Sep 17 00:00:00 2001 From: Ryan Breen Date: Tue, 3 Feb 2026 07:08:43 -0500 Subject: [PATCH] fix(arm64): achieve full Copy-on-Write parity with x86-64 This commit fixes multiple ARM64 CoW implementation issues to achieve complete parity with x86-64. All 6 CoW tests now pass on ARM64. Fixes implemented: - Add CoW statistics counters to ARM64 page fault handler - Add TLB flush (tlbi vmalle1is) after TTBR0 switch in context_switch.rs, syscall_entry.rs - ARM64 requires explicit TLB invalidation unlike x86-64 - Fix fork stack copy to use HHDM physical addresses instead of virtual addresses (TTBR0 still points to parent during copy) - Implement cleanup_cow_frames for ARM64 to properly decrement reference counts when process exits - Fix sbrk check in libbreenix to use >= instead of == for page alignment - Fix cow_oom_test to enable OOM after fork (fork requires frame allocation) - Fix ARM64 exception handler to terminate userspace processes with SIGSEGV on unhandled data aborts instead of hanging Test results (ARM64): - cow_readonly_test: PASS - cow_signal_test: PASS - cow_cleanup_test: PASS - cow_sole_owner_test: PASS - cow_stress_test: PASS - cow_oom_test: PASS Co-Authored-By: Claude Opus 4.5 --- docs/planning/ARM64_TEST_CATALOG.md | 407 ++++++++++++++++++ .../src/arch_impl/aarch64/context_switch.rs | 32 +- kernel/src/arch_impl/aarch64/exception.rs | 62 ++- kernel/src/arch_impl/aarch64/syscall_entry.rs | 117 ++++- kernel/src/interrupts.rs | 65 +-- kernel/src/memory/cow_stats.rs | 63 +++ kernel/src/memory/layout.rs | 10 +- kernel/src/memory/mod.rs | 1 + kernel/src/process/manager.rs | 89 +++- kernel/src/process/process.rs | 59 ++- libs/libbreenix/src/memory.rs | 4 +- userspace/tests/cow_oom_test.rs | 72 ++-- 12 files changed, 835 insertions(+), 146 deletions(-) create mode 100644 docs/planning/ARM64_TEST_CATALOG.md create mode 100644 kernel/src/memory/cow_stats.rs diff --git a/docs/planning/ARM64_TEST_CATALOG.md b/docs/planning/ARM64_TEST_CATALOG.md new file mode 100644 index 00000000..d6581477 --- /dev/null +++ b/docs/planning/ARM64_TEST_CATALOG.md @@ -0,0 +1,407 @@ +# ARM64 Test Catalog + +Generated: 2026-02-03 +Kernel: aarch64-breenix +Based on: arm64-parity.md (last updated 2026-02-02) + +## Summary + +| Metric | Count | Notes | +|--------|-------|-------| +| **Total test binaries** | 106 | All .elf files in `userspace/tests/aarch64/` | +| **Passing (genuine)** | ~46 | Tests with verified implementations | +| **Passing (suspicious)** | 4 | Tests may pass for wrong reasons | +| **Failing** | ~30 | Known failures per arm64-parity.md | +| **Not testable** | ~10 | Network/timing-dependent tests | +| **Pass rate** | 62.5% (50/80) | Per arm64-parity.md test run | + +## Implementation Quality Assessment + +### Overall Assessment: GENUINE + +The ARM64 implementation shares syscall code with x86-64 via the shared `kernel/src/syscall/` modules. The ARM64-specific code in `kernel/src/arch_impl/aarch64/syscall_entry.rs` is a dispatcher that routes to these shared implementations. This architectural decision means: + +1. **Most syscalls are genuine** - They use the same implementation as x86-64 +2. **Fork/exec are ARM64-specific** - But they are properly implemented with CoW, page table creation, etc. +3. **Two testing syscalls are stubbed** - COW_STATS and SIMULATE_OOM return ENOSYS on ARM64 + +### Red Flags Found + +| Issue | Location | Impact | Assessment | +|-------|----------|--------|------------| +| COW_STATS stub | `syscall_entry.rs:745-747` | 2 tests | **KNOWN LIMITATION** - returns ENOSYS | +| SIMULATE_OOM stub | `syscall_entry.rs:745-747` | 1 test | **KNOWN LIMITATION** - returns ENOSYS | +| Network retries | Test code | ~10 tests | **NOT CHEATING** - legitimate retry for async ops | + +## Test Categories + +### Process Management + +| Test | Status | Genuine? | Notes | +|------|--------|----------|-------| +| fork_test | PASS | YES | Full CoW fork with independent memory spaces | +| fork_memory_test | PASS | YES | Verifies CoW memory isolation | +| fork_state_test | PASS | YES | Verifies register state preserved across fork | +| fork_pending_signal_test | PASS | YES | Signal inheritance in fork | +| waitpid_test | PASS | YES | Uses shared sys_waitpid implementation | +| wnohang_timing_test | PASS | YES | WNOHANG non-blocking wait | +| exec_from_ext2_test | PASS | YES | Full ELF loading from ext2 filesystem | +| exec_argv_test | PASS | YES | Argv passing via stack | +| exec_stack_argv_test | PASS | YES | Stack-based argv setup | +| argv_test | PASS | YES | Command-line argument parsing | + +### Signals + +| Test | Status | Genuine? | Notes | +|------|--------|----------|-------| +| signal_test | PASS | YES | Basic signal delivery | +| signal_handler_test | PASS | YES | User-defined signal handlers execute | +| signal_return_test | PASS | YES | sigreturn restores context | +| signal_fork_test | PASS | YES | Signals + fork interaction | +| signal_exec_test | PASS | YES | Signal disposition on exec | +| sigchld_test | PASS | YES | SIGCHLD on child exit | +| sigchld_job_test | PASS | YES | Job control with SIGCHLD | +| pause_test | PASS | YES | pause() blocks until signal | +| alarm_test | FAIL | - | Timing-sensitive, may timeout | +| sigsuspend_test | PASS | YES | Atomic signal mask + wait | +| itimer_test | PASS | YES | Interval timers | +| ctrl_c_test | FAIL | - | TTY/terminal control | +| kill_process_group_test | FAIL | - | Process group signals | + +### Pipes and IPC + +| Test | Status | Genuine? | Notes | +|------|--------|----------|-------| +| pipe_fork_test | PASS | YES | Pipe communication between parent/child | +| pipe_concurrent_test | PASS | YES | Multiple readers/writers | +| pipe2_test | PASS | YES | pipe2() with flags | +| shell_pipe_test | PASS | YES | Shell-style pipeline | +| pipeline_test | PASS | YES | Multi-stage pipelines | +| fifo_test | FAIL | - | Named pipes (requires filesystem support) | + +### Unix Domain Sockets + +| Test | Status | Genuine? | Notes | +|------|--------|----------|-------| +| unix_socket_test | FAIL | - | socketpair returns EFAULT | +| unix_named_socket_test | PASS | YES | Abstract namespace sockets work | + +### Network (TCP/UDP) + +| Test | Status | Genuine? | Notes | +|------|--------|----------|-------| +| udp_socket_test | FAIL | - | Loopback issues | +| tcp_socket_test | FAIL | - | Loopback blocking | +| tcp_blocking_test | FAIL | - | Blocking recv | +| tcp_client_test | FAIL | - | External connection | +| blocking_recv_test | FAIL | - | Blocking I/O | +| concurrent_recv_stress | FAIL | - | Stress test | +| nonblock_eagain_test | PASS | YES | Non-blocking returns EAGAIN | +| dns_test | PASS* | YES | Requires network - external DNS | +| http_test | PASS* | YES | Requires network - external HTTP | + +*Network tests pass when virtio-net is configured correctly. + +### File Descriptors + +| Test | Status | Genuine? | Notes | +|------|--------|----------|-------| +| dup_test | PASS | YES | Full dup() implementation with verification | +| fcntl_test | PASS | YES | F_GETFD, F_SETFD, F_GETFL, F_SETFL | +| cloexec_test | FAIL | - | FD_CLOEXEC on exec | +| nonblock_test | PASS | YES | O_NONBLOCK flag | +| lseek_test | PASS | YES | File position seeking | + +### Filesystem + +| Test | Status | Genuine? | Notes | +|------|--------|----------|-------| +| file_read_test | PASS | YES | ext2 file read implemented | +| getdents_test | PASS | YES | Directory listing | +| fs_write_test | PASS* | PARTIAL | Requires writable ext2 | +| fs_rename_test | FAIL | - | Rename not implemented | +| fs_large_file_test | FAIL | - | Large file support | +| fs_directory_test | PASS | YES | mkdir/rmdir | +| fs_link_test | FAIL | - | Hard links | +| fs_block_alloc_test | FAIL | - | Block allocation | +| access_test | PASS | YES | File access checks | +| devfs_test | PASS | YES | /dev filesystem | +| cwd_test | PASS | YES | getcwd/chdir | + +### Memory (CoW) + +| Test | Status | Genuine? | Notes | +|------|--------|----------|-------| +| test_mmap | PASS | YES | Memory mapping works | +| cow_signal_test | PASS | YES | CoW + signals | +| cow_cleanup_test | PASS | YES | Page cleanup on exit | +| cow_sole_owner_test | FAIL | NO | **Requires COW_STATS syscall** | +| cow_stress_test | PASS | YES | 128-page CoW stress test | +| cow_readonly_test | PASS | YES | Read-only CoW pages | +| cow_oom_test | PASS | PARTIAL | OOM handling (SIMULATE_OOM stubbed) | + +### TTY/PTY + +| Test | Status | Genuine? | Notes | +|------|--------|----------|-------| +| tty_test | PASS | YES | Basic TTY operations | +| job_control_test | PASS | YES | Job control signals | +| session_test | PASS | YES | Session management | +| job_table_test | PASS | YES | Job table operations | +| pty_test | PASS | YES | Pseudo-terminal | + +### Time + +| Test | Status | Genuine? | Notes | +|------|--------|----------|-------| +| clock_gettime_test | PASS | YES | Uses CNTVCT_EL0 counter | + +### Coreutils + +| Test | Status | Genuine? | Notes | +|------|--------|----------|-------| +| true_test | PASS | YES | exit(0) | +| false_test | PASS | YES | exit(1) | +| head_test | PASS | YES | First N lines | +| tail_test | PASS | YES | Last N lines | +| wc_test | PASS | YES | Word count | +| which_test | PASS | YES | PATH lookup | +| cat_test | PASS | YES | File concatenation | +| ls_test | PASS | YES | Directory listing | +| mkdir_argv_test | PASS | YES | mkdir with args | +| echo_argv_test | PASS | YES | echo with args | +| rm_argv_test | PASS | YES | rm with args | +| cp_mv_argv_test | PASS | YES | cp/mv operations | + +### Graphics + +| Test | Status | Genuine? | Notes | +|------|--------|----------|-------| +| fbinfo_test | PASS | YES | Framebuffer info syscall | + +## Detailed Analysis + +### Verified Genuine Implementations + +#### fork_test +**Status**: PASS +**Genuine**: YES +**Implementation**: `kernel/src/arch_impl/aarch64/syscall_entry.rs:sys_fork_aarch64()` + +**What it tests**: +- Process forking with proper register state capture +- CoW memory mapping +- Child returns 0, parent returns child PID +- exec() in child process + +**Verification**: +- Examined `sys_fork_aarch64()` - captures full Aarch64ExceptionFrame +- Creates new ProcessPageTable with CoW mappings +- Uses `manager.fork_process_aarch64()` which calls shared fork implementation +- Child thread added to scheduler with proper context +- NOT A STUB - over 100 lines of real implementation + +**Confidence**: HIGH + +#### clock_gettime_test +**Status**: PASS +**Genuine**: YES +**Implementation**: `kernel/src/arch_impl/aarch64/syscall_entry.rs:sys_clock_gettime()` + +**What it tests**: +- CLOCK_MONOTONIC returns valid time +- Time advances between calls +- Sub-millisecond precision (ARM64 uses CNTVCT_EL0) +- Nanoseconds not suspiciously aligned +- Monotonicity maintained + +**Verification**: +- Uses `crate::time::get_monotonic_time_ns()` - shared time module +- ARM64 uses CNTVCT_EL0 counter (generic timer) +- NOT a stub - returns actual hardware timestamps + +**Confidence**: HIGH + +#### cow_stress_test +**Status**: PASS +**Genuine**: YES +**Implementation**: Uses shared CoW page fault handler + +**What it tests**: +- Allocates 128 pages (512KB) via sbrk +- Fills with parent pattern +- Forks child +- Child writes to all 128 pages (triggers 128 CoW faults) +- Verifies parent memory unchanged +- Parent also writes to verify isolation + +**Verification**: +- Test does actual memory allocation and writes +- Verifies data patterns across processes +- CoW fault handling is shared code + +**Confidence**: HIGH + +#### signal_handler_test +**Status**: PASS +**Genuine**: YES +**Implementation**: Uses shared signal delivery code + +**What it tests**: +- Register SIGUSR1 handler with sigaction +- Send signal to self with kill() +- Verify handler actually executes +- Handler sets global flag that is checked + +**Verification**: +- Test uses static HANDLER_CALLED flag +- Handler must execute to set the flag +- Cannot pass without handler execution + +**Confidence**: HIGH + +### Known Limitations (Not Cheating) + +#### COW_STATS / SIMULATE_OOM Syscalls +**Status**: ENOSYS on ARM64 +**Location**: `syscall_entry.rs:745-747` + +```rust +syscall_nums::COW_STATS | syscall_nums::SIMULATE_OOM => { + (-38_i64) as u64 // -ENOSYS +} +``` + +**Impact**: +- `cow_sole_owner_test` - Cannot verify sole owner optimization counter +- `cow_oom_test` - Cannot simulate OOM conditions + +**Assessment**: This is a **known limitation**, not cheating. The tests that need these syscalls are documented as failing. The core CoW functionality works - only the diagnostic syscalls are missing. + +### Suspicious Tests (May Pass For Wrong Reasons) + +#### cow_oom_test +**Status**: PASS +**Assessment**: PARTIAL + +The test passes but cannot truly test OOM handling because SIMULATE_OOM returns ENOSYS. The test has fallback logic that allows it to pass without OOM simulation: + +```rust +// From the test - it accepts the syscall not being available +match memory::cow_stats() { + Some(s) => s, + None => { + io::print(" FAIL: Could not get initial CoW stats\n"); + // ... exits with failure + } +}; +``` + +However, looking at the actual test code, it does fail properly when cow_stats() returns None. The issue is that the test is designed to work around the limitation, not that it's "cheating". + +#### Network Tests with Retries +**Status**: Various +**Assessment**: LEGITIMATE + +Many network tests have retry loops: +```rust +for retry in 0..MAX_LOOPBACK_RETRIES { + match accept(server_fd, None) { + Ok(fd) if fd >= 0 => { ... } + Err(EAGAIN) => { ... retry ... } + } +} +``` + +This is **NOT cheating** - it's correct behavior for async network operations. The tests do fail if retries are exhausted. + +## Failure Categories + +Per arm64-parity.md: + +| Category | Count | Root Cause | Priority | +|----------|-------|------------|----------| +| argc/argv setup | ~4 | Initial process args | P3 | +| Signal/process bugs | ~8 | Various syscall issues | P4 | +| COW syscall ENOSYS | ~2 | Not implemented | P5 | +| Network issues | ~6 | Loopback/blocking | P4 | +| Filesystem write | ~6 | Needs writable disk | P2 (FIXED) | +| Other | ~4 | Various | P6 | + +## Recommendations + +1. **Implement COW_STATS for ARM64** - Low effort, enables 2 more tests +2. **Fix socketpair EFAULT** - Blocking unix_socket_test +3. **Debug loopback networking** - Many TCP/UDP tests affected +4. **Verify argc/argv initial process setup** - ~4 tests blocked + +## Test Infrastructure Notes + +### Running Tests +```bash +# Build ARM64 kernel +cargo build --release --features testing --target aarch64-breenix.json \ + -Z build-std=core,alloc -Z build-std-features=compiler-builtins-mem \ + -p kernel --bin kernel-aarch64 + +# Run boot test +./docker/qemu/run-aarch64-boot-test-native.sh + +# Run full test suite +./docker/qemu/run-aarch64-test-suite.sh --all +``` + +### Test Output Locations +- Boot test: `/tmp/breenix_aarch64_boot_native/serial.txt` +- Test suite: `/tmp/breenix_arm64_test_results/*.txt` + +### Test Success Markers +Tests print specific markers: +- `*_TEST_PASSED` or `exit(0)` for success +- `*_TEST_FAILED` or `exit(1)` for failure + +## Appendix: All Test Binaries + +Total: 106 .elf files + +### Test Programs (*_test.elf) +``` +access_test.elf alarm_test.elf argv_test.elf +blocking_recv_test.elf cat_test.elf clock_gettime_test.elf +cloexec_test.elf cow_cleanup_test.elf cow_oom_test.elf +cow_readonly_test.elf cow_signal_test.elf cow_sole_owner_test.elf +cow_stress_test.elf cp_mv_argv_test.elf ctrl_c_test.elf +cwd_test.elf devfs_test.elf dns_test.elf +dup_test.elf echo_argv_test.elf exec_argv_test.elf +exec_from_ext2_test.elf exec_stack_argv_test.elf false_test.elf +fbinfo_test.elf fcntl_test.elf fifo_test.elf +file_read_test.elf fork_memory_test.elf fork_pending_signal_test.elf +fork_state_test.elf fork_test.elf fs_block_alloc_test.elf +fs_directory_test.elf fs_large_file_test.elf fs_link_test.elf +fs_rename_test.elf fs_write_test.elf getdents_test.elf +head_test.elf http_test.elf itimer_test.elf +job_control_test.elf job_table_test.elf kill_process_group_test.elf +ls_test.elf lseek_test.elf mkdir_argv_test.elf +nonblock_eagain_test.elf nonblock_test.elf pause_test.elf +pipe_concurrent_test.elf pipe_fork_test.elf pipe2_test.elf +pipeline_test.elf pty_test.elf rm_argv_test.elf +session_test.elf shell_pipe_test.elf sigchld_job_test.elf +sigchld_test.elf signal_exec_check.elf signal_exec_test.elf +signal_fork_test.elf signal_handler_test.elf signal_return_test.elf +signal_test.elf sigsuspend_test.elf tail_test.elf +tcp_blocking_test.elf tcp_client_test.elf tcp_socket_test.elf +test_mmap.elf true_test.elf tty_test.elf +udp_socket_test.elf unix_named_socket_test.elf unix_socket_test.elf +waitpid_test.elf wc_test.elf which_test.elf +wnohang_timing_test.elf +``` + +### Utility Programs (non-test) +``` +bounce.elf cat.elf cp.elf demo.elf +echo.elf false.elf head.elf hello_time.elf +hello_world.elf init_shell.elf ls.elf mkdir.elf +mv.elf resolution.elf rm.elf rmdir.elf +simple_exit.elf spinner.elf tail.elf telnetd.elf +true.elf wc.elf which.elf +``` diff --git a/kernel/src/arch_impl/aarch64/context_switch.rs b/kernel/src/arch_impl/aarch64/context_switch.rs index 18818d99..6e2ae4ce 100644 --- a/kernel/src/arch_impl/aarch64/context_switch.rs +++ b/kernel/src/arch_impl/aarch64/context_switch.rs @@ -599,21 +599,20 @@ fn switch_ttbr0_if_needed(_thread_id: u64) { // NOTE: No logging - context switch path must be lock-free unsafe { - // Write new TTBR0 + // CRITICAL: On ARM64, changing TTBR0 does NOT automatically flush the TLB + // (unlike x86-64's CR3). We MUST flush the TLB after switching TTBR0, + // otherwise the parent process may use stale TLB entries from the child + // after a fork/exit cycle, causing CoW memory corruption. core::arch::asm!( - "msr ttbr0_el1, {}", + "dsb ishst", // Ensure previous stores complete + "msr ttbr0_el1, {}", // Set new page table + "isb", // Synchronize context + "tlbi vmalle1is", // FLUSH ENTIRE TLB - critical for CoW correctness + "dsb ish", // Ensure TLB flush completes + "isb", // Synchronize instruction stream in(reg) next_ttbr0, options(nomem, nostack) ); - - // Memory barriers required after page table switch - // DSB ISH: Ensure the write to TTBR0 is complete - // ISB: Flush instruction pipeline - core::arch::asm!( - "dsb ish", - "isb", - options(nomem, nostack, preserves_flags) - ); } // Update saved process TTBR0 @@ -793,11 +792,14 @@ fn check_and_deliver_signals_for_current_thread_arm64(frame: &mut Aarch64Excepti if let Some(ref page_table) = process.page_table { let ttbr0_value = page_table.level_4_frame().start_address().as_u64(); unsafe { - // Write new TTBR0 + // CRITICAL: Flush TLB after TTBR0 switch for CoW correctness core::arch::asm!( - "msr ttbr0_el1, {}", - "dsb ish", - "isb", + "dsb ishst", // Ensure previous stores complete + "msr ttbr0_el1, {}", // Set new page table + "isb", // Synchronize context + "tlbi vmalle1is", // FLUSH ENTIRE TLB + "dsb ish", // Ensure TLB flush completes + "isb", // Synchronize instruction stream in(reg) ttbr0_value, options(nomem, nostack) ); diff --git a/kernel/src/arch_impl/aarch64/exception.rs b/kernel/src/arch_impl/aarch64/exception.rs index 39ad62fc..d21d39f9 100644 --- a/kernel/src/arch_impl/aarch64/exception.rs +++ b/kernel/src/arch_impl/aarch64/exception.rs @@ -79,13 +79,59 @@ pub extern "C" fn handle_sync_exception(frame: *mut Aarch64ExceptionFrame, esr: return; } - // Not a CoW fault or couldn't be handled - print debug info and hang - let frame = unsafe { &*frame }; + // Not a CoW fault or couldn't be handled + let frame_ref = unsafe { &mut *frame }; crate::serial_println!("[exception] Data abort at address {:#x}", far); - crate::serial_println!(" ELR: {:#x}, ESR: {:#x}", frame.elr, esr); + crate::serial_println!(" ELR: {:#x}, ESR: {:#x}", frame_ref.elr, esr); crate::serial_println!(" ISS: {:#x} (WnR={}, DFSC={:#x})", iss, (iss >> 6) & 1, iss & 0x3F); - // Hang on unhandled data abort + + // Check if from userspace (EL0) - SPSR[3:0] indicates source EL + let from_el0 = (frame_ref.spsr & 0xF) == 0; + + if from_el0 { + // From userspace - terminate the process with SIGSEGV + crate::serial_println!("[exception] Terminating userspace process with SIGSEGV"); + + // Get current TTBR0 to find the process + let ttbr0: u64; + unsafe { + core::arch::asm!("mrs {}, ttbr0_el1", out(reg) ttbr0, options(nomem, nostack)); + } + let page_table_phys = ttbr0 & !0xFFFF_0000_0000_0FFF; + + // Find and terminate the process + let mut terminated = false; + crate::process::with_process_manager(|pm| { + if let Some((pid, process)) = pm.find_process_by_cr3_mut(page_table_phys) { + let name = process.name.clone(); + crate::serial_println!("[exception] Killing process {} (PID {}) due to data abort", + name, pid.as_u64()); + pm.exit_process(pid, -11); // SIGSEGV exit code + terminated = true; + } else { + crate::serial_println!("[exception] Could not find process with TTBR0={:#x}", page_table_phys); + } + }); + + if terminated { + // Mark scheduler needs reschedule + crate::task::scheduler::set_need_resched(); + + // Switch scheduler to idle thread + crate::task::scheduler::switch_to_idle(); + + // Modify exception frame to return to idle loop + // The idle loop runs in EL1 and will handle rescheduling + frame_ref.elr = crate::arch_impl::aarch64::idle_loop_arm64 as *const () as u64; + frame_ref.spsr = 0x3c5; // EL1h, interrupts enabled + + // Return to idle loop via ERET + return; + } + } + + // From kernel or couldn't terminate - hang loop { unsafe { core::arch::asm!("wfi"); } } } @@ -422,6 +468,7 @@ fn exception_class_name(ec: u32) -> &'static str { /// Returns false if this wasn't a CoW fault or couldn't be handled fn handle_cow_fault_arm64(far: u64, iss: u32) -> bool { use crate::memory::arch_stub::{VirtAddr, Page, Size4KiB}; + use crate::memory::cow_stats; use crate::memory::frame_allocator::allocate_frame; use crate::memory::frame_metadata::{frame_decref, frame_is_shared}; use crate::memory::process_memory::{is_cow_page, make_private_flags}; @@ -437,6 +484,9 @@ fn handle_cow_fault_arm64(far: u64, iss: u32) -> bool { return false; } + // Track CoW fault count + cow_stats::TOTAL_FAULTS.fetch_add(1, core::sync::atomic::Ordering::Relaxed); + let faulting_addr = VirtAddr::new(far); // Get current TTBR0 (user page table base) @@ -458,6 +508,7 @@ fn handle_cow_fault_arm64(far: u64, iss: u32) -> bool { // Try to acquire process manager lock match crate::process::try_manager() { Some(mut guard) => { + cow_stats::MANAGER_PATH.fetch_add(1, core::sync::atomic::Ordering::Relaxed); let pm = match guard.as_mut() { Some(pm) => pm, None => return false, @@ -519,6 +570,7 @@ fn handle_cow_fault_arm64(far: u64, iss: u32) -> bool { options(nostack) ); } + cow_stats::SOLE_OWNER_OPT.fetch_add(1, core::sync::atomic::Ordering::Relaxed); crate::serial_println!("[COW] Made sole-owner page writable"); return true; } @@ -568,6 +620,7 @@ fn handle_cow_fault_arm64(far: u64, iss: u32) -> bool { ); } + cow_stats::PAGES_COPIED.fetch_add(1, core::sync::atomic::Ordering::Relaxed); crate::serial_println!( "[COW] Copied page from {:#x} to {:#x}", old_frame.start_address().as_u64(), @@ -577,6 +630,7 @@ fn handle_cow_fault_arm64(far: u64, iss: u32) -> bool { true } None => { + cow_stats::DIRECT_PATH.fetch_add(1, core::sync::atomic::Ordering::Relaxed); crate::serial_println!("[COW] Manager lock held, cannot handle"); false } diff --git a/kernel/src/arch_impl/aarch64/syscall_entry.rs b/kernel/src/arch_impl/aarch64/syscall_entry.rs index de7dcf92..6a69772b 100644 --- a/kernel/src/arch_impl/aarch64/syscall_entry.rs +++ b/kernel/src/arch_impl/aarch64/syscall_entry.rs @@ -181,10 +181,14 @@ fn check_and_deliver_signals_aarch64(frame: &mut Aarch64ExceptionFrame) { if let Some(ref page_table) = process.page_table { let ttbr0 = page_table.level_4_frame().start_address().as_u64(); unsafe { + // CRITICAL: Flush TLB after TTBR0 switch for CoW correctness core::arch::asm!( - "dsb ishst", - "msr ttbr0_el1, {}", - "isb", + "dsb ishst", // Ensure previous stores complete + "msr ttbr0_el1, {}", // Set new page table + "isb", // Synchronize context + "tlbi vmalle1is", // FLUSH ENTIRE TLB + "dsb ish", // Ensure TLB flush completes + "isb", // Synchronize instruction stream in(reg) ttbr0, options(nostack) ); @@ -741,9 +745,12 @@ fn dispatch_syscall( } } - // Testing syscalls (stubs) - syscall_nums::COW_STATS | syscall_nums::SIMULATE_OOM => { - (-38_i64) as u64 // -ENOSYS + // Testing/diagnostic syscalls + syscall_nums::COW_STATS => { + sys_cow_stats_aarch64(arg1) + } + syscall_nums::SIMULATE_OOM => { + sys_simulate_oom_aarch64(arg1) } syscall_nums::GETPID => sys_getpid(), @@ -1360,6 +1367,104 @@ extern "C" { ) -> !; } +// ============================================================================= +// Testing/diagnostic syscall implementations for ARM64 +// ============================================================================= + +/// CowStatsResult structure returned by sys_cow_stats +/// Matches the layout expected by userspace +#[repr(C)] +struct CowStatsResultAarch64 { + total_faults: u64, + manager_path: u64, + direct_path: u64, + pages_copied: u64, + sole_owner_opt: u64, +} + +/// sys_cow_stats - Get Copy-on-Write statistics (for testing) - ARM64 implementation +/// +/// This syscall is used to verify that the CoW optimization paths are working. +/// It returns the current CoW statistics to userspace. +/// +/// Parameters: +/// - stats_ptr: pointer to a CowStatsResult structure in userspace +/// +/// Returns: 0 on success, negative error code on failure +fn sys_cow_stats_aarch64(stats_ptr: u64) -> u64 { + use crate::memory::cow_stats; + + if stats_ptr == 0 { + return (-14_i64) as u64; // -EFAULT - null pointer + } + + // Validate the address is in userspace + if !crate::memory::layout::is_valid_user_address(stats_ptr) { + log::error!("sys_cow_stats_aarch64: Invalid userspace address {:#x}", stats_ptr); + return (-14_i64) as u64; // -EFAULT + } + + // Get the current stats from the shared module + let stats = cow_stats::get_stats(); + + // Copy to userspace + unsafe { + let user_stats = stats_ptr as *mut CowStatsResultAarch64; + (*user_stats).total_faults = stats.total_faults; + (*user_stats).manager_path = stats.manager_path; + (*user_stats).direct_path = stats.direct_path; + (*user_stats).pages_copied = stats.pages_copied; + (*user_stats).sole_owner_opt = stats.sole_owner_opt; + } + + log::debug!( + "sys_cow_stats_aarch64: total={}, manager={}, direct={}, copied={}, sole_owner={}", + stats.total_faults, + stats.manager_path, + stats.direct_path, + stats.pages_copied, + stats.sole_owner_opt + ); + + 0 +} + +/// sys_simulate_oom - Enable or disable OOM simulation (for testing) - ARM64 implementation +/// +/// This syscall is used to test the kernel's behavior when frame allocation fails +/// during Copy-on-Write page faults. When OOM simulation is enabled, all frame +/// allocations will return None, causing CoW faults to fail and processes to be +/// terminated with SIGSEGV. +/// +/// Parameters: +/// - enable: 1 to enable OOM simulation, 0 to disable +/// +/// Returns: 0 on success, -ENOSYS if testing feature is not compiled in +/// +/// # Safety +/// Only enable OOM simulation briefly for testing! Extended OOM simulation will +/// crash the kernel because it affects ALL frame allocations. +fn sys_simulate_oom_aarch64(enable: u64) -> u64 { + #[cfg(feature = "testing")] + { + if enable != 0 { + crate::memory::frame_allocator::enable_oom_simulation(); + log::info!("sys_simulate_oom_aarch64: OOM simulation ENABLED"); + } else { + crate::memory::frame_allocator::disable_oom_simulation(); + log::info!("sys_simulate_oom_aarch64: OOM simulation disabled"); + } + 0 + } + + #[cfg(not(feature = "testing"))] + { + let _ = enable; // suppress unused warning + log::warn!("sys_simulate_oom_aarch64: testing feature not compiled in"); + (-38_i64) as u64 // -ENOSYS - function not implemented + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/kernel/src/interrupts.rs b/kernel/src/interrupts.rs index 4f1c32fd..9666b0d4 100644 --- a/kernel/src/interrupts.rs +++ b/kernel/src/interrupts.rs @@ -636,69 +636,8 @@ extern "x86-interrupt" fn invalid_opcode_handler(stack_frame: InterruptStackFram /// IMPORTANT: This function uses try_manager() to avoid deadlock when called /// during signal delivery (which holds the process manager lock). If the lock /// is held, we handle the CoW fault directly by manipulating page tables via CR3. -/// Copy-on-Write statistics for testing and debugging -pub mod cow_stats { - use core::sync::atomic::{AtomicU64, Ordering}; - - /// Total CoW faults handled - pub static TOTAL_FAULTS: AtomicU64 = AtomicU64::new(0); - /// Faults handled via process manager (normal path) - pub static MANAGER_PATH: AtomicU64 = AtomicU64::new(0); - /// Faults handled via direct page table manipulation (lock-held path) - pub static DIRECT_PATH: AtomicU64 = AtomicU64::new(0); - /// Pages that were copied (frame was shared) - pub static PAGES_COPIED: AtomicU64 = AtomicU64::new(0); - /// Pages made writable without copy (sole owner optimization) - pub static SOLE_OWNER_OPT: AtomicU64 = AtomicU64::new(0); - - /// Get current CoW statistics - #[allow(dead_code)] - pub fn get_stats() -> CowStats { - CowStats { - total_faults: TOTAL_FAULTS.load(Ordering::Relaxed), - manager_path: MANAGER_PATH.load(Ordering::Relaxed), - direct_path: DIRECT_PATH.load(Ordering::Relaxed), - pages_copied: PAGES_COPIED.load(Ordering::Relaxed), - sole_owner_opt: SOLE_OWNER_OPT.load(Ordering::Relaxed), - } - } - - /// Reset all statistics (for testing) - #[allow(dead_code)] - pub fn reset_stats() { - TOTAL_FAULTS.store(0, Ordering::Relaxed); - MANAGER_PATH.store(0, Ordering::Relaxed); - DIRECT_PATH.store(0, Ordering::Relaxed); - PAGES_COPIED.store(0, Ordering::Relaxed); - SOLE_OWNER_OPT.store(0, Ordering::Relaxed); - } - - /// CoW statistics snapshot - #[allow(dead_code)] - #[derive(Debug, Clone, Copy)] - pub struct CowStats { - pub total_faults: u64, - pub manager_path: u64, - pub direct_path: u64, - pub pages_copied: u64, - pub sole_owner_opt: u64, - } - - #[allow(dead_code)] - impl CowStats { - /// Print statistics to serial output - pub fn print(&self) { - crate::serial_println!( - "[COW STATS] total={} manager={} direct={} copied={} sole_owner={}", - self.total_faults, - self.manager_path, - self.direct_path, - self.pages_copied, - self.sole_owner_opt - ); - } - } -} +/// Copy-on-Write statistics - re-export from architecture-independent module +pub use crate::memory::cow_stats; fn handle_cow_fault( faulting_addr: VirtAddr, diff --git a/kernel/src/memory/cow_stats.rs b/kernel/src/memory/cow_stats.rs new file mode 100644 index 00000000..6cb32c14 --- /dev/null +++ b/kernel/src/memory/cow_stats.rs @@ -0,0 +1,63 @@ +//! Architecture-independent Copy-on-Write statistics +//! +//! This module provides tracking for CoW page fault handling across all +//! architectures. Both x86_64 and ARM64 use these counters. + +use core::sync::atomic::{AtomicU64, Ordering}; + +/// Total CoW faults handled +pub static TOTAL_FAULTS: AtomicU64 = AtomicU64::new(0); +/// Faults handled via process manager (normal path) +pub static MANAGER_PATH: AtomicU64 = AtomicU64::new(0); +/// Faults handled via direct page table manipulation (lock-held path) +pub static DIRECT_PATH: AtomicU64 = AtomicU64::new(0); +/// Pages that were copied (frame was shared) +pub static PAGES_COPIED: AtomicU64 = AtomicU64::new(0); +/// Pages made writable without copy (sole owner optimization) +pub static SOLE_OWNER_OPT: AtomicU64 = AtomicU64::new(0); + +/// Get current CoW statistics +pub fn get_stats() -> CowStats { + CowStats { + total_faults: TOTAL_FAULTS.load(Ordering::Relaxed), + manager_path: MANAGER_PATH.load(Ordering::Relaxed), + direct_path: DIRECT_PATH.load(Ordering::Relaxed), + pages_copied: PAGES_COPIED.load(Ordering::Relaxed), + sole_owner_opt: SOLE_OWNER_OPT.load(Ordering::Relaxed), + } +} + +/// Reset all statistics (for testing) +#[allow(dead_code)] +pub fn reset_stats() { + TOTAL_FAULTS.store(0, Ordering::Relaxed); + MANAGER_PATH.store(0, Ordering::Relaxed); + DIRECT_PATH.store(0, Ordering::Relaxed); + PAGES_COPIED.store(0, Ordering::Relaxed); + SOLE_OWNER_OPT.store(0, Ordering::Relaxed); +} + +/// CoW statistics snapshot +#[derive(Debug, Clone, Copy)] +pub struct CowStats { + pub total_faults: u64, + pub manager_path: u64, + pub direct_path: u64, + pub pages_copied: u64, + pub sole_owner_opt: u64, +} + +#[allow(dead_code)] +impl CowStats { + /// Print statistics to serial output + pub fn print(&self) { + crate::serial_println!( + "[COW STATS] total={} manager={} direct={} copied={} sole_owner={}", + self.total_faults, + self.manager_path, + self.direct_path, + self.pages_copied, + self.sole_owner_opt + ); + } +} diff --git a/kernel/src/memory/layout.rs b/kernel/src/memory/layout.rs index 1228e775..c417d7b4 100644 --- a/kernel/src/memory/layout.rs +++ b/kernel/src/memory/layout.rs @@ -340,10 +340,18 @@ pub fn is_user_stack_address(addr: u64) -> bool { } // ARM64: stack is in high user range (lower half canonical space). +// Note: On ARM64, stacks are allocated starting at USER_STACK_REGION_START (the top) +// and growing down, so actual stack addresses are BELOW USER_STACK_REGION_START. +// The valid range is [USER_STACK_REGION_START - max_stack_size, USER_STACK_REGION_START). +// We allow up to 1MB of stack space per process to account for multiple stacks. #[cfg(target_arch = "aarch64")] #[inline] pub fn is_user_stack_address(addr: u64) -> bool { - addr >= USER_STACK_REGION_START && addr < USER_STACK_REGION_END + // Stack region extends downward from USER_STACK_REGION_START + // Allow addresses from START-1MB to START + const MAX_STACK_REGION_SIZE: u64 = 1024 * 1024; // 1 MB for multiple stacks + let region_bottom = USER_STACK_REGION_START.saturating_sub(MAX_STACK_REGION_SIZE); + addr >= region_bottom && addr < USER_STACK_REGION_START } /// Check if an address is in userspace mmap region diff --git a/kernel/src/memory/mod.rs b/kernel/src/memory/mod.rs index 31b8b91f..c40b318f 100644 --- a/kernel/src/memory/mod.rs +++ b/kernel/src/memory/mod.rs @@ -1,3 +1,4 @@ +pub mod cow_stats; pub mod frame_allocator; pub mod frame_metadata; pub mod heap; diff --git a/kernel/src/process/manager.rs b/kernel/src/process/manager.rs index a0f91d84..08dcf061 100644 --- a/kernel/src/process/manager.rs +++ b/kernel/src/process/manager.rs @@ -1271,29 +1271,94 @@ impl ProcessManager { child_sp_aligned ); - // Copy the parent's stack contents to the child's stack + // Copy the parent's stack contents to the child's stack using HHDM-based physical access. + // CRITICAL: We cannot use virtual addresses directly because TTBR0 still points to the + // parent's page table. Writing to child virtual addresses would actually write to parent + // physical frames. Instead, we must: + // 1. Translate parent VA -> parent PA using parent's page table + // 2. Translate child VA -> child PA using child's page table + // 3. Copy using HHDM addresses: (HHDM_BASE + phys_addr) if parent_stack_used > 0 && parent_stack_used <= (64 * 1024) { - let parent_stack_src = parent_sp as *const u8; - let child_stack_dst = child_sp_aligned as *mut u8; - log::debug!( - "ARM64 fork: Copying {} bytes of stack from {:#x} to {:#x}", + "ARM64 fork: Copying {} bytes of stack from {:#x} to {:#x} via HHDM", parent_stack_used, parent_sp, child_sp_aligned ); - unsafe { - core::ptr::copy_nonoverlapping( - parent_stack_src, - child_stack_dst, - parent_stack_used as usize, + let hhdm_base = crate::arch_impl::aarch64::constants::HHDM_BASE; + + // Get parent's page table for address translation + let parent_page_table = self + .processes + .get(&parent_pid) + .and_then(|p| p.page_table.as_ref()) + .ok_or("Parent process page table not found for stack copy")?; + + // Get child's page table for address translation + let child_page_table = child_process + .page_table + .as_ref() + .ok_or("Child process page table not found for stack copy")?; + + // Copy stack page by page + // Start from the page containing SP (page-align down) + let start_page_addr = parent_sp & !0xFFF; + let mut parent_page_addr = start_page_addr; + let parent_stack_top_u64 = parent_thread.stack_top.as_u64(); + let child_stack_top_u64 = child_stack_top.as_u64(); + + while parent_page_addr < parent_stack_top_u64 { + // Calculate corresponding child page address (same offset from stack top) + let offset_from_top = parent_stack_top_u64 - parent_page_addr; + let child_page_addr = child_stack_top_u64 - offset_from_top; + + // Translate parent virtual address to physical + let parent_phys = match parent_page_table.translate_page(VirtAddr::new(parent_page_addr)) { + Some(phys) => phys, + None => { + log::warn!( + "ARM64 fork: parent stack page {:#x} not mapped, skipping", + parent_page_addr + ); + parent_page_addr += 4096; + continue; + } + }; + + // Translate child virtual address to physical + let child_phys = match child_page_table.translate_page(VirtAddr::new(child_page_addr)) { + Some(phys) => phys, + None => { + log::error!( + "ARM64 fork: child stack page {:#x} not mapped!", + child_page_addr + ); + return Err("Child stack page not mapped"); + } + }; + + // Copy via HHDM (kernel's direct physical memory mapping) + let parent_hhdm_addr = (hhdm_base + parent_phys.as_u64()) as *const u8; + let child_hhdm_addr = (hhdm_base + child_phys.as_u64()) as *mut u8; + + unsafe { + core::ptr::copy_nonoverlapping(parent_hhdm_addr, child_hhdm_addr, 4096); + } + + log::trace!( + "ARM64 fork: copied stack page {:#x} (phys {:#x}) -> {:#x} (phys {:#x})", + parent_page_addr, + parent_phys.as_u64(), + child_page_addr, + child_phys.as_u64() ); + + parent_page_addr += 4096; } log::info!( - "ARM64 fork: Copied {} bytes of stack from parent to child", - parent_stack_used + "ARM64 fork: Copied stack pages from parent to child via HHDM" ); } diff --git a/kernel/src/process/process.rs b/kernel/src/process/process.rs index f7f90823..79decb29 100644 --- a/kernel/src/process/process.rs +++ b/kernel/src/process/process.rs @@ -487,14 +487,61 @@ impl Process { } } - /// Clean up Copy-on-Write frame references (ARM64 stub) + /// Clean up Copy-on-Write frame references when process exits (ARM64) + /// + /// Walks all user pages in the process's page table and decrements their + /// reference counts. Frames that are no longer shared (refcount reaches 0) + /// are returned to the frame allocator for reuse. #[cfg(not(target_arch = "x86_64"))] fn cleanup_cow_frames(&mut self) { - // ARM64 stub - CoW not yet implemented - log::debug!( - "Process {}: CoW cleanup not implemented for ARM64", - self.id.as_u64() - ); + use crate::memory::frame_allocator::deallocate_frame; + use crate::memory::frame_metadata::frame_decref; + use crate::memory::arch_stub::{PageTableFlags, PhysFrame}; + + // Get the page table for this process + let page_table = match self.page_table.as_ref() { + Some(pt) => pt, + None => { + log::debug!( + "Process {}: No page table to clean up", + self.id.as_u64() + ); + return; + } + }; + + let mut freed_count = 0; + let mut shared_count = 0; + + // Walk all user pages and decrement refcounts + let _ = page_table.walk_mapped_pages(|_virt_addr, phys_addr, flags| { + // Only process user-accessible pages + if !flags.contains(PageTableFlags::USER_ACCESSIBLE) { + return; + } + + let frame = PhysFrame::containing_address(phys_addr); + + // Decrement reference count + // If this was the last reference (frame was tracked and refcount reached 0), + // deallocate the frame. frame_decref returns false for untracked frames + // to prevent corruption from freeing potentially in-use frames. + if frame_decref(frame) { + deallocate_frame(frame); + freed_count += 1; + } else { + shared_count += 1; + } + }); + + if freed_count > 0 || shared_count > 0 { + log::debug!( + "Process {}: CoW cleanup - freed {} frames, {} still shared", + self.id.as_u64(), + freed_count, + shared_count + ); + } } /// Check if process is terminated diff --git a/libs/libbreenix/src/memory.rs b/libs/libbreenix/src/memory.rs index 7f2efc89..b12e53d5 100644 --- a/libs/libbreenix/src/memory.rs +++ b/libs/libbreenix/src/memory.rs @@ -51,7 +51,9 @@ pub fn get_brk() -> u64 { pub fn sbrk(size: usize) -> *mut u8 { let current = get_brk(); let new_break = brk(current + size as u64); - if new_break == current + size as u64 { + // Check that the new break is >= requested (kernel may page-align up) + // On failure, kernel returns the old break which will be < current + size + if new_break >= current + size as u64 { current as *mut u8 } else { core::ptr::null_mut() diff --git a/userspace/tests/cow_oom_test.rs b/userspace/tests/cow_oom_test.rs index 77967752..11534434 100644 --- a/userspace/tests/cow_oom_test.rs +++ b/userspace/tests/cow_oom_test.rs @@ -7,13 +7,16 @@ //! //! Test mechanism: //! 1. Parent allocates and writes to heap memory (establishes page) -//! 2. Parent enables OOM simulation via syscall -//! 3. Parent forks child (heap pages become CoW-shared) -//! 4. Parent immediately disables OOM simulation (so parent doesn't crash) -//! 5. Child attempts to write to heap (triggers CoW fault) -//! 6. CoW fault handler tries to allocate frame, fails (OOM simulation) -//! 7. Kernel terminates child with SIGSEGV (exit code -11) -//! 8. Parent verifies child was killed by SIGSEGV, not normal exit +//! 2. Parent forks child (heap pages become CoW-shared) +//! 3. Child enables OOM simulation (AFTER fork succeeds - fork needs frames for page tables) +//! 4. Child attempts to write to heap (triggers CoW fault) +//! 5. CoW fault handler tries to allocate frame, fails (OOM simulation) +//! 6. Kernel terminates child with SIGSEGV (exit code -11) +//! 7. Parent verifies child was killed by SIGSEGV, not normal exit +//! +//! IMPORTANT: OOM must be enabled AFTER fork, not before! Fork requires frame +//! allocation for the child's page tables. If OOM is enabled before fork, +//! fork() fails with ENOMEM and we never reach the CoW test scenario. //! //! Expected behavior: //! - Child is killed with exit status indicating SIGSEGV (not normal exit) @@ -142,40 +145,37 @@ pub extern "C" fn _start() -> ! { } io::print(" Heap allocated and initialized\n"); - // Step 3: Enable OOM simulation BEFORE fork - io::print("\nStep 3: Enabling OOM simulation\n"); - let oom_result = memory::simulate_oom(true); - if oom_result != 0 { - io::print(" FAIL: simulate_oom(true) returned "); - print_signed(oom_result as i64); - io::print("\n"); - io::print("COW_OOM_TEST_FAILED\n"); - process::exit(1); - } - io::print(" OOM simulation enabled\n"); - - // Step 4: Fork - child inherits CoW-shared pages - // Fork should succeed because it doesn't allocate new page frames - // (it shares parent's pages with CoW semantics) - io::print("\nStep 4: Forking process...\n"); + // Step 3: Fork FIRST - child inherits CoW-shared pages + // IMPORTANT: Fork must happen BEFORE OOM is enabled because fork() + // requires frame allocation for the child's page tables. + io::print("\nStep 3: Forking process...\n"); let fork_result = process::fork(); if fork_result < 0 { io::print(" FAIL: fork() failed with error "); print_signed(fork_result); io::print("\n"); - // Disable OOM before exiting - memory::simulate_oom(false); io::print("COW_OOM_TEST_FAILED\n"); process::exit(1); } if fork_result == 0 { // ========== CHILD PROCESS ========== - // OOM simulation is still active (inherited from parent) - // Any CoW fault will fail due to frame allocation failure - - io::print("[CHILD] Process started, about to trigger CoW fault...\n"); + io::print("[CHILD] Process started\n"); + + // Step 4 (child): Enable OOM simulation AFTER fork succeeded + // Now that we have our own page tables, enable OOM so the CoW + // fault during heap write will fail. + io::print("[CHILD] Enabling OOM simulation...\n"); + let oom_result = memory::simulate_oom(true); + if oom_result != 0 { + io::print("[CHILD] FAIL: simulate_oom(true) returned "); + print_signed(oom_result as i64); + io::print("\n"); + io::print("COW_OOM_TEST_FAILED\n"); + process::exit(98); // Different code to distinguish this failure + } + io::print("[CHILD] OOM simulation enabled\n"); // Attempt to write to heap - this triggers a CoW page fault // With OOM simulation active, allocate_frame() returns None @@ -190,17 +190,13 @@ pub extern "C" fn _start() -> ! { process::exit(99); // Special exit code to indicate test failure } else { // ========== PARENT PROCESS ========== + // Parent never has OOM enabled - only the child does io::print(" Forked child PID: "); print_number(fork_result as u64); io::print("\n"); - // Step 5: Immediately disable OOM simulation so parent doesn't crash - io::print("\nStep 5: Disabling OOM simulation in parent\n"); - memory::simulate_oom(false); - io::print(" OOM simulation disabled\n"); - - // Step 6: Wait for child and verify it was killed by SIGSEGV - io::print("\nStep 6: Waiting for child...\n"); + // Step 4 (parent): Wait for child and verify it was killed by SIGSEGV + io::print("\nStep 4: Waiting for child...\n"); let mut status: i32 = 0; let wait_result = process::waitpid(fork_result as i32, &mut status as *mut i32, 0); @@ -216,8 +212,8 @@ pub extern "C" fn _start() -> ! { print_signed(status as i64); io::print("\n"); - // Step 7: Verify child was killed by signal (not normal exit) - io::print("\nStep 7: Verifying child was killed by SIGSEGV\n"); + // Step 5: Verify child was killed by signal (not normal exit) + io::print("\nStep 5: Verifying child was killed by SIGSEGV\n"); if process::wifexited(status) { let exit_code = process::wexitstatus(status);