Use proper g++ options, including proper C++ std=gnu++11

[ianfixes]

Issue Summary

via https://github.com/ianfixes/arduino_ci/issues/140#issuecomment-656966055

My initial reverse-engineer of arduino compilation used the wrong compiler options: -std=c++0x.

Here is a dump of actual arduino compilation commands:

/home/.../snap/arduino/41/.arduino15/packages/arduino/tools/arm-none-eabi-gcc/7-2017q4/bin/arm-none-eabi-g++ 
-mcpu=cortex-m0plus 
-mthumb 
-c 
-g 
-Os 
-Wall 
-Wextra
-Wno-expansion-to-defined 
-std=gnu++11 
-ffunction-sections 
-fdata-sections 
-fno-threadsafe-statics 
-nostdlib 
--param max-inline-insns-single=500 
-fno-rtti 
-fno-exceptions 
-MMD 
-DF_CPU=48000000L 
-DARDUINO=10813 
-DARDUINO_SAMD_MKRWIFI1010 
-DARDUINO_ARCH_SAMD 
-DUSE_ARDUINO_MKR_PIN_LAYOUT 
-D__SAMD21G18A__ 
-DUSB_VID=0x2341 
-DUSB_PID=0x8054 
-DUSBCON 
"-DUSB_MANUFACTURER=\"Arduino LLC\"" 
"-DUSB_PRODUCT=\"Arduino MKR WiFi 1010\"" 
-DUSE_BQ24195L_PMIC 
-I/home/.../snap/arduino/41/.arduino15/packages/arduino/tools/CMSIS/4.5.0/CMSIS/Include/ 
-I/home/.../snap/arduino/41/.arduino15/packages/arduino/tools/CMSIS-Atmel/1.2.0/CMSIS/Device/ATMEL/ 
-I/home/.../snap/arduino/41/.arduino15/packages/arduino/hardware/samd/1.8.6/cores/arduino 
-I/home/.../snap/arduino/41/.arduino15/packages/arduino/hardware/samd/1.8.6/variants/mkrwifi1010 
-I/home/.../snap/arduino/current/Arduino/libraries/TaskScheduler/src 
-I/home/.../snap/arduino/current/Arduino/libraries/WiFiNINA/src 
-I/home/.../snap/arduino/current/Arduino/libraries/RTCZero/src 
-I/home/.../snap/arduino/current/Arduino/libraries/StreamUtils/src 
-I/home/.../snap/arduino/41/.arduino15/packages/arduino/hardware/samd/1.8.6/libraries/SPI 
/home/.../snap/arduino/current/Arduino/libraries/RTCZero/src/RTCZero.cpp 
-o /tmp/arduino_build_827430/libraries/RTCZero/RTCZero.cpp.o

Of these, the following look like they should be swapped in immediately:

• -std=gnu++11
• -Wno-expansion-to-defined
• -nostdlib

This will likely change the nature of the workaround for things like String.h and Stream.h, possibly allowing (or requiring, depending on how you look at it) a ground-up rewrite of those.

[matthijskooijman]

Getting the compilation options right is a good plan. However, we certainly do not need to copy all options, since a lot of them are only needed to make things work on an Arduino, or make things more efficient, etc. Also, not all Arduino cores and boards use the same options, so we're going to find a common subset of the different cores and use that. And then maybe additional options can be configured in a per-core or per-board manner? Maybe it would even make sense to put all the options in the (default) config file even?

I can imagine something like:

global:
  flags: -std=gnu++11 -DARDUINO=100

packages:
  arduino:samd:
    url: ...
    flags: -DARDUINO_ARCH_SAMD

boards:
  zero:
    ...
    flags: -DARDUINO_SAMD_ZERO

This would allow three levels of compiler flags: Global, per-package, per board. I've now put some defines in flags for illustrating, but those should probably be in defines instead (and I guess the defines could also be at all three levels).

I also wonder if this would need maybe two values: flags and extra_flags? Then flags can contain the defaults from default.yaml, which can either be completely replaced by putting flags in your .arduino-ci.yaml, or you can add extra flags by putting extra_flags in your .arduino-ci.yaml.

Furthermore, we might want to consider allowing specifying filetype-specific compilation flags too. Arduino cores have different recipes (compiler commandlines) for C, CPP and assembler files and the final link, which might be relevant here as well (though that does require separate object building #113, or letting arduino-cli build #147).

Flags

-mcpu=cortex-m0plus 
-mthumb 

These select the ARM-processor to use, so not relevant.

-c 

This tells to compile but not link, so this would be hardcoded if we go for #113.

-g 

This enables debugging symbols, probably good to include (in case someone wants to debug their unittst with gdb or similar).

-Os 

This enables size optimizations. Enabling some kind of optimization for the unittests might be good (though not required), but maybe not size. Maybe -Og is a good compromise, IRC that enables all optimizations that do not complicate step-by-step debugging.

-Wall 
-Wextra
-Wno-expansion-to-defined 

These are warning options, might be good to make them user-configurable (by default, the Java IDE disables warnings, so enabling them by default would be useful, but might also produce a lot of warnings all of a sudden).

-std=gnu++11

This determines the language standard version. Switching to gnu++11 would indeed be good, since most Arduino cores now use this. However, the currently used -std=c++0x is actually not far from this. C++0x was the working name for the new C++ version when they still expected it to be final before 2010, but they ended up finalizing it only in 2011, so -std=c++11 and -std=c++0x is exactly the same option. Of course, Arduino uses gnu++11 rather than c++11, which enables some GNU-specific extensions that might be used by some Arduino code, so this should indeed be switched.

-ffunction-sections 
-fdata-sections 

These make sure that functions and variables end up in different sections, so they can be optimized away with --gc-sections at link time.

-fno-threadsafe-statics 
-nostdlib 
--param max-inline-insns-single=500 
-fno-rtti 
-fno-exceptions 
-MMD 

-Wno-expansion-to-defined

This disables a specific warning, of which I wonder if it really should be disabled (it looks like a meaningful warning). It turns out this is only needed when using the Atmel CMSIS code (which we don't), and not even really then: arduino/ArduinoCore-samd#556

-nostdlib

This removes the standard libc libraries, which is probably needed when running on SAMD to get complete control of the startup process or so. When running unittests, this option would probably just break the build, so I don't think we need this here.

Then there's a bunch of defines, where we should pick a small selection, only -DARDUINO is really common, most others are board-specific, I already added a bunch of those in #144. We could probably bump the ARDUINO version, number, maybe to 1.8.0 or so? This is a bit of a weird define now, since e.g. arduino-cli has different version numbers than the Java IDE and core versions are no longer tied closely to IDE versions, so sketches should not really rely on this number anymore (ideally, cores would specify an API version, but that's not common practice yet, I think).

So, I think we need: -g -Og -std=gnu++11 -DARDUINO=10800

[ianfixes]

A bunch of what you're suggesting is already implemented, see: https://github.com/ianfixes/arduino_ci/blob/master/REFERENCE.md#defining-new-arduino-platforms

This behavior is configured in https://github.com/ianfixes/arduino_ci/blob/master/lib/arduino_ci/cpp_library.rb

I prefer not to use -Og in place of -O1, as that's in there specifically to aid in libasan-enabled debugging when available.

The main expected impact is the -no-stdlib change, since without it I had run into collisions between Arduino and standard libraries. I'd be surprised if the other flags couldn't be used immediately.

[matthijskooijman]

I prefer not to use -Og in place of -O1, as that's in there specifically to aid in libasan-enabled debugging when available.

I was suggesting this for the non-libasan build, since I guess that custom options for that would be needed anyway (though I haven't looked at how this libasan stuff works in detail). Also, I wonder if libasan wouldn't benefit from -Og rather than -O1, maybe the same "do not jumble debug symbols with optimizations" applies to libasan as well. OTOH, -O1 might be a subset of -Og, dunno exactly.

The main expected impact is the -no-stdlib change, since without it I had run into collisions between Arduino and standard libraries. I'd be surprised if the other flags couldn't be used immediately.

But I don't think this should be changed/added? Running with -no-stdlib would, I think, completely prevent using some standard library components, such as libc functions and startup constructor calls. Can you expand on the collisions you saw? How to reproduce that?

[ianfixes]

the same "do not jumble debug symbols with optimizations" applies to libasan as well

I believe so, although I don't recall what documentation I read that would have supported that view. In any case, I think we're on the same page about the optimizer really not being necessary for space or performance reasons -- it would all be to drive various features of segfault analysis during unit testing.

stdlib problems

My memory on this is fuzzy, as this library was built up by just running into problems (at every level) and then working around them. I might be confusing stdlib problems with arduino "standard library" problems -- in other words, mocking the built-in stuff. AVR Math was definitely on that list.

Also, if we compile using the Arduino CLI, I'm not sure what options we have to mock all the built-in classes. I guess we'll find out 😄