nrf5: Enlarge stack to fix thread join overruns (#2189)

* nrf5: Enlarge stack to fix thread join overruns

As of b15c292 ("[nrf5-lock] start joiner role on boot (#1962)"),
we are using too much stack space in timer task. The timer task has a 1k
stack and logging along uses a 256 byte stack buffer.

The code in
GenericThreadStackManagerImpl_FreeRTOS<ImplClass>::OnJoinerTimer should
be moved off the timer task. In the meantime increase the stack size
to avoid overruns in the thread joiner.

Also enable the option configCHECK_FOR_STACK_OVERFLOW, and while we're
here also enable configUSE_MALLOC_FAILED_HOOK. These diagnostic options
are invaluable for saving debugging time.

Since logging uses significant stack space, try to catch stack overflows
in the platform LogV(). This fires reliably in OnJoinerTimer prior
to enlarging the stack.

Fixes #2187

* Reduce timer task memory to 2k

* Fix the stack size in EFR32 as well

examples/lighting-app/nrf5/BUILD.gn

@@ -92,6 +92,7 @@ executable("lighting_app") {
     "${chip_root}/src/setup_payload",
     "${chip_root}/third_party/openthread/platforms/nrf528xx:libnordicsemi_nrf52840_radio_driver_softdevice",
     "${chip_root}/third_party/openthread/platforms/nrf528xx:libopenthread-nrf52840-softdevice-sdk",
+    "${nrf5_platform_dir}/app/support:freertos_debugging_hooks",
     "${nrf5_platform_dir}/app/support:freertos_newlib_lock_support",
     "${nrf5_platform_dir}/app/support:freertos_newlib_lock_support_test",
     "${openthread_root}:libopenthread-cli-ftd",

examples/lighting-app/nrf5/Makefile

@@ -54,6 +54,7 @@ SRCS = \
     $(CHIP_ROOT)/src/app/clusters/on-off-server/on-off.c \
     $(NRF5_PLATFORM_DIR)/app/support/CXXExceptionStubs.cpp \
     $(NRF5_PLATFORM_DIR)/app/support/nRF5Sbrk.c \
+    $(NRF5_PLATFORM_DIR)/app/support/FreeRTOSDebuggingHooks.c \
     $(NRF5_PLATFORM_DIR)/app/support/FreeRTOSNewlibLockSupport.c \
     $(NRF5_PLATFORM_DIR)/app/support/FreeRTOSNewlibLockSupport_test.c \
     $(NRF5_PLATFORM_DIR)/app/Server.cpp \

examples/lock-app/efr32/include/FreeRTOSConfig.h

@@ -186,7 +186,7 @@ runs at 32768/8=4096Hz.  Ensure the tick rate is a multiple of the clock. */
 #define configUSE_TIMERS (1)
 #define configTIMER_TASK_PRIORITY (configMAX_PRIORITIES - 1) /* Highest priority */
 #define configTIMER_QUEUE_LENGTH (10)
-#define configTIMER_TASK_STACK_DEPTH (configMINIMAL_STACK_SIZE)
+#define configTIMER_TASK_STACK_DEPTH (1024)
 
 /* Cortex-M specific definitions. */
 #ifdef __NVIC_PRIO_BITS

examples/lock-app/nrf5/BUILD.gn

@@ -93,6 +93,7 @@ executable("lock_app") {
     "${chip_root}/src/setup_payload",
     "${chip_root}/third_party/openthread/platforms/nrf528xx:libnordicsemi_nrf52840_radio_driver_softdevice",
     "${chip_root}/third_party/openthread/platforms/nrf528xx:libopenthread-nrf52840-softdevice-sdk",
+    "${nrf5_platform_dir}/app/support:freertos_debugging_hooks",
     "${nrf5_platform_dir}/app/support:freertos_newlib_lock_support",
     "${nrf5_platform_dir}/app/support:freertos_newlib_lock_support_test",
     "${openthread_root}:libopenthread-cli-ftd",

examples/lock-app/nrf5/Makefile

@@ -54,6 +54,7 @@ SRCS = \
     $(CHIP_ROOT)/src/app/clusters/on-off-server/on-off.c \
     $(NRF5_PLATFORM_DIR)/app/support/CXXExceptionStubs.cpp \
     $(NRF5_PLATFORM_DIR)/app/support/nRF5Sbrk.c \
+    $(NRF5_PLATFORM_DIR)/app/support/FreeRTOSDebuggingHooks.c \
     $(NRF5_PLATFORM_DIR)/app/support/FreeRTOSNewlibLockSupport.c \
     $(NRF5_PLATFORM_DIR)/app/support/FreeRTOSNewlibLockSupport_test.c \
     $(NRF5_PLATFORM_DIR)/app/Server.cpp \

examples/platform/nrf528xx/app/project_include/FreeRTOSConfig.h

@@ -82,8 +82,6 @@
 /* Hook function related definitions. */
 #define configUSE_IDLE_HOOK 0
 #define configUSE_TICK_HOOK 0
-#define configCHECK_FOR_STACK_OVERFLOW 0
-#define configUSE_MALLOC_FAILED_HOOK 0
 
 /* Run time and task stats gathering related definitions. */
 #define configGENERATE_RUN_TIME_STATS 0
@@ -97,7 +95,9 @@
 #define configUSE_TIMERS 1
 #define configTIMER_TASK_PRIORITY (2)
 #define configTIMER_QUEUE_LENGTH 32
-#define configTIMER_TASK_STACK_DEPTH (256)
+
+// TODO: Fix high stack usage in GenericThreadStackManagerImpl_FreeRTOS
+#define configTIMER_TASK_STACK_DEPTH (512)
 
 /* Tickless Idle configuration. */
 #define configEXPECTED_IDLE_TIME_BEFORE_SLEEP 2
@@ -116,7 +116,10 @@
 #define configASSERT(x) ASSERT(x)
 #endif
 
+/* Diagnostic options. */
 #ifndef NDEBUG
+#define configCHECK_FOR_STACK_OVERFLOW 2
+#define configUSE_MALLOC_FAILED_HOOK 1
 #define configUSE_LIST_DATA_INTEGRITY_CHECK_BYTES 1
 #endif
 

examples/platform/nrf528xx/app/support/BUILD.gn

@@ -35,3 +35,12 @@ source_set("freertos_newlib_lock_support_test") {
 
   public_deps = [ ":freertos_newlib_lock_support" ]
 }
+
+source_set("freertos_debugging_hooks") {
+  sources = [
+    "FreeRTOSDebuggingHooks.c",
+    "FreeRTOSDebuggingHooks.h",
+  ]
+
+  public_deps = [ "${nrf5_sdk_build_root}:nrf5_sdk" ]
+}

examples/platform/nrf528xx/app/support/FreeRTOSDebuggingHooks.c

@@ -0,0 +1,29 @@
+/*
+ *
+ *    Copyright (c) 2020 Project CHIP Authors
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+#include "FreeRTOSDebuggingHooks.h"
+
+__attribute__((optimize("no-optimize-sibling-calls"))) void vApplicationStackOverflowHook(xTaskHandle pxTask,
+                                                                                          signed char * pcTaskName)
+{
+    ASSERT(0);
+}
+
+__attribute__((optimize("no-optimize-sibling-calls"))) void vApplicationMallocFailedHook(void)
+{
+    ASSERT(0);
+}

examples/platform/nrf528xx/app/support/FreeRTOSDebuggingHooks.h

@@ -0,0 +1,34 @@
+/*
+ *
+ *    Copyright (c) 2020 Project CHIP Authors
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+#include <FreeRTOS.h>
+#include <nrf_assert.h>
+#include <task.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+// Called by FreeRTOS when the stack overflows.
+void vApplicationStackOverflowHook(xTaskHandle pxTask, signed char * pcTaskName);
+
+// Called by FreeRTOS when malloc fails.
+void vApplicationMallocFailedHook(void);
+
+#ifdef __cplusplus
+} // extern "C"
+#endif

src/platform/EFR32/Logging.cpp

@@ -237,6 +237,13 @@ extern "C" void LwIPLog(const char * aFormat, ...)
     }
 
     PrintLog(formattedMsg);
+
+#if configCHECK_FOR_STACK_OVERFLOW
+    // Force a stack overflow check.
+    if (xTaskGetSchedulerState() != taskSCHEDULER_NOT_STARTED)
+        taskYIELD();
+#endif
+
     // Let the application know that a log message has been emitted.
     DeviceLayer::OnLogOutput();
 #endif // EFR32_LOG_ENABLED
@@ -289,6 +296,12 @@ extern "C" void otPlatLog(otLogLevel aLogLevel, otLogRegion aLogRegion, const ch
         }
 
         PrintLog(formattedMsg);
+
+#if configCHECK_FOR_STACK_OVERFLOW
+        // Force a stack overflow check.
+        if (xTaskGetSchedulerState() != taskSCHEDULER_NOT_STARTED)
+            taskYIELD();
+#endif
     }
 
     // Let the application know that a log message has been emitted.

src/platform/nRF5/Logging.cpp

@@ -24,8 +24,8 @@
 
 #define NRF_LOG_MODULE_NAME chip
 
-#include "nrf_log.h"
-
+#include <FreeRTOS.h>
+#include <nrf_log.h>
 #include <platform/internal/CHIPDeviceLayerInternal.h>
 #include <support/logging/CHIPLogging.h>
 
@@ -123,6 +123,12 @@ void LogV(uint8_t module, uint8_t category, const char * msg, va_list v)
                 NRF_LOG_DEBUG("%s", NRF_LOG_PUSH(formattedMsg));
                 break;
             }
+
+#if configCHECK_FOR_STACK_OVERFLOW
+            // Force a stack overflow check.
+            if (xTaskGetSchedulerState() != taskSCHEDULER_NOT_STARTED)
+                taskYIELD();
+#endif
         }
 
         // Let the application know that a log message has been emitted.
@@ -215,6 +221,12 @@ extern "C" void otPlatLog(otLogLevel aLogLevel, otLogRegion aLogRegion, const ch
             NRF_LOG_DEBUG("%s", NRF_LOG_PUSH(formattedMsg));
             break;
         }
+
+#if configCHECK_FOR_STACK_OVERFLOW
+        // Force a stack overflow check.
+        if (xTaskGetSchedulerState() != taskSCHEDULER_NOT_STARTED)
+            taskYIELD();
+#endif
     }
 
     // Let the application know that a log message has been emitted.