Merge pull request #1322 from NickeZ/nickez/remove-semihosting

Nickez/remove semihosting

.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."

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
 

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

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)

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

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)

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

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

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).
  */