Skip to content

Commit

Permalink
Merge branch 'coredump_traverse_state_lists_v5.2' into 'release/v5.2'
Browse files Browse the repository at this point in the history 
Improve the probability of accessing healthy TCBs (v5.2)

See merge request espressif/esp-idf!28662
  • Loading branch information
@dobairoland
dobairoland committed Feb 18, 2024
2 parents 0d3486b + 054f33b commit 04ce3af
Show file tree
Hide file tree
Showing 16 changed files with 287 additions and 198 deletions.
54 changes: 37 additions & 17 deletions components/espcoredump/include_core_dump/esp_core_dump_common.h
View file
@@ -1,12 +1,13 @@
/*
* SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
#ifndef ESP_CORE_DUMP_COMMON_H_
#define ESP_CORE_DUMP_COMMON_H_

#include "freertos/FreeRTOS.h"
#include "esp_private/freertos_debug.h"
#include "esp_app_format.h"
#include "esp_core_dump_types.h"

Expand All @@ -30,22 +31,6 @@ typedef enum {
COREDUMP_MEMORY_START = COREDUMP_MEMORY_DRAM
} coredump_region_t;

/**
* @brief Get the (FreeRTOS) task handle for the current task.
*
* @return Task handle of the current task.
*/
core_dump_task_handle_t esp_core_dump_get_current_task_handle(void);

/**
* @brief Get next task handle of a given handle.
*
* @param handle Task handle to get the next handle from.
*
* @return Next task handle.
*/
core_dump_task_handle_t esp_core_dump_get_next_task(core_dump_task_handle_t handle);

/**
* @brief Get a task snapshot from a given handle.
*
Expand Down Expand Up @@ -123,6 +108,16 @@ static inline uint32_t esp_core_dump_get_tcb_len(void)
sizeof(StaticTask_t);
}

/**
* @brief Get the (FreeRTOS) task handle for the current task.
*
* @return Task handle of the current task.
*/
static inline core_dump_task_handle_t esp_core_dump_get_current_task_handle(void)
{
return (core_dump_task_handle_t) xTaskGetCurrentTaskHandleForCore(xPortGetCoreID());
}

/**
* @brief Get the length, in bytes, of a given memory location. Padding is
* taken into account in this calculation.
Expand All @@ -139,6 +134,31 @@ static inline uint32_t esp_core_dump_get_memory_len(uint32_t start, uint32_t end
return (len + sizeof(uint32_t) - 1) & ~(sizeof(uint32_t) - 1);
}

/**
* @brief Initialize the task iterator to start traversing task lists.
*/
static inline void esp_core_dump_task_iterator_init(TaskIterator_t *iter)
{
if (iter) {
iter->uxCurrentListIndex = 0;
iter->pxNextListItem = NULL;
iter->pxTaskHandle = NULL;
}
}

/**
* @brief Get the next task using the task iterator
*
* This function retrieves the next task in the traversal sequence.
*
* @param task_iterator Pointer to the task iterator structure.
*
* @return The index of the current task list. Returns -1 if all tasks have been traversed.
*/
static inline int esp_core_dump_task_iterator_next(TaskIterator_t *task_iterator)
{
return xTaskGetNext(task_iterator);
}

#ifdef __cplusplus
}
Expand Down
46 changes: 27 additions & 19 deletions components/espcoredump/src/core_dump_binary.c
View file
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -91,19 +91,20 @@ esp_err_t esp_core_dump_write_binary(core_dump_write_config_t *write_cfg)
core_dump_header_t hdr = { 0 };
core_dump_task_header_t task_hdr = { 0 };
core_dump_mem_seg_header_t mem_seg = { 0 };
void *task = NULL;
TaskIterator_t task_iter;
void *cur_task = NULL;

// Verifies all tasks in the snapshot
esp_core_dump_reset_tasks_snapshots_iter();
while ((task = esp_core_dump_get_next_task(task))) {
if (!esp_core_dump_get_task_snapshot(task, &task_hdr, &mem_seg)) {
esp_core_dump_task_iterator_init(&task_iter);
while (esp_core_dump_task_iterator_next(&task_iter) != -1) {
if (!esp_core_dump_get_task_snapshot(task_iter.pxTaskHandle, &task_hdr, &mem_seg)) {
bad_tasks_num++;
continue;
}
hdr.tasks_num++;
if (task == esp_core_dump_get_current_task_handle()) {
cur_task = task;
if (task_iter.pxTaskHandle == esp_core_dump_get_current_task_handle()) {
cur_task = task_iter.pxTaskHandle;
ESP_COREDUMP_LOG_PROCESS("Task %x %x is first crashed task.", cur_task, task_hdr.tcb_addr);
}
ESP_COREDUMP_LOG_PROCESS("Stack len = %lu (%x %x)", task_hdr.stack_end-task_hdr.stack_start,
Expand All @@ -128,11 +129,17 @@ esp_err_t esp_core_dump_write_binary(core_dump_write_config_t *write_cfg)
// Check if current task TCB is broken
if (cur_task == NULL) {
ESP_COREDUMP_LOG_PROCESS("The current crashed task is broken.");
cur_task = esp_core_dump_get_next_task(NULL);
if (cur_task == NULL) {
ESP_COREDUMP_LOGE("No valid tasks in the system!");
return ESP_FAIL;
}
esp_core_dump_task_iterator_init(&task_iter);
while (esp_core_dump_task_iterator_next(&task_iter) != -1) {
if (task_iter.pxTaskHandle != NULL) {
cur_task = task_iter.pxTaskHandle;
break;
}
}
if (cur_task == NULL) {
ESP_COREDUMP_LOGE("No valid tasks in the system!");
return ESP_FAIL;
}
}

// Add user memory regions data size
Expand Down Expand Up @@ -196,15 +203,16 @@ esp_err_t esp_core_dump_write_binary(core_dump_write_config_t *write_cfg)
}
}
// Write all other tasks in the snapshot
task = NULL;
while ((task = esp_core_dump_get_next_task(task))) {
if (!esp_core_dump_get_task_snapshot(task, &task_hdr, NULL))
esp_core_dump_task_iterator_init(&task_iter);
while (esp_core_dump_task_iterator_next(&task_iter) != -1) {
if (!esp_core_dump_get_task_snapshot(task_iter.pxTaskHandle, &task_hdr, NULL))
continue;
// Skip first crashed task
if (task == cur_task) {
if (task_iter.pxTaskHandle == cur_task) {
continue;
}
ESP_COREDUMP_LOGD("Save task %x (TCB:%x, stack:%x..%x)", task, task_hdr.tcb_addr, task_hdr.stack_start, task_hdr.stack_end);
ESP_COREDUMP_LOGD("Save task %x (TCB:%x, stack:%x..%x)",
task_iter.pxTaskHandle, task_hdr.tcb_addr, task_hdr.stack_start, task_hdr.stack_end);
err = esp_core_dump_save_task(write_cfg, &task_hdr);
if (err != ESP_OK) {
ESP_COREDUMP_LOGE("Failed to save core dump task %x, error=%d!",
Expand All @@ -215,10 +223,10 @@ esp_err_t esp_core_dump_write_binary(core_dump_write_config_t *write_cfg)

// Save interrupted stacks of the tasks
// Actually there can be tasks interrupted at the same time, one on every core including the crashed one.
task = NULL;
esp_core_dump_reset_tasks_snapshots_iter();
while ((task = esp_core_dump_get_next_task(task))) {
if (!esp_core_dump_get_task_snapshot(task, &task_hdr, &mem_seg))
esp_core_dump_task_iterator_init(&task_iter);
while (esp_core_dump_task_iterator_next(&task_iter) != -1) {
if (!esp_core_dump_get_task_snapshot(task_iter.pxTaskHandle, &task_hdr, &mem_seg))
continue;
if (mem_seg.size > 0) {
ESP_COREDUMP_LOG_PROCESS("Save interrupted task stack %lu bytes @ %x",
Expand Down
10 changes: 0 additions & 10 deletions components/espcoredump/src/core_dump_common.c
View file
Expand Up @@ -194,11 +194,6 @@ inline void esp_core_dump_reset_tasks_snapshots_iter(void)
esp_core_dump_reset_fake_stacks();
}

inline void *esp_core_dump_get_next_task(void *handle)
{
return pxTaskGetNext(handle);
}

bool esp_core_dump_get_task_snapshot(void *handle, core_dump_task_header_t *task,
core_dump_mem_seg_header_t *interrupted_stack)
{
Expand Down Expand Up @@ -319,9 +314,4 @@ inline bool esp_core_dump_in_isr_context(void)
#endif // CONFIG_ESP_TASK_WDT_EN
}

inline core_dump_task_handle_t esp_core_dump_get_current_task_handle()
{
return (core_dump_task_handle_t) xTaskGetCurrentTaskHandleForCPU(xPortGetCoreID());
}

#endif
21 changes: 11 additions & 10 deletions components/espcoredump/src/core_dump_elf.c
View file
Expand Up @@ -393,6 +393,7 @@ static int elf_process_note_segment(core_dump_elf_t *self, int notes_size)
static int elf_process_tasks_regs(core_dump_elf_t *self)
{
core_dump_task_header_t task_hdr = { 0 };
TaskIterator_t task_iter;
void *task = NULL;
int len = 0;
int ret = 0;
Expand All @@ -413,11 +414,11 @@ static int elf_process_tasks_regs(core_dump_elf_t *self)
// processes PR_STATUS and register dump for each task
// each call to the processing function appends PR_STATUS note into note segment
// and writes data or updates the segment note header accordingly (if phdr is set)
task = NULL;
while ((task = esp_core_dump_get_next_task(task))) {
if (task == esp_core_dump_get_current_task_handle()) {
continue; // skip current task (already processed)
}
esp_core_dump_task_iterator_init(&task_iter);
while (esp_core_dump_task_iterator_next(&task_iter) != -1) {
task = task_iter.pxTaskHandle;
if (!task || task == esp_core_dump_get_current_task_handle()) // skip current task (already processed)
continue;
if (esp_core_dump_get_task_snapshot(task, &task_hdr, NULL)) {
ret = elf_add_regs(self, &task_hdr);
if (self->elf_stage == ELF_STAGE_PLACE_HEADERS) {
Expand Down Expand Up @@ -453,9 +454,9 @@ static int elf_save_task(core_dump_elf_t *self, core_dump_task_header_t *task)
static int elf_write_tasks_data(core_dump_elf_t *self)
{
int elf_len = 0;
void *task = NULL;
core_dump_task_header_t task_hdr = { 0 };
core_dump_mem_seg_header_t interrupted_stack = { 0 };
TaskIterator_t task_iter;
int ret = ELF_PROC_ERR_OTHER;
uint16_t tasks_num = 0;
uint16_t bad_tasks_num = 0;
Expand All @@ -468,17 +469,17 @@ static int elf_write_tasks_data(core_dump_elf_t *self)
ESP_COREDUMP_LOG_PROCESS("================ Processing task data ================");
// processes all task's stack data and writes segment data into partition
// if flash configuration is set
task = NULL;
esp_core_dump_reset_tasks_snapshots_iter();
while ((task = esp_core_dump_get_next_task(task))) {
esp_core_dump_task_iterator_init(&task_iter);
while (esp_core_dump_task_iterator_next(&task_iter) != -1) {
tasks_num++;
if (!esp_core_dump_get_task_snapshot(task, &task_hdr, &interrupted_stack)) {
if (!esp_core_dump_get_task_snapshot(task_iter.pxTaskHandle, &task_hdr, &interrupted_stack)) {
bad_tasks_num++;
continue;
}
ret = elf_save_task(self, &task_hdr);
ELF_CHECK_ERR((ret > 0), ret,
"Task %x, TCB write failed, return (%d).", task, ret);
"Task %x, TCB write failed, return (%d).", task_iter.pxTaskHandle, ret);
elf_len += ret;
if (interrupted_stack.size > 0) {
ESP_COREDUMP_LOG_PROCESS("Add interrupted task stack %lu bytes @ %x",
Expand Down
1 change: 1 addition & 0 deletions components/freertos/FreeRTOS-Kernel-SMP/portable/linux/include/freertos/portmacro.h
View file
Expand Up @@ -291,6 +291,7 @@ void vPortExitCriticalIDF(void);

// ----------------------- System --------------------------

bool portVALID_LIST_MEM(const void *ptr);
bool portVALID_TCB_MEM(const void *ptr);
bool portVALID_STACK_MEM(const void *ptr);

Expand Down
4 changes: 4 additions & 0 deletions components/freertos/FreeRTOS-Kernel-SMP/portable/linux/port.c
View file
Expand Up @@ -600,6 +600,10 @@ unsigned long ulPortGetRunTime( void )
return ( unsigned long ) xTimes.tms_utime;
}
/*-----------------------------------------------------------*/
bool portVALID_LIST_MEM(const void *ptr)
{
return true;
}

bool portVALID_TCB_MEM(const void *ptr)
{
Expand Down
12 changes: 12 additions & 0 deletions components/freertos/FreeRTOS-Kernel-SMP/portable/riscv/include/freertos/portmacro.h
View file
Expand Up @@ -319,6 +319,17 @@ void vPortSetStackWatchpoint(void *pxStackStart);

// -------------------- Heap Related -----------------------

/**
* @brief Checks if a given piece of memory can be used to store a FreeRTOS list
*
* - Defined in heap_idf.c
*
* @param ptr Pointer to memory
* @return true Memory can be used to store a List
* @return false Otherwise
*/
bool xPortCheckValidListMem(const void *ptr);

/**
* @brief Checks if a given piece of memory can be used to store a task's TCB
*
Expand All @@ -341,6 +352,7 @@ bool xPortCheckValidTCBMem(const void *ptr);
*/
bool xPortcheckValidStackMem(const void *ptr);

#define portVALID_LIST_MEM(ptr) xPortCheckValidListMem(ptr)
#define portVALID_TCB_MEM(ptr) xPortCheckValidTCBMem(ptr)
#define portVALID_STACK_MEM(ptr) xPortcheckValidStackMem(ptr)

Expand Down
12 changes: 12 additions & 0 deletions components/freertos/FreeRTOS-Kernel-SMP/portable/xtensa/include/freertos/portmacro.h
View file
Expand Up @@ -365,6 +365,17 @@ void vPortSetStackWatchpoint(void *pxStackStart);

// -------------------- Heap Related -----------------------

/**
* @brief Checks if a given piece of memory can be used to store a FreeRTOS list
*
* - Defined in heap_idf.c
*
* @param ptr Pointer to memory
* @return true Memory can be used to store a List
* @return false Otherwise
*/
bool xPortCheckValidListMem(const void *ptr);

/**
* @brief Checks if a given piece of memory can be used to store a task's TCB
*
Expand All @@ -387,6 +398,7 @@ bool xPortCheckValidTCBMem(const void *ptr);
*/
bool xPortcheckValidStackMem(const void *ptr);

#define portVALID_LIST_MEM(ptr) xPortCheckValidListMem(ptr)
#define portVALID_TCB_MEM(ptr) xPortCheckValidTCBMem(ptr)
#define portVALID_STACK_MEM(ptr) xPortcheckValidStackMem(ptr)

Expand Down
12 changes: 12 additions & 0 deletions components/freertos/FreeRTOS-Kernel/portable/linux/include/freertos/portmacro_idf.h
View file
Expand Up @@ -48,6 +48,17 @@ static inline BaseType_t IRAM_ATTR xPortGetCoreID(void)
return (BaseType_t) 0;
}

/**
* @brief Checks if a given piece of memory can be used to store a FreeRTOS list
*
* - Defined in heap_idf.c
*
* @param ptr Pointer to memory
* @return true Memory can be used to store a List
* @return false Otherwise
*/
bool xPortCheckValidListMem(const void *ptr);

/**
* @brief Checks if a given piece of memory can be used to store a task's TCB
*
Expand All @@ -70,6 +81,7 @@ bool xPortCheckValidTCBMem(const void *ptr);
*/
bool xPortcheckValidStackMem(const void *ptr);

#define portVALID_LIST_MEM(ptr) xPortCheckValidListMem(ptr)
#define portVALID_TCB_MEM(ptr) xPortCheckValidTCBMem(ptr)
#define portVALID_STACK_MEM(ptr) xPortcheckValidStackMem(ptr)

Expand Down
12 changes: 12 additions & 0 deletions components/freertos/FreeRTOS-Kernel/portable/riscv/include/freertos/portmacro.h
View file
Expand Up @@ -688,6 +688,17 @@ FORCE_INLINE_ATTR bool xPortCanYield(void)

// -------------------- Heap Related -----------------------

/**
* @brief Checks if a given piece of memory can be used to store a FreeRTOS list
*
* - Defined in heap_idf.c
*
* @param ptr Pointer to memory
* @return true Memory can be used to store a List
* @return false Otherwise
*/
bool xPortCheckValidListMem(const void *ptr);

/**
* @brief Checks if a given piece of memory can be used to store a task's TCB
*
Expand All @@ -710,6 +721,7 @@ bool xPortCheckValidTCBMem(const void *ptr);
*/
bool xPortcheckValidStackMem(const void *ptr);

#define portVALID_LIST_MEM(ptr) xPortCheckValidListMem(ptr)
#define portVALID_TCB_MEM(ptr) xPortCheckValidTCBMem(ptr)
#define portVALID_STACK_MEM(ptr) xPortcheckValidStackMem(ptr)

Expand Down

0 comments on commit 04ce3af

Please sign in to comment.