Skip to content

Commit

Permalink
nrf5: Enlarge stack to fix thread join overruns
Browse files Browse the repository at this point in the history
As of b15c292 ("[nrf5-lock] start joiner role on boot (project-chip#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.

Depends on project-chip#2185

Fixes project-chip#2187
  • Loading branch information
mspang committed Aug 16, 2020
1 parent 9f10829 commit b0fd684
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 5 deletions.
1 change: 1 addition & 0 deletions examples/lighting-app/nrf5/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
Expand Down
1 change: 1 addition & 0 deletions examples/lighting-app/nrf5/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
1 change: 1 addition & 0 deletions examples/lock-app/nrf5/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions examples/lock-app/nrf5/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
9 changes: 9 additions & 0 deletions examples/platform/nrf528xx/app/support/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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" ]
}
29 changes: 29 additions & 0 deletions examples/platform/nrf528xx/app/support/FreeRTOSDebuggingHooks.c
Original file line number Diff line number Diff line change
@@ -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);
}
34 changes: 34 additions & 0 deletions examples/platform/nrf528xx/app/support/FreeRTOSDebuggingHooks.h
Original file line number Diff line number Diff line change
@@ -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
10 changes: 8 additions & 2 deletions src/platform/nRF5/Logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit b0fd684

Please sign in to comment.