Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default stack size is tiny, and the stack canary is turned off! #315

Closed
fake-name opened this issue Apr 16, 2017 · 12 comments
Closed

Default stack size is tiny, and the stack canary is turned off! #315

fake-name opened this issue Apr 16, 2017 · 12 comments

Comments

@fake-name
Copy link

fake-name commented Apr 16, 2017

Basically, application code runs in a freertos task with a 4k BYTE stack (!). It's extremely easy to overrun this with any sort of web/string parsing.

Additionally, it appears that code is compiled by default with CONFIG_FREERTOS_CHECK_STACKOVERFLOW_NONE, which means that when you do overflow the stack, you only find out when, at a some random point, freertos discovers that it's internal data structures have been corrupted, and aborts.

Since I assume that most people using arduino-esp32 aren't going to be creating more freertos tasks, it'd probably be a decent idea to give the main task most of the available memory. I set it to 32Kbytes and it seems to run without issue.

Additionally, I think it's probably a good idea to turn on stack overflow checking by default as well. It does introduce a small performance hit, but it's probably safe to assume that people who need that much speed are able to determine that they need to turn it off, and it makes debugging for everyone else MUCH easier.

The stack is set in main.cpp. I'm not sure where CONFIG_FREERTOS_CHECK_STACKOVERFLOW_NONE is set, I reordered the directives in FreeRTOSConfig.h, and added a compiler directive to define CONFIG_FREERTOS_CHECK_STACKOVERFLOW_CANARY as a workaround.

Note that due to platform oddments, apparently the stack size is specified in bytes, while the freertos documentation generally states it's specified in machine words. See here: https://www.esp32.com/viewtopic.php?t=900#p3879

@me-no-dev
Copy link
Member

I have never been able to overrun the stack and you are the first to report issues :)
It is a good idea though and I will add it to the next update.
I am locally testing stack overflow canary also so that will probably get turned on as well.

@igrr
Copy link
Member

igrr commented Apr 16, 2017

Also suggest turning on the "Enable watchpoint at the end of stack" option, as it will produce more meaningful backtraces (pointing to the place where stack overflow has actually happened).

@me-no-dev
Copy link
Member

Actually CONFIG_FREERTOS_CHECK_STACKOVERFLOW_CANARY is already enabled :) https://github.com/espressif/arduino-esp32/blob/master/tools/sdk/include/config/sdkconfig.h#L104

@me-no-dev
Copy link
Member

@igrr did you mean Set a debug watchpoint as a stack overflow check?

@fake-name
Copy link
Author

Hrm, I wonder if the fact that it seems to be off is because I'm using arduino-esp32 through the platformio toolkit. The platformio build config definitely has the stack checks disabled. I'll open a bug report there, then.

On the topic of nice-to-have stuff for dealing with freertos, I'd also suggest attaching some of the various exception hooks freertos provides. Having a function that prints a error to the console for vApplicationMallocFailedHook and vApplicationStackOverflowHook can be very helpful.

Anyways, the reason I was overrunning the stack was because I was allocating a few relatively large buffers (~1Kbyte) on the stack, for some network code I was writing. The only symptoms of the overrun was a random crash sometimes later when FreeRTOS went to context switch, and some of it's task management linked-lists were corrupted by the overflow.

@me-no-dev
Copy link
Member

I'm closing this because those options are enabled in the current and future builds :)

@fake-name
Copy link
Author

Coo. Yeah, the problem wound up being further up my dependency tree. Sorry about that!

@ankgt
Copy link

ankgt commented Nov 16, 2018

Hrm, I wonder if the fact that it seems to be off is because I'm using arduino-esp32 through the platformio toolkit. The platformio build config definitely has the stack checks disabled. I'll open a bug report there, then.

@igrr I am running arduino-ep32 through platformio too. Can you which file I need to inspect to check if this flag is enabled/disabled on my development machine?

Also, it would be great if you can let me what you did to set the main arduino FreeRTOS stack size to greater then 4K.

Thanks!

@bangonkali
Copy link

It would be interesting to see having the stack size as configurable via platform.ini for users of Platform IO. How do you guys suggest I implement something like this? I need to have a stack size of 8192 * 2 for my app to work.

It seems Arduino JSON is taking up a lot on my end. 😑However, it may be my particular use case that is causing this as I make char c[2000] buffers quite a few times.

/* /Users/<user>/.platformio/packages/framework-arduinoespressif32/cores/esp32/main.cpp */
#include "freertos/FreeRTOS.h"
#include "freertos/task.h"
#include "esp_task_wdt.h"
#include "Arduino.h"

TaskHandle_t loopTaskHandle = NULL;

#if CONFIG_AUTOSTART_ARDUINO

#if CONFIG_FREERTOS_UNICORE
#define ARDUINO_RUNNING_CORE 0
#else
#define ARDUINO_RUNNING_CORE 1
#endif

bool loopTaskWDTEnabled;

void loopTask(void *pvParameters)
{
    setup();
    for(;;) {
        if(loopTaskWDTEnabled){
            esp_task_wdt_reset();
        }
        loop();
    }
}

extern "C" void app_main()
{
    loopTaskWDTEnabled = false;
    initArduino();
    xTaskCreatePinnedToCore(loopTask, "loopTask", 8192 * 2, NULL, 1, &loopTaskHandle, ARDUINO_RUNNING_CORE);
}

#endif

@atanisoft
Copy link
Collaborator

@bangonkali I'd suggest opening a new issue rather than commenting on one that has been closed for well over a year now.

As for ArduinoJson, switch to a DynamicJsonBuffer and try allocating memory at runtime rather than static initializer for the char [2000].

As for PlatformIO and stack size, that would likely need to be implemented in PlatformIO AND arduino-esp32 and I don't see that happening as it is not a common use case. A solution would be replicate the xTaskCreatePinnedToCore in your setup() method and at the end of setup call vTaskDelete(NULL); to kill the loopTask created by arduino-esp32.

@bangonkali
Copy link

@atanisoft thanks for the reply on this old thread! Highly appreciated. I learned a lot. 👌

As for ArduinoJson, switch to a DynamicJsonBuffer and try allocating memory at runtime rather than static initializer for the char [2000].

Applied this on my work and was able to go back to the default stack. I also used Assistant, which really helped.

As for PlatformIO and stack size, that would likely need to be implemented in PlatformIO AND arduino-esp32 and I don't see that happening as it is not a common use case. A solution would be replicate the xTaskCreatePinnedToCore in your setup() method and at the end of setup call vTaskDelete(NULL); to kill the loopTask created by arduino-esp32.

Thanks for this advice. Helps me to understand how to handle RTOS tasks a little bit more.

@playground
Copy link

@bangonkali did you get it to work in PlatformIO?
I'm getting this same error with M5Stack Atom Echo Smart Speaker
Guru Meditation Error: Core 1 panic'ed (Unhandled debug exception) Debug exception reason: Stack canary watchpoint triggered (loopTask)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants