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

Usage of __PRETTY_FUNCTION__ cause exception on version 2.5.1+ #6383

Closed
gaborauth opened this issue Aug 5, 2019 · 10 comments · Fixed by #6450
Closed

Usage of __PRETTY_FUNCTION__ cause exception on version 2.5.1+ #6383

gaborauth opened this issue Aug 5, 2019 · 10 comments · Fixed by #6450

Comments

@gaborauth
Copy link

Platform

  • Hardware: WEMOS D1 mini
  • Core Version: 2.5.0, 2.5.1, 2.5.2
  • Development Env: Arduino IDE
  • Operating System: Ubuntu

Settings in IDE

  • Module: Generic ESP8266 Module
  • Flash Mode: dout
  • Flash Size: 1MB
  • lwip Variant: v2 Lower Memory
  • Reset Method: ck
  • Flash Frequency: 40Mhz
  • CPU Frequency: 160MHz
  • Upload Using: SERIAL
  • Upload Speed: 115200

Problem Description

Expected output of the sketch (version 2.5.0):

void loop()

Sketch output on 2.5.1 and 2.5.2:

Exception (3):
epc1=0x402024cd epc2=0x00000000 epc3=0x00000000 excvaddr=0x4023a764 depc=0x00000000

>>>stack>>>

ctx: cont
sp: 3ffffda0 end: 3fffffc0 offset: 01a0
3fffff40:  40202839 00000001 3ffef484 3ffee28c  
3fffff50:  3fffdad0 00000000 3ffee234 402010cc  
3fffff60:  3ffe8790 3ffee2e4 3ffe84f4 402012a9  
3fffff70:  4023a764 3ffee28c 402016e0 3fffefa0  
3fffff80:  40201c4e 000003e8 3ffee234 402012e1  
3fffff90:  feefeffe 00000000 3ffee25c 40201051  
3fffffa0:  feefeffe feefeffe feefeffe 40201790  
3fffffb0:  feefeffe feefeffe 3ffe84f4 40100459  
<<<stack<<<

Sketch

void setup() {
    Serial.begin(115200);
}

void loop() {
    delay(1000);
    Serial.println(__PRETTY_FUNCTION__);
}

Related PR?

Maybe: #5758

@d-a-v
Copy link
Collaborator

d-a-v commented Aug 5, 2019

This is related to #6368 (comment) and #6384. A fix will come soon.

@earlephilhower
Copy link
Collaborator

Should be fixed, now, in git head. If not, please re-open.

@cr1st1p
Copy link

cr1st1p commented Aug 25, 2019

This is related to #6368 (comment) and #6384. A fix will come soon.

Hmm
My understanding is that a fix is done somewhere in WString.cpp.
But I wouldn't be so sure that Serial.println(const char* or const char[]) (which should be the function used for Serial.println(__PRETTY_FUNCTION__) does any WString use in there.
Even though __PRETTY_FUNCTION__ was moved to 'rodata' in Feb, that doesn't mean it was converted, for the compiler, from its 'const char []' type to 'const FlashStringHelper*' type so that other functions know what to do with it.

So, working with functions that are using standard string pointers (char*, char[]) could crash.
Think 'strcpy(dest, __PRETTY_FUNCTION__)' , 'strcat(dest, __PRETTY_FUNCTION__).
My concrete crashing example: std::string s(__PRETTY_FUNCTION__)

@earlephilhower
Copy link
Collaborator

You make some good points, but on this big they'll be overlooked. Can you open a new one with, so it will show up on the active list, please.

@cr1st1p
Copy link

cr1st1p commented Aug 26, 2019

@earlephilhower "Should be fixed in git head" - did it actually fix the problem?
I couldn't test myself, but I thought the fix was about WString.concat and not WString() constructor, in case even that gets called by Serial.println(__PRETTY_FUNCTION__) .

I'm asking because __PRETTY_FUNCTION__ translates to const char [] which would make Serial.println() use this implementation variant size_t println(const char[]); and not something with WString.

Let me know if I'm wrong else please reopen bug?
Thank you.

@earlephilhower
Copy link
Collaborator

earlephilhower commented Aug 26, 2019

Yeah, I was on my phone so couldn't get a good read on what was being discussed here.

There was a release with an updated SDK blob which handled the HW exceptions caused by non-32b reads on progmem, which was when this change was done. It saves 100s of bytes of heap due to the very long names you get in template expansion and some assertions in the code. The pre3.0 SDK, however, busted everything else so it was backed out.

Any of the *printf routines silently handle progmem addresses for format or parameter strings. In this case, the expedient thing to do is make is Serial.prinf("%s\n", __PRETTY_FUNCTION__)in your code for now.

Print::println() uses bytewise reads and not the STDC formatting so has a problem. A patch to make Print::write use pgm_read_byte would be a simple fix for this peculiar case.

earlephilhower added a commit to earlephilhower/Arduino that referenced this issue Aug 26, 2019
Adjust the ::write implementation in Print and its overridden copy in
UART to allow it to silentely accept PROGMEM strings (since there is no
write_P macro).

Fixes esp8266#6383
@cr1st1p
Copy link

cr1st1p commented Aug 27, 2019

@earlephilhower
Not everybody is using __PRETTY_FUNCTION__ only in Serial.print* and Print::println.
My own code is not using it like that, for example (a bigger logging library, that should be able to send data to other logging engines as well).

I found a mention about __PRETTY_FUNCTION__ being moved only on github's changelog !?

Can we please at least ensure this information and how to treat the change is more prominently present?
In some Arduino esp8266 compiler dedicated page (if there is one) and at least in https://arduino-esp8266.readthedocs.io/en/latest/PROGMEM.html ?

Thank you.

@earlephilhower
Copy link
Collaborator

The current PR is not going to hit 100% of the cases, true, but we're shooting for as wide coverage as possible with the least code borkage. Unfortunately on an embedded system like this with such a weird address space we have to make tradeoffs like this.

Better docs always sounds like a good idea! Would you like to do the doc PR?

@cr1st1p
Copy link

cr1st1p commented Aug 27, 2019

@earlephilhower Will try these days. PROGMEM.rst, git master branch, right?

@earlephilhower
Copy link
Collaborator

Yes. Just fork, git checkout -b my-branch-name, do your edits, push, and then you can make a PR right from the github website. Looking forward to it!

earlephilhower added a commit that referenced this issue Aug 28, 2019
Adjust the ::write implementation in Print and its overridden copy in
UART to allow it to silentely accept PROGMEM strings (since there is no
write_P macro).

Fixes #6383
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

Successfully merging a pull request may close this issue.

4 participants