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.

Sign up for GitHub

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

Jump to bottom

v3.0.0: Calling digitalWrite() inside ISR May Crash Due to Lack of IRAM_ATTR in _stopPWM() #8043

Closed
6 tasks done
SadaleNet opened this issue May 16, 2021 · 6 comments · Fixed by #8048
Closed
6 tasks done

v3.0.0: Calling digitalWrite() inside ISR May Crash Due to Lack of IRAM_ATTR in _stopPWM() #8043

SadaleNet opened this issue May 16, 2021 · 6 comments · Fixed by #8048
Labels
component: core type: bug waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Milestone
3.0.1

Comments

@SadaleNet
Copy link
Contributor

SadaleNet commented May 16, 2021 •
edited
Loading

Basic Infos

  • This issue complies with the issue POLICY doc.
  • I have read the documentation at readthedocs and the issue is not addressed there.
  • I have tested that the issue is present in current master branch (aka latest git).
  • I have searched the issue tracker for a similar issue.
  • If there is a stack dump, I have decoded it.
  • I have filled out all fields below.

Platform

  • Hardware: [ESP-WROOM-02]
  • Core Version: [3.0.0 release]
  • Development Env: [Arduino IDE]
  • Operating System: [Linux]

Settings in IDE

  • Module: [Generic ESP8266 Module]
  • Flash Mode: [qio]
  • Flash Size: [4MB]
  • lwip Variant: [v2 Lower Memory]
  • Reset Method: [nodemcu]
  • Flash Frequency: [40Mhz]
  • CPU Frequency: [80Mhz]
  • Upload Using: [SERIAL]
  • Upload Speed: [115200] (serial upload only)

Problem Description

digitalWrite() calls _stopPWM(), which lacks the IRAM_ATTR attribute.

When digitalWrite() is called inside an ISR handler, it may have to wait for the content get fetched from the external flash, which'd block the ISR handler, which may cause the system to crash.

Relevant code: https://github.com/esp8266/Arduino/blob/90f611f/cores/esp8266/core_esp8266_waveform_pwm.cpp#L263

This bug can be easily reproduced if you read the flash filesystem while running digitalWrite() in ISR frequently. For the ease of implementation, the MCVE uses the ESP8266TimerInterrupt library.

This bug is new in v3.0.0 and couldn't be reproduced in v2.7.4.

MCVE Sketch

#include <ESP8266TimerInterrupt.h> // https://github.com/khoih-prog/ESP8266TimerInterrupt
#include <LittleFS.h>
#define TEST_PIN (15)
#define FILENAME "foobarbaz"
#define FILE_CONTENT "foobarbaz"
ESP8266Timer esp8266timer;

static IRAM_ATTR void timer_isr_handler() {
  static int state = LOW;
  digitalWrite(TEST_PIN, state);
  state = !state;
}

void setup() {
  LittleFS.begin();
  Serial.begin(115200);
  // Create a file in the LittleFS filesystem to be read in loop()
  File f = LittleFS.open(FILENAME, "w");
  f.write(FILE_CONTENT, sizeof(FILE_CONTENT));
  f.close();

  // Initialization
  pinMode(TEST_PIN, OUTPUT);
  if (!esp8266timer.attachInterruptInterval(1 * 1000, timer_isr_handler)) { // Triggered once every millisecond
    Serial.println("Error: Failed to attach timer interrupt handler!");
    delay(1000);
    ESP.restart();
  }

  delay(5000); // Rate limiting: Prevent the flash from wearing out from frequent f.write() calls.
  Serial.println("ready!");
}

void loop() {
  // Print the content from a file in the filesystem with Serial
  File f = LittleFS.open(FILENAME, "r");
  if(f) {
    char buf[sizeof(FILE_CONTENT)];
    f.read((uint8_t*)buf, sizeof(FILE_CONTENT));
    Serial.print("File content read: ");
    Serial.println(buf);
    f.close();
  }
  delay(100);
}

Stack Dump


Exception 0: Illegal instruction
PC: 0x40209884: _stopPWM(uint8_t) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/core_esp8266_waveform_pwm.cpp line 264
EXCVADDR: 0x00000000

Decoding stack results
0x40100400: __digitalWrite(uint8_t, uint8_t) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/core_esp8266_wiring_digital.cpp line 86
0x401001e0: timer1_isr_handler(void*, void*) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/core_esp8266_timer.cpp line 37
0x4010011d: timer_isr_handler() at /home/username/Desktop/link to usb-7-segment/firmware/stoppwm_bug/stoppwm_bug.ino line 11
0x40100228: timer1_isr_handler(void*, void*) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/core_esp8266_timer.cpp line 44
0x40100e86: __wrap_spi_flash_read(uint32_t, uint32_t*, size_t) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/core_esp8266_phy.cpp line 309
0x40100e86: __wrap_spi_flash_read(uint32_t, uint32_t*, size_t) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/core_esp8266_phy.cpp line 309
0x40209180: EspClass::flashRead(unsigned int, unsigned int*, unsigned int) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/Esp.cpp line 954
0x40209207: EspClass::flashRead(unsigned int, unsigned char*, unsigned int) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/Esp.cpp line 933
0x40100e86: __wrap_spi_flash_read(uint32_t, uint32_t*, size_t) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/core_esp8266_phy.cpp line 309
0x40209180: EspClass::flashRead(unsigned int, unsigned int*, unsigned int) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/Esp.cpp line 954
0x40209207: EspClass::flashRead(unsigned int, unsigned char*, unsigned int) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/Esp.cpp line 933
0x40100e86: __wrap_spi_flash_read(uint32_t, uint32_t*, size_t) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/core_esp8266_phy.cpp line 309
0x40209180: EspClass::flashRead(unsigned int, unsigned int*, unsigned int) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/Esp.cpp line 954
0x40209207: EspClass::flashRead(unsigned int, unsigned char*, unsigned int) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/Esp.cpp line 933
0x401001a8: ets_post(uint8, ETSSignal, ETSParam) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/core_esp8266_main.cpp line 181
0x40100100: std::function ::operator()() const at /home/username/.arduino15/packages/esp8266/tools/xtensa-lx106-elf-gcc/3.0.0-newlib4.0.0-gnu23-48f7b08/xtensa-lx106-elf/include/c++/10.2.0/bits/std_function.h line 617
0x402075fa: __yield() at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/core_esp8266_main.cpp line 116
0x40208045: flash_hal_read(unsigned int, unsigned int, unsigned char*) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/flash_hal.cpp line 36
0x40205dba: littlefs_impl::LittleFSImpl::lfs_flash_read(lfs_config const*, unsigned int, unsigned int, void*, unsigned int) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/libraries/LittleFS/src/LittleFS.cpp line 171
0x40201330: lfs_bd_read at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/libraries/LittleFS/src/../lib/littlefs/lfs.c line 101
0x402018a1: lfs_dir_fetchmatch at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/libraries/LittleFS/src/../lib/littlefs/lfs.c line 891
0x40205dba: littlefs_impl::LittleFSImpl::lfs_flash_read(lfs_config const*, unsigned int, unsigned int, void*, unsigned int) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/libraries/LittleFS/src/LittleFS.cpp line 171
0x40201e94: lfs_dir_find at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/libraries/LittleFS/src/../lib/littlefs/lfs.c line 1238
0x402020f8: lfs_dir_find_match at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/libraries/LittleFS/src/../lib/littlefs/lfs.c line 1140
0x40204554: lfs_dir_commit_commit at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/libraries/LittleFS/src/../lib/littlefs/lfs.c line 1560
0x40204266: lfs_file_rawopencfg at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/libraries/LittleFS/src/../lib/littlefs/lfs.c line 2472
0x40204ced: lfs_file_open at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/libraries/LittleFS/src/../lib/littlefs/lfs.c line 5058
0x4020684f: littlefs_impl::LittleFSImpl::open(char const*, fs::OpenMode, fs::AccessMode) at /home/username/.arduino15/packages/esp8266/tools/xtensa-lx106-elf-gcc/3.0.0-newlib4.0.0-gnu23-48f7b08/xtensa-lx106-elf/include/c++/10.2.0/bits/shared_ptr_base.h line 1324
0x401008a7: umm_free_core(umm_heap_context_t*, void*) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/umm_malloc/umm_malloc.cpp line 549
0x40206c33: fs::FS::open(char const*, char const*) at /home/username/.arduino15/packages/esp8266/tools/xtensa-lx106-elf-gcc/3.0.0-newlib4.0.0-gnu23-48f7b08/xtensa-lx106-elf/include/c++/10.2.0/bits/shared_ptr_base.h line 1324
0x40206dd0: HardwareSerial::write(unsigned char const*, unsigned int) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/HardwareSerial.h line 193
0x40206ddc: HardwareSerial::write(unsigned char const*, unsigned int) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/HardwareSerial.h line 193
0x40206dd0: HardwareSerial::write(unsigned char const*, unsigned int) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/HardwareSerial.h line 193
0x40206f74: Print::write(char const*) at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/Print.h line 59
0x40206fc0: Print::println() at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/Print.h line 57
0x4020549f: loop() at /home/username/Desktop/link to usb-7-segment/firmware/stoppwm_bug/stoppwm_bug.ino line 31
0x402055b5: setup() at /home/username/.arduino15/packages/esp8266/tools/xtensa-lx106-elf-gcc/3.0.0-newlib4.0.0-gnu23-48f7b08/xtensa-lx106-elf/include/c++/10.2.0/bits/shared_ptr_base.h line 1183
0x40205f48: fs::FS::_defaultTimeCB() at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/FS.h line 249
0x4020767c: loop_wrapper() at /home/username/.arduino15/packages/esp8266/hardware/esp8266/3.0.0/cores/esp8266/core_esp8266_main.cpp line 201
@d-a-v
Copy link
Collaborator

d-a-v commented May 17, 2021

Did you try to add IRAM_ATTR there ?

@d-a-v d-a-v added component: core type: bug waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. labels May 17, 2021
@d-a-v d-a-v added this to the 3.0.1 milestone May 17, 2021
@SadaleNet
Copy link
Contributor Author

SadaleNet commented May 17, 2021 •
edited
Loading

@d-a-v Yes. I've tried it on my local ESP8266 Arduino core installation. It worked. That's how I found that it's the problem of lack of IRAM_ATTR in _stopPWM().

@d-a-v
Copy link
Collaborator

d-a-v commented May 17, 2021

Thanks !
We will be glad to accept a pull-request fixing this bug if you plan to do it soon !
Or we can do it right now if you prefer.

@SadaleNet
Copy link
Contributor Author

@d-a-v Cool. I'll do it today. Will send you a pull request after I'm done.

SadaleNet added a commit to SadaleNet/Arduino that referenced this issue May 17, 2021
@SadaleNet
Added IRAM_ATTR to _stopPWM() so that digitalWrite() would work insid…
7527117 
…e ISR. Fixes esp8266#8043
@SadaleNet SadaleNet mentioned this issue May 17, 2021
Merged
@SadaleNet
Copy link
Contributor Author

@d-a-v Pull request ready. Please review.

@d-a-v d-a-v closed this as completed in 25e1b3b May 17, 2021
@dok-net
Copy link
Contributor

dok-net commented May 17, 2021

@d-a-v I think there are more code paths depending on the selected PWM generator that need the attribute. Can this issue be reopened, please?

This was referenced May 17, 2021
Merged
Open
@Dilbert66 Dilbert66 mentioned this issue Jul 4, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: core type: bug waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Projects
None yet
Milestone
3.0.1
3 participants
@SadaleNet @d-a-v @dok-net