Skip to content

Code does not compile when compiling without optimisations -Og instead of optimize small -Os #5115

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

Closed
jantje opened this issue Sep 9, 2018 · 8 comments
Assignees
Milestone

Comments

@jantje
Copy link

jantje commented Sep 9, 2018

edit: quoting updated
Hi
When trying to make a hardware debug platform.txt I bumped into the compilation problem below.

'Building file: C:\eclipse\arduinoPlugin\packages\esp8266\hardware\esp8266\2.4.2\cores\esp8266\core_esp8266_postmortem.c'
'Starting C compile'
"C:\eclipse\arduinoPlugin\packages\esp8266\tools\xtensa-lx106-elf-gcc\1.20.0-26-gb404fb9-2/bin/xtensa-lx106-elf-gcc" -D__ets__ -DICACHE_FLASH -U__STRICT_ANSI__ "-IC:\eclipse\arduinoPlugin\packages\esp8266\hardware\esp8266\2.4.2/tools/sdk/include" "-IC:\eclipse\arduinoPlugin\packages\esp8266\hardware\esp8266\2.4.2/tools/sdk/lwip2/include" "-IC:\eclipse\arduinoPlugin\packages\esp8266\hardware\esp8266\2.4.2/tools/sdk/libc/xtensa-lx106-elf/include" "-IL:/test/runtime-EclipseApplication_eng/esp/ff/core" -c -Wall -Wextra   -DDEBUG -Og -ggdb -g -Wpointer-arith -Wno-implicit-function-declaration -Wl,-EL -fno-inline-functions -nostdlib -mlongcalls -mtext-section-literals -falign-functions=4 -MMD -std=gnu99 -ffunction-sections -fdata-sections -DF_CPU=160000000L -DLWIP_OPEN_SRC -DTCP_MSS=536 -DDEBUG_ESP_PORT=Serial  -DARDUINO=10802 -DARDUINO_ESP8266_GENERIC -DARDUINO_ARCH_ESP8266 -DARDUINO_BOARD="\"ESP8266_GENERIC\"" -DLED_BUILTIN=9  -DESP8266   -I"C:\eclipse\arduinoPlugin\packages\esp8266\hardware\esp8266\2.4.2\cores\esp8266" -I"C:\eclipse\arduinoPlugin\packages\esp8266\hardware\esp8266\2.4.2\variants\generic" -I"C:\Users\jan\git\hardware\jantje\ESP8266\libraries\gdb" -MMD -MP -MF"core\core\core_esp8266_postmortem.c.d" -MT"core\core\core_esp8266_postmortem.c.o" -D__IN_ECLIPSE__=1 "C:\eclipse\arduinoPlugin\packages\esp8266\hardware\esp8266\2.4.2\cores\esp8266\core_esp8266_postmortem.c"  -o  "core\core\core_esp8266_postmortem.c.o"
C:\eclipse\arduinoPlugin\packages\esp8266\hardware\esp8266\2.4.2\cores\esp8266\core_esp8266_postmortem.c:83:23: error: istr causes a section type conflict with iram_read_byte
     static const char istr[] ICACHE_RAM_ATTR = (str); \
                       ^
C:\eclipse\arduinoPlugin\packages\esp8266\hardware\esp8266\2.4.2\cores\esp8266\core_esp8266_postmortem.c:190:5: note: in expansion of macro 'ets_printf_P'
     ets_printf_P("<<<stack<<<\n");
     ^
C:\eclipse\arduinoPlugin\packages\esp8266\hardware\esp8266\2.4.2\cores\esp8266\core_esp8266_postmortem.c:66:29: note: 'iram_read_byte' was declared here
 static char ICACHE_RAM_ATTR iram_read_byte (const char *addr) {
                             ^
core\core\subdir.mk:356: recipe for target 'core\core\core_esp8266_postmortem.c.o' failed
make: *** [core\core\core_esp8266_postmortem.c.o] Error 1

The problem lies into the fact that the define ICACHE_RAM_ATTR is used 2 times for a different variable.
Apparently optimized for small optimizes this to 1 variable which makes this problem is not visible when using -Os. Obviously optimizing for debugging does not allow this.

This can be fixed by changing the line

    static const char istr[] ICACHE_RAM_ATTR = (str);

to

    static const char istr[] __attribute__((section(".iram.text.2"))) = (str);

I didn't want to make this a push request because I am not sure "the change is an acceptable solution" as yesterday I had not heard about something called
__attribute__((section(XXX)))

Best regards
Jantje

@earlephilhower
Copy link
Collaborator

@jantje Give PR #5116 a try, it applies the same changes I did to fix this for PROGMEM variables to the section names of all ICACHE variables.

@jantje
Copy link
Author

jantje commented Sep 10, 2018

@earlephilhower
Thanks for the quick response. However I can not test the PR as I work from the release versions (read without using git)
But adding LINE should indeed do the trick.

@jjsuwa
Copy link
Contributor

jjsuwa commented Sep 10, 2018

Hi again,

#define ICACHE_FLASH_ATTR   __attribute__((section(".irom0.text")))
#define ICACHE_RAM_ATTR     __attribute__((section(".iram.text")))
#define ICACHE_RODATA_ATTR  __attribute__((section(".irom.text")))

The original is of course OK, but

#define ICACHE_FLASH_ATTR   __attribute__((section(".irom0.text.")))
#define ICACHE_RAM_ATTR     __attribute__((section(".iram.text.")))
#define ICACHE_RODATA_ATTR  __attribute__((section(".irom.text.")))

is not. ;-)

@earlephilhower
Copy link
Collaborator

Good catch. It does seem compile, link, and run, actually, w/o the period (tested using #gcc pragme optimize("Og")), but I'll re-add it.

@earlephilhower
Copy link
Collaborator

I take it back, I read your comment wrong. The version with the periods is already there and is similar to PROGMEM, so there's no problem with my PR as-is.

earlephilhower added a commit to earlephilhower/Arduino that referenced this issue Sep 10, 2018
Similar to PROGMEM changes, make the name of each ICACHE_* variable/fuction
unique to avoid issues with section conflicts.

Fixes esp8266#5115
@earlephilhower
Copy link
Collaborator

D'oh, too early, I see the problem you're pointing out is in the linker scripts, mybad! Updated the PR.

@devyte devyte added this to the 2.5.0 milestone Sep 11, 2018
devyte pushed a commit that referenced this issue Sep 11, 2018
Similar to PROGMEM changes, make the name of each ICACHE_* variable/fuction
unique to avoid issues with section conflicts.

Fixes #5115
@jantje
Copy link
Author

jantje commented Sep 11, 2018

Thanks :-)

@earlephilhower
Copy link
Collaborator

Reopening since there seems to be some issue with the new linker sections and the patch is presently undone.

earlephilhower added a commit to earlephilhower/Arduino that referenced this issue Sep 19, 2018
Similar to PROGMEM changes, make the name of each ICACHE_* variable/fuction
unique to avoid issues with section conflicts.

Also rename the generated LD linker script to avoid issue with older copies
of the eagle.app.v6.common.ld which were generated by the build process
in a global directory before being moved to the {build.path}.  The linker
would use the older, generated *.ld file instead of the generated one, which
would lead to runtime failures on some systems and cause the VTABLE location
to not correspond to the IDE menu selection.

Fixes esp8266#5115, and is an update to esp8266#5117 and esp8266#5116.
devyte pushed a commit that referenced this issue Sep 21, 2018
* Move ICACHE_* to unique sections, local LD script

Similar to PROGMEM changes, make the name of each ICACHE_* variable/fuction
unique to avoid issues with section conflicts.

Also rename the generated LD linker script to avoid issue with older copies
of the eagle.app.v6.common.ld which were generated by the build process
in a global directory before being moved to the {build.path}.  The linker
would use the older, generated *.ld file instead of the generated one, which
would lead to runtime failures on some systems and cause the VTABLE location
to not correspond to the IDE menu selection.

Fixes #5115, and is an update to #5117 and #5116.

* Update boards.txt.py and platform.io build
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants