From 881157e1ed5e7e789be9ba6f4cb63fd4f7232961 Mon Sep 17 00:00:00 2001
From: Jeroen Domburg <jeroen@espressif.com>
Date: Tue, 10 Jan 2017 13:05:19 +0800
Subject: [PATCH] Add documentation to panic handler functions, move watchpoint
 stuff from tasks.c to port.c, account for non-32-bytes-aligned stacks when
 setting watchpoint, add debug reason explanation to panic handler

---
 components/esp32/include/esp_panic.h          | 33 ++++++++++++++++-
 components/esp32/panic.c                      | 37 +++++++++++++++++--
 .../freertos/include/freertos/portable.h      |  7 ++++
 components/freertos/port.c                    | 16 ++++++++
 components/freertos/tasks.c                   |  3 +-
 5 files changed, 89 insertions(+), 7 deletions(-)

diff --git a/components/esp32/include/esp_panic.h b/components/esp32/include/esp_panic.h
index ce4ea3c50..aa83c6d38 100644
--- a/components/esp32/include/esp_panic.h
+++ b/components/esp32/include/esp_panic.h
@@ -16,16 +16,47 @@
 
 #include "esp_err.h"
 
+
+/**
+ * @brief If an OCD is connected over JTAG. set breakpoint 0 to the given function 
+ *        address. Do nothing otherwise.
+ * @param data  Pointer to the target breakpoint position
+ */
+
 void esp_set_breakpoint_if_jtag(void *fn);
 
 #define ESP_WATCHPOINT_LOAD 0x40000000
 #define ESP_WATCHPOINT_STORE 0x80000000
 #define ESP_WATCHPOINT_ACCESS 0xC0000000
 
+/**
+ * @brief Set a watchpoint to break/panic when a certain memory range is accessed.
+ *
+ * @param no Watchpoint number. On the ESP32, this can be 0 or 1.
+ * @param adr Base address to watch
+ * @param size Size of the region, starting at the base address, to watch. Must
+ *             be one of 2^n, with n in [0..6].
+ * @param flags One of ESP_WATCHPOINT_* flags
+ *
+ * @return ESP_ERR_INVALID_ARG on invalid arg, ESP_OK otherwise
+ *
+ * @warning The ESP32 watchpoint hardware watches a region of bytes by effectively 
+ *          masking away the lower n bits for a region with size 2^n. If adr does
+ *          not have zero for these lower n bits, you may not be watching the
+ *          region you intended.
+ */
 esp_err_t esp_set_watchpoint(int no, void *adr, int size, int flags);
+
+
+/**
+ * @brief Clear a watchpoint
+ *
+ * @param no Watchpoint to clear
+ *
+ */
 void esp_clear_watchpoint(int no);
 
 
 #endif
 
-#endif
\ No newline at end of file
+#endif
diff --git a/components/esp32/panic.c b/components/esp32/panic.c
index b4a84aee8..c5b18870a 100644
--- a/components/esp32/panic.c
+++ b/components/esp32/panic.c
@@ -164,8 +164,37 @@ void panicHandler(XtExcFrame *frame)
     panicPutStr("Guru Meditation Error: Core ");
     panicPutDec(xPortGetCoreID());
     panicPutStr(" panic'ed (");
-    panicPutStr(reason);
-    panicPutStr(")\r\n");
+    if (!abort_called) {
+        panicPutStr(reason);
+        panicPutStr(")\r\n");
+            if (regs[20]==PANIC_RSN_DEBUGEXCEPTION) {
+                int debugRsn;
+                asm("rsr.debugcause %0":"=r"(debugRsn));
+                panicPutStr("Debug exception reason: ");
+                if (debugRsn&XCHAL_DEBUGCAUSE_ICOUNT_MASK) panicPutStr("SingleStep ");
+                if (debugRsn&XCHAL_DEBUGCAUSE_IBREAK_MASK) panicPutStr("HwBreakpoint ");
+                if (debugRsn&XCHAL_DEBUGCAUSE_DBREAK_MASK) {
+                    //Unlike what the ISA manual says, this core seemingly distinguishes from a DBREAK
+                    //reason caused by watchdog 0 and one caused by watchdog 1 by setting bit 8 of the
+                    //debugcause if the cause is watchdog 1 and clearing it if it's watchdog 0.
+                    if (debugRsn&(1<<8)) {
+#if CONFIG_FREERTOS_WATCHPOINT_END_OF_STACK
+                        panicPutStr("Stack canary watchpoint triggered ");
+#else
+                        panicPutStr("Watchpoint 1 triggered ");
+#endif
+                    } else {
+                        panicPutStr("Watchpoint 0 triggered ");
+                    }
+                }
+                if (debugRsn&XCHAL_DEBUGCAUSE_BREAK_MASK) panicPutStr("BREAK instr ");
+                if (debugRsn&XCHAL_DEBUGCAUSE_BREAKN_MASK) panicPutStr("BREAKN instr ");
+                if (debugRsn&XCHAL_DEBUGCAUSE_DEBUGINT_MASK) panicPutStr("DebugIntr ");
+                panicPutStr("\r\n");
+            }
+        } else {
+            panicPutStr("abort)\r\n");
+        }
 
     if (esp_cpu_in_ocd_debug_mode()) {
         asm("break.n 1");
@@ -363,11 +392,11 @@ esp_err_t esp_set_watchpoint(int no, void *adr, int size, int flags)
     if (flags&(~0xC0000000)) return ESP_ERR_INVALID_ARG;
     int dbreakc=0x3F;
     //We support watching 2^n byte values, from 1 to 64. Calculate the mask for that.
-    for (x=0; x<6; x++) {
+    for (x=0; x<7; x++) {
         if (size==(1<<x)) break;
         dbreakc<<=1;
     }
-    if (x==6) return ESP_ERR_INVALID_ARG;
+    if (x==7) return ESP_ERR_INVALID_ARG;
     //Mask mask and add in flags.
     dbreakc=(dbreakc&0x3f)|flags;
 
diff --git a/components/freertos/include/freertos/portable.h b/components/freertos/include/freertos/portable.h
index 9ed378a8a..b05755da4 100644
--- a/components/freertos/include/freertos/portable.h
+++ b/components/freertos/include/freertos/portable.h
@@ -187,6 +187,13 @@ void vPortEndScheduler( void ) PRIVILEGED_FUNCTION;
 
 void vPortYieldOtherCore( BaseType_t coreid) PRIVILEGED_FUNCTION;
 
+
+/*
+ Callback to set a watchpoint on the end of the stack. Called every context switch to change the stack
+ watchpoint around.
+ */
+void vPortSetStackWatchpoint( void* pxStackStart );
+
 /*
  * The structures and methods of manipulating the MPU are contained within the
  * port layer.
diff --git a/components/freertos/port.c b/components/freertos/port.c
index e5898038b..c778950d6 100644
--- a/components/freertos/port.c
+++ b/components/freertos/port.c
@@ -394,3 +394,19 @@ void vPortFirstTaskHook(TaskFunction_t function) {
 #endif
 
 
+void vPortSetStackWatchpoint( void* pxStackStart ) {
+	//Set watchpoint 1 to watch the last 32 bytes of the stack.
+	//Unfortunately, the Xtensa watchpoints can't set a watchpoint on a random [base - base+n] region because
+	//the size works by masking off the lowest address bits. For that reason, we futz a bit and watch the lowest 32
+	//bytes of the stack we can actually watch. In general, this can cause the watchpoint to be triggered at most
+	//28 bytes early. The value 32 is chosen because it's larger than the stack canary, which in FreeRTOS is 20 bytes.
+	//This way, we make sure we trigger before/when the stack canary is corrupted, not after.
+	int addr=(int)pxStackStart;
+	addr=(addr+31)&(~31);
+	esp_set_watchpoint(1, (char*)addr, 32, ESP_WATCHPOINT_STORE);
+}
+
+
+
+
+
diff --git a/components/freertos/tasks.c b/components/freertos/tasks.c
index 72e7a1d33..64031cfbc 100644
--- a/components/freertos/tasks.c
+++ b/components/freertos/tasks.c
@@ -2811,8 +2811,7 @@ void vTaskSwitchContext( void )
 		traceTASK_SWITCHED_IN();
 
 #if CONFIG_FREERTOS_WATCHPOINT_END_OF_STACK
-		//Set watchpoint 1 to watch the last 32 bytes of the stack.
-		esp_set_watchpoint(1, pxCurrentTCB[xPortGetCoreID()]->pxStack, 32, ESP_WATCHPOINT_STORE);
+		vPortSetStackWatchpoint(pxCurrentTCB[xPortGetCoreID()]->pxStack);
 #endif