Merge branch 'master' into raw-syscalls I cleaned up the #![feature(min_const_generics)] statements because the toolchain update made them unnecessary.
diff --git a/Makefile b/Makefile index a37e40c..b8d1f70 100644 --- a/Makefile +++ b/Makefile
@@ -84,12 +84,19 @@ # Important: This tests a platform without atomic instructions. PLATFORM=opentitan cargo build --release --target=riscv32imc-unknown-none-elf --examples -p libtock -p libtock_core +# Arguments to pass to cargo to exclude libtock_runtime and crates that depend +# on libtock_runtime. Used when we need to build a crate for the host OS, as +# libtock_runtime only suppors running on Tock. +EXCLUDE_RUNTIME := --exclude libtock_runtime + .PHONY: test test: examples test-qemu-hifive LIBTOCK_PLATFORM=nrf52 PLATFORM=nrf52 cargo fmt --all -- --check PLATFORM=nrf52 cargo clippy --all-targets --exclude libtock_runtime --workspace LIBTOCK_PLATFORM=hifive1 cargo clippy --target=riscv32imac-unknown-none-elf -p libtock_runtime - PLATFORM=nrf52 cargo miri test --exclude libtock_runtime --workspace + PLATFORM=nrf52 cargo miri test $(EXCLUDE_RUNTIME) --workspace + MIRIFLAGS="-Zmiri-symbolic-alignment-check -Zmiri-track-raw-pointers" \ + PLATFORM=nrf52 cargo miri test $(EXCLUDE_RUNTIME) --workspace echo '[ SUCCESS ] libtock-rs tests pass' .PHONY: analyse-stack-sizes
diff --git a/core/platform/src/command_return_tests.rs b/core/platform/src/command_return_tests.rs index 439f9e7..85583ad 100644 --- a/core/platform/src/command_return_tests.rs +++ b/core/platform/src/command_return_tests.rs
@@ -130,7 +130,7 @@ assert_eq!(command_return.get_failure_2_u32(), None); assert_eq!( command_return.get_failure_u64(), - Some((ErrorCode::Busy, 0x00001003_00001002)) + Some((ErrorCode::Busy, 0x0000_1003_0000_1002)) ); assert_eq!(command_return.get_success_u32(), None); assert_eq!(command_return.get_success_2_u32(), None); @@ -240,7 +240,10 @@ assert_eq!(command_return.get_failure_u64(), None); assert_eq!(command_return.get_success_u32(), None); assert_eq!(command_return.get_success_2_u32(), None); - assert_eq!(command_return.get_success_u64(), Some(0x00001002_00001001)); + assert_eq!( + command_return.get_success_u64(), + Some(0x0000_1002_0000_1001) + ); assert_eq!(command_return.get_success_3_u32(), None); assert_eq!(command_return.get_success_u32_u64(), None); assert_eq!(command_return.return_variant(), return_variant::SUCCESS_U64); @@ -299,7 +302,7 @@ assert_eq!(command_return.get_success_3_u32(), None); assert_eq!( command_return.get_success_u32_u64(), - Some((1001, 0x00001003_00001002)) + Some((1001, 0x0000_1003_0000_1002)) ); assert_eq!( command_return.return_variant(),
diff --git a/core/platform/src/lib.rs b/core/platform/src/lib.rs index 78293e3..e189975 100644 --- a/core/platform/src/lib.rs +++ b/core/platform/src/lib.rs
@@ -1,5 +1,5 @@ -#![feature(min_const_generics)] // TODO: Remove when toolchain updated. #![no_std] +#![warn(unsafe_op_in_unsafe_fn)] mod async_traits; mod command_return;
diff --git a/core/runtime/src/lib.rs b/core/runtime/src/lib.rs index bd890ea..f705c45 100644 --- a/core/runtime/src/lib.rs +++ b/core/runtime/src/lib.rs
@@ -21,8 +21,6 @@ #![feature(asm)] #![no_std] #![warn(unsafe_op_in_unsafe_fn)] -// TODO: Remove after toolchain upgrade, these have been stabilized. -#![feature(min_const_generics, unsafe_block_in_unsafe_fn)] mod startup;
diff --git a/core/src/entry_point/start_item_arm.rs b/core/src/entry_point/start_item_arm.rs index 894088e..5d88d6b 100644 --- a/core/src/entry_point/start_item_arm.rs +++ b/core/src/entry_point/start_item_arm.rs
@@ -1,18 +1,12 @@ -use core::hint; - /// Tock programs' entry point. Called by the kernel at program start. Sets up /// the stack then calls rust_start() for the remainder of setup. #[doc(hidden)] #[no_mangle] #[naked] #[link_section = ".start"] -pub unsafe extern "C" fn _start( - app_start: usize, - mem_start: usize, - _memory_len: usize, - app_heap_break: usize, -) -> ! { - llvm_asm!(" +pub unsafe extern "C" fn _start() -> ! { + asm!( + " // Because ROPI-RWPI support in LLVM/rustc is incomplete, Rust // applications must be statically linked. An offset between the // location the program is linked at and its actual location in flash @@ -102,12 +96,8 @@ mov r2, r8 // Call rust_start - bl rust_start" - : // No output operands - : "{r0}"(app_start), "{r1}"(mem_start), "{r3}"(app_heap_break) // Input operands - : "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r8", "r12", - "cc", "memory" // Clobbers - : "volatile" // Options - ); - hint::unreachable_unchecked() + bl rust_start", + // No clobbers because we don't return. + options(noreturn), + ) }
diff --git a/core/src/entry_point/start_item_riscv32.rs b/core/src/entry_point/start_item_riscv32.rs index f8a0e19..ec63591 100644 --- a/core/src/entry_point/start_item_riscv32.rs +++ b/core/src/entry_point/start_item_riscv32.rs
@@ -1,5 +1,3 @@ -use core::hint; - /// Tock programs' entry point. Called by the kernel at program start. Sets up /// the stack then calls rust_start() for the remainder of setup. #[doc(hidden)] @@ -14,12 +12,12 @@ // Due to Rust issue: https://github.com/rust-lang/rust/issues/42779 we can't have // args to the function pub unsafe extern "C" fn _start() -> ! { - llvm_asm!( - // Compute the stack top. - // - // struct hdr* myhdr = (struct hdr*) app_start; - // uint32_t stacktop = (((uint32_t) mem_start + myhdr->stack_size + 7) & 0xfffffff8); - "lw t0, 36(a0) // t0 = myhdr->stack_size + asm!( + // Compute the stack top. + // + // struct hdr* myhdr = (struct hdr*) app_start; + // uint32_t stacktop = (((uint32_t) mem_start + myhdr->stack_size + 7) & 0xfffffff8); + "lw t0, 36(a0) // t0 = myhdr->stack_size addi t0, t0, 7 // t0 = myhdr->stack_size + 7 add t0, t0, a1 // t0 = mem_start + myhdr->stack_size + 7 li t1, 7 // t1 = 7 @@ -89,14 +87,10 @@ mv s0, sp // Set the frame pointer to sp. mv a1, s1 // second arg is stacktop mv a2, t1 // third arg is app_heap_break that we told the kernel - jal rust_start" - : // No output operands - : - : "memory", "x10", "x11", "x12", "x13", "x14", "x15", "x16", "x17", - "x5", "x6", "x7", "x28", "x29", "x30", "x31", "x1" // Clobbers - : "volatile" // Options - ); - hint::unreachable_unchecked(); + jal rust_start", + // No clobbers needed for a noreturn asm! block. + options(noreturn), + ) } /// Ensure an abort symbol exists.
diff --git a/core/src/lib.rs b/core/src/lib.rs index 717dc41..6281d18 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs
@@ -1,4 +1,4 @@ -#![feature(lang_items, llvm_asm, naked_functions)] +#![feature(asm, lang_items, llvm_asm, naked_functions)] #![cfg_attr(any(target_arch = "arm", target_arch = "riscv32"), no_std)] #![cfg_attr(feature = "alloc", feature(alloc_error_handler))]
diff --git a/doc/MiriTips.md b/doc/MiriTips.md new file mode 100644 index 0000000..5c88964 --- /dev/null +++ b/doc/MiriTips.md
@@ -0,0 +1,42 @@ +Miri Tips +========= + +`libtock-rs` runs most of its unit tests under +[Miri](https://github.com/rust-lang/miri) to detect undefined behavior. However, +Miri does not detect all undefined behavior encountered. This document lists +techniques for maximizing the amount of undefined behavior identified by Miri. + +## Miri flags + +Miri has [configuration +flags](https://github.com/rust-lang/miri#miri--z-flags-and-environment-variables), +which can be set using the `MIRIFLAGS` environment variable. We run the tests +both with and without `-Zmiri-track-raw-pointers`, as both versions of Stacked +Borrows has undefined behavior the other does not (see [this +discussion](https://github.com/rust-lang/miri/pull/1748) for more information). +On the run with `-Zmiri-track-raw-pointers`, we also set +`-Zmiri-symbolic-alignment-check` to make Miri's alignment check more pendantic. + +## Detecting invalid values + +Miri does not always detect undefined behavior when an invalid value is created +but not used. Here are some cases that Miri currently accepts: + +1. Unused reference to an invalid value + (https://github.com/rust-lang/miri/issues/1638) +1. Unused uninitialized value (https://github.com/rust-lang/miri/issues/1340) +1. Slices pointing to invalid values + (https://github.com/rust-lang/miri/issues/1240) + +Note that copying the value around (including passing it to functions) does +*not* count as a use for the purpose of this check. + +For types that implement `core::fmt::Debug` (i.e. most types), you can check a +value is valid by attempting to debug-print it: + +```rust +format!("{:?}", value); +``` + +If `value` is invalid, then Miri will identify undefined behavior in the above +code.
diff --git a/layout_generic.ld b/layout_generic.ld index 0bbef87..3a77ad7 100644 --- a/layout_generic.ld +++ b/layout_generic.ld
@@ -24,9 +24,15 @@ /* Section for just the app crt0 header. * This must be first so that the app can find it. */ - .crt0_header : + /* Runtime setup logic. The crt0_header symbol is used by the entry point + * assembly. We have to include start here rather than .text because + * otherwise elf2tab fails to recognize that the process binary's flash + * region should start at the beginning of .start. + */ + .start : { _beginning = .; /* Start of the app in flash. */ + crt0_header = .; /** * Populate the header expected by `crt0`: * @@ -68,6 +74,8 @@ * between the header and subsequent .data section. It's unclear why, * but LLD is aligning sections to a multiple of 32 bytes. */ . = ALIGN(32); + + *(.start) } > FLASH =0xFF /* Text section, Code! */ @@ -75,7 +83,6 @@ { . = ALIGN(4); _text = .; - KEEP (*(.start)) *(.text*) *(.rodata*) KEEP (*(.syscalls)) @@ -140,30 +147,10 @@ { } > FLASH - /* ARM Exception support - * - * This contains compiler-generated support for unwinding the stack, - * consisting of key-value pairs of function addresses and information on - * how to unwind stack frames. - * https://wiki.linaro.org/KenWerner/Sandbox/libunwind?action=AttachFile&do=get&target=libunwind-LDS.pdf - * - * .ARM.exidx is sorted, so has to go in its own output section. - * - * __NOTE__: It's at the end because we currently don't actually serialize - * it to the binary in elf2tbf. If it was before the RAM sections, it would - * through off our calculations of the header. - */ - PROVIDE_HIDDEN (__exidx_start = .); - .ARM.exidx : - { - /* (C++) Index entries for section unwinding */ - *(.ARM.exidx* .gnu.linkonce.armexidx.*) - } > FLASH - PROVIDE_HIDDEN (__exidx_end = .); - + /* Sections we do not need. */ /DISCARD/ : { - *(.eh_frame) + *(.ARM.exidx .eh_frame) } }
diff --git a/rust-toolchain b/rust-toolchain index 0a379e7..1674405 100644 --- a/rust-toolchain +++ b/rust-toolchain
@@ -1,7 +1,7 @@ [toolchain] # See https://rust-lang.github.io/rustup-components-history/ for a list of # recently nightlies and what components are available for them. -channel = "nightly-2020-08-20" +channel = "nightly-2021-03-25" components = ["clippy", "miri", "rustfmt"] targets = ["thumbv7em-none-eabi", "riscv32imac-unknown-none-elf",
diff --git a/src/ble_composer.rs b/src/ble_composer.rs index 589dfc3..d470378 100644 --- a/src/ble_composer.rs +++ b/src/ble_composer.rs
@@ -15,7 +15,7 @@ } impl BlePayload { - pub fn add(&mut self, kind: u8, content: &[u8]) -> Result<(), ()> { + pub fn add(&mut self, kind: u8, content: &[u8]) -> Result<(), Overflow> { self.check_can_write_num_bytes(content.len() + 2)?; self.bytes[self.occupied] = (content.len() + 1) as u8; @@ -26,7 +26,7 @@ Ok(()) } - pub fn add_flag(&mut self, flag: u8) -> Result<(), ()> { + pub fn add_flag(&mut self, flag: u8) -> Result<(), Overflow> { self.check_can_write_num_bytes(3)?; self.bytes[self.occupied] = 2; @@ -36,7 +36,7 @@ Ok(()) } - pub fn add_service_payload(&mut self, uuid: [u8; 2], content: &[u8]) -> Result<(), ()> { + pub fn add_service_payload(&mut self, uuid: [u8; 2], content: &[u8]) -> Result<(), Overflow> { self.check_can_write_num_bytes(4 + content.len())?; self.bytes[self.occupied] = (content.len() + 3) as u8; self.bytes[self.occupied + 1] = gap_types::SERVICE_DATA; @@ -49,11 +49,11 @@ Ok(()) } - fn check_can_write_num_bytes(&self, number: usize) -> Result<(), ()> { + fn check_can_write_num_bytes(&self, number: usize) -> Result<(), Overflow> { if self.occupied + number <= self.bytes.len() { Ok(()) } else { - Err(()) + Err(Overflow) } } } @@ -72,6 +72,10 @@ } } +// Error type returned when the buffer is too full to perform an operation. +#[derive(Debug)] +pub struct Overflow; + #[cfg(test)] mod test { use super::*;
diff --git a/src/debug/mod.rs b/src/debug/mod.rs index 85b4337..68d32eb 100644 --- a/src/debug/mod.rs +++ b/src/debug/mod.rs
@@ -28,7 +28,7 @@ write_as_hex(&mut buffer[12..22], *address); for index in 0..4 { let byte = *(address as *const u8).offset(index); - let byte_is_printable_char = byte >= 0x20 && byte < 0x80; + let byte_is_printable_char = (0x20..0x80).contains(&byte); if byte_is_printable_char { buffer[23 + index as usize] = byte; }
diff --git a/src/sensors/mod.rs b/src/sensors/mod.rs index f3a0e7a..f9196d2 100644 --- a/src/sensors/mod.rs +++ b/src/sensors/mod.rs
@@ -46,9 +46,9 @@ } } - impl Into<i32> for $type_name { - fn into(self) -> i32 { - self.value + impl From<$type_name> for i32 { + fn from(sensor: $type_name) -> i32 { + sensor.value } }
diff --git a/test_runner/src/main.rs b/test_runner/src/main.rs index b21b945..5ab9c1a 100644 --- a/test_runner/src/main.rs +++ b/test_runner/src/main.rs
@@ -53,8 +53,8 @@ } fn test_succeeded(input: String, failed_tests: &mut Vec<String>) -> Option<bool> { - let success = input.find("[ OK ]").is_some(); - let failure = input.find("[ FAILURE ]").is_some(); + let success = input.contains("[ OK ]"); + let failure = input.contains("[ FAILURE ]"); let input = input.replace("[ OK ]", ""); let input = input.replace("[ FAILURE ]", ""); let input = input.trim();
diff --git a/tools/print_sizes/src/main.rs b/tools/print_sizes/src/main.rs index dcd7777..12f05c4 100644 --- a/tools/print_sizes/src/main.rs +++ b/tools/print_sizes/src/main.rs
@@ -117,18 +117,21 @@ .expect("No examples found"); let section_width = 7; - // TODO: We do not currently print out .rodata's size. Currently, the linker - // script embeds .rodata in .text, so we don't see it as a separate section - // here. We should modify the linker script to put .rodata in its own - // section. Until that is done, .rodata's size will be counted as part of - // .text, so we'll just print .text's size for now. println!( - "{0:1$} {2:3$} {4:>7$} {5:>7$} {6:>7$}", - "Example", name_width, "Architecture", arch_width, ".bss", ".data", ".text", section_width + "{0:1$} {2:3$} {4:>8$} {5:>8$} {6:>8$} {7:>8$}", + "Example", + name_width, + "Architecture", + arch_width, + ".bss", + ".data", + ".text", + ".rodata", + section_width ); for data in example_data { println!( - "{0:1$} {2:3$} {4:7$} {5:7$} {6:7$}", + "{0:1$} {2:3$} {4:8$} {5:8$} {6:8$} {7:8$}", data.name, name_width, data.arch, @@ -136,6 +139,7 @@ data.sizes.bss, data.sizes.data, data.sizes.text, + data.sizes.rodata, section_width ); }