espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.72k stars 7.3k forks source link

Calling a virtual method in an ISR when flash is being read panics (IDFGH-1116) #3434

Open judge2005 opened 5 years ago

judge2005 commented 5 years ago

Environment

Problem Description

If I have an ISR that calls a virtual method on a class, and a task is reading from flash, the ESP32 panics. It works fine if the method is not virtual.

Expected Behavior

It doesn't panic

Actual Behavior

It panics

Steps to repropduce

Compile and run the code below (it uses the Arduino framework)

Code to reproduce this issue

#include "Arduino.h"
#include "esp_spi_flash.h"
#include "nvs.h"
#include "nvs_flash.h"

hw_timer_t *timer;

class Test {
public:
    // If this is NOT virtual, everything works
    virtual void IRAM_ATTR callMe();
};

void Test::callMe() {

}

Test *test = new Test();

void IRAM_ATTR isr() {
    test->callMe();
}

void setup() {
    Serial.begin(115200);
    nvs_flash_init();
    timer = timerBegin(0, 80, true);
    timerAttachInterrupt(timer, isr, true);
    timerAlarmWrite(timer, 100, true); 
    timerAlarmEnable(timer);
}

void loop() {
    const uint32_t n = SPI_FLASH_SEC_SIZE;
    uint32_t val_read;
    for (uint32_t offset = 0; offset < n; offset += 4) {
        if (spi_flash_read(offset, (uint8_t *) &val_read, 4) != ESP_OK) {
            break;
        }
    }

    delay(10);
}

Other items if possible

igrr commented 5 years ago

Could you please post the panic handler output, obtained using IDF monitor? It will contain a bit more information, such as the specific address which triggers the cache access error.

tim-nordell-nimbelink commented 5 years ago

@judge2005 , @igrr -

I'll also add a note that a virtual function reads the vtable entry out of flash, so you're reading flash within the ISR. The documentation here (https://docs.espressif.com/projects/esp-idf/en/latest/api-guides/general-notes.html#iram-instruction-ram) states:

If the ISR is placed into IRAM, all constant data used by the ISR and functions called from ISR (including, but not limited to, const char arrays), must be placed into DRAM using DRAM_ATTR.

(Granted, it seems as though you should be able to place constant data in flash even if the ISR is marked as being in IRAM; the consequence seems like it should only be that isn't gaining the full benefit of being in IRAM. I'm not sure if there is another rationale.)

judge2005 commented 5 years ago

@judge2005 , @igrr -

I'll also add a note that a virtual function reads the vtable entry out of flash, so you're reading flash within the ISR. The documentation here (https://docs.espressif.com/projects/esp-idf/en/latest/api-guides/general-notes.html#iram-instruction-ram) states:

If the ISR is placed into IRAM, all constant data used by the ISR and functions called from ISR (including, but not limited to, const char arrays), must be placed into DRAM using DRAM_ATTR.

(Granted, it seems as though you should be able to place constant data in flash even if the ISR is marked as being in IRAM; the consequence seems like it should only be that isn't gaining the full benefit of being in IRAM. I'm not sure if there is another rationale.)

I was thinking that the vtable was probably in flash. I don't suppose there could be any way to force it into IRAM? Now or in the future?

igrr commented 5 years ago

It should be possible to move the vtable into DRAM using the linker script generation feature. However, it is not available in Arduino IDE (you will have to use Arduino as IDF component).

judge2005 commented 5 years ago

It should be possible to move the vtable into DRAM using the linker script generation feature. However, it is not available in Arduino IDE (you will have to use Arduino as IDF component).

Do you have a link to how to do this? I'm not using the Arduino IDE. I am using eclipse, so I may be able to incorporate this into my build process. However, I guess it would be nice to be able to make this happen with a directive like DRAM_ATTR on the class, as I can't guarantee that others using my code would use my development environment!

Thanks

me-no-dev commented 5 years ago

Instructions to use Arduino as IDF component: https://github.com/espressif/arduino-esp32/blob/master/docs/esp-idf_component.md You have no way to guarantee the same env on your users though.

judge2005 commented 4 years ago

I see that the ESP8266 Arduino platform has this as a build option. Would it be possible to do this with the ESP32 platform?