From b0844b01bc21c04d445703e97f43488b423abbd8 Mon Sep 17 00:00:00 2001
From: Angus Gratton <gus@projectgus.com>
Date: Sat, 7 May 2016 18:21:13 +1000
Subject: [PATCH] Add abort() implementation

Also reduces the IRAM footprint of the fatal exception handler, as only
the prelude (which disables interrupts & enables the flash mapping) is
in IRAM now.

Closes #54, relevant to #133.
---
 core/app_main.c                               |  8 +-
 core/debug_dumps.c                            | 88 ++++++++++++++++---
 core/include/debug_dumps.h                    |  9 +-
 core/include/xtensa_ops.h                     | 14 +++
 core/newlib_syscalls.c                        |  2 +-
 .../unaligned_load/unaligned_load.c           |  2 +-
 examples/http_get_mbedtls/http_get_mbedtls.c  |  6 +-
 examples/tls_server/tls_server.c              |  8 +-
 lwip/include/arch/cc.h                        |  2 +-
 9 files changed, 106 insertions(+), 33 deletions(-)

diff --git a/core/app_main.c b/core/app_main.c
index 6804ed9..fe33785 100644
--- a/core/app_main.c
+++ b/core/app_main.c
@@ -40,8 +40,6 @@ void user_init(void);
 #define RTCMEM_BACKUP_PHY_VER  31
 #define RTCMEM_SYSTEM_PP_VER   62
 
-#define halt()  while (1) {}
-
 extern uint32_t _bss_start;
 extern uint32_t _bss_end;
 
@@ -100,11 +98,11 @@ static void IRAM get_otp_mac_address(uint8_t *buf) {
     if (!(otp_flags & 0x8000)) {
         //FIXME: do we really need this check?
         printf("Firmware ONLY supports ESP8266!!!\n");
-        halt();
+        abort();
     }
     if (otp_id0 == 0 && otp_id1 == 0) {
         printf("empty otp\n");
-        halt();
+        abort();
     }
     if (otp_flags & 0x1000) {
         // If bit 12 is set, it indicates that the vendor portion of the MAC
@@ -258,7 +256,7 @@ static void zero_bss(void) {
 static void init_networking(uint8_t *phy_info, uint8_t *mac_addr) {
     if (sdk_register_chipv6_phy(phy_info)) {
         printf("FATAL: sdk_register_chipv6_phy failed");
-        halt();
+        abort();
     }
     uart_set_baud(0, 74906);
     uart_set_baud(1, 74906);
diff --git a/core/debug_dumps.c b/core/debug_dumps.c
index 5878ae8..7bea87a 100644
--- a/core/debug_dumps.c
+++ b/core/debug_dumps.c
@@ -1,5 +1,5 @@
 /* Code for dumping status/debug output/etc, including fatal
- * exception handling.
+ * exception handling & abort implementation.
  *
  * Part of esp-open-rtos
  *
@@ -20,7 +20,49 @@
 #include "espressif/esp_common.h"
 #include "sdk_internal.h"
 
-void dump_excinfo(void) {
+/* Forward declarations */
+static void IRAM fatal_handler_prelude(void);
+/* Inner parts of crash handlers marked noinline to ensure they don't inline into IRAM. */
+static void __attribute__((noinline)) __attribute__((noreturn)) fatal_exception_handler_inner(uint32_t *sp);
+static void __attribute__((noinline)) __attribute__((noreturn)) abort_handler_inner(uint32_t *caller, uint32_t *sp);
+
+/* fatal_exception_handler called from any unhandled user exception
+ *
+ * (similar to a hard fault on other processor architectures)
+ *
+ * This function is run from IRAM, but the majority of the handler
+ * runs from flash after fatal_handler_prelude ensures it is mapped
+ * safely.
+ */
+void IRAM __attribute__((noreturn)) fatal_exception_handler(uint32_t *sp) {
+    fatal_handler_prelude();
+    fatal_exception_handler_inner(sp);
+}
+
+/* Abort implementation
+ *
+ * Replaces the weak-linked abort implementation provided by newlib libc.
+ *
+ * Disable interrupts, enable flash mapping, dump stack & caller
+ * address, restart.
+ *
+ * This function is run from IRAM, but the majority of the abort
+ * handler runs from flash after fatal_handler_prelude ensures it is
+ * mapped safely.
+ *
+ */
+void IRAM abort(void) {
+    uint32_t *sp, *caller;
+    RETADDR(caller);
+    /* abort() caller is one instruction before our return address */
+    caller = (uint32_t *)((intptr_t)caller - 3);
+    SP(sp);
+    fatal_handler_prelude();
+    abort_handler_inner(caller, sp);
+}
+
+/* Dump exception information from special function registers */
+static void dump_excinfo(void) {
     uint32_t exccause, epc1, epc2, epc3, excvaddr, depc, excsave1;
     uint32_t excinfo[8];
 
@@ -50,9 +92,13 @@ void dump_excinfo(void) {
     sdk_system_rtc_mem_write(0, excinfo, 32);
 }
 
-/* There's a lot of smart stuff we could do while dumping stack
-   but for now we just dump a likely looking section of stack
-   memory
+/* dump stack memory (frames above sp) to stdout
+
+   There's a lot of smart stuff we could do while dumping stack
+   but for now we just dump what looks like our stack region.
+
+   Probably dumps more memory than it needs to, the first instance of
+   0xa5a5a5a5 probably constitutes the end of our stack.
 */
 void dump_stack(uint32_t *sp) {
     printf("\nStack: SP=%p\n", sp);
@@ -68,10 +114,10 @@ void dump_stack(uint32_t *sp) {
     }
 }
 
-/* Dump exception status registers as stored above 'sp'
-   by the interrupt handler preamble
+/* Dump normal registers that were stored above 'sp'
+   by the exception handler preamble
 */
-void dump_exception_registers(uint32_t *sp) {
+void dump_registers_in_exception_handler(uint32_t *sp) {
     uint32_t excsave1;
     uint32_t *saved = sp - (0x50 / sizeof(uint32_t));
     printf("Registers:\n");
@@ -84,7 +130,11 @@ void dump_exception_registers(uint32_t *sp) {
     printf("SAR %08x\n", saved[0x13]);
 }
 
-void IRAM fatal_exception_handler(uint32_t *sp) {
+
+/* Prelude ensures exceptions/NMI off and flash is mapped, allowing
+   calls to non-IRAM functions.
+*/
+static void IRAM fatal_handler_prelude(void) {
     if (!sdk_NMIIrqIsOn) {
         vPortEnterCritical();
         do {
@@ -93,9 +143,15 @@ void IRAM fatal_exception_handler(uint32_t *sp) {
     }
     Cache_Read_Disable();
     Cache_Read_Enable(0, 0, 1);
+}
+
+/* Main part of fatal exception handler, is run from flash to save
+   some IRAM.
+*/
+static void fatal_exception_handler_inner(uint32_t *sp) {
     dump_excinfo();
     if (sp) {
-        dump_exception_registers(sp);
+        dump_registers_in_exception_handler(sp);
         dump_stack(sp);
     }
     uart_flush_txfifo(0);
@@ -103,3 +159,15 @@ void IRAM fatal_exception_handler(uint32_t *sp) {
     sdk_system_restart_in_nmi();
     while(1) {}
 }
+
+/* Main part of abort handler, can be run from flash to save some
+   IRAM.
+*/
+static void abort_handler_inner(uint32_t *caller, uint32_t *sp) {
+    printf("abort() invoked at %p.\n", caller);
+    dump_stack(sp);
+    uart_flush_txfifo(0);
+    uart_flush_txfifo(1);
+    sdk_system_restart_in_nmi();
+    while(1) {}
+}
diff --git a/core/include/debug_dumps.h b/core/include/debug_dumps.h
index d918043..1a76d26 100644
--- a/core/include/debug_dumps.h
+++ b/core/include/debug_dumps.h
@@ -17,13 +17,6 @@ void dump_stack(uint32_t *sp);
 
    Probably not useful to be called in other contexts.
 */
-void fatal_exception_handler(uint32_t *sp);
-
-/* Dump the current exception register state.
-
-   Probably mostly useful when called from fatal exception handler.
-*/
-void dump_excinfo(void);
-
+void __attribute__((noreturn)) fatal_exception_handler(uint32_t *sp);
 
 #endif
diff --git a/core/include/xtensa_ops.h b/core/include/xtensa_ops.h
index 49d1fbe..1ef68e9 100644
--- a/core/include/xtensa_ops.h
+++ b/core/include/xtensa_ops.h
@@ -14,6 +14,20 @@
 // GCC macros for reading, writing, and exchanging Xtensa processor special
 // registers:
 
+/* Read stack pointer to variable.
+ *
+ * Note that the compiler will push a stack frame (minimum 16 bytes)
+ * in the prelude of a C function that calls any other functions.
+ */
+#define SP(var) asm volatile ("mov %0, a1" : "=r" (var));
+
+/* Read the function return address to a variable.
+ *
+ * Depends on the containing function being simple enough that a0 is
+ * being used as a working register.
+ */
+#define RETADDR(var) asm volatile ("mov %0, a0" : "=r" (var))
+
 #define RSR(var, reg) asm volatile ("rsr %0, " #reg : "=r" (var));
 #define WSR(var, reg) asm volatile ("wsr %0, " #reg : : "r" (var));
 #define XSR(var, reg) asm volatile ("xsr %0, " #reg : "+r" (var));
diff --git a/core/newlib_syscalls.c b/core/newlib_syscalls.c
index b414481..0d4b95e 100644
--- a/core/newlib_syscalls.c
+++ b/core/newlib_syscalls.c
@@ -25,7 +25,7 @@ IRAM caddr_t _sbrk_r (struct _reent *r, int incr)
        if (heap_end + incr > stack_ptr)
        {
        _write (1, "_sbrk: Heap collided with stack\n", 32);
-       while(1) {}
+       abort();
        }
     */
     heap_end += incr;
diff --git a/examples/experiments/unaligned_load/unaligned_load.c b/examples/experiments/unaligned_load/unaligned_load.c
index 258b4d0..4244804 100644
--- a/examples/experiments/unaligned_load/unaligned_load.c
+++ b/examples/experiments/unaligned_load/unaligned_load.c
@@ -305,7 +305,7 @@ static void test_system_interaction()
     }
     uint32_t ticks = xTaskGetTickCount() - start;
     printf("Timer interaction test PASSED after %dms.\n", ticks*portTICK_RATE_MS);
-    while(1) {}
+    abort();
 }
 
 /* The following "sanity tests" are designed to try to execute every code path
diff --git a/examples/http_get_mbedtls/http_get_mbedtls.c b/examples/http_get_mbedtls/http_get_mbedtls.c
index 017f8fc..7aad0e3 100644
--- a/examples/http_get_mbedtls/http_get_mbedtls.c
+++ b/examples/http_get_mbedtls/http_get_mbedtls.c
@@ -115,7 +115,7 @@ void http_get_task(void *pvParameters)
                                     strlen(pers))) != 0)
     {
         printf(" failed\n  ! mbedtls_ctr_drbg_seed returned %d\n", ret);
-        while(1) {} /* todo: replace with abort() */
+        abort();
     }
 
     printf(" ok\n");
@@ -129,7 +129,7 @@ void http_get_task(void *pvParameters)
     if(ret < 0)
     {
         printf(" failed\n  !  mbedtls_x509_crt_parse returned -0x%x\n\n", -ret);
-        while(1) {} /* todo: replace with abort() */
+        abort();
     }
 
     printf(" ok (%d skipped)\n", ret);
@@ -138,7 +138,7 @@ void http_get_task(void *pvParameters)
     if((ret = mbedtls_ssl_set_hostname(&ssl, WEB_SERVER)) != 0)
     {
         printf(" failed\n  ! mbedtls_ssl_set_hostname returned %d\n\n", ret);
-        while(1) {} /* todo: replace with abort() */
+        abort();
     }
 
     /*
diff --git a/examples/tls_server/tls_server.c b/examples/tls_server/tls_server.c
index d253bc9..9030dc0 100644
--- a/examples/tls_server/tls_server.c
+++ b/examples/tls_server/tls_server.c
@@ -85,7 +85,7 @@ void tls_server_task(void *pvParameters)
                                     strlen(pers))) != 0)
     {
         printf(" failed\n  ! mbedtls_ctr_drbg_seed returned %d\n", ret);
-        while(1) {} /* todo: replace with abort() */
+        abort();
     }
 
     printf(" ok\n");
@@ -99,7 +99,7 @@ void tls_server_task(void *pvParameters)
     if(ret < 0)
     {
         printf(" failed\n  !  mbedtls_x509_crt_parse returned -0x%x\n\n", -ret);
-        while(1) {} /* todo: replace with abort() */
+        abort();
     }
 
     printf(" ok (%d skipped)\n", ret);
@@ -109,7 +109,7 @@ void tls_server_task(void *pvParameters)
     if(ret != 0)
     {
         printf(" failed\n ! mbedtls_pk_parse_key returned - 0x%x\n\n", -ret);
-        while(1) { } /*todo: replace with abort() */
+        abort();
     }
 
     printf(" ok\n");
@@ -134,7 +134,7 @@ void tls_server_task(void *pvParameters)
     if( ( ret = mbedtls_ssl_conf_own_cert( &conf, &srvcert, &pkey ) ) != 0 )
     {
         printf( " failed\n  ! mbedtls_ssl_conf_own_cert returned %d\n\n", ret );
-        while(1) { }
+        abort();
     }
 
     mbedtls_ssl_conf_rng(&conf, mbedtls_ctr_drbg_random, &ctr_drbg);
diff --git a/lwip/include/arch/cc.h b/lwip/include/arch/cc.h
index a09ea0c..04dab26 100644
--- a/lwip/include/arch/cc.h
+++ b/lwip/include/arch/cc.h
@@ -94,7 +94,7 @@ typedef int sys_prot_t;
         _Pragma("GCC diagnostic pop")                   \
     } while(0)
 #define LWIP_PLATFORM_ASSERT(x) do { printf("Assertion \"%s\" failed at line %d in %s\n", \
-					    x, __LINE__, __FILE__); while(1) {} } while(0)
+                                            x, __LINE__, __FILE__); abort(); } while(0)
 
 #define LWIP_ERROR(message, expression, handler) do { if (!(expression)) { \
   printf("Assertion \"%s\" failed at line %d in %s\n", message, __LINE__, __FILE__); \