earlephilhower / newlib-xtensa

newlib-xtensa fork intended for esp8266
GNU General Public License v2.0
5 stars 7 forks source link

PSTR within templated code #4

Closed mikee47 closed 4 years ago

mikee47 commented 4 years ago

GCC silently disregards attributes in templated code (https://github.com/esp8266/Arduino/pull/6294#issuecomment-560576979)

Test case:

template <typename T>
void templateTest(T x)
{
    Serial.print("x = ");
    Serial.println(x);
    Serial.println(PSTR("This text ends up in RAM (.rodata) because GCC silently disregards attributes in templated code."));
}

This can be fixed by adding to linker script .irom0.text output section:

    /* Inline flash strings PSTR() within templated code */
    *(.rodata._ZZ*__c)
    *(.rodata._ZZ*__c_*)

With this in mind, a safer/more descriptive name than __c would be preferable, such as __pstr__.

mikee47 commented 4 years ago

Further thought. The above linker script changes would also catch regular PSTR usage, which means PROGMEM doesn't need to be applied to it. That would emit it to the regular .rodata section and be coalesced automatically before the linker moves it into .irom0.text.

Filter can be simplified to: *(.rodata.*__pstr__*) or just *(*__pstr__*)

earlephilhower commented 4 years ago

With the new PSTR macros, we're already placing every string into a single block of dedupable strings by the section name itself. That is, on the Arduino core, we don't have the lines you mentioned above. Everything is done w/no chance of naming conflict by the *(.irom0.pstr.*) line.

I think if you use the macro as-defined here and not the older one you've got in Sming, it may actually take care of the template stuff, too, since the assembly emitted itself is making the .section XXXX in the intermediate .S file, not GCC or its template munger.

mikee47 commented 4 years ago

This is the intermediate assembly generated for the above test case:

    .weak   _ZZ12templateTestIiEvT_E8__pstr__
    .section    .rodata._ZZ12templateTestIiEvT_E8__pstr__,"aG",@progbits,_ZZ12templateTestIiEvT_E8__pstr__,comdat
    .align  4
    .type   _ZZ12templateTestIiEvT_E8__pstr__, @object
    .size   _ZZ12templateTestIiEvT_E8__pstr__, 92
_ZZ12templateTestIiEvT_E8__pstr__:
    .string "This PSTR text ends up in RAM because GCC silently disregards attributes in templated code."
mikee47 commented 4 years ago

With a regular (non-templated) function, I get this:

    .section    ".irom0.pstr.test-string.cpp.17.155", "aSM", @progbits, 1 #,"a"
    .align  4
    .type   _ZZ15nonTemplateTestiE8__pstr__, @object
    .size   _ZZ15nonTemplateTestiE8__pstr__, 66
_ZZ15nonTemplateTestiE8__pstr__:
    .string "This PSTR text is stored correctly as it's not in templated code."
mikee47 commented 4 years ago

This is a known issue with GCC and templates. https://answers.launchpad.net/gcc-arm-embedded/+question/615576

The problem with using attributes and templates is overloading. Because attributes don't mangle var names, overloading becomes problematic in C++. I'm guessing thats why the section attribute on a template var is ignored. See some notes here - https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html.

earlephilhower commented 4 years ago

Think this was taken care of in #3. Since GCC drops the section hint, we have to match by name (now __pstr__).