From 495b32b7e0c7ce20082d88869d1728ef6f8aa035 Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Mon, 2 Mar 2020 16:02:03 +0100 Subject: [PATCH 1/4] Add feature to track allocations in libtock-rs and print statistics to the console. --- Cargo.toml | 1 + deploy.py | 8 ++ patches/libtock-rs/07-debug_allocations.patch | 98 +++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 patches/libtock-rs/07-debug_allocations.patch diff --git a/Cargo.toml b/Cargo.toml index 2994acf..03dc034 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ std = ["cbor/std", "crypto/std", "crypto/derive_debug"] debug_ctap = ["crypto/derive_debug"] with_ctap1 = ["crypto/with_ctap1"] panic_console = ["libtock/panic_console"] +debug_allocations = ["libtock/debug_allocations"] [dev-dependencies] elf2tab = "0.4.0" diff --git a/deploy.py b/deploy.py index 1dc3fc8..85515a6 100755 --- a/deploy.py +++ b/deploy.py @@ -366,6 +366,14 @@ if __name__ == '__main__': "output messages before starting blinking the LEDs on the " "board."), ) + app_commands.add_argument( + "--debug-allocations", + action="append_const", + const="debug_allocations", + dest="features", + help=("The console will be used to output allocator statistics every " + "yime an allocation/deallocation happens."), + ) app_commands.add_argument( "--no-u2f", action=RemoveConstAction, diff --git a/patches/libtock-rs/07-debug_allocations.patch b/patches/libtock-rs/07-debug_allocations.patch new file mode 100644 index 0000000..0b874be --- /dev/null +++ b/patches/libtock-rs/07-debug_allocations.patch @@ -0,0 +1,98 @@ +diff --git a/Cargo.toml b/Cargo.toml +index 386a9ed..af3c5db 100644 +--- a/Cargo.toml ++++ b/Cargo.toml +@@ -10,6 +10,7 @@ linked_list_allocator = { version = "0.6.6", default-features = false } + + [features] + panic_console = [] ++debug_allocations = [] + + [dev-dependencies] + corepack = { version = "0.4.0", default-features = false, features = ["alloc"] } +diff --git a/src/entry_point.rs b/src/entry_point.rs +index 2fe5c40..c978ee5 100644 +--- a/src/entry_point.rs ++++ b/src/entry_point.rs +@@ -368,22 +368,77 @@ pub unsafe extern "C" fn rust_start(app_start: usize, stacktop: usize, app_heap_ + } + } + ++#[cfg(feature = "debug_allocations")] ++use core::fmt::Write; ++#[cfg(feature = "debug_allocations")] ++use core::sync::atomic; ++#[cfg(feature = "debug_allocations")] ++use core::sync::atomic::AtomicUsize; ++ + #[cfg(any(target_arch = "arm", target_arch = "riscv32"))] + #[global_allocator] +-static ALLOCATOR: TockAllocator = TockAllocator; ++static ALLOCATOR: TockAllocator = TockAllocator::new(); + + static mut HEAP: Heap = Heap::empty(); + +-struct TockAllocator; ++struct TockAllocator { ++ #[cfg(feature = "debug_allocations")] ++ count: AtomicUsize, ++ #[cfg(feature = "debug_allocations")] ++ size: AtomicUsize, ++} ++ ++impl TockAllocator { ++ const fn new() -> TockAllocator { ++ TockAllocator { ++ #[cfg(feature = "debug_allocations")] ++ count: AtomicUsize::new(0), ++ #[cfg(feature = "debug_allocations")] ++ size: AtomicUsize::new(0), ++ } ++ } ++} + + unsafe impl GlobalAlloc for TockAllocator { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { +- HEAP.allocate_first_fit(layout) ++ let ptr = HEAP ++ .allocate_first_fit(layout) + .ok() +- .map_or(0 as *mut u8, |allocation| allocation.as_ptr()) ++ .map_or(ptr::null_mut(), NonNull::as_ptr); ++ #[cfg(feature = "debug_allocations")] ++ { ++ self.count.fetch_add(1, atomic::Ordering::SeqCst); ++ self.size.fetch_add(layout.size(), atomic::Ordering::SeqCst); ++ writeln!( ++ crate::console::Console::new(), ++ "alloc[{}, {}] = {:?} ({} ptrs, {} bytes)", ++ layout.size(), ++ layout.align(), ++ ptr, ++ self.count.load(atomic::Ordering::SeqCst), ++ self.size.load(atomic::Ordering::SeqCst) ++ ) ++ .unwrap(); ++ } ++ ptr + } + + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { ++ #[cfg(feature = "debug_allocations")] ++ { ++ self.count.fetch_sub(1, atomic::Ordering::SeqCst); ++ self.size.fetch_sub(layout.size(), atomic::Ordering::SeqCst); ++ writeln!( ++ crate::console::Console::new(), ++ "dealloc[{}, {}] = {:?} ({} ptrs, {} bytes)", ++ layout.size(), ++ layout.align(), ++ ptr, ++ self.count.load(atomic::Ordering::SeqCst), ++ self.size.load(atomic::Ordering::SeqCst) ++ ) ++ .unwrap(); ++ } + HEAP.deallocate(NonNull::new_unchecked(ptr), layout) + } + } From 0547a02b3fc3d3199795aa8c3b3d08bfbf258399 Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Mon, 2 Mar 2020 16:09:33 +0100 Subject: [PATCH 2/4] Typo. --- deploy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deploy.py b/deploy.py index 85515a6..2b2eefc 100755 --- a/deploy.py +++ b/deploy.py @@ -372,7 +372,7 @@ if __name__ == '__main__': const="debug_allocations", dest="features", help=("The console will be used to output allocator statistics every " - "yime an allocation/deallocation happens."), + "time an allocation/deallocation happens."), ) app_commands.add_argument( "--no-u2f", From 9f6207f5a51e0f6f552ee0d062dd3f1c7ec4c8f2 Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Mon, 2 Mar 2020 16:16:55 +0100 Subject: [PATCH 3/4] Add more cargo check combinations, to include the panic_console and debug_allocations features. --- .github/workflows/cargo_check.yml | 18 ++++++++++++++++++ run_desktop_tests.sh | 3 +++ 2 files changed, 21 insertions(+) diff --git a/.github/workflows/cargo_check.yml b/.github/workflows/cargo_check.yml index 36110a9..60b96d1 100644 --- a/.github/workflows/cargo_check.yml +++ b/.github/workflows/cargo_check.yml @@ -46,12 +46,30 @@ jobs: command: check args: --target thumbv7em-none-eabi --release --features debug_ctap + - name: Check OpenSK panic_console + uses: actions-rs/cargo@v1 + with: + command: check + args: --target thumbv7em-none-eabi --release --features panic_console + + - name: Check OpenSK debug_allocations + uses: actions-rs/cargo@v1 + with: + command: check + args: --target thumbv7em-none-eabi --release --features debug_allocations + - name: Check OpenSK debug_ctap,with_ctap1 uses: actions-rs/cargo@v1 with: command: check args: --target thumbv7em-none-eabi --release --features debug_ctap,with_ctap1 + - name: Check OpenSK debug_ctap,with_ctap1,panic_console,debug_allocations + uses: actions-rs/cargo@v1 + with: + command: check + args: --target thumbv7em-none-eabi --release --features debug_ctap,with_ctap1,panic_console,debug_allocations + - name: Check examples uses: actions-rs/cargo@v1 with: diff --git a/run_desktop_tests.sh b/run_desktop_tests.sh index a4682b2..e3f2b1e 100755 --- a/run_desktop_tests.sh +++ b/run_desktop_tests.sh @@ -28,7 +28,10 @@ echo "Checking that CTAP2 builds properly..." cargo check --release --target=thumbv7em-none-eabi cargo check --release --target=thumbv7em-none-eabi --features with_ctap1 cargo check --release --target=thumbv7em-none-eabi --features debug_ctap +cargo check --release --target=thumbv7em-none-eabi --features panic_console +cargo check --release --target=thumbv7em-none-eabi --features debug_allocations cargo check --release --target=thumbv7em-none-eabi --features debug_ctap,with_ctap1 +cargo check --release --target=thumbv7em-none-eabi --features debug_ctap,with_ctap1,panic_console,debug_allocations echo "Checking that examples build properly..." cargo check --release --target=thumbv7em-none-eabi --examples From 7703ddb44ce79db48d9f09e6e50135ca6230bf94 Mon Sep 17 00:00:00 2001 From: Guillaume Endignoux Date: Tue, 3 Mar 2020 19:32:25 +0100 Subject: [PATCH 4/4] Add comment about AtomicUsize. --- patches/libtock-rs/07-debug_allocations.patch | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/patches/libtock-rs/07-debug_allocations.patch b/patches/libtock-rs/07-debug_allocations.patch index 0b874be..13d2eaf 100644 --- a/patches/libtock-rs/07-debug_allocations.patch +++ b/patches/libtock-rs/07-debug_allocations.patch @@ -11,10 +11,10 @@ index 386a9ed..af3c5db 100644 [dev-dependencies] corepack = { version = "0.4.0", default-features = false, features = ["alloc"] } diff --git a/src/entry_point.rs b/src/entry_point.rs -index 2fe5c40..c978ee5 100644 +index 2fe5c40..545d163 100644 --- a/src/entry_point.rs +++ b/src/entry_point.rs -@@ -368,22 +368,77 @@ pub unsafe extern "C" fn rust_start(app_start: usize, stacktop: usize, app_heap_ +@@ -368,22 +368,82 @@ pub unsafe extern "C" fn rust_start(app_start: usize, stacktop: usize, app_heap_ } } @@ -33,6 +33,11 @@ index 2fe5c40..c978ee5 100644 static mut HEAP: Heap = Heap::empty(); -struct TockAllocator; ++// With the "debug_allocations" feature, we use `AtomicUsize` to store the ++// statistics because: ++// - it is `Sync`, so we can use it in a static object (the allocator), ++// - it implements interior mutability, so we can use it in the allocator ++// methods (that take an immutable `&self` reference). +struct TockAllocator { + #[cfg(feature = "debug_allocations")] + count: AtomicUsize,