From 5c5644dfc2c358066f860d9ba24e1f3f79b560c1 Mon Sep 17 00:00:00 2001 From: Niklas Dusenlund Date: Mon, 11 Nov 2024 11:10:29 +0100 Subject: [PATCH 1/2] Makefile: Remove semihosting targets Also remove the documentation that mentions those targets --- .ci/ci | 4 -- Makefile | 18 +------- scripts/jlink-gdb-debug/.gdbinit | 8 ---- scripts/jlink-gdb-debug/README.md | 71 ------------------------------- 4 files changed, 1 insertion(+), 100 deletions(-) delete mode 100644 scripts/jlink-gdb-debug/.gdbinit delete mode 100644 scripts/jlink-gdb-debug/README.md diff --git a/.ci/ci b/.ci/ci index abfd7d2e2..47678769f 100755 --- a/.ci/ci +++ b/.ci/ci @@ -60,10 +60,6 @@ make -j8 firmware make -j8 firmware-btc make -j8 factory-setup -# Semihosting -make -j8 bootloader-semihosting -make -j8 firmware-semihosting - # Disallow some symbols in the final binary that we don't want. if arm-none-eabi-nm build/bin/firmware.elf | grep -q "float_to_decimal_common_shortest"; then echo "Rust fmt float formatting like {.1} adds significant binary bloat." diff --git a/Makefile b/Makefile index f7ada1f80..071a902fb 100644 --- a/Makefile +++ b/Makefile @@ -38,11 +38,6 @@ build-build-rust-unit-tests/Makefile: cd build-build-rust-unit-tests && cmake .. -DCOVERAGE=OFF -DSANITIZE_ADDRESS=OFF -DSANITIZE_UNDEFINED=OFF $(MAKE) -C py/bitbox02 -build-semihosting/Makefile: - mkdir -p build-semihosting - cd build-semihosting && cmake .. -DCMAKE_TOOLCHAIN_FILE=arm.cmake -DSEMIHOSTING=ON - ${MAKE} -C py/bitbox02 - # Directory for building for "host" machine according to gcc convention build: build/Makefile @@ -54,20 +49,13 @@ build-build: build-build/Makefile # address santizers when they link code compiled with gcc. build-build-rust-unit-tests: build-build-rust-unit-tests/Makefile -# Directory for building for "host" machine but with semihosting enbled -build-semihosting: build-semihosting/Makefile - firmware: | build # Generate python bindings for protobuf for test scripts $(MAKE) -C build firmware.elf -firmware-semihosting: | build-semihosting - $(MAKE) -C build-semihosting firmware.elf firmware-btc: | build $(MAKE) -C build firmware-btc.elf bootloader: | build $(MAKE) -C build bootloader.elf -bootloader-semihosting: | build-semihosting - $(MAKE) -C build-semihosting bootloader-development.elf bootloader-development: | build $(MAKE) -C build bootloader-development.elf bootloader-development-locked: | build @@ -110,8 +98,6 @@ flash-dev-firmware: ./py/load_firmware.py build/bin/firmware.bin --debug jlink-flash-bootloader-development: | build JLinkExe -if SWD -device ATSAMD51J20 -speed 4000 -autoconnect 1 -CommanderScript ./build/scripts/bootloader-development.jlink -jlink-flash-bootloader-semihosting: | build-semihosting - JLinkExe -if SWD -device ATSAMD51J20 -speed 4000 -autoconnect 1 -CommanderScript ./build-semihosting/scripts/bootloader-development.jlink jlink-flash-bootloader-development-locked: | build JLinkExe -if SWD -device ATSAMD51J20 -speed 4000 -autoconnect 1 -CommanderScript ./build/scripts/bootloader-development-locked.jlink jlink-flash-bootloader: | build @@ -126,8 +112,6 @@ jlink-flash-firmware-btc: | build JLinkExe -if SWD -device ATSAMD51J20 -speed 4000 -autoconnect 1 -CommanderScript ./build/scripts/firmware-btc.jlink jlink-flash-factory-setup: | build JLinkExe -if SWD -device ATSAMD51J20 -speed 4000 -autoconnect 1 -CommanderScript ./build/scripts/factory-setup.jlink -jlink-flash-firmware-semihosting: | build-semihosting - JLinkExe -if SWD -device ATSAMD51J20 -speed 4000 -autoconnect 1 -CommanderScript ./build-semihosting/scripts/firmware.jlink dockerinit: ./scripts/container.sh build --pull --force-rm --no-cache -t shiftcrypto/firmware_v2:$(shell cat .containerversion) . dockerpull: @@ -144,4 +128,4 @@ prepare-tidy: | build build-build make -C build rust-cbindgen make -C build-build rust-cbindgen clean: - rm -rf build build-build build-semihosting build-build-rust-unit-tests + rm -rf build build-build build-build-rust-unit-tests diff --git a/scripts/jlink-gdb-debug/.gdbinit b/scripts/jlink-gdb-debug/.gdbinit deleted file mode 100644 index 5460f9bba..000000000 --- a/scripts/jlink-gdb-debug/.gdbinit +++ /dev/null @@ -1,8 +0,0 @@ -# Start a JLink gdb server with the following command: -# $ JLinkGDBServer -if SWD -device ATSAMD51J20 -speed 4000 -ir -# See I/O using telnet: -# $ telnet localhost 2333 -file ../../build-semihosting/bin/firmware.elf -target remote :2331 -monitor semihosting enable -monitor reset diff --git a/scripts/jlink-gdb-debug/README.md b/scripts/jlink-gdb-debug/README.md deleted file mode 100644 index 3e012560d..000000000 --- a/scripts/jlink-gdb-debug/README.md +++ /dev/null @@ -1,71 +0,0 @@ -# Debugging with JLinkGDBServer - -If the firmware is compiled with semihosting support, the input and output is redirected to the -developers computer. See instructions below. - -## Compile and flash firmware with semihosting support - -In the root directory of the project run: - -``` -make firmware-semihosting -make jlink-flash-firmware-semihosting -``` - - -## Configure GDB - -If you want to use the `.gdbinit` helper script you need to allow it by creating `~/.gdbinit` with -the following content: - -``` -set auto-load safe-path / -``` - -## Run JLinkGDBServer - -Start the JLink GDB server with the following command: - -``` -JLinkGDBServer -if SWD -device ATSAMD51J20 -speed 4000 -ir -``` - -## Run GDB (with arm support) - -The GDB init file, `.gdbinit`, contains the following commands: - -``` -file ../../build/bin/firmware-semihosting.elf -target remote :2331 -monitor semihosting enable -monitor reset -``` - -First it will load the symbols from `firmware-semihosting.elf` then it will connect to the JLink GDB Server. -After that it will enable "semihosting" to redirect IO to the JLink GDB Server. Finally it will reset the -device. - -Change your current working directory to the directory with the `.gdbinit` file and run `gdb`. - -``` -cd /scripts/jlink-gdb-debug -gdb-multiarch -``` - -## See input and output - -In a new terminal, run `telnet localhost 2333` to connect to the IO of the device through the GDB -Server. - -``` -$ telnet localhost 2333 -Trying 127.0.0.1... -Connected to localhost. -Escape character is '^]'. -SEGGER J-Link GDB Server V6.44h - Terminal output channel -``` - - -## Start debugging - -To start debugging run the command `continue` in GDB. From 8ff9ad4c535594f5593d4d5a64984d2fa5b8db6e Mon Sep 17 00:00:00 2001 From: Niklas Dusenlund Date: Mon, 11 Nov 2024 11:30:11 +0100 Subject: [PATCH 2/2] Remove semihosting define --- CMakeLists.txt | 5 ----- src/CMakeLists.txt | 22 ++++------------------ src/firmware.c | 2 -- src/hardfault.c | 1 - src/platform/platform_init.c | 7 ------- src/util.c | 18 ++++++++---------- src/util.h | 36 ------------------------------------ 7 files changed, 12 insertions(+), 79 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c89847337..757c84469 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -61,14 +61,9 @@ option(COVERAGE "Compile with test coverage flags." OFF) option(SANITIZE_ADDRESS "Compile with asan." OFF) option(SANITIZE_UNDEFINED "Compile with ubsan." OFF) option(CMAKE_VERBOSE_MAKEFILE "Verbose build." OFF) -option(SEMIHOSTING "Enable semihosting." OFF) # Generate compile_command.json (for tidy and other tools) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) -if(SEMIHOSTING AND NOT CMAKE_CROSSCOMPILING) - message(FATAL_ERROR "It doesn't make sense to semihost for the build machine") -endif() - #----------------------------------------------------------------------------- # Force out-of-source build diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 9e5870c2d..2680f4f47 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -418,15 +418,8 @@ if(CMAKE_CROSSCOMPILING) # Select the smaller version of libc called nano. target_compile_options(${elf} PRIVATE --specs=nano.specs) target_link_libraries(${elf} PRIVATE --specs=nano.specs) - if(SEMIHOSTING) - # Select an implementation of the system calls that can communicate with the debugger - target_compile_options(${elf} PRIVATE --specs=rdimon.specs) - target_link_libraries(${elf} PRIVATE --specs=rdimon.specs) - target_compile_definitions(${elf} PRIVATE SEMIHOSTING) - else() - target_compile_options(${elf} PRIVATE --specs=nosys.specs) - target_link_libraries(${elf} PRIVATE --specs=nosys.specs) - endif() + target_compile_options(${elf} PRIVATE --specs=nosys.specs) + target_link_libraries(${elf} PRIVATE --specs=nosys.specs) endforeach(bootloader) foreach(bootloader ${BOOTLOADERS-BITBOX02}) @@ -475,15 +468,8 @@ if(CMAKE_CROSSCOMPILING) # Select the smaller version of libc called nano. target_compile_options(${elf} PRIVATE --specs=nano.specs) target_link_libraries(${elf} PRIVATE --specs=nano.specs) - if(SEMIHOSTING) - # Select an implementation of the system calls that can communicate with the debugger - target_compile_options(${elf} PRIVATE --specs=rdimon.specs) - target_link_libraries(${elf} PRIVATE --specs=rdimon.specs) - target_compile_definitions(${elf} PRIVATE SEMIHOSTING) - else() - target_compile_options(${elf} PRIVATE --specs=nosys.specs) - target_link_libraries(${elf} PRIVATE --specs=nosys.specs) - endif() + target_compile_options(${elf} PRIVATE --specs=nosys.specs) + target_link_libraries(${elf} PRIVATE --specs=nosys.specs) target_link_libraries(${elf} PRIVATE ${firmware}_rust_c) endforeach(firmware) diff --git a/src/firmware.c b/src/firmware.c index 9b797d389..feaee8734 100644 --- a/src/firmware.c +++ b/src/firmware.c @@ -22,7 +22,6 @@ #include "qtouch.h" #include "screen.h" #include "ui/screen_stack.h" -#include "util.h" #include "workflow/idle_workflow.h" #include "workflow/orientation_screen.h" @@ -39,7 +38,6 @@ int main(void) qtouch_init(); common_main(); bitbox02_smarteeprom_init(); - traceln("%s", "Device initialized"); orientation_screen_blocking(); idle_workflow_blocking(); firmware_main_loop(); diff --git a/src/hardfault.c b/src/hardfault.c index 0c5522501..05a87638b 100644 --- a/src/hardfault.c +++ b/src/hardfault.c @@ -35,7 +35,6 @@ void MemManage_Handler(void) void Abort(const char* msg) { screen_print_debug(msg, 0); - traceln("Aborted: %s", msg); usb_stop(); #if !defined(TESTING) #if defined(BOOTLOADER) diff --git a/src/platform/platform_init.c b/src/platform/platform_init.c index b10b25432..df6fcaee5 100644 --- a/src/platform/platform_init.c +++ b/src/platform/platform_init.c @@ -18,16 +18,9 @@ #if !defined(BOOTLOADER) #include "sd_mmc/sd_mmc_start.h" #endif -#include "util.h" - -extern void initialise_monitor_handles(void); void platform_init(void) { -#if defined(SEMIHOSTING) - initialise_monitor_handles(); - traceln("%s", "Semihosting enabled"); -#endif oled_init(); #if !defined(BOOTLOADER) sd_mmc_start(); diff --git a/src/util.c b/src/util.c index 0cd5cf65f..620d4ed34 100644 --- a/src/util.c +++ b/src/util.c @@ -29,7 +29,15 @@ void util_zero(volatile void* dst, size_t len) // the data type. #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdiscarded-qualifiers" +// ifdef here so that we don't have to use -Wno-unknown-pragmas on GCC +#ifdef __clang__ +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wincompatible-pointer-types-discards-qualifiers" +#endif rust_util_zero(rust_util_bytes_mut(dst, len)); +#ifdef __clang__ +#pragma clang diagnostic pop +#endif #pragma GCC diagnostic pop } @@ -39,16 +47,6 @@ void util_uint8_to_hex(const uint8_t* in_bin, const size_t in_len, char* out) rust_util_bytes(in_bin, in_len), rust_util_bytes_mut((uint8_t*)out, in_len * 2 + 1)); } -#if defined(SEMIHOSTING) -char* util_uint8_to_hex_alloc(const uint8_t* in_bin, const size_t in_len) -{ - void* out = malloc(in_len * 2 + 1); - rust_util_uint8_to_hex( - rust_util_bytes(in_bin, in_len), rust_util_bytes_mut((uint8_t*)out, in_len * 2 + 1)); - return (char*)out; -} -#endif - void util_cleanup_str(char** str) { util_zero(*str, strlens(*str)); diff --git a/src/util.h b/src/util.h index 4103e6932..90c65ce6c 100644 --- a/src/util.h +++ b/src/util.h @@ -61,10 +61,6 @@ void util_zero(volatile void* dst, size_t len); // `out` must be of size in_len*2+1. Use BB_HEX_SIZE() to compute the size. void util_uint8_to_hex(const uint8_t* in_bin, size_t in_len, char* out); -// This function is only compiled when we compile with SEMIHOSTING, it is convenient when debugging -// to print byte arrays as hex. -char* util_uint8_to_hex_alloc(const uint8_t* in_bin, size_t in_len); - #define BB_HEX_SIZE(in_bin) (sizeof((in_bin)) * 2 + 1) void util_cleanup_str(char** str); @@ -85,38 +81,6 @@ void util_cleanup_64(uint8_t** buf); uint8_t* __attribute__((__cleanup__(util_cleanup_64))) var##_clean __attribute__((unused)) = \ var; -/** - * Tracing tools. Only enabled in semihosting builds - * - * Since we are using C99 it is necessary to provide at least 1 argument to "...". To print a - * string, provide the format string "%s" and your string as the second argument. - * - * "do {} while" is a hack to make a macro work like a function in some cases. - * - * stderr is not buffered and takes forever to print stdout is used instead. - * - * SOURCE_PATH_SIZE is a definition provided from the command line which is the length of the path - * to the project directory. - */ - -#if defined(SEMIHOSTING) -#define LOG_LEVEL 1 -#else -#define LOG_LEVEL 0 -#endif -#define FILENAME (&__FILE__[SOURCE_PATH_SIZE]) - -#define trace(format, ...) \ - do { \ - if (LOG_LEVEL > 0) fprintf(stdout, "%s:%d: " format, FILENAME, __LINE__, __VA_ARGS__); \ - } while (0) - -#define traceln(format, ...) \ - do { \ - if (LOG_LEVEL > 0) \ - fprintf(stdout, "%s:%d: " format "\n", FILENAME, __LINE__, __VA_ARGS__); \ - } while (0) - /** * Struct definining a rw buffer (buffer + length). */