Micro OPT #132
Micro OPT #132
Conversation
WalkthroughReworks x86_64 interrupt/exception handling with structured fault analysis and IRQ mapping, adds a syscall entry/dispatcher, replaces inline-asm atomics with portable builtins and a large atomic API, adjusts build flags to PIE/PIC, updates GRUB for BIOS/EFI branches, and condenses ROADMAP documentation. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor CPU
participant IDT as InterruptHandler
participant IRQ as IRQ Controller
participant NET as Net_Poll
participant FA as FaultAnalysis
participant PANIC as KernelPanic
CPU->>IDT: InterruptHandler(regs)
alt Hardware IRQ (vector >= 32)
IDT->>IRQ: acknowledge / route
alt Timer IRQ (vector 32)
IDT->>NET: Net_Poll()
NET-->>IDT: return
IDT-->>CPU: EOI + return
else Known IRQ handlers (32..47)
IDT-->>CPU: handle + EOI + return
else Unhandled IRQ
IDT->>IDT: log unhandled IRQ
IDT-->>CPU: EOI + return
end
else CPU Exception (vector < 32)
alt Page Fault (14)
IDT->>FA: HandlePageFault(regs) -> FaultResult
alt FAULT_HANDLED / FAULT_RETRY
IDT-->>CPU: return / retry
else FAULT_KILL_PROCESS / FAULT_PANIC_KERNEL
IDT->>FA: AnalyzeFault(regs) -> FaultContext
FA-->>IDT: ctx
IDT->>PANIC: panic(ctx.fault_reason)
end
else Other critical exceptions (6,8,13...)
IDT->>FA: AnalyzeFault(regs) -> FaultContext
FA-->>IDT: ctx
IDT->>PANIC: panic(ctx.fault_reason)
end
end
sequenceDiagram
autonumber
participant User as Userland
participant CPU
participant SysEntry as SyscallEntry (ASM)
participant SysHandler as SyscallHandler (C)
participant Kernel as Kernel (services)
User->>CPU: int 0x80 / syscall
CPU->>SysEntry: trap to SyscallEntry
SysEntry->>SysHandler: marshal regs -> SyscallHandler(syscall_num, a1,a2,a3)
SysHandler->>Kernel: perform syscall (e.g., SYS_WRITE, SYS_EXIT)
Kernel-->>SysHandler: result
SysHandler-->>SysEntry: return value (rax)
SysEntry-->>CPU: restore + iretq -> Userland
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
arch/x86_64/interrupts/Interrupts.c (1)
22-28: Move APIC tick increment before MLFQSchedulekernel/sched/MLFQ.c reads APICticks (extern/return at kernel/sched/MLFQ.c:170–171) but arch/x86_64/interrupts/Interrupts.c declares APICticks (line 15) and calls AtomicInc(&APICticks) only after MLFQSchedule (line 25); move AtomicInc(&APICticks) to immediately before MLFQSchedule so the scheduler observes the current tick.
🧹 Nitpick comments (2)
docs/ROADMAP.md (1)
3-3: Fix heading level hierarchy.Starting with h3 (
###) without h2 violates markdown heading hierarchy.-### Goals for v0.0.2-development4 +## Goals for v0.0.2-development4arch/x86_64/interrupts/Interrupts.c (1)
61-61: Unreachable code - comment is misleading.The comment "should not be possible as already clamped" is incorrect. This default case IS reachable for any interrupt >= 48.
Either handle high interrupts explicitly or update the comment:
- default: break; // fallback to CPU exceptions (should not be possible as already clamped) + default: + // Unknown high interrupt number (>=48) + PrintKernelWarning("[IRQ] Unknown interrupt: "); + PrintKernelInt(regs->interrupt_number); + PrintKernelWarning("\n"); + ApicSendEoi(); + return;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
arch/x86_64/interrupts/Interrupts.c(4 hunks)docs/ROADMAP.md(1 hunks)grub.cfg(1 hunks)meson.build(2 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: README.md:9-9
Timestamp: 2025-09-01T01:10:22.123Z
Learning: VoidFrame's interrupt handlers in arch/x86_64/interrupts/Interrupts.asm only save basic GPRs and segment registers, missing critical registers for deep fault tracing: control registers (CR0/CR2/CR3/CR4), SIMD state (XMM/YMM/ZMM), debug registers (DR0-DR7), and extended FPU state. The CPU initialization code enables SSE/AVX/XSAVE features but interrupt handlers don't preserve this extended state. Enhanced fault tracing requires selective extended state saving for critical exceptions (ISR 6, 8, 13, 14) while keeping lightweight handling for regular interrupts.
📚 Learning: 2025-08-31T07:06:37.003Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#101
File: kernel/etc/Shell.c:829-837
Timestamp: 2025-08-31T07:06:37.003Z
Learning: assembler-0 is working on optimizing ramfs performance in VoidFrame kernel and wants to implement proper filesystem characteristics like faster lookups, better memory management, and advanced features like copy-on-write and caching.
Applied to files:
docs/ROADMAP.md
📚 Learning: 2025-08-12T03:19:52.873Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#73
File: kernel/elf/ELFloader.c:0-0
Timestamp: 2025-08-12T03:19:52.873Z
Learning: VoidFrame kernel now has complete UEFI boot support with hybrid BIOS/UEFI ISO creation capability. The existing VBE framebuffer implementation requires no changes as it uses Multiboot2 framebuffer tags and direct memory-mapped access, making it inherently UEFI-compatible. Build process: ninja + ./scripts/build_hybrid_iso.sh creates dual-platform bootable ISO.
Applied to files:
docs/ROADMAP.mdgrub.cfg
📚 Learning: 2025-08-12T03:19:52.873Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#73
File: kernel/elf/ELFloader.c:0-0
Timestamp: 2025-08-12T03:19:52.873Z
Learning: User assembler-0 has successfully implemented UEFI boot support for VoidFrame kernel. The VBE framebuffer implementation is fully UEFI-compatible using Multiboot2 framebuffer tags and direct memory-mapped access, requiring no kernel code changes - only bootloader configuration updates.
Applied to files:
docs/ROADMAP.mdgrub.cfg
📚 Learning: 2025-08-12T01:43:39.847Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#72
File: arch/x86_64/syscall/SyscallEntry.asm:24-30
Timestamp: 2025-08-12T01:43:39.847Z
Learning: User prefers to defer syscall implementation complexity until next year and is considering making a ring 0 kernel instead of continuing with user-mode syscall approach in the VoidFrame kernel project.
Applied to files:
docs/ROADMAP.md
📚 Learning: 2025-08-16T05:28:34.925Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#90
File: kernel/memory/MemOps.c:103-124
Timestamp: 2025-08-16T05:28:34.925Z
Learning: In VoidFrame kernel, the virtual address space management uses a simple linear allocator (kernel_space.next_vaddr) without any reclamation mechanism. VMemFree and VMemUnmapMMIO clear page tables but don't free virtual address space for reuse, causing eventual virtual address space exhaustion. The solution requires implementing a free list with coalescing in the VirtAddrSpace structure and updating allocation/deallocation functions to reuse freed ranges.
Applied to files:
docs/ROADMAP.md
📚 Learning: 2025-09-01T01:10:22.123Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: README.md:9-9
Timestamp: 2025-09-01T01:10:22.123Z
Learning: VoidFrame's interrupt handlers in arch/x86_64/interrupts/Interrupts.asm only save basic GPRs and segment registers, missing critical registers for deep fault tracing: control registers (CR0/CR2/CR3/CR4), SIMD state (XMM/YMM/ZMM), debug registers (DR0-DR7), and extended FPU state. The CPU initialization code enables SSE/AVX/XSAVE features but interrupt handlers don't preserve this extended state. Enhanced fault tracing requires selective extended state saving for critical exceptions (ISR 6, 8, 13, 14) while keeping lightweight handling for regular interrupts.
Applied to files:
arch/x86_64/interrupts/Interrupts.c
📚 Learning: 2025-08-13T07:26:56.892Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#79
File: drivers/PCI/PCI.c:154-154
Timestamp: 2025-08-13T07:26:56.892Z
Learning: VoidFrame kernel uses direct interrupt dispatch in arch/x86_64/interrupts/Interrupts.c rather than RegisterInterruptHandler pattern. Hardware interrupts are handled in switch cases by interrupt number (32+ for IRQs), with proper PIC EOI handling required.
Applied to files:
arch/x86_64/interrupts/Interrupts.c
📚 Learning: 2025-08-31T14:19:11.366Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: docs/ARCHITECHTURE.md:1-3
Timestamp: 2025-08-31T14:19:11.366Z
Learning: assembler-0 wants to implement deep fault tracing for VoidFrame kernel exceptions (Page Fault, General Protection Fault, Invalid Opcode) instead of just panicking. The current interrupt handler has complete register state available in the Registers struct (defined in arch/x86_64/cpu/Cpu.h) including RBP for stack walking, RIP for fault location, and error codes for analysis.
Applied to files:
arch/x86_64/interrupts/Interrupts.c
📚 Learning: 2025-09-01T01:10:22.123Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: README.md:9-9
Timestamp: 2025-09-01T01:10:22.123Z
Learning: The current VoidFrame Registers struct in arch/x86_64/cpu/Cpu.h only saves basic GPRs, segment registers, and exception context (interrupt_number, error_code, rip, cs, rflags, rsp, ss). It's missing critical registers for deep fault tracing: SIMD registers (XMM/YMM/ZMM), control registers (CR0/CR2/CR3/CR4), debug registers (DR0-DR7), and extended FPU state. The struct has a "DO NOT TOUCH" comment indicating it must match interrupt ASM stack layout.
Applied to files:
arch/x86_64/interrupts/Interrupts.c
📚 Learning: 2025-08-10T06:22:28.816Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#62
File: kernel/core/Panic.c:69-79
Timestamp: 2025-08-10T06:22:28.816Z
Learning: In VoidFrame kernel, panic functions must call "asm volatile("cli")" as the very first instruction to disable interrupts immediately. Without this, if interrupts are enabled (after "sti"), an interrupt can fire during panic handling causing the panic to fail, hang, or behave unpredictably. This must be done in ALL panic entry points: Panic(), PanicWithCode(), PanicWithContext(), PanicFromInterrupt(), and at the start of KernelPanicHandler().
Applied to files:
arch/x86_64/interrupts/Interrupts.c
📚 Learning: 2025-08-10T06:22:28.816Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#62
File: kernel/core/Panic.c:69-79
Timestamp: 2025-08-10T06:22:28.816Z
Learning: In VoidFrame kernel, panic functions must disable interrupts with "cli" as the very first operation, before any other code executes. If interrupts are enabled (after "sti"), an interrupt can fire between the panic call and the cli instruction in KernelPanicHandler, causing the panic to fail or behave unpredictably.
Applied to files:
arch/x86_64/interrupts/Interrupts.c
🧬 Code graph analysis (1)
arch/x86_64/interrupts/Interrupts.c (1)
mm/trace/StackTrace.c (2)
AnalyzeFault(169-194)PrintDetailedFaultInfo(196-234)
🪛 markdownlint-cli2 (0.17.2)
docs/ROADMAP.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
meson.build (1)
303-303: LGTM! Commented BIOS option aligns with dual-platform support.The commented OVMF BIOS option provides a useful testing path for UEFI boot, complementing the BIOS/EFI branching in grub.cfg.
docs/ROADMAP.md (1)
1-10: LGTM! Concise roadmap with clear priorities.The simplified roadmap effectively communicates high-level goals. The focus on KASLR support, EEVDF scheduling, and unified process management shows good architectural direction.
arch/x86_64/interrupts/Interrupts.c (3)
64-108: Excellent structured fault handling for page faults!The new page fault handler with
FaultResultenum provides clear, maintainable control flow. The separation between recoverable user-space faults and kernel panics is well implemented.Note: I see from the learnings that you previously wanted deep fault tracing with extended register state. The current implementation with
AnalyzeFaultandPrintDetailedFaultInfois a good step. If you need the extended state (CR2, XSAVE, etc.) mentioned in the learnings, that would require updates to the assembly interrupt handlers.
110-128: Good fault analysis implementation!The unified fault analysis approach for critical exceptions (Invalid Opcode, Double Fault, GPF) with
AnalyzeFaultand structured context is a significant improvement over ad-hoc panic messages.
55-60: Verify and either implement handlers or document passthrough for IRQs 35–43 and 45Repo search found no drivers binding to IRQs 35–43 or 45; confirm whether PCI/MSI/MSI‑X, ACPI or VM passthrough can assign these vectors — if so, add per-device ISRs in the appropriate drivers; otherwise add a clear comment in arch/x86_64/interrupts/Interrupts.c (and suppress or rate-limit the warning).
Relevant locations: arch/x86_64/interrupts/Interrupts.c (lines 55–60), drivers/APIC.c (vector mapping), drivers/ISA/ISA.h, drivers/PS2.c, drivers/Ide.h — no bindings to 35–43/45 found in drivers/.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
arch/x86_64/interrupts/Interrupts.c (1)
135-151: Bug: exception message subtracts 32 (prints negative/garbage).Exceptions are 0–31; don’t offset. Also avoid FormatS static buffer in IRQ context.
- PanicFromInterrupt(FormatS("Unhandled exception %d", regs->interrupt_number - 32), regs); + PanicFromInterrupt(FormatS("Unhandled exception %u", regs->interrupt_number), regs);Optional (safer in ISR): avoid FormatS static buffer
- PanicFromInterrupt(FormatS("Unhandled exception %u", regs->interrupt_number), regs); + char msg[64]; + snprintf(msg, sizeof(msg), "Unhandled exception %u", (unsigned)regs->interrupt_number); + PanicFromInterrupt(msg, regs);
🧹 Nitpick comments (5)
kernel/core/Kernel.c (1)
516-522: Boot log prints OK; consider gating behind a debug flag.These address prints are useful but noisy on real boots. Suggest wrapping with a boot debug guard (e.g., VF_DEBUG_BOOT).
+#ifdef VF_DEBUG_BOOT PrintKernel("KernelMainHigherHalf addr: "); PrintKernelHex((uint64_t)&KernelMainHigherHalf); PrintKernel(", calculated entry: "); PrintKernelHex(higher_half_entry); PrintKernel("\n"); +#endifarch/x86_64/interrupts/Interrupts.h (1)
6-7: Make ToIRQ return a signed type to represent -1 clearly.Returning -1 via uint64_t yields UINT64_MAX; error checks become brittle.
-uint64_t ToIRQ(uint64_t irn); +int ToIRQ(uint64_t irn);arch/x86_64/interrupts/Interrupts.c (3)
18-20: Match ToIRQ’s return type to a signed int and keep range checks.-uint64_t ToIRQ(const uint64_t irn) { - return irn >= 32 && irn <= 255 ? irn - 32 : -1; +int ToIRQ(const uint64_t irn) { + return (irn >= 32 && irn <= 255) ? (int)(irn - 32) : -1; }
79-88: Guard against NULL current process before dereference.In early/edge faults, MLFQGetCurrentProcess() can be NULL.
- PrintKernelWarning("Killing process "); - PrintKernelInt(MLFQGetCurrentProcess()->pid); - PrintKernelWarning(" due to page fault\n"); - MLFQKillCurrentProcess("Page Fault (segmentation fault)"); + Process* cur = MLFQGetCurrentProcess(); + if (cur) { + PrintKernelWarning("Killing process "); + PrintKernelInt(cur->pid); + PrintKernelWarning(" due to page fault\n"); + MLFQKillCurrentProcess("Page Fault (segmentation fault)"); + } else { + PrintKernelWarning("No current process; escalating to kernel panic\n"); + PanicFromInterrupt("Page Fault with no current process", regs); + }
27-67: IRQ path verified; handlers present for 32,33,34,44,46,47 — 35–43 & 45 are passthrough (warn + EOI).Approve as-is; add handlers for any additional devices you expect to support.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
arch/x86_64/interrupts/Interrupts.c(5 hunks)arch/x86_64/interrupts/Interrupts.h(1 hunks)kernel/atomic/Atomics.c(1 hunks)kernel/atomic/Atomics.h(1 hunks)kernel/core/Kernel.c(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: README.md:9-9
Timestamp: 2025-09-01T01:10:22.123Z
Learning: VoidFrame's interrupt handlers in arch/x86_64/interrupts/Interrupts.asm only save basic GPRs and segment registers, missing critical registers for deep fault tracing: control registers (CR0/CR2/CR3/CR4), SIMD state (XMM/YMM/ZMM), debug registers (DR0-DR7), and extended FPU state. The CPU initialization code enables SSE/AVX/XSAVE features but interrupt handlers don't preserve this extended state. Enhanced fault tracing requires selective extended state saving for critical exceptions (ISR 6, 8, 13, 14) while keeping lightweight handling for regular interrupts.
📚 Learning: 2025-08-17T08:16:43.539Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#93
File: kernel/memory/VMem.c:72-79
Timestamp: 2025-08-17T08:16:43.539Z
Learning: In VoidFrame kernel, VMemGetPhysAddr and related page table walking functions were causing panics because they assumed identity mapping existed for physical addresses < IDENTITY_MAP_SIZE (4GB), but the bootloader doesn't provide this 1:1 mapping. The fix is to remove all identity mapping assumptions and always use PHYS_TO_VIRT() for page table access in functions like VMemGetPageTablePhys, VMemMap, VMemMapHuge, VMemGetPhysAddr, VMemUnmap, VMemMapMMIO, and VMemUnmapMMIO.
Applied to files:
kernel/core/Kernel.c
📚 Learning: 2025-08-13T07:19:55.274Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#79
File: drivers/PCI/PCI.c:154-154
Timestamp: 2025-08-13T07:19:55.274Z
Learning: VMemUnmapMMIO in VoidFrame kernel should use the same page table access pattern as the working VMemUnmap function: `uint64_t* pt_table = (uint64_t*)(pt_phys + KERNEL_VIRTUAL_OFFSET);` rather than PHYS_TO_VIRT macro or identity mapping checks.
Applied to files:
kernel/core/Kernel.c
📚 Learning: 2025-08-13T07:26:56.892Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#79
File: drivers/PCI/PCI.c:154-154
Timestamp: 2025-08-13T07:26:56.892Z
Learning: VoidFrame kernel uses direct interrupt dispatch in arch/x86_64/interrupts/Interrupts.c rather than RegisterInterruptHandler pattern. Hardware interrupts are handled in switch cases by interrupt number (32+ for IRQs), with proper PIC EOI handling required.
Applied to files:
arch/x86_64/interrupts/Interrupts.c
📚 Learning: 2025-09-01T01:10:22.123Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: README.md:9-9
Timestamp: 2025-09-01T01:10:22.123Z
Learning: VoidFrame's interrupt handlers in arch/x86_64/interrupts/Interrupts.asm only save basic GPRs and segment registers, missing critical registers for deep fault tracing: control registers (CR0/CR2/CR3/CR4), SIMD state (XMM/YMM/ZMM), debug registers (DR0-DR7), and extended FPU state. The CPU initialization code enables SSE/AVX/XSAVE features but interrupt handlers don't preserve this extended state. Enhanced fault tracing requires selective extended state saving for critical exceptions (ISR 6, 8, 13, 14) while keeping lightweight handling for regular interrupts.
Applied to files:
arch/x86_64/interrupts/Interrupts.c
📚 Learning: 2025-08-31T14:19:11.366Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: docs/ARCHITECHTURE.md:1-3
Timestamp: 2025-08-31T14:19:11.366Z
Learning: assembler-0 wants to implement deep fault tracing for VoidFrame kernel exceptions (Page Fault, General Protection Fault, Invalid Opcode) instead of just panicking. The current interrupt handler has complete register state available in the Registers struct (defined in arch/x86_64/cpu/Cpu.h) including RBP for stack walking, RIP for fault location, and error codes for analysis.
Applied to files:
arch/x86_64/interrupts/Interrupts.c
📚 Learning: 2025-09-01T01:10:22.123Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: README.md:9-9
Timestamp: 2025-09-01T01:10:22.123Z
Learning: The current VoidFrame Registers struct in arch/x86_64/cpu/Cpu.h only saves basic GPRs, segment registers, and exception context (interrupt_number, error_code, rip, cs, rflags, rsp, ss). It's missing critical registers for deep fault tracing: SIMD registers (XMM/YMM/ZMM), control registers (CR0/CR2/CR3/CR4), debug registers (DR0-DR7), and extended FPU state. The struct has a "DO NOT TOUCH" comment indicating it must match interrupt ASM stack layout.
Applied to files:
arch/x86_64/interrupts/Interrupts.c
📚 Learning: 2025-08-10T06:22:28.816Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#62
File: kernel/core/Panic.c:69-79
Timestamp: 2025-08-10T06:22:28.816Z
Learning: In VoidFrame kernel, panic functions must call "asm volatile("cli")" as the very first instruction to disable interrupts immediately. Without this, if interrupts are enabled (after "sti"), an interrupt can fire during panic handling causing the panic to fail, hang, or behave unpredictably. This must be done in ALL panic entry points: Panic(), PanicWithCode(), PanicWithContext(), PanicFromInterrupt(), and at the start of KernelPanicHandler().
Applied to files:
arch/x86_64/interrupts/Interrupts.c
📚 Learning: 2025-08-10T06:22:28.816Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#62
File: kernel/core/Panic.c:69-79
Timestamp: 2025-08-10T06:22:28.816Z
Learning: In VoidFrame kernel, panic functions must disable interrupts with "cli" as the very first operation, before any other code executes. If interrupts are enabled (after "sti"), an interrupt can fire between the panic call and the cli instruction in KernelPanicHandler, causing the panic to fail or behave unpredictably.
Applied to files:
arch/x86_64/interrupts/Interrupts.c
🧬 Code graph analysis (4)
arch/x86_64/interrupts/Interrupts.h (1)
arch/x86_64/interrupts/Interrupts.c (1)
ToIRQ(18-20)
kernel/core/Kernel.c (1)
kernel/etc/Console.c (2)
PrintKernel(174-192)PrintKernelHex(228-251)
kernel/atomic/Atomics.h (1)
kernel/atomic/Atomics.c (37)
AtomicInc(10-12)AtomicDec(13-15)AtomicCmpxchg(16-19)AtomicRead(20-22)AtomicFetchAdd(25-27)AtomicFetchSub(28-30)AtomicExchange(31-33)AtomicStore(34-36)AtomicReadRelaxed(38-40)AtomicReadAcquire(41-43)AtomicStoreRelaxed(44-46)AtomicStoreRelease(47-49)AtomicBitTestAndSet(51-55)AtomicBitTestAndClear(56-60)AtomicFetchOr(61-63)AtomicFetchAnd(64-66)AtomicFetchXor(67-69)AtomicThreadFenceAcquire(71-71)AtomicThreadFenceRelease(72-72)AtomicThreadFenceSeqCst(73-73)AtomicInc64(76-78)AtomicDec64(79-81)AtomicFetchAdd64(82-84)AtomicFetchSub64(85-87)AtomicExchange64(88-90)AtomicCmpxchg64(91-93)AtomicRead64(94-96)AtomicStore64(97-99)AtomicReadRelaxed64(100-102)AtomicReadAcquire64(103-105)AtomicStoreRelaxed64(106-108)AtomicStoreRelease64(109-111)AtomicBitTestAndSet64(112-116)AtomicBitTestAndClear64(117-121)AtomicFetchOr64(122-124)AtomicFetchAnd64(125-127)AtomicFetchXor64(128-130)
arch/x86_64/interrupts/Interrupts.c (2)
mm/trace/StackTrace.c (2)
AnalyzeFault(169-194)PrintDetailedFaultInfo(196-234)kernel/etc/Format.c (1)
FormatS(288-295)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
kernel/atomic/Atomics.h (1)
34-38: Fences API looks good and portable.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
meson.build (1)
40-42: PIE/PIC flags conflict with-mcmodel=kernelEnabling
-fPIEand-fPICalongside-mcmodel=kernelcreates conflicting code generation models. The kernel memory model assumes static addresses above 0xFFFFFFFF80000000, while PIE/PIC generates position-independent code expecting relocation.Apply this diff to fix the conflict:
- '-fPIE', - '-fPIC', + '-fno-pie', + '-fno-pic', '-mcmodel=kernel',Based on past learnings,
-mcmodel=largecauses crashes during virtual address switching because it generates absolute 64-bit addresses incompatible with early boot code. Stick with-mcmodel=kerneland-fno-pie/-fno-picfor stability.
🧹 Nitpick comments (5)
kernel/sched/MLFQ.c (1)
442-442: Harden logging of “reason”.If reason could ever be non-kernel or NULL, PrintKernelSuccessF("%s") risks a fault. At minimum, guard NULL; ideally constrain to trusted literals/enums.
@@ static void __attribute__((visibility("hidden"))) ASTerminate(uint32_t pid, const char* reason) { - PrintKernelSuccessF("Astra: Terminated PID %d with reason %s\n", pid, reason); + if (!reason) reason = "unknown"; + PrintKernelSuccessF("Astra: Terminated PID %d with reason %s\n", pid, reason);meson.build (1)
306-306: Consider enabling UEFI boot option for testing.The commented UEFI BIOS option suggests you're preparing for UEFI support. When ready to test, ensure the OVMF firmware path exists on your system.
arch/x86_64/syscall/Syscall.c (1)
21-23: SYS_READ stub needs TODO documentation.The empty implementation should at least document its placeholder status.
case SYS_READ: - + // TODO: Implement read syscall with proper fd handling return 0;arch/x86_64/syscall/SyscallEntry.asm (1)
25-29: Syscall argument mapping doesn't match standard x86-64 ABI.The current mapping uses non-standard registers for syscall arguments. The x86-64 syscall ABI typically uses:
rax=syscall_num,rdi=arg1,rsi=arg2,rdx=arg3,r10=arg4,r8=arg5,r9=arg6.Your implementation maps:
rbx → arg1(should berdi)rcx → arg2(should bersi)r8 → arg3(should berdx)If this is intentional for your kernel's ABI, document it clearly. Otherwise, consider aligning with standard conventions:
- ; System call convention: rax=syscall_num, rdi=arg1, rsi=arg2, rdx=arg3 - mov rdi, rax ; syscall number - mov rsi, rbx ; arg1 - mov rdx, rcx ; arg2 - mov rcx, r8 ; arg3 + ; System call convention (custom): rax=syscall_num, rbx=arg1, rcx=arg2, r8=arg3 + ; Map to C calling convention: rdi=syscall_num, rsi=arg1, rdx=arg2, rcx=arg3 + mov rdi, rax ; syscall number + mov rsi, rbx ; arg1 + mov rdx, rcx ; arg2 + mov rcx, r8 ; arg3arch/x86_64/interrupts/Interrupts.asm (1)
298-298: Clarify or remove unused isr80 (vector 0x80)Interrupts.asm defines ISR_NOERRCODE 80 and emits
dq isr80(arch/x86_64/interrupts/Interrupts.asm:298,562) and Idt.c declaresextern void isr80()(arch/x86_64/idt/Idt.c:92), but the IDT gate for vector 80 is set toSyscallEntry(arch/x86_64/idt/Idt.c:368; arch/x86_64/syscall/SyscallEntry.asm). Remove or document the unusedisr80(or aliasSyscallEntry->isr80/ update the IDT) to avoid confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
arch/x86_64/idt/Idt.c(3 hunks)arch/x86_64/interrupts/Interrupts.asm(2 hunks)arch/x86_64/syscall/Syscall.c(1 hunks)arch/x86_64/syscall/Syscall.h(1 hunks)arch/x86_64/syscall/SyscallEntry.asm(1 hunks)kernel/sched/MLFQ.c(2 hunks)meson.build(5 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: README.md:9-9
Timestamp: 2025-09-01T01:10:22.123Z
Learning: VoidFrame's interrupt handlers in arch/x86_64/interrupts/Interrupts.asm only save basic GPRs and segment registers, missing critical registers for deep fault tracing: control registers (CR0/CR2/CR3/CR4), SIMD state (XMM/YMM/ZMM), debug registers (DR0-DR7), and extended FPU state. The CPU initialization code enables SSE/AVX/XSAVE features but interrupt handlers don't preserve this extended state. Enhanced fault tracing requires selective extended state saving for critical exceptions (ISR 6, 8, 13, 14) while keeping lightweight handling for regular interrupts.
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: docs/ARCHITECHTURE.md:1-3
Timestamp: 2025-08-31T14:19:11.366Z
Learning: assembler-0 wants to implement deep fault tracing for VoidFrame kernel exceptions (Page Fault, General Protection Fault, Invalid Opcode) instead of just panicking. The current interrupt handler has complete register state available in the Registers struct (defined in arch/x86_64/cpu/Cpu.h) including RBP for stack walking, RIP for fault location, and error codes for analysis.
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: README.md:9-9
Timestamp: 2025-09-01T01:10:22.123Z
Learning: The current VoidFrame Registers struct in arch/x86_64/cpu/Cpu.h only saves basic GPRs, segment registers, and exception context (interrupt_number, error_code, rip, cs, rflags, rsp, ss). It's missing critical registers for deep fault tracing: SIMD registers (XMM/YMM/ZMM), control registers (CR0/CR2/CR3/CR4), debug registers (DR0-DR7), and extended FPU state. The struct has a "DO NOT TOUCH" comment indicating it must match interrupt ASM stack layout.
Learnt from: assembler-0
PR: assembler-0/VoidFrame#93
File: kernel/memory/VMem.c:72-79
Timestamp: 2025-08-17T08:16:43.539Z
Learning: In VoidFrame kernel, VMemGetPhysAddr and related page table walking functions were causing panics because they assumed identity mapping existed for physical addresses < IDENTITY_MAP_SIZE (4GB), but the bootloader doesn't provide this 1:1 mapping. The fix is to remove all identity mapping assumptions and always use PHYS_TO_VIRT() for page table access in functions like VMemGetPageTablePhys, VMemMap, VMemMapHuge, VMemGetPhysAddr, VMemUnmap, VMemMapMMIO, and VMemUnmapMMIO.
📚 Learning: 2025-09-01T01:10:22.123Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: README.md:9-9
Timestamp: 2025-09-01T01:10:22.123Z
Learning: VoidFrame's interrupt handlers in arch/x86_64/interrupts/Interrupts.asm only save basic GPRs and segment registers, missing critical registers for deep fault tracing: control registers (CR0/CR2/CR3/CR4), SIMD state (XMM/YMM/ZMM), debug registers (DR0-DR7), and extended FPU state. The CPU initialization code enables SSE/AVX/XSAVE features but interrupt handlers don't preserve this extended state. Enhanced fault tracing requires selective extended state saving for critical exceptions (ISR 6, 8, 13, 14) while keeping lightweight handling for regular interrupts.
Applied to files:
arch/x86_64/interrupts/Interrupts.asmarch/x86_64/syscall/Syscall.harch/x86_64/idt/Idt.c
📚 Learning: 2025-09-02T02:37:06.993Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#106
File: kernel/sched/MLFQ.h:153-154
Timestamp: 2025-09-02T02:37:06.993Z
Learning: In VoidFrame MLFQ.c, the ProcessRuntimePath cleanup should happen for both normal exit (ProcessExitStub → TerminateProcess with TERM_NORMAL) and kill (MLFQKillProcess → TerminateProcess with TERM_KILLED) since both go through the same TerminateProcess function which has VfsDelete cleanup code when VF_CONFIG_PROCINFO_AUTO_CLEANUP is enabled. If normal exit leaves directories but kill doesn't, this suggests VfsDelete might be failing silently for normal exits, or there's a race condition in the cleanup logic.
Applied to files:
kernel/sched/MLFQ.c
📚 Learning: 2025-09-06T05:31:15.868Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#111
File: kernel/sched/MLFQ.c:1514-1514
Timestamp: 2025-09-06T05:31:15.868Z
Learning: In the MLFQ scheduler, removing MLFQYield() from critical system processes like DynamoX and Astra improves system responsiveness, indicating that the yield mechanism may be too aggressive for system-critical processes that need quick response times.
Applied to files:
kernel/sched/MLFQ.c
📚 Learning: 2025-09-02T00:55:58.224Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#106
File: kernel/sched/MLFQ.c:691-694
Timestamp: 2025-09-02T00:55:58.224Z
Learning: The VoidFrame MLFQ scheduler has a critical bug where ProcessRuntimePath uses FormatS() which returns a static buffer, causing all processes to share the same path string. This results in process directory cleanup operations affecting the wrong directories - killing one process can delete another process's /Runtime/Processes directory because they all point to the same FormatS static buffer.
Applied to files:
kernel/sched/MLFQ.c
📚 Learning: 2025-09-01T01:10:22.123Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: README.md:9-9
Timestamp: 2025-09-01T01:10:22.123Z
Learning: The current VoidFrame Registers struct in arch/x86_64/cpu/Cpu.h only saves basic GPRs, segment registers, and exception context (interrupt_number, error_code, rip, cs, rflags, rsp, ss). It's missing critical registers for deep fault tracing: SIMD registers (XMM/YMM/ZMM), control registers (CR0/CR2/CR3/CR4), debug registers (DR0-DR7), and extended FPU state. The struct has a "DO NOT TOUCH" comment indicating it must match interrupt ASM stack layout.
Applied to files:
arch/x86_64/syscall/Syscall.h
📚 Learning: 2025-08-13T07:26:56.892Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#79
File: drivers/PCI/PCI.c:154-154
Timestamp: 2025-08-13T07:26:56.892Z
Learning: VoidFrame kernel uses direct interrupt dispatch in arch/x86_64/interrupts/Interrupts.c rather than RegisterInterruptHandler pattern. Hardware interrupts are handled in switch cases by interrupt number (32+ for IRQs), with proper PIC EOI handling required.
Applied to files:
arch/x86_64/syscall/Syscall.h
📚 Learning: 2025-09-18T12:25:39.184Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#132
File: meson.build:40-41
Timestamp: 2025-09-18T12:25:39.184Z
Learning: The `-mcmodel=large` compiler flag generates absolute 64-bit addresses that are incompatible with early boot code that runs at physical addresses before virtual memory is enabled. When switching to virtual addressing, the kernel crashes because memory references point to invalid addresses during the transition. Use `-mcmodel=kernel` for kernels that run in canonical address space instead.
Applied to files:
meson.build
📚 Learning: 2025-08-31T14:19:11.366Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#103
File: docs/ARCHITECHTURE.md:1-3
Timestamp: 2025-08-31T14:19:11.366Z
Learning: assembler-0 wants to implement deep fault tracing for VoidFrame kernel exceptions (Page Fault, General Protection Fault, Invalid Opcode) instead of just panicking. The current interrupt handler has complete register state available in the Registers struct (defined in arch/x86_64/cpu/Cpu.h) including RBP for stack walking, RIP for fault location, and error codes for analysis.
Applied to files:
meson.build
📚 Learning: 2025-08-13T08:43:54.750Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#80
File: README.md:7-8
Timestamp: 2025-08-13T08:43:54.750Z
Learning: User assembler-0 is working on VoidFrame kernel with a tight deadline (2-3 days before summer ends) and specifically requested runtime safety checks similar to __stack_chk_fail but for deadlocks and other kernel issues. The kernel already has extensive safety mechanisms including stack canaries, guard pages, heap integrity checks, and process security validation.
Applied to files:
meson.build
📚 Learning: 2025-07-08T00:36:50.297Z
Learnt from: assembler-0
PR: assembler-0/VoidFrame#2
File: Kernel/Syscall.c:44-44
Timestamp: 2025-07-08T00:36:50.297Z
Learning: In IdtSetGate function calls, the parameters are: (interrupt_vector, handler_address, selector, flags). The interrupt vector (like 0x80) is the first argument, and the selector (like 0x08 for kernel code segment) is the third argument.
Applied to files:
arch/x86_64/idt/Idt.c
🧬 Code graph analysis (3)
kernel/sched/MLFQ.c (1)
kernel/etc/Console.c (1)
PrintKernelSuccessF(280-287)
arch/x86_64/syscall/Syscall.c (2)
kernel/sched/MLFQ.c (2)
MLFQKillCurrentProcess(1943-1947)MLFQYield(1001-1004)kernel/etc/Console.c (1)
PrintKernel(174-192)
arch/x86_64/syscall/Syscall.h (1)
arch/x86_64/syscall/Syscall.c (1)
SyscallHandler(5-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
kernel/sched/MLFQ.c (1)
1945-1946: LGTM: one-liner is fine.Behavior unchanged; simpler read.
arch/x86_64/syscall/Syscall.h (1)
1-18: LGTM! Well-structured syscall interface header.The header provides a clean, minimal syscall interface with proper include guards and standard syscall numbers. The use of 0x80 for the syscall vector follows classic x86 conventions.
arch/x86_64/syscall/SyscallEntry.asm (1)
50-50: Stack cleanup is correct — original RAX is saved and intentionally skipped.
arch/x86_64/syscall/SyscallEntry.asm pushes rax at the top (push rax, ~line 9) and later uses add rsp, 8 (line 50) to drop it; Idt.c registers SyscallEntry at vector 0x80. No change required.
| IdtSetGate(78, (uint64_t)isr78, kernelCodeSegment, flags); | ||
| IdtSetGate(79, (uint64_t)isr79, kernelCodeSegment, flags); | ||
| // IdtSetGate(80, (uint64_t)isr80, kernelCodeSegment, flags); | ||
| IdtSetGate(80, (uint64_t)SyscallEntry, SYSCALL_SEGMENT_SELECTOR, IDT_INTERRUPT_GATE_KERNEL); |
There was a problem hiding this comment.
🧩 Analysis chain
Good syscall gate implementation, but ensure consistency.
The syscall gate correctly uses the dedicated SyscallEntry handler with appropriate constants from Syscall.h.
Verify that the syscall entry correctly preserves the kernel state:
🏁 Script executed:
#!/bin/bash
# Verify the SyscallEntry implementation matches expected register preservation
echo "=== Checking SyscallEntry register preservation ==="
rg -A30 "^SyscallEntry:" arch/x86_64/syscall/SyscallEntry.asm
# Verify syscall handler is properly integrated
echo "=== Checking syscall integration ==="
ast-grep --pattern 'SyscallHandler($$$)' Length of output: 648
Fix incorrect argument setup in SyscallEntry (arch/x86_64/syscall/SyscallEntry.asm)
The entry saves registers but the moves that prepare SyscallHandler args (e.g. mov rdi, rax; mov rsi, rbx; mov rdx, rcx; mov rcx, r8) contradict the comment (rax=syscall_num, rdi=arg1, rsi=arg2, rdx=arg3) and will pass wrong values. Replace/reorder these moves so the original user registers are copied into the correct System V argument registers (rdi=syscall_num, rsi=arg1, rdx=arg2, rcx=arg3, r8=arg4, r9=arg5) or update the handler/comment to match, and ensure push/pop symmetry and proper RIP/RFLAGS handling.
🤖 Prompt for AI Agents
In arch/x86_64/idt/Idt.c around line 368 the syscall entry is wired to
SyscallEntry; the review points out that arch/x86_64/syscall/SyscallEntry.asm
currently moves registers incorrectly and will pass wrong values to
SyscallHandler. Edit SyscallEntry.asm to copy the original user registers into
System V argument registers with the correct mapping: rdi = syscall_num, rsi =
arg1, rdx = arg2, rcx = arg3, r8 = arg4, r9 = arg5 (or alternatively update the
handler/comment to match any deliberate different convention), ensure the
sequence of moves reflects that mapping (no overwrites), keep push/pop symmetry
for saved registers, and preserve correct RIP/RFLAGS handling on entry/return so
the stack layout and flag restoration remain correct.
| case SYS_WRITE: | ||
| // arg1 = fd (ignored for now), arg2 = buffer, arg3 = count | ||
| if (arg1 == 1) { // stdout | ||
| const char* buffer = (char*)arg2; | ||
| PrintKernel(buffer); | ||
| return arg3; | ||
| } | ||
| return -1; |
There was a problem hiding this comment.
Add bounds checking for buffer access in SYS_WRITE.
The SYS_WRITE implementation dereferences arg2 as a buffer pointer without validation. This could cause kernel panics if userspace passes invalid pointers.
Apply this diff to add basic validation:
case SYS_WRITE:
// arg1 = fd (ignored for now), arg2 = buffer, arg3 = count
if (arg1 == 1) { // stdout
+ if (!arg2 || arg3 > MAX_SYSCALL_BUFFER_SIZE) {
+ return -1; // EINVAL
+ }
const char* buffer = (char*)arg2;
+ // TODO: Add user/kernel memory boundary check
PrintKernel(buffer);
return arg3;
}
return -1;Consider implementing proper user-to-kernel buffer copying mechanism to prevent security vulnerabilities.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case SYS_WRITE: | |
| // arg1 = fd (ignored for now), arg2 = buffer, arg3 = count | |
| if (arg1 == 1) { // stdout | |
| const char* buffer = (char*)arg2; | |
| PrintKernel(buffer); | |
| return arg3; | |
| } | |
| return -1; | |
| case SYS_WRITE: | |
| // arg1 = fd (ignored for now), arg2 = buffer, arg3 = count | |
| if (arg1 == 1) { // stdout | |
| if (!arg2 || arg3 > MAX_SYSCALL_BUFFER_SIZE) { | |
| return -1; // EINVAL | |
| } | |
| const char* buffer = (char*)arg2; | |
| // TODO: Add user/kernel memory boundary check | |
| PrintKernel(buffer); | |
| return arg3; | |
| } | |
| return -1; |
🤖 Prompt for AI Agents
In arch/x86_64/syscall/Syscall.c around lines 12 to 19, the SYS_WRITE path
dereferences arg2 without validation; replace the direct dereference with
user-pointer and length validation, reject or truncate writes that exceed a
reasonable max (or the process's user memory limits), and copy data from
user-space into a kernel buffer using the existing safe user-to-kernel copy
helper (e.g., copy_from_user/get_user loop or a validate_user_pointer helper)
rather than accessing arg2 directly; on invalid pointer or failed copy return
-EFAULT and on success return the actual number of bytes copied/printed.
There was a problem hiding this comment.
bruh, i just add syscalls so its kinda posix compliant, im not adding ring 3
There was a problem hiding this comment.
@assembler-0, I understand your approach of getting basic POSIX syscalls working first! That makes sense for incremental kernel development.
For now, you could add a simple null pointer check without needing ring 3 protection:
case SYS_WRITE:
if (arg1 == 1) { // stdout
if (!arg2) return -1; // Just check for null
const char* buffer = (char*)arg2;
PrintKernel(buffer);
return arg3;
}
return -1;The full memory protection can definitely wait until you implement proper user/kernel separation later. This gives you basic POSIX compliance without the complexity.
✏️ Learnings added
Learnt from: assembler-0
PR: assembler-0/VoidFrame#132
File: arch/x86_64/syscall/Syscall.c:12-19
Timestamp: 2025-09-19T11:55:43.301Z
Learning: In the VoidFrame kernel project, assembler-0 is implementing basic POSIX-compliant syscalls in ring 0 without user/kernel memory protection, prioritizing getting basic functionality working before adding full security features.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: assembler-0
PR: assembler-0/VoidFrame#72
File: arch/x86_64/syscall/SyscallEntry.asm:24-30
Timestamp: 2025-08-12T01:43:39.847Z
Learning: User prefers to defer syscall implementation complexity until next year and is considering making a ring 0 kernel instead of continuing with user-mode syscall approach in the VoidFrame kernel project.
| PrintKernel("System: Cleanup successful\n"); | ||
| } | ||
| #endif | ||
| PrintKernelSuccessF("Astra: Terminated PID %d with reason %s\n", pid, reason); |
There was a problem hiding this comment.
Fix enqueue-before-ZOMBIE race in ASTerminate.
ASTerminate enqueues the slot before setting state to PROC_ZOMBIE. MLFQCleanupTerminatedProcess explicitly skips entries not in PROC_ZOMBIE, so a concurrent cleanup can drop the entry and leak resources. TerminateProcess uses the safe order (ZOMBIE -> fence -> enqueue). Mirror that here.
Apply:
@@ static void __attribute__((visibility("hidden"))) ASTerminate(uint32_t pid, const char* reason) {
- AddToTerminationQueueAtomic(slot);
- proc->state = PROC_ZOMBIE;
+ // Set ZOMBIE before enqueue to avoid cleanup seeing a DYING proc and skipping it
+ proc->state = PROC_ZOMBIE;
+ __atomic_thread_fence(__ATOMIC_SEQ_CST);
+ AddToTerminationQueueAtomic(slot);Committable suggestion skipped: line range outside the PR's diff.
1 participant
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores