From b1c8df2a7f3a19616bc6dce04b66f02a727b3355 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Fri, 26 Oct 2018 16:08:51 +0200 Subject: [PATCH 1/3] Don't compile the interrupts module on Windows to fix cargo test --- .../posts/06-cpu-exceptions/index.md | 30 +++++++++++++++++++ src/interrupts.rs | 6 ++++ 2 files changed, 36 insertions(+) diff --git a/blog/content/second-edition/posts/06-cpu-exceptions/index.md b/blog/content/second-edition/posts/06-cpu-exceptions/index.md index 8c48b129..189e32d7 100644 --- a/blog/content/second-edition/posts/06-cpu-exceptions/index.md +++ b/blog/content/second-edition/posts/06-cpu-exceptions/index.md @@ -494,6 +494,36 @@ The [`Ordering`] parameter specifies the desired guarantees of the atomic operat [`Ordering`]: https://doc.rust-lang.org/core/sync/atomic/enum.Ordering.html +### Fixing `cargo test` on Windows + +The `x86-interrupt` calling convention has an annoying problem: There is a bug in LLVM that leads to a "LLVM ERROR: offset is not a multiple of 16" when compiling a function with the `x86-interrupt` calling convention _for a Windows target_. Normally this is no problem, since we only compile for our custom `x86_64-blog_os.json` target. But `cargo test` compiles our crate for the host system, so the error occurs if the host system is Windows. + +To fix this problem, we add a conditional compilation attribute, so that the `x86-interrupt` functions are not compiled on Windows systems. We don't have any unit tests that rely on the `interrupts` module, so we can simply skip compilation of the whole module: + +```rust +// in src/interrupts.rs + +// LLVM throws an error if a function with the +// x86-interrupt calling convention is compiled +// for a Windows system. +#![cfg(not(windows))] +``` + +The bang ("!") after the hash ("#") indicates that this is an [inner attribute] and applies to the module we're in. Without the bang it would only apply to the next item in the file. Note that inner attributes must be right at the beginning of a module. + +[inner attribute]: https://doc.rust-lang.org/reference/attributes.html + +We could achieve the same effect by using an _outer_ attribute (without a bang) in our `lib.rs`: + +```rust +// in src/lib.rs + +#[cfg(not(windows))] // no bang ("!") after the hash ("#") +pub mod interrupts; +``` + +Both variants have the exact same effects, so it comes down to personal preference which one to use. I prefer the inner attribute in this case because it does not clutter our `lib.rs` with a workaround for a LLVM bug, but either way is fine. + ## Too much Magic? The `x86-interrupt` calling convention and the [`InterruptDescriptorTable`] type made the exception handling process relatively straightforward and painless. If this was too much magic for you and you like to learn all the gory details of exception handling, we got you covered: Our [“Handling Exceptions with Naked Functions”] series shows how to handle exceptions without the `x86-interrupt` calling convention and also creates its own IDT type. Historically, these posts were the main exception handling posts before the `x86-interrupt` calling convention and the `x86_64` crate existed. Note that these posts are based on the [first edition] of this blog and might be out of date. diff --git a/src/interrupts.rs b/src/interrupts.rs index a43a5991..b1cdf323 100644 --- a/src/interrupts.rs +++ b/src/interrupts.rs @@ -1,3 +1,9 @@ +// The x86-interrupt calling convention leads to the following LLVM error +// when compiled for a Windows target: "offset is not a multiple of 16". This +// happens for example when running `cargo test` on Windows. To avoid this +// problem we skip compilation of this module on Windows. +#![cfg(not(windows))] + use gdt; use pic8259_simple::ChainedPics; use spin; From d802763867ef316f1e0daa8d99d011ca1711be3f Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Fri, 26 Oct 2018 16:28:56 +0200 Subject: [PATCH 2/3] Appveyor: Only run `cargo test` on x86_64 Many parts of the `x86_64` library are not available on 32-bit x86 and we currently don't do much target dependent conditional compilation in blog_os since we only support a single architecture at the moment. --- .appveyor.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.appveyor.yml b/.appveyor.yml index 5f40248e..e0c30afb 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -91,5 +91,6 @@ before_test: # environment variable. test_script: - bootimage build - - cargo test + - if %target%==x86_64-pc-windows-gnu cargo test + - if %target%==x86_64-pc-windows-msvc cargo test - bootimage test From b8206b895f56ca94931dabaa82206ddab49b6014 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Fri, 26 Oct 2018 16:44:42 +0200 Subject: [PATCH 3/3] Move PICS import into _start function --- .../second-edition/posts/08-hardware-interrupts/index.md | 4 +++- src/main.rs | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/blog/content/second-edition/posts/08-hardware-interrupts/index.md b/blog/content/second-edition/posts/08-hardware-interrupts/index.md index d228cc7d..e2fdcfaf 100644 --- a/blog/content/second-edition/posts/08-hardware-interrupts/index.md +++ b/blog/content/second-edition/posts/08-hardware-interrupts/index.md @@ -147,12 +147,14 @@ Until now nothing happened because interrupts are still disabled in the CPU conf #[cfg(not(test))] #[no_mangle] pub extern "C" fn _start() -> ! { + use blog_os::interrupts::PICS; // new + println!("Hello World{}", "!"); blog_os::gdt::init(); blog_os::interrupts::init_idt(); unsafe { PICS.lock().initialize() }; - x86_64::instructions::interrupts::enable(); // new + x86_64::instructions::interrupts::enable(); // new println!("It did not crash!"); loop {} diff --git a/src/main.rs b/src/main.rs index 71dfc701..c9890b1b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,7 +6,6 @@ extern crate blog_os; extern crate x86_64; -use blog_os::interrupts::PICS; use core::panic::PanicInfo; /// This function is the entry point, since the linker looks for a function @@ -14,6 +13,8 @@ use core::panic::PanicInfo; #[cfg(not(test))] #[no_mangle] // don't mangle the name of this function pub extern "C" fn _start() -> ! { + use blog_os::interrupts::PICS; + println!("Hello World{}", "!"); blog_os::gdt::init();