refactor(core): optimize assert/fatal_error for reduced flash footprint

[no changelog]
trezor · Jun 17, 2024 · 02c608a · 02c608a
1 parent 306465a
commit 02c608a
Show file tree

Hide file tree

Showing 14 changed files with 106 additions and 117 deletions.
diff --git a/core/embed/lib/assert.h b/core/embed/lib/assert.h
@@ -0,0 +1,55 @@
+/*
+ * This file is part of the Trezor project, https://trezor.io/
+ *
+ * Copyright (c) SatoshiLabs
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef LIB_ASSERT_H
+#define LIB_ASSERT_H
+
+// This file overrides the standard `assert` macro to
+// save space in flash memory.
+//
+// This file will be included instead of the standard assert.h
+// as it is passed to the compiler with -include before the
+// paths to the standard libraries.
+//
+// Our custom assert macro eliminates printing of the
+// expression and prints only a short file name and line number.
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#ifndef NDEBUG
+
+void __attribute__((noreturn))
+__fatal_error(const char *msg, const char *file, int line);
+
+#define assert(expr) \
+  ((expr) ? (void)0 : __fatal_error("Assert", __FILE_NAME__, __LINE__))
+
+#else
+
+#define assert(expr) ((void)0)
+
+#endif
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif  // LIB_ASSERT_H
diff --git a/core/embed/lib/error_handling.c b/core/embed/lib/error_handling.c
@@ -64,28 +64,24 @@ void __attribute__((noreturn)) error_shutdown(const char *message) {
 }
 
 void __attribute__((noreturn))
-__fatal_error(const char *expr, const char *msg, const char *file, int line,
-              const char *func) {
+__fatal_error(const char *msg, const char *file, int line) {
 #ifdef FANCY_FATAL_ERROR
-  char buf[256] = {0};
-  mini_snprintf(buf, sizeof(buf), "%s: %d", file, line);
-  error_shutdown(msg != NULL ? msg : buf);
+  if (msg == NULL) {
+    char buf[128] = {0};
+    mini_snprintf(buf, sizeof(buf), "%s:%d", file, line);
+    msg = buf;
+  }
+  error_shutdown(msg);
 #else
   display_orientation(0);
   term_set_color(COLOR_WHITE, COLOR_FATAL_ERROR);
   term_printf("\nINTERNAL ERROR:\n");
-  if (expr) {
-    term_printf("expr: %s\n", expr);
-  }
   if (msg) {
     term_printf("msg : %s\n", msg);
   }
   if (file) {
     term_printf("file: %s:%d\n", file, line);
   }
-  if (func) {
-    term_printf("func: %s\n", func);
-  }
 #ifdef SCM_REVISION
   const uint8_t *rev = (const uint8_t *)SCM_REVISION;
   term_printf("rev : %02x%02x%02x%02x%02x\n", rev[0], rev[1], rev[2], rev[3],
@@ -97,13 +93,6 @@ __fatal_error(const char *expr, const char *msg, const char *file, int line,
 #endif
 }
 
-#ifndef NDEBUG
-void __assert_func(const char *file, int line, const char *func,
-                   const char *expr) {
-  __fatal_error(expr, "assert failed", file, line, func);
-}
-#endif
-
 void __attribute__((noreturn)) show_wipe_code_screen(void) {
   error_shutdown_ex("WIPE CODE ENTERED",
                     "All data has been erased from the device",

diff --git a/core/embed/lib/error_handling.h b/core/embed/lib/error_handling.h
@@ -35,15 +35,12 @@ void __attribute__((noreturn)) error_shutdown(const char *message);
 
 // Do not use this function directly, use the `ensure()` macro instead.
 void __attribute__((noreturn))
-__fatal_error(const char *expr, const char *msg, const char *file, int line,
-              const char *func);
+__fatal_error(const char *msg, const char *file, int line);
 
 // Checks for an expression and if it is false, shows an error message
 // and shuts down the device.
 #define ensure(expr, msg) \
-  (((expr) == sectrue)    \
-       ? (void)0          \
-       : __fatal_error(#expr, msg, __FILE__, __LINE__, __func__))
+  (((expr) == sectrue) ? (void)0 : __fatal_error(msg, __FILE_NAME__, __LINE__))
 
 // Shows WIPE CODE ENTERED screeen and shuts down the device.
 void __attribute__((noreturn)) show_wipe_code_screen(void);

diff --git a/core/embed/rust/src/lib.rs b/core/embed/rust/src/lib.rs
@@ -53,9 +53,9 @@ fn panic_debug(panic_info: &core::panic::PanicInfo) -> ! {
     // Filling at least the file and line information, if available.
     // TODO: find out how to display message from panic_info.message()
     if let Some(location) = panic_info.location() {
-        trezorhal::fatal_error::__fatal_error("", "rs", location.file(), location.line(), "");
+        trezorhal::fatal_error::__fatal_error("rs", location.file(), location.line());
     } else {
-        trezorhal::fatal_error::__fatal_error("", "rs", "", 0, "");
+        trezorhal::fatal_error::__fatal_error("rs", "", 0);
     }
 }
 

diff --git a/core/embed/rust/src/trezorhal/fatal_error.rs b/core/embed/rust/src/trezorhal/fatal_error.rs
@@ -34,17 +34,14 @@ pub fn error_shutdown(title: &str, msg: &str, footer: &str) -> ! {
 /// Shows an error message on the screen and shuts down the device.
 /// In debug mode, also prints the error message to the console.
 #[inline(never)] // saves few kilobytes of flash
-pub fn __fatal_error(_expr: &str, msg: &str, _file: &str, _line: u32, _func: &str) -> ! {
+pub fn __fatal_error(msg: &str, _file: &str, _line: u32) -> ! {
     #[cfg(feature = "debug")]
     {
         dbg_println!("=== FATAL ERROR");
 
         if _line != 0 {
             dbg_println!("Location: {}:{}", _file, _line);
         }
-        if !_expr.is_empty() {
-            dbg_println!("Expression: {}", _expr);
-        }
         if !msg.is_empty() {
             dbg_println!("Message: {}", msg);
         }
@@ -56,49 +53,31 @@ pub fn __fatal_error(_expr: &str, msg: &str, _file: &str, _line: u32, _func: &st
 }
 
 pub trait UnwrapOrFatalError<T> {
-    fn unwrap_or_fatal_error(self, expr: &str, msg: &str, file: &str, line: u32, func: &str) -> T;
+    fn unwrap_or_fatal_error(self, msg: &str, file: &str, line: u32) -> T;
 }
 
 impl<T> UnwrapOrFatalError<T> for Option<T> {
-    fn unwrap_or_fatal_error(self, expr: &str, msg: &str, file: &str, line: u32, func: &str) -> T {
+    fn unwrap_or_fatal_error(self, msg: &str, file: &str, line: u32) -> T {
         match self {
             Some(x) => x,
-            None => __fatal_error(expr, msg, file, line, func),
+            None => __fatal_error(msg, file, line),
         }
     }
 }
 
 impl<T, E> UnwrapOrFatalError<T> for Result<T, E> {
-    fn unwrap_or_fatal_error(self, expr: &str, msg: &str, file: &str, line: u32, func: &str) -> T {
+    fn unwrap_or_fatal_error(self, msg: &str, file: &str, line: u32) -> T {
         match self {
             Ok(x) => x,
-            Err(_) => __fatal_error(expr, msg, file, line, func),
+            Err(_) => __fatal_error(msg, file, line),
         }
     }
 }
 
-macro_rules! function_name {
-    () => {{
-        #[cfg(not(feature = "bootloader"))]
-        {
-            fn f() {}
-            fn type_name_of<T>(_: T) -> &'static str {
-                core::any::type_name::<T>()
-            }
-            let name = type_name_of(f);
-            name.get(..name.len() - 3).unwrap_or("")
-        }
-        #[cfg(feature = "bootloader")]
-        {
-            ""
-        }
-    }};
-}
-
 macro_rules! unwrap {
     ($e:expr, $msg:expr) => {{
         use crate::trezorhal::fatal_error::UnwrapOrFatalError;
-        $e.unwrap_or_fatal_error(stringify!($e), $msg, file!(), line!(), function_name!())
+        $e.unwrap_or_fatal_error($msg, file!(), line!())
     }};
     ($expr:expr) => {
         unwrap!($expr, "unwrap failed")
@@ -108,19 +87,13 @@ macro_rules! unwrap {
 macro_rules! ensure {
     ($what:expr, $error:expr) => {
         if !($what) {
-            crate::trezorhal::fatal_error::__fatal_error(
-                stringify!($what),
-                $error,
-                file!(),
-                line!(),
-                function_name!(),
-            );
+            crate::trezorhal::fatal_error::__fatal_error($error, file!(), line!());
         }
     };
 }
 
 macro_rules! fatal_error {
     ($msg:expr) => {{
-        crate::trezorhal::fatal_error::__fatal_error("", $msg, file!(), line!(), function_name!());
+        crate::trezorhal::fatal_error::__fatal_error($msg, file!(), line!());
     }};
 }
diff --git a/legacy/common.c b/legacy/common.c
@@ -33,8 +33,7 @@ uint8_t HW_ENTROPY_DATA[HW_ENTROPY_LEN];
 static HMAC_DRBG_CTX drbg_ctx;
 
 void __attribute__((noreturn))
-__fatal_error(const char *expr, const char *msg, const char *file, int line_num,
-              const char *func) {
+__fatal_error(const char *msg, const char *file, int line_num) {
   const BITMAP *icon = &bmp_icon_error;
   char line[128] = {0};
   int y = icon->height + 3;
@@ -44,7 +43,7 @@ __fatal_error(const char *expr, const char *msg, const char *file, int line_num,
   oledDrawStringCenter(OLED_WIDTH / 2, (icon->height - FONT_HEIGHT) / 2 + 1,
                        "FATAL  ERROR", FONT_STANDARD);
 
-  snprintf(line, sizeof(line), "Expr: %s", expr ? expr : "(null)");
+  snprintf(line, sizeof(line), "Expr: %s", "(null)");
   oledDrawString(0, y, line, FONT_STANDARD);
   y += FONT_HEIGHT + 1;
 
@@ -59,7 +58,7 @@ __fatal_error(const char *expr, const char *msg, const char *file, int line_num,
   oledDrawString(0, y, label, FONT_STANDARD);
   y += FONT_HEIGHT + 1;
 
-  snprintf(line, sizeof(line), "Func: %s", func ? func : "(null)");
+  snprintf(line, sizeof(line), "Func: %s", "(null)");
   oledDrawString(0, y, line, FONT_STANDARD);
   y += FONT_HEIGHT + 1;
 
@@ -80,7 +79,10 @@ error_shutdown(const char *line1, const char *line2, const char *line3,
 #ifndef NDEBUG
 void __assert_func(const char *file, int line, const char *func,
                    const char *expr) {
-  __fatal_error(expr, "assert failed", file, line, func);
+  (void)func;
+  (void)expr;
+
+  __fatal_error("assert failed", file, line);
 }
 #endif
 

diff --git a/legacy/common.h b/legacy/common.h
@@ -28,18 +28,15 @@
 extern uint8_t HW_ENTROPY_DATA[HW_ENTROPY_LEN];
 
 void __attribute__((noreturn))
-__fatal_error(const char *expr, const char *msg, const char *file, int line,
-              const char *func);
+__fatal_error(const char *msg, const char *file, int line);
 void __attribute__((noreturn))
 error_shutdown(const char *line1, const char *line2, const char *line3,
                const char *line4);
 void show_wipe_code_screen(void);
 void show_pin_too_many_screen(void);
 
 #define ensure(expr, msg) \
-  (((expr) == sectrue)    \
-       ? (void)0          \
-       : __fatal_error(#expr, msg, __FILE__, __LINE__, __func__))
+  (((expr) == sectrue) ? (void)0 : __fatal_error(msg, __FILE__, __LINE__))
 
 void hal_delay(uint32_t ms);
 

diff --git a/legacy/firmware/trezor.c b/legacy/firmware/trezor.c
@@ -45,13 +45,13 @@
 #ifdef USE_SECP256K1_ZKP
 void secp256k1_default_illegal_callback_fn(const char *str, void *data) {
   (void)data;
-  __fatal_error(NULL, str, __FILE__, __LINE__, __func__);
+  __fatal_error(str, __FILE__, __LINE__);
   return;
 }
 
 void secp256k1_default_error_callback_fn(const char *str, void *data) {
   (void)data;
-  __fatal_error(NULL, str, __FILE__, __LINE__, __func__);
+  __fatal_error(str, __FILE__, __LINE__);
   return;
 }
 #endif

diff --git a/storage/storage.c b/storage/storage.c
@@ -158,9 +158,8 @@ static const uint8_t FALSE_BYTE = 0x00;
 static const uint32_t TRUE_WORD = 0xC35A69A5;
 static const uint32_t FALSE_WORD = 0x3CA5965A;
 
-static void __handle_fault(const char *msg, const char *file, int line,
-                           const char *func);
-#define handle_fault(msg) (__handle_fault(msg, __FILE__, __LINE__, __func__))
+static void __handle_fault(const char *msg, const char *file, int line);
+#define handle_fault(msg) (__handle_fault(msg, __FILE_NAME__, __LINE__))
 
 static uint32_t pin_to_int(const uint8_t *pin, size_t pin_len);
 static secbool storage_upgrade(void);
@@ -1356,15 +1355,14 @@ void storage_wipe(void) {
   init_wiped_storage();
 }
 
-static void __handle_fault(const char *msg, const char *file, int line,
-                           const char *func) {
+static void __handle_fault(const char *msg, const char *file, int line) {
   CONFIDENTIAL static secbool in_progress = secfalse;
 
   // If fault handling is already in progress, then we are probably facing a
   // fault injection attack, so wipe.
   if (secfalse != in_progress) {
     storage_wipe();
-    __fatal_error("Fault detected", msg, file, line, func);
+    __fatal_error(msg, file, line);
   }
 
   // We use the PIN fail counter as a fault counter. Increment the counter,
@@ -1373,19 +1371,19 @@ static void __handle_fault(const char *msg, const char *file, int line,
   uint32_t ctr = 0;
   if (sectrue != pin_get_fails(&ctr)) {
     storage_wipe();
-    __fatal_error("Fault detected", msg, file, line, func);
+    __fatal_error(msg, file, line);
   }
 
   if (sectrue != storage_pin_fails_increase()) {
     storage_wipe();
-    __fatal_error("Fault detected", msg, file, line, func);
+    __fatal_error(msg, file, line);
   }
 
   uint32_t ctr_new = 0;
   if (sectrue != pin_get_fails(&ctr_new) || ctr + 1 != ctr_new) {
     storage_wipe();
   }
-  __fatal_error("Fault detected", msg, file, line, func);
+  __fatal_error(msg, file, line);
 }
 
 /*

diff --git a/storage/tests/c/common.c b/storage/tests/c/common.c
@@ -28,21 +28,14 @@ void __shutdown(void) {
   exit(3);
 }
 
-void __fatal_error(const char *expr, const char *msg, const char *file,
-                   int line, const char *func) {
+void __fatal_error(const char *msg, const char *file, int line) {
   printf("\nFATAL ERROR:\n");
-  if (expr) {
-    printf("expr: %s\n", expr);
-  }
   if (msg) {
     printf("msg : %s\n", msg);
   }
   if (file) {
     printf("file: %s:%d\n", file, line);
   }
-  if (func) {
-    printf("func: %s\n", func);
-  }
   __shutdown();
 }