CONFIG_ESP_COREDUMP_MAX_TASKS_NUM ignored (not used) (IDFGH-13516)

valkuc commented 3 weeks ago

Answers checklist.

[X] I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
[X] I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
[X] I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

v5.3

Espressif SoC revision.

ESP32-C3

Operating System used.

Windows

How did you build your project?

Command line with idf.py

If you are using Windows, please specify command line type.

CMD

Development Kit.

QEMU

Power Supply used.

USB

What is the expected behavior?

Core dump should have not more than CONFIG_ESP_COREDUMP_MAX_TASKS_NUM tasks in it, according to documentation https://docs.espressif.com/projects/esp-idf/en/v5.3/esp32c3/api-reference/kconfig.html#config-esp-coredump-max-tasks-num

What is the actual behavior?

Core dump contains all tasks.

Steps to reproduce.

Using hello-world project, configure Core dump component with max 1 task and dump to console.
Raise an error, for example with
```
char *a=NULL;
char b = a[1];
printf(b);
```
Look at console to see all threads dumped.

Debug Logs.

No response

More Information.

I did not find in the espcoredump component code any references to CONFIG_ESP_COREDUMP_MAX_TASKS_NUM, so, looks like it is just not used.

erhankur commented 2 weeks ago

@valkuc Thanks for reporting the unused config variable. We will add it.

gerekon commented 1 week ago

@erhankur Maybe we can remove this variable at all. As for me it does not make big sense. It was used to keep arrays of task snapshots. But later on we refactored coredump components significantly and use task snapshot iterator instead, therefore got rid of that variable.

valkuc commented 1 week ago

@erhankur Maybe we can remove this variable at all. As for me it does not make big sense. It was used to keep arrays of task snapshots. But later on we refactored coredump components significantly and use task snapshot iterator instead, therefore got rid of that variable.

What will happen if application have so much threads that they will not fit into core dump partition? Did your refactoring cover this scenario?

gerekon commented 1 week ago

@valkuc I think it is better to get slightly different approach here. Need to make coredump smart enough to try to save at least crashed task. I think in most cases that will be enough to diagnose problem. The next prio tasks could be the ones currently working on other cores. And the least prio are all other tasks. So crashed task will be saved into coredump with high level of probability. The others do not seem so important.

Disadvantages of CONFIG_ESP_COREDUMP_MAX_TASKS_NUM

You need to rebuild app. That's could impossible when coredump is enabled in the FW working on released product/device.
It is hard to predict if certain value for CONFIG_ESP_COREDUMP_MAX_TASKS_NUM will guarantee that resulting coredump will fit into partition. There can be dynamically created tasks with various stack sizes.

valkuc commented 1 week ago

@gerekon I like this. It would be great if the core dump component will be able to work in such way - try to save as much as possible in provided partition starting with the highest priority info and ending with the lowest priority info. And if I understand correctly, in the current implementation (SDK v5.3) the core dump will not be saved at all if the dump size is larger than the partition, right?

gerekon commented 1 week ago

And if I understand correctly, in the current implementation (SDK v5.3) the core dump will not be saved at all if the dump size is larger than the partition, right?

Yes, that's true. https://github.com/espressif/esp-idf/blob/6673376297b921d438790d195014b860eaf8bb17/components/espcoredump/src/core_dump_flash.c#L265

espressif / esp-idf