robert-w-gries / rxinu

Rust implementation of Xinu educational operating system
Apache License 2.0
34 stars 4 forks source link

Musical notes are printed instead of "IT DID NOT CRASH!" (WIP: To be copied to blog_os) #14

Closed robert-w-gries closed 6 years ago

robert-w-gries commented 6 years ago

The initial problem

After implementing interrupts in my kernel, I noticed a strange issue where the kernel did not seem to properly print the stack trace of a page fault.

[image of broken exception printing]()

Narrowing down the issue to printing

This issue is not specific to just printing exception stack frames. I also see the issue when printing "It did not crash!". The issue always causes a page fault cpu exception:

[image of cpu exception]()

The virtual address that caused the exception points to a string that seemed to be in a sane location that should have not caused a page fault. Even weirder was that the page fault came up intermittently.

Garbage musical notes (undefined behavior?)

I played around with the kernel to narrow down where the issue seems to be triggered. The trigger point appears to be the loading of the IDT. To force the issue to appear, I added an #[inline(always)] before the interrupts::init function and added several print methods. I then saw the following strange behavior:

[image of weird printing musical notes]()

Workaround?

I was stumped for several months on how to fix this issue. Nothing I tried seemed to fix it until I tried removing the lazy static definition of the IDT and moving to a static mut.

diff --git a/arch/x86/x86_64/src/interrupts/mod.rs b/arch/x86/x86_64/src/interrupts/mod.rs
index 34954a8..5f900dc 100644
--- a/arch/x86/x86_64/src/interrupts/mod.rs
+++ b/arch/x86/x86_64/src/interrupts/mod.rs
@@ -8,6 +8,7 @@ mod irq;

 const DOUBLE_FAULT_IST_INDEX: usize = 0;

+/*
 lazy_static! {
     static ref IDT: Idt = {
         let mut idt = Idt::new();
@@ -29,11 +30,15 @@ lazy_static! {
         idt
     };
 }
+*/
+
+static mut IDT: Option<Idt> = None;

 static TSS: Once<TaskStateSegment> = Once::new();
 static GDT: Once<gdt::Gdt> = Once::new();

 /// Initialize double fault stack and load gdt and idt 
+#[inline(always)]
 pub fn init(memory_controller: &mut MemoryController) {
     use x86_64::VirtualAddress;
     use x86_64::structures::gdt::SegmentSelector;
@@ -64,9 +69,34 @@ pub fn init(memory_controller: &mut MemoryController) {
         // reload code segment register and load TSS
         set_cs(code_selector);
         load_tss(tss_selector);
-    }

-    IDT.load();
+        // setup IDT
+        IDT = Some(Idt::new());
+        let idt = IDT.as_mut().unwrap();
+        idt.breakpoint.set_handler_fn(breakpoint_handler);
+        idt.divide_by_zero.set_handler_fn(divide_by_zero_handler);
+        idt.invalid_opcode.set_handler_fn(invalid_opcode_handler);
+        idt.page_fault.set_handler_fn(page_fault_handler);
+
+        /// The set_stack_index method is unsafe because the caller must ensure
+        /// that the used index is valid and not already used for another exception.
+        unsafe {
+            idt.double_fault.set_handler_fn(double_fault_handler)
+                .set_stack_index(DOUBLE_FAULT_IST_INDEX as u16);
+        }
+
+        idt.interrupts[2].set_handler_fn(irq::cascade);
+        idt.interrupts[3].set_handler_fn(irq::com2);
+        idt.interrupts[4].set_handler_fn(irq::com1);
+
+        idt.load();
+        println!("TEST!");
+        println!("TEST!");
+        println!("TEST!");
+        println!("TEST!");
+        println!("TEST!");
+        println!("TEST!");
+    }
 }

With this change, the kernel does not hit the random page fault and weird behavior. Please let me know if this is just hiding the real issue or if this is an actual workaround. I am also suspecting the issue is related to the lazy_static crate instead of this repository.

EDIT: This is a duplicate of #11