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

Move all PROGMEM to their own section #5048

Merged
merged 1 commit into from
Aug 15, 2018

Conversation

earlephilhower
Copy link
Collaborator

According to the GCC man page, section attributes should only be used
for global variables. However, the PROGMEM and ICACHE_RODATA macros use
this variable decorator even for local variables. Most of the time it works,
but when a static or inlined function tries to use a PROGMEM/PSTR/etc.
variable the compiler can throw an error like:
error: XXX causes a section type conflict with YYY

Change the PROGMEM macro to emit a section name that is unique (a combo
of the file, line, and counter variables to ensure uniqueness). The
standard linker script will place them properly in .IROM without
any changes.

Fixes #5036 and others.

@earlephilhower
Copy link
Collaborator Author

This could also allow the single array mimetable.cpp to move back into the static function it started in, but I didn't want to change more than the minimum as this is a small change with potentially large effect.

According to the GCC man page, __section__ attributes should only be used
for global variables.  However, the PROGMEM and ICACHE_RODATA macros use
this variable decorator even for local variables.  Most of the time it works,
but when a static or inlined function tries to use a PROGMEM/PSTR/etc.
variable the compiler can throw an error like:
  error: XXX causes a section type conflict with YYY

Change the PROGMEM macro to emit a section name that is unique (a combo
of the file, line, and counter variables to ensure uniqueness).  The
standard linker script will place them properly in .IROM without
any changes.

Fixes esp8266#5036 and others.
Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome side effect (shrinks bin)

@devyte devyte merged commit 56b98fd into esp8266:master Aug 15, 2018
@jjsuwa
Copy link
Contributor

jjsuwa commented Aug 15, 2018

#define PROGMEM __attribute__((section( ".irom.text." __FILE__ "." __STRINGIZE(__LINE__) "." __STRINGIZE(__COUNTER__))))

When __FILE__ has non-symbol chars such as space (e.g. "C:\Program Files (x86)\Arduino\hardware\esp8266com\esp8266\libraries\ESP8266WiFi\src\BearSSLHelpers.cpp"), the compilation will always be failed.

C:\Users\Administrator\AppData\Local\Temp\ccYdv0jj.s: Assembler messages:
C:\Users\Administrator\AppData\Local\Temp\ccYdv0jj.s:4348: Error: junk at end of line, first unrecognized character is `F'

@earlephilhower
Copy link
Collaborator Author

Ah, poop. I did test on my Windows VM, but for me the Arduino libs were installed under C:\Users\... so I didn't catch this since there were no spaces.

Fast fix is to drop __FILE__ from the generated GUID, at the loss of much entropy. Let me see if there's some other way, or something else unique per file that's not . Since this got merged, would you please open a new bug with your quote, @jjsuwa, so we can track it?

@earlephilhower
Copy link
Collaborator Author

@jjsuwa Can you try #5049 out? I manually forced a space in the string and it was able to compile successfully with this one change.

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 this pull request may close these issues.

5 participants