ets_printf may not be safe for ISR debug printing
- Dominant language
- C++
- Stars
- 16.7k
- Forks
- 13.1k
- PR merge metrics
- No merged PRs in 30d
Description
### Basic Infos
- [X] This issue complies with the [issue POLICY doc](https://github.com/esp8266/Arduino/blob/master/POLICY.md).
- [X] I have read the documentation at [readthedocs](https://arduino-esp8266.readthedocs.io/en/latest) and the issue is not addressed there.
- [X] I have tested that the issue is present in current master branch (aka latest git).
- [X] I have searched the issue tracker for a similar issue.
- [ ] If there is a stack dump, I have decoded it.
- [X] I have filled out all fields below.
#### Platform
- Hardware: [ESP8285 device]
- Core Version: [f2de9e130c66b00cc9e9d8b4e9144b0f5621607d]
- Development Env: [Arduino IDE]
- Operating System: [Ubuntu]
### Settings in IDE
- Module: [Generic ESP8285 Module]
- Flash Mode: [dout]
- Flash Size: [1MB]
- 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
I have seen in several places where `ets_printf()` is the recommended function call for debug printing from an ISR; however, from what I see in the current master I don't think this is true anymore.
`ets_printf()` uses the character print function installed by `ets_install_putc1()`. At the start of `setup()` the print function address resides in IRAM. Once `Serial.begin()` is called that print function address resides in flash. The MCVE below shows the function address under various situations and the resulting output across _Serial_ and _Serial1_.
It looks to me that of these options, the safest ISR debug print solution is to use `ets_uart_printf()` and/or `ets_putc()`. Both of these functions reside in ROM and neither uses the print function installed by `ets_install_putc1()`. They use an internal ROM function for printing, which requires a prior call to `uart_buff_switch()` to select a UART.
### [MCVE](https://stackoverflow.com/help/mcve) Sketch
```cpp
#include
#include
#include
// Boot ROM print functions
extern "C" int ets_uart_printf(const char *format, ...) __attribute__ ((format (printf, 1, 2)));
extern "C" void ets_write_char(char c);
extern "C" void uart_buff_switch(uint8_t);
typedef void (*fn_putc1_t)(char);
// Boot ROM data area reference
// Location where ets_install_putc1 saves function address
#define installed_putc1 ((fn_putc1_t)(*(void **)0x3fffdd48))
void print_test() {
char c;
size_t i;
os_printf(" os_print()\n");
os_printf_plus(" os_print_plus()\n");
ets_printf(" ets_printf()\n");
for (i=0; (c=" installed_putc1()\n"[i]); i++) installed_putc1(c);
for (i=0; (c=" ets_write_char()\n"[i]); i++) ets_write_char(c);
ets_uart_printf(" ets_uart_printf()\n");
for (i=0; (c=" ets_putc()\n"[i]); i++) ets_putc(c);
}
void setup() {
fn_putc1_t original_putc1 = installed_putc1;
WiFi.mode(WIFI_OFF);
Serial.begin (115200);
Serial1.begin(115200);
Serial.printf ("\nOriginal value at setup() putc1 = %p\n", original_putc1);
Serial1.printf("\nOriginal value at setup() putc1 = %p\n", original_putc1);
Serial.printf ("\nAfter Serial.begin(115200) putc1 = %p\n", installed_putc1);
Serial1.printf("\nAfter Serial.begin(115200) putc1 = %p\n", installed_putc1);
print_test();
Serial.setDebugOutput(true);
Serial.printf ("\nSerial.setDebugOutput(true) putc1 = %p\n", installed_putc1);
Serial1.printf("\nSerial.setDebugOutput(true) putc1 = %p\n", installed_putc1);
print_test();
Serial1.setDebugOutput(true);
Serial1.printf("\nSerial1.setDebugOutput(true) putc1 = %p\n", installed_putc1);
Serial.printf ("\nSerial1.setDebugOutput(true) putc1 = %p\n", installed_putc1);
print_test();
Serial1.setDebugOutput(false);
Serial.printf ("\nSerial1.setDebugOutput(false) putc1 = %p\n", installed_putc1);
Serial1.printf("\nSerial1.setDebugOutput(false) putc1 = %p\n", installed_putc1);
print_test();
uart_buff_switch(0);
Serial.printf ("\nuart_buff_switch(0) putc1 = %p\n", installed_putc1);
Serial1.printf("\nuart_buff_switch(0) putc1 = %p\n", installed_putc1);
print_test();
uart_buff_switch(1);
Serial.printf ("\nuart_buff_switch(1) putc1 = %p\n", installed_putc1);
Serial1.printf("\nuart_buff_switch(1) putc1 = %p\n", installed_putc1);
print_test();
uart_buff_switch(0);
Serial.printf ("\nuart_buff_switch(0) putc1 = %p\n", installed_putc1);
Serial1.printf("\nuart_buff_switch(0) putc1 = %p\n", installed_putc1);
print_test();
}
void loop() {}
```
### Debug Messages
**From Serial**
```
Original value at setup() putc1 = 0x401049e0
After Serial.begin(115200) putc1 = 0x40203d40
ets_uart_printf()
ets_putc()
Serial.setDebugOutput(true) putc1 = 0x40203024
os_print()
os_print_plus()
ets_printf()
installed_putc1()
ets_write_char()
ets_uart_printf()
ets_putc()
Serial1.setDebugOutput(true) putc1 = 0x40202fe8
ets_uart_printf()
ets_putc()
Serial1.setDebugOutput(false) putc1 = 0x40203d40
ets_uart_printf()
ets_putc()
uart_buff_switch(0) putc1 = 0x40203d40
ets_uart_printf()
ets_putc()
uart_buff_switch(1) putc1 = 0x40203d40
uart_buff_switch(0) putc1 = 0x40203d40
ets_uart_printf()
ets_putc()
```
**From Serial1**
```
Original value at setup() putc1 = 0x401049e0
After Serial.begin(115200) putc1 = 0x40203d40
Serial.setDebugOutput(true) putc1 = 0x40203024
Serial1.setDebugOutput(true) putc1 = 0x40202fe8
os_print()
os_print_plus()
ets_printf()
installed_putc1()
ets_write_char()
Serial1.setDebugOutput(false) putc1 = 0x40203d40
uart_buff_switch(0) putc1 = 0x40203d40
uart_buff_switch(1) putc1 = 0x40203d40
ets_uart_printf()
ets_putc()
uart_buff_switch(0) putc1 = 0x40203d40
```
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.