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

Upgrade to GCC 10.1 toolchain #6294

Merged
merged 85 commits into from
Jul 7, 2020
Merged

Conversation

earlephilhower
Copy link
Collaborator

@earlephilhower earlephilhower commented Jul 13, 2019

All examples I've tested run fine. GDB is working. No warnings.

Fixes #3352

@earlephilhower
Copy link
Collaborator Author

Lots of parenthesis warning, but basic examples like "Blink.ino" and "BearSSL_Validation.ino" run fine as far as I can see now. Still optimization on exceptions and removing the warnings which are causing the plain Arduino CI builds to fail.

Remove dependencies on earlier forked GNU utilities (gcc-xtensa,
binutils-gdb-xtensa) and just use GCC sources, unmodified (except for
patches in the esp-quick-toolchain directories).
GDB works with pure GNU GCC and pure GNU binutils now.  Still warnings
galore, but tested with the example sketch in the docs.
Fix some warnings present in GCC8/9 in the IPAddress code

In AddressListIterator there was a copy constructor which simply copied
the structure bit-for-bit.  That's the default operation, so remove it
to avoid the warning there.

IPAddress, add a default copy constructor since the other copy
constructors are simply parsing from one format into a native ip_addr_t.

@d-a-v, can you give these a look over and see if they're good (since
IP stuff is really your domain).
These both use shared-ptrs to handle refcnts to allocated data, so using
the default copy constructor is fine (and has been in use for a long
time).
Make GCC think _heap_start is large enough to avoid the basic (and
incorrect) bounds-checking warnings it produces.  The size chosen is
arbitrary and does not affect the actual size of the heap in any way.
@earlephilhower
Copy link
Collaborator Author

@d-a-v, could you check the AddrList and IPAddr changes?

@d-a-v
Copy link
Collaborator

d-a-v commented Jul 13, 2019

Comparing a sketch with .text1 fitting issue, it appears file _divdi3.o takes much space in iram

      0x3df /path/to/libgcc.a(_divdi3.o)

This symbol is not here with gcc4.8.
Apart from that, I also run BearSSLValidation and it works great here too.
That's a great leap for espkind !

Instead of a bogus size, use an indefinite size for the heap to avoid
GCC warnings
@earlephilhower
Copy link
Collaborator Author

earlephilhower commented Jul 14, 2019

Thanks, @d-a-v. I haven't been looking at the iram yet, still trying to get rid of the warnings. Are you OK with the changes to IP classes here? 3afe9e3

-edit- Just added the divdi3.o to the nm remove stage, we'll see how it goes on the next toolchain rebuild.

The callback function is defined to take a (void*) as parameter, but our
templates let users use anything that fits inside sizeof(void*) to be
passed in.  Add pragmas to stop GCC warnings about this, since we
already check the size of the type will fit in the allocated space.
Manually delete the divdi3.so from the libgcc.a library by running the
updated EQT's 9.1-post script.
Exceptions are broken on all builds (GCC4.8-9.1) due to the removal of
the PROGMEM non-32b read exception handler (added in the unstable
pre3.0.0).

Build the exception code with -mforce-l32 and patch
accordingly to avoid LoadStore errors.

Apply patches to select portions of the regex lib which use _stype_
(which is now in flash).
@earlephilhower
Copy link
Collaborator Author

@mcspr and @Jason2866 , I've hacked in the toolchain update in the PIO CI tests. Can you take a gander and see if there's something else you would suggest doing vs just overwriting the toolchain as the tests/platformio.sh does now.

mv ~/.platformio/packages/toolchain-xtensa/package.json .save
rm -rf ~/.platformio/packages/toolchain-xtensa
mv $TRAVIS_BUILD_DIR/tools/xtensa-lx106-elf ~/.platformio/packages/toolchain-xtensa
mv .save ~/.platformio/packages/toolchain-xtensa/package.json
python -c "import json; import os; fp=open(os.path.expanduser('~/.platformio/platforms/espressif8266/platform.json'), 'r+'); data=json.load(fp); data['packages']['framework-arduinoespressif8266']['version'] = '*'; fp.seek(0); fp.truncate(); json.dump(data, fp); fp.close()"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Regarding platform installation:

  • pio platform install has --without-package switch, this can avoid fetching the platform (toolchain-xtensa)
  • the same "*" glob for toolchain-extensa package described in the platform.json needs to be here?

Script itself uses pio ci down below, can't reach with github comment UI:

  • if get.py can instead of immediately fetching stuff just print the url, then "--board nodemcu_v2 --verbose" can be amended with the -O "platform_packages = toolchain-xtensa @ url" (shell-script quoting might become weird though). but, that would work only when package.json is inside the tarball / zip
  • manual fetch is also an option, package.json can just be generated (see root repo's package.json as an example)

@Jason2866
Copy link
Contributor

Jason2866 commented Jun 10, 2020

@earlephilhower time for some work load on your servers. 😆 Since you have already made the changes to include the package.json in your tarball generating script. So you could do it the way @mcspr has described. And i would like it a lot too because this gives everyone the possibilty to use the GCC 10.1 build chain easily with PlatformIO 👍☺
At the moment i manually add the package.json and upload on my github. Ugly...

@earlephilhower
Copy link
Collaborator Author

I'll adjust things to use the URL package once I get a chance to rebuild the toolchain. There were a couple of PIO issues squeaked out with the hacked version, though, which have been fixed.

Add a menu to enable GCC's built-in stack smash protection.  When a
subroutine goes past its end of stack, generate a crashdump on function
exit like:

````
GCC detected stack overrun
Stack corrupted, stack smash detected.

>>>stack>>>

ctx: cont
sp: 3fffff20 end: 3fffffc0 offset: 0000
3fffff20:  40202955 00000001 0000001c 4020287e
3fffff30:  feefeffe 000000fd 00000000 00000000
...
<<<stack<<<
````

Disabled by default because there is a small per-function code overhead
(and CPU time if the function is called very frequently and is very
small).

BearSSL and LWIP are not built using stack smash detection, yet.
Report a fake exception to have the exception decoder print the actual
faulting function.  This won't tell you where in the function the issue
happened, but it will tell you the function name first and foremost.
@Gizmodo
Copy link

Gizmodo commented Jun 20, 2020

@d-a-v When do you plan to merge PR? I can't wait for C++17

@devyte
Copy link
Collaborator

devyte commented Jun 21, 2020

You can check the milestone for issues and PRs on the right. This is targeted for 3.0.0. That means it will be merged after 2.7.2, which is very close at hand.

@d-a-v
Copy link
Collaborator

d-a-v commented Jun 21, 2020

@Gizmodo it is included in the alpha release

@Jason2866
Copy link
Contributor

Jason2866 commented Jun 21, 2020

@Gizmodo
I did a PlatformIO setup for testing the Project Tasmota with GCC10.1
You can use it with this entry in Platformio.ini
platform = https://github.com/Jason2866/platform-espressif8266.git#new_gcc
You can take a look at: https://github.com/Jason2866/Tasmota/tree/Gcc10.1

@hreintke
Copy link
Contributor

hreintke commented Jul 3, 2020

@earlephilhower

There is a difference between the current an gcc9 toolchain in the .d files that are created.
These .d files are used by make in the build process.

Current :

T1.cpp.o: ..\T1.cpp ..\T1.h C:\temp\ArduinoC11\cores\esp8266/Arduino.h \
 C:/temp/ArduinoC11/tools/sdk/libc/xtensa-lx106-elf/include/stdlib.h \
 C:/temp/ArduinoC11/tools/sdk/libc/xtensa-lx106-elf/include/machine/ieeefp.h \
 C:/temp/ArduinoC11/tools/sdk/libc/xtensa-lx106-elf/include/_ansi.h \
 C:/temp/ArduinoC11/tools/sdk/libc/xtensa-lx106-elf/include/newlib.h \
 C:/temp/ArduinoC11/tools/sdk/libc/xtensa-lx106-elf/include/sys/config.h \
 C:/temp/ArduinoC11/tools/sdk/libc/xtensa-lx106-elf/include/sys/features.h \
 C:/temp/ArduinoC11/tools/sdk/libc/xtensa-lx106-elf/include/xtensa/config/core-isa.h \

gcc9

T1.cpp.o: ..\T1.cpp ..\T1.h C\:\temp\ArduinoC17\cores\esp8266/Arduino.h \
 C\:/temp/ArduinoC17/tools/sdk/libc/xtensa-lx106-elf/include/stdlib.h \
 C\:/temp/ArduinoC17/tools/sdk/libc/xtensa-lx106-elf/include/machine/ieeefp.h \
 C\:/temp/ArduinoC17/tools/sdk/libc/xtensa-lx106-elf/include/_ansi.h \
 C\:/temp/ArduinoC17/tools/sdk/libc/xtensa-lx106-elf/include/newlib.h \
 C\:/temp/ArduinoC17/tools/sdk/libc/xtensa-lx106-elf/include/sys/config.h \
 C\:/temp/ArduinoC17/tools/sdk/libc/xtensa-lx106-elf/include/sys/features.h \
 C\:/temp/ArduinoC17/tools/sdk/libc/xtensa-lx106-elf/include/xtensa/config/core-isa.h \

There is an additional \ after the first C

In my eclipse sloeber development environment that results in not being able to do incremental builds.

           Finished prerequisites of target file '..\T1.h'.
          No recipe for '..\T1.h' and no prerequisites actually changed.
          No need to remake target '..\T1.h'.
          Considering target file 'C\:\temp\ArduinoC17\cores\esp8266/Arduino.h'.
           File 'C\:\temp\ArduinoC17\cores\esp8266/Arduino.h' does not exist.
           Finished prerequisites of target file 'C\:\temp\ArduinoC17\cores\esp8266/Arduino.h'.
          Must remake target 'C\:\temp\ArduinoC17\cores\esp8266/Arduino.h'.
make: *** No rule to make target 'C\:\temp\ArduinoC17\cores\esp8266/Arduino.h', needed by 'T1.cpp.o'.  Stop.
"C:/temp/Sloeber/arduinoPlugin/tools/make/make --debug=verbose all" terminated with exit code 2. Build might be incomplete.

Is there a way to revert to the old format ?

@earlephilhower
Copy link
Collaborator Author

That's pretty strange. It's not a dependency format so much as an escaping of paths issue. I doubt there's anything you can do to make GCC not escape the colon in makefiles.

What make version do you have installed? Looking thru gnu.org I can see in gcc 4.x has an unescape_char(...,':') in the depfiles parser, but that is not present in the 3.x branch. No guarantees this fixes your problem, but it does look very promising.

@hreintke
Copy link
Contributor

hreintke commented Jul 4, 2020

Sloeber eclipse has make 3.8.2 in it's distribution.
In contact with him now probably moving to 4.2.1 which doesn't have problems with the new format.

Thanks for pointing in the right direction.

@earlephilhower
Copy link
Collaborator Author

Here goes nothing...

@earlephilhower earlephilhower merged commit d979b57 into esp8266:master Jul 7, 2020
@mhightower83
Copy link
Contributor

mhightower83 commented Jul 13, 2020

FWIW: The optimization does appear to have been improved. Code that constructs a slightly obscured divide by 0, as some of the examples I do to show the exception processing, may now report an HWDT event instead. In my case, the 10.1 compiler figured it out and replaced it with a break 1, 15. My workaround was to add a noinline to my local divide a / b function that I call to generate the exception.

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.

error: array used as initializer Multiline raw Strings not parsed in Macro
10 participants