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
);
}
