Array bounds warning building Host with GCC 8

[mikee47]

See #1692 for initial problem report.

The issue is with the LOAD_PSTR macro in FakePgmSpace.h, which copies data from flash to RAM in 4-byte words. Byte-copying from flash is very inefficient so this is an optimisation for the Esp8266.

This issue has arised now we can compile the code using more up-to-date tools. From https://gcc.gnu.org/gcc-8/changes.html:

The -Warray-bounds option has been improved to detect more instances of out-of-bounds array indices and pointer offsets. For example, negative or excessive indices into flexible array members and string literals are detected.

This is the output from the pre-processor for the offending code:

  case EVENT_STAMODE_CONNECTED:
  (__extension__({
    static const char log_string[] = "%u " "connect to ssid %s, channel %d" "\r\n";
    char fmtbuf[(((sizeof(log_string)) + 3) & ~3)] __attribute__((aligned(4)));
    memcpy(fmtbuf, log_string, sizeof(fmtbuf));;
    m_printf(fmtbuf, system_get_time(), evt->event_info.connected.ssid, evt->event_info.connected.channel);
  }));

We're attempting to do a memcpy using the size of the allocated buffer, which is longer than the actual data stored in flash. That doesn't matter on the Esp8266 as we know it's safe to read at least to the next word boundary, but it's not good practice.

[mikee47]

@frankdownunder Could you give this PR a check to ensure it solves your problem?

[frankdownunder]

@mikee47 I will do it tomorrow - should I look at
https://github.com/mikee47/Sming/tree/fix/LOAD_PSTR_overrun ??

[mikee47]

Thanks, yes that's the one. Did you manage to get anything compiled for Host yet?

[frankdownunder]

my branch is 'origin/fix/LOAD_PSTR_overrun'.
Those compiler errors are gone, but unfortunately there are some new ones:
Libraries/RCSwitch/RCSwitch.cpp: In member function ‘char* RCSwitch::getCodeWordB(int, int, boolean)’:
Libraries/RCSwitch/RCSwitch.cpp:288:12: error: invalid conversion from ‘char’ to ‘char*’ [-fpermissive]
return '\0';
^~~~
Libraries/RCSwitch/RCSwitch.cpp: In member function ‘char* RCSwitch::getCodeWordC(char, int, int, boolean)’:
Libraries/RCSwitch/RCSwitch.cpp:351:12: error: invalid conversion from ‘char’ to ‘char*’ [-fpermissive]
return '\0';
^~~~
Libraries/RCSwitch/RCSwitch.cpp: In member function ‘char* RCSwitch::getCodeWordD(char, int, boolean)’:
Libraries/RCSwitch/RCSwitch.cpp:421:20: error: invalid conversion from ‘char’ to ‘char*’ [-fpermissive]
return '\0';
^~~~
Libraries/RCSwitch/RCSwitch.cpp:440:20: error: invalid conversion from ‘char’ to ‘char*’ [-fpermissive]
return '\0';

Plus also I got
In file included from Libraries/Adafruit_GFX/Adafruit_GFX.cpp:38:
Libraries/Adafruit_GFX/glcdfont.c:10: error: "PROGMEM" redefined [-Werror]
#define PROGMEM

In file included from System/include/debug_progmem.h:20,
from Arch/Host/Components/esp_hal/include/esp_systemapi.h:43,
from Arch/Host/System/include/user_config.h:7,
from Wiring/Arduino.h:5,
from Libraries/Adafruit_GFX/Adafruit_GFX.h:5,
from Libraries/Adafruit_GFX/Adafruit_GFX.cpp:34:
Wiring/FakePgmSpace.h:150: note: this is the location of the previous definition
#define PROGMEM attribute((aligned(4)))

so some more patches needed for the Libraries it would seem

[frankdownunder]

If I workaround past those compiler errors, the buid goes on to give me this:

C+ Libraries/OneWire/OneWire.cpp
C+ Libraries/APA102/apa102.cpp
AR out/build/Host/Linux/libsming.a
mkdir -p out/build/Host/Linux/Arch/Host/Components/lwip
CMake Error: The source directory "/Arch/Host/Components/lwip/Linux" does not exist.

[mikee47]

Tools A little background on the systems I'm using to develop and test:

Linux - gcc (Ubuntu 7.4.0-1ubuntu1~18.04) 7.4.0. I'm running Ubuntu 18.04.2 LTS.

The Travis build I had to update it was using g++ (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4, which failed miserably, so updated to g++ (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609.

Windows - gcc (MinGW.org GCC-6.3.0-1) 6.3.0.

Similarly appveyor was running the default MinGW install gcc version 5.3.0 (GCC), which didn't work, so added a mingw-update to get it to the above 6.3.0.

I would suggest that the above versions are the most likely ones to be found on users' dev systems, certainly for Windows users, so straying too far from those versions and we're likely to run into problems.

==> @frankdownunder For the time being, is it hassle to drop back to 7.4?

Libraries These the perhaps the biggest problem facing a migration to Esp32. I checked the Host build against all the libraries and made a spot decision whether to fix or to exclude it. Whilst the libraries may build, ArduinoJson is the only one which I can say will actually work! All the others will need a closer looking at - some will certainly not work and probably crash as they do direct memory writes, etc.

==> May I suggest for the time being just set ARDUINO_LIBRARIES=ArduinoJson ?

CI Testing It does appear we get some valuable additional checking using GCC 8+. Should we then add additional builds under those compilers? I say add since it's unsafe to presume that a successful test under, say, gcc 8.3.0 (Feb 19) or even GCC 9 will guarantee we won't have issues with earlier versions.

[mikee47]

CMake Error: The source directory "/Arch/Host/Components/lwip/Linux" does not exist.

@frankdownunder If you're still getting this error, could you do:

make submodules-clean
make lwip V=1

Should highlight what's going on

[frankdownunder]

I did this, still working wit your branch on your fork, and got
make: *** No rule to make target 'lwip'. Stop.

[frankdownunder]

You asked for the time being, is it hassle to drop back to 7.4?
That's no problem. I reckon we can workaround the issues I have anyway, so don't hold things up on my account.

[mikee47]

I've got a suspicion you have ENABLE_CUSTOM_LWIP defined in your environment... If so, please remove it and try again.

[slaff]

I've got a suspicion you have ...

He for sure does not have STRICT=1 set. Without it the default behaviour is to treat re-definition as an error.

Libraries/Adafruit_GFX/glcdfont.c:10: error: "PROGMEM" redefined [-Werror]
#define PROGMEM

[frankdownunder]

I fixed the two issues I mentioned - see PR #1719