From 03a57c568ac915152bb6e0233565bdab73035208 Mon Sep 17 00:00:00 2001 From: Michael Spang Date: Sat, 15 Aug 2020 20:24:12 -0400 Subject: [PATCH] nrf5: Enlarge stack to fix thread join overruns As of b15c292ee ("[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::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 --- examples/lighting-app/nrf5/BUILD.gn | 1 + examples/lighting-app/nrf5/Makefile | 1 + examples/lock-app/nrf5/BUILD.gn | 1 + examples/lock-app/nrf5/Makefile | 1 + .../app/project_include/FreeRTOSConfig.h | 9 +++-- .../platform/nrf528xx/app/support/BUILD.gn | 9 +++++ .../app/support/FreeRTOSDebuggingHooks.c | 29 ++++++++++++++++ .../app/support/FreeRTOSDebuggingHooks.h | 34 +++++++++++++++++++ src/platform/nRF5/Logging.cpp | 10 ++++-- 9 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 examples/platform/nrf528xx/app/support/FreeRTOSDebuggingHooks.c create mode 100644 examples/platform/nrf528xx/app/support/FreeRTOSDebuggingHooks.h diff --git a/examples/lighting-app/nrf5/BUILD.gn b/examples/lighting-app/nrf5/BUILD.gn index 998438996e2558..50825fd9f08dfc 100644 --- a/examples/lighting-app/nrf5/BUILD.gn +++ b/examples/lighting-app/nrf5/BUILD.gn @@ -93,6 +93,7 @@ executable("lighting_app") { "${chip_root}/third_party/openthread/platforms/nrf528xx:libopenthread-nrf52840-softdevice-sdk", "${openthread_root}:libopenthread-cli-ftd", "${openthread_root}:libopenthread-ftd", + "${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", ] diff --git a/examples/lighting-app/nrf5/Makefile b/examples/lighting-app/nrf5/Makefile index f054b3602b1318..55a75d90120214 100644 --- a/examples/lighting-app/nrf5/Makefile +++ b/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 \ diff --git a/examples/lock-app/nrf5/BUILD.gn b/examples/lock-app/nrf5/BUILD.gn index dd7555bdf22745..ef36ede26d952c 100644 --- a/examples/lock-app/nrf5/BUILD.gn +++ b/examples/lock-app/nrf5/BUILD.gn @@ -92,6 +92,7 @@ executable("lock_app") { "${chip_root}/src/lib", "${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", diff --git a/examples/lock-app/nrf5/Makefile b/examples/lock-app/nrf5/Makefile index d07f487e360d98..049e2d8ddd5ec0 100644 --- a/examples/lock-app/nrf5/Makefile +++ b/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 \ diff --git a/examples/platform/nrf528xx/app/project_include/FreeRTOSConfig.h b/examples/platform/nrf528xx/app/project_include/FreeRTOSConfig.h index e234ba6a5219b7..c479c37f0a0586 100644 --- a/examples/platform/nrf528xx/app/project_include/FreeRTOSConfig.h +++ b/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 (1024) /* 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 diff --git a/examples/platform/nrf528xx/app/support/BUILD.gn b/examples/platform/nrf528xx/app/support/BUILD.gn index 6c002e1203c0d6..a25cbc96f85a62 100644 --- a/examples/platform/nrf528xx/app/support/BUILD.gn +++ b/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" ] +} diff --git a/examples/platform/nrf528xx/app/support/FreeRTOSDebuggingHooks.c b/examples/platform/nrf528xx/app/support/FreeRTOSDebuggingHooks.c new file mode 100644 index 00000000000000..7c34363aef8be3 --- /dev/null +++ b/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); +} diff --git a/examples/platform/nrf528xx/app/support/FreeRTOSDebuggingHooks.h b/examples/platform/nrf528xx/app/support/FreeRTOSDebuggingHooks.h new file mode 100644 index 00000000000000..e5ea6eb7aeb26a --- /dev/null +++ b/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 +#include +#include + +#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 diff --git a/src/platform/nRF5/Logging.cpp b/src/platform/nRF5/Logging.cpp index 4c524e7472a5eb..52223d5ca24b58 100644 --- a/src/platform/nRF5/Logging.cpp +++ b/src/platform/nRF5/Logging.cpp @@ -24,8 +24,8 @@ #define NRF_LOG_MODULE_NAME chip -#include "nrf_log.h" - +#include +#include #include #include @@ -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.