Merge #55

55: Version 0.6.0 r=phil-opp a=phil-opp

The main change of this PR is to remove the `#[cfg(not(test))]` attribute from the `entry_point` macro. This makes it possible to use the macro together with custom test frameworks and `cargo xtest`. Since this is a breaking change, this PR bumps the version to 0.6.0.

This PR also adds two additional checks to the build script:

- check that the kernel exists early to provide a better error message
- check that the kernel has a non-empty text section
  - an empty text section can occur if no entry point is set
  - this turns a runtime error (bootloader panics when loading the kernel) into a compile time error

Co-authored-by: Philipp Oppermann <[email protected]>

Author: bors[bot]

Cargo.lock

@@ -2,7 +2,7 @@ cargo-features = ["publish-lockfile"]
 
 [package]
 name = "bootloader"
-version = "0.5.3"
+version = "0.6.0"
 authors = ["Philipp Oppermann <[email protected]>"]
 license = "MIT/Apache-2.0"
 description = "An experimental pure-Rust x86 bootloader."

Cargo.toml

@@ -1,3 +1,17 @@
+# 0.6.0
+
+## Breaking
+
+- Don't set the `#[cfg(not(test))]` attribute for the entry point function in the `entry_point` macro
+    - With custom test frameworks, it's possible to use the normal entry point also in test environments
+    - To get the old behavior, you can add the `#[cfg(not(test))]` attribute to the `entry_point` invocation
+
+## Other
+
+- Additional assertions for the passed `KERNEL` executable
+    - check that the executable exists (for better error messages)
+    - check that the executable has a non-empty text section (an empty text section occurs when no entry point is set)
+
 # 0.5.3
 
 - Mention minimal required bootimage version in error message when `KERNEL` environment variable is not set.

Changelog.md

@@ -27,6 +27,12 @@ fn main() {
         }
     });
 
+    // check that the kernel file exists
+    assert!(
+        kernel.exists(),
+        format!("KERNEL does not exist: {}", kernel.display())
+    );
+
     let kernel_file_name = kernel
         .file_name()
         .expect("KERNEL has no valid file name")
@@ -36,6 +42,7 @@ fn main() {
     let kernel_out_path = out_dir.join(format!("kernel_bin-{}.o", kernel_file_name));
     let kernel_archive_path = out_dir.join(format!("libkernel_bin-{}.a", kernel_file_name));
 
+    // get access to llvm tools shipped in the llvm-tools-preview rustup component
     let llvm_tools = match llvm_tools::LlvmTools::new() {
         Ok(tools) => tools,
         Err(llvm_tools::Error::NotFound) => {
@@ -49,10 +56,28 @@ fn main() {
             process::exit(1);
         }
     };
+
+    // check that kernel executable has code in it
+    let llvm_size = llvm_tools
+        .tool(&llvm_tools::exe("llvm-size"))
+        .expect("llvm-size not found in llvm-tools");
+    let mut cmd = Command::new(llvm_size);
+    cmd.arg(&kernel);
+    let output = cmd.output().expect("failed to run llvm-size");
+    let output_str = String::from_utf8_lossy(&output.stdout);
+    let second_line_opt = output_str.lines().skip(1).next();
+    let second_line = second_line_opt.expect("unexpected llvm-size line output");
+    let text_size_opt = second_line.split_ascii_whitespace().next();
+    let text_size = text_size_opt.expect("unexpected llvm-size output");
+    if text_size == "0" {
+        panic!("Kernel executable has an empty text section. Perhaps the entry point was set incorrectly?\n\n\
+            Kernel executable at `{}`\n", kernel.display());
+    }
+
+    // wrap the kernel executable as binary in a new ELF file
     let objcopy = llvm_tools
         .tool(&llvm_tools::exe("llvm-objcopy"))
         .expect("llvm-objcopy not found in llvm-tools");
-
     let mut cmd = Command::new(objcopy);
     cmd.arg("-I").arg("binary");
     cmd.arg("-O").arg("elf64-x86-64");
@@ -79,6 +104,7 @@ fn main() {
         process::exit(1);
     }
 
+    // create an archive for linking
     let ar = llvm_tools
         .tool(&llvm_tools::exe("llvm-ar"))
         .unwrap_or_else(|| {
@@ -97,12 +123,14 @@ fn main() {
         process::exit(1);
     }
 
-    println!("cargo:rerun-if-env-changed=KERNEL");
-    println!("cargo:rerun-if-changed={}", kernel.display());
-    println!("cargo:rerun-if-changed=build.rs");
+    // pass link arguments to rustc
     println!("cargo:rustc-link-search=native={}", out_dir.display());
     println!(
         "cargo:rustc-link-lib=static=kernel_bin-{}",
         kernel_file_name
     );
+
+    println!("cargo:rerun-if-env-changed=KERNEL");
+    println!("cargo:rerun-if-changed={}", kernel.display());
+    println!("cargo:rerun-if-changed=build.rs");
 }

build.rs

@@ -22,7 +22,6 @@ pub mod bootinfo;
 #[macro_export]
 macro_rules! entry_point {
     ($path:path) => {
-        #[cfg(not(test))]
         #[export_name = "_start"]
         pub extern "C" fn __impl_start(boot_info: &'static $crate::bootinfo::BootInfo) -> ! {
             // validate the signature of the program entry point