From b595d26cb33733b53fff2483e183d90dfe547d26 Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Fri, 23 Jun 2023 13:51:36 +0200 Subject: [PATCH 1/2] ci: add miri --- .github/workflows/_build-rust.yml | 12 ++++++++++++ .github/workflows/rust.yml | 11 +++++++++++ multiboot2-header/src/builder/header.rs | 1 + multiboot2-header/src/builder/information_request.rs | 1 + multiboot2-header/src/builder/traits.rs | 1 + multiboot2/src/boot_loader_name.rs | 3 ++- multiboot2/src/builder/information.rs | 1 + multiboot2/src/builder/mod.rs | 2 +- multiboot2/src/command_line.rs | 3 ++- multiboot2/src/lib.rs | 10 ++++++++++ multiboot2/src/module.rs | 3 ++- multiboot2/src/smbios.rs | 1 + 12 files changed, 45 insertions(+), 4 deletions(-) diff --git a/.github/workflows/_build-rust.yml b/.github/workflows/_build-rust.yml index 02f21b28..7152072b 100644 --- a/.github/workflows/_build-rust.yml +++ b/.github/workflows/_build-rust.yml @@ -50,6 +50,11 @@ on: required: false default: true description: Execute tests. + do-miri: + type: boolean + required: false + default: false + description: Execute unit tests with miri. jobs: rust: @@ -104,3 +109,10 @@ jobs: Expand-Archive .\cargo-nextest.zip cp .\cargo-nextest/cargo-nextest.exe . .\cargo-nextest.exe nextest run --features ${{ inputs.features }} --no-default-features + - name: Unit Test with Miri + if: inputs.do-miri + # "--tests" so that the doctests are skipped. Currently, the doctest + # in miri fails. + run: | + rustup component add miri + cargo miri test --tests diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 996416ba..a5a1a217 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -161,3 +161,14 @@ jobs: - run: cd integration-test && nix-shell --run "echo OK" && cd .. # Now, run the actual test. - run: cd integration-test && nix-shell --run ./run.sh && cd .. + + miri: + name: tests with miri (nightly) + needs: build_nightly + uses: ./.github/workflows/_build-rust.yml + with: + rust-version: nightly + do-style-check: false + do-test: false + do-miri: true + features: builder,unstable diff --git a/multiboot2-header/src/builder/header.rs b/multiboot2-header/src/builder/header.rs index d406d13e..0ab65d28 100644 --- a/multiboot2-header/src/builder/header.rs +++ b/multiboot2-header/src/builder/header.rs @@ -356,6 +356,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn test_builder() { // Step 1/2: Build Header let mb2_hdr_data = create_builder().build(); diff --git a/multiboot2-header/src/builder/information_request.rs b/multiboot2-header/src/builder/information_request.rs index 65ca22fd..543e7cc9 100644 --- a/multiboot2-header/src/builder/information_request.rs +++ b/multiboot2-header/src/builder/information_request.rs @@ -98,6 +98,7 @@ mod tests { use crate::{HeaderTagFlag, InformationRequestHeaderTag, MbiTagType, MbiTagTypeId}; #[test] + #[cfg_attr(miri, ignore)] fn test_builder() { let builder = InformationRequestHeaderTagBuilder::new(HeaderTagFlag::Required) .add_ir(MbiTagType::EfiMmap) diff --git a/multiboot2-header/src/builder/traits.rs b/multiboot2-header/src/builder/traits.rs index 67bd2fb6..f936c4e8 100644 --- a/multiboot2-header/src/builder/traits.rs +++ b/multiboot2-header/src/builder/traits.rs @@ -51,6 +51,7 @@ mod tests { use super::*; #[test] + #[cfg_attr(miri, ignore)] fn test_as_bytes() { struct Foobar { a: u32, diff --git a/multiboot2/src/boot_loader_name.rs b/multiboot2/src/boot_loader_name.rs index 5e9863fc..3785658a 100644 --- a/multiboot2/src/boot_loader_name.rs +++ b/multiboot2/src/boot_loader_name.rs @@ -82,7 +82,7 @@ mod tests { const MSG: &str = "hello"; - /// Returns the tag structure in bytes in native endian format. + /// Returns the tag structure in bytes in little endian format. fn get_bytes() -> std::vec::Vec { // size is: 4 bytes for tag + 4 bytes for size + length of null-terminated string let size = (4 + 4 + MSG.as_bytes().len() + 1) as u32; @@ -101,6 +101,7 @@ mod tests { /// Tests to parse a string with a terminating null byte from the tag (as the spec defines). #[test] + #[cfg_attr(miri, ignore)] fn test_parse_str() { let tag = get_bytes(); let tag = unsafe { &*tag.as_ptr().cast::() }; diff --git a/multiboot2/src/builder/information.rs b/multiboot2/src/builder/information.rs index adc23415..80122bf1 100644 --- a/multiboot2/src/builder/information.rs +++ b/multiboot2/src/builder/information.rs @@ -408,6 +408,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn test_builder() { // Step 1/2: Build MBI let mb2i_data = create_builder().build(); diff --git a/multiboot2/src/builder/mod.rs b/multiboot2/src/builder/mod.rs index d9b1b0d0..6e8392f9 100644 --- a/multiboot2/src/builder/mod.rs +++ b/multiboot2/src/builder/mod.rs @@ -139,7 +139,7 @@ mod tests { let tag_type_id = 1337_u32; let content = "hallo"; - let tag = unsafe { BoxedDst::::new(tag_type_id, content.as_bytes()) }; + let tag = BoxedDst::::new(tag_type_id, content.as_bytes()); assert_eq!(tag.typ, tag_type_id); assert_eq!(tag.size as usize, METADATA_SIZE + content.len()); assert_eq!(tag.string(), Ok(content)); diff --git a/multiboot2/src/command_line.rs b/multiboot2/src/command_line.rs index fc15cbb3..07200023 100644 --- a/multiboot2/src/command_line.rs +++ b/multiboot2/src/command_line.rs @@ -91,7 +91,7 @@ mod tests { const MSG: &str = "hello"; - /// Returns the tag structure in bytes in native endian format. + /// Returns the tag structure in bytes in little endian format. fn get_bytes() -> std::vec::Vec { // size is: 4 bytes for tag + 4 bytes for size + length of null-terminated string let size = (4 + 4 + MSG.as_bytes().len() + 1) as u32; @@ -110,6 +110,7 @@ mod tests { /// Tests to parse a string with a terminating null byte from the tag (as the spec defines). #[test] + #[cfg_attr(miri, ignore)] fn test_parse_str() { let tag = get_bytes(); let tag = unsafe { &*tag.as_ptr().cast::() }; diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index f67294e3..8036f53f 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -661,6 +661,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn name_tag() { #[repr(C, align(8))] struct Bytes([u8; 32]); @@ -695,6 +696,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn framebuffer_tag_rgb() { // direct RGB mode test: // taken from GRUB2 running in QEMU at @@ -755,6 +757,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn framebuffer_tag_indexed() { // indexed mode test: // this is synthetic, as I can't get QEMU @@ -826,6 +829,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn vbe_info_tag() { //Taken from GRUB2 running in QEMU. #[repr(C, align(8))] @@ -996,6 +1000,7 @@ mod tests { /// Tests to parse a MBI that was statically extracted from a test run with /// GRUB as bootloader. #[test] + #[cfg_attr(miri, ignore)] fn grub2() { #[repr(C, align(8))] struct Bytes([u8; 960]); @@ -1411,6 +1416,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn elf_sections() { #[repr(C, align(8))] struct Bytes([u8; 168]); @@ -1487,6 +1493,7 @@ mod tests { } #[test] + #[cfg_attr(miri, ignore)] fn efi_memory_map() { #[repr(C, align(8))] struct Bytes([u8; 72]); @@ -1565,6 +1572,7 @@ mod tests { /// Example for a custom tag. #[test] + #[cfg_attr(miri, ignore)] fn get_custom_tag_from_mbi() { const CUSTOM_TAG_ID: u32 = 0x1337; @@ -1626,6 +1634,7 @@ mod tests { /// Example for a custom DST tag. #[test] + #[cfg_attr(miri, ignore)] fn get_custom_dst_tag_from_mbi() { const CUSTOM_TAG_ID: u32 = 0x1337; @@ -1703,6 +1712,7 @@ mod tests { /// Tests that `get_tag` can consume multiple types that implement `Into` #[test] + #[cfg_attr(miri, ignore)] fn get_tag_into_variants() { #[repr(C, align(8))] struct Bytes([u8; 32]); diff --git a/multiboot2/src/module.rs b/multiboot2/src/module.rs index 9661d889..b105cb59 100644 --- a/multiboot2/src/module.rs +++ b/multiboot2/src/module.rs @@ -131,7 +131,7 @@ mod tests { const MSG: &str = "hello"; - /// Returns the tag structure in bytes in native endian format. + /// Returns the tag structure in bytes in little endian format. fn get_bytes() -> std::vec::Vec { // size is: 4 bytes for tag + 4 bytes for size + length of null-terminated string // 4 bytes mod_start + 4 bytes mod_end @@ -153,6 +153,7 @@ mod tests { /// Tests to parse a string with a terminating null byte from the tag (as the spec defines). #[test] + #[cfg_attr(miri, ignore)] fn test_parse_str() { let tag = get_bytes(); let tag = unsafe { &*tag.as_ptr().cast::() }; diff --git a/multiboot2/src/smbios.rs b/multiboot2/src/smbios.rs index bd7eff27..9f5ce0d8 100644 --- a/multiboot2/src/smbios.rs +++ b/multiboot2/src/smbios.rs @@ -77,6 +77,7 @@ mod tests { /// Test to parse a given tag. #[test] + #[cfg_attr(miri, ignore)] fn test_parse() { let tag = get_bytes(); let tag = unsafe { &*tag.as_ptr().cast::() }; From 19adde195c358d78448b802c5a642d24550d122b Mon Sep 17 00:00:00 2001 From: Philipp Schuster Date: Fri, 23 Jun 2023 13:59:42 +0200 Subject: [PATCH 2/2] multiboot2: remove unused align(8) structs I decided so as they only make things more complicated for miri. As we have mature builder and parser structs guaranteeing alignment, we do not have to create additional confusion. These align(8) markers do not have value-add here besides being informational. They are only useful if no crazy pointer magic is involved and Rust constructs these types "in the Rust-typical way". --- multiboot2/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/multiboot2/src/lib.rs b/multiboot2/src/lib.rs index 8036f53f..88c712a7 100644 --- a/multiboot2/src/lib.rs +++ b/multiboot2/src/lib.rs @@ -145,7 +145,7 @@ impl core::error::Error for MbiLoadError {} /// The basic header of a boot information. #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[repr(C, align(8))] +#[repr(C)] pub struct BootInformationHeader { // size is multiple of 8 total_size: u32, @@ -1638,7 +1638,7 @@ mod tests { fn get_custom_dst_tag_from_mbi() { const CUSTOM_TAG_ID: u32 = 0x1337; - #[repr(C, align(8))] + #[repr(C)] #[derive(crate::Pointee)] struct CustomTag { tag: TagTypeId,