Function declaration isn't a prototype [-Wstrict-prototypes]

[TD-er]

Basic Infos

• This issue complies with the issue POLICY doc.
• I have read the documentation at readthedocs and the issue is not addressed there.
• I have tested that the issue is present in current master branch (aka latest git).
• I have searched the issue tracker for a similar issue.
• If there is a stack dump, I have decoded it.
• I have filled out all fields below.

Platform

• Core Version: [Core 2.6.2]
• Development Env: [Platformio]
• Operating System: [Windows]

Problem Description

As discussed here: platformio/platform-espressif8266#166 (comment)

The suggested fix by @ascillato for PlatformIO build issues is to add (void) in function declarations without parameters, just like in C.

This made me look into the reasons behind it and I came across this warning option for gcc:

To get gcc to warn about empty parameter lists, use -Wstrict-prototypes :

Warn if a function is declared or defined without specifying the argument types. (An old-style function definition is permitted without a warning if preceded by a declaration which specifies the argument types.)

So I applied it also to my PlatformIO environment and came across a number of warnings in the code of this library.

C:\Users\gijs\.platformio\packages\framework-arduinoespressif8266@src-84f070b88e6ae21c2e01a78d7310fa63\cores\esp8266/twi.h:47:1: warning: function declaration isn't a prototype [-Wstrict-prototypes]
 uint8_t twi_status();
 ^


In file included from C:\Users\gijs\.platformio\packages\framework-arduinoespressif8266@src-84f070b88e6ae21c2e01a78d7310fa63\cores\esp8266/Arduino.h:40:0,
                 from lib\Adafruit_NeoPixel\esp8266.c:7:
C:\Users\gijs\.platformio\packages\framework-arduinoespressif8266@src-84f070b88e6ae21c2e01a78d7310fa63\cores\esp8266/core_esp8266_features.h:89:1: warning: function declaration isn't a prototype [-Wstrict-prototypes]
Compiling .pio\build\normal_ESP8266_4M1M\libe2c\ESP8266mDNS\LEAmDNS_Helpers.cpp.o
 inline uint32_t esp_get_cycle_count() __attribute__((always_inline));
 ^

C:\Users\gijs\.platformio\packages\framework-arduinoespressif8266@src-84f070b88e6ae21c2e01a78d7310fa63\cores\esp8266/core_esp8266_features.h:90:17: warning: function declaration isn't a prototype [-Wstrict-prototypes]
Compiling .pio\build\normal_ESP8266_4M1M\libe2c\ESP8266mDNS\LEAmDNS.cpp.o
 inline uint32_t esp_get_cycle_count() {
                 ^

In file included from lib\Adafruit_NeoPixel\esp8266.c:7:0:
Compiling .pio\build\normal_ESP8266_4M1M\lib88f\NewPingESP8266\NewPingESP8266.cpp.o
C:\Users\gijs\.platformio\packages\framework-arduinoespressif8266@src-84f070b88e6ae21c2e01a78d7310fa63\cores\esp8266/Arduino.h:142:1: warning: function declaration isn't a prototype [-Wstrict-prototypes]
 void ets_intr_lock();
Compiling .pio\build\normal_ESP8266_4M1M\lib06e\ESP8266WiFi\ESP8266WiFiSTA-WPS.cpp.o
 ^
Compiling .pio\build\normal_ESP8266_4M1M\lib725\ESP8266HTTPClient\ESP8266HTTPClient.cpp.o
C:\Users\gijs\.platformio\packages\framework-arduinoespressif8266@src-84f070b88e6ae21c2e01a78d7310fa63\cores\esp8266/Arduino.h:143:1: warning: function declaration isn't a prototype [-Wstrict-prototypes]
 void ets_intr_unlock();
 ^
C:\Users\gijs\.platformio\packages\framework-arduinoespressif8266@src-84f070b88e6ae21c2e01a78d7310fa63\cores\esp8266/Arduino.h:176:1: warning: function declaration isn't a prototype [-Wstrict-prototypes]
 int atexit(void (*func)()) __attribute__((weak));
 ^

The fixes are rather trivial, but I wonder also whether these kind of issues can lead to build issues as described in the linked issue of PlatformIO ?

[earlephilhower]

It's a warning, and the parameter lists for the functions are all empty, so I really doubt it has any effect on the actual generated code. IMHO the linked PIO issue is something completely different.

Have you done any tests with Arduino builds adding -Wstrict-prototypes to platform.local.txt?

[earlephilhower]

I can guarantee it's not an issue with the Arduino core.

For G++, -Wstrict-prototypes is invalid anyway since C++ requires it to be on:
cc1plus: warning: command line option '-Wstrict-prototypes' is valid for C/ObjC but not for C++ [enabled by default]

In the Arduino core, we have vanishingly few .c files anymore, we migrated everything to .cpp a while back.

earle@server:~/Arduino/hardware/esp8266com/esp8266$ find libraries  -name "*.c" 
libraries/GDBStub/src/internal/gdbstub.c
libraries/LittleFS/src/lfs_util.c
libraries/LittleFS/src/lfs.c
libraries/LittleFS/lib/littlefs/lfs_util.c
libraries/LittleFS/lib/littlefs/lfs.c
libraries/LittleFS/lib/littlefs/emubd/lfs_emubd.c
libraries/SPISlave/src/hspi_slave.c
libraries/TFT_Touch_Shield_V2/font.c
earle@server:~/Arduino/hardware/esp8266com/esp8266$ find cores -name "*.c" 
cores/esp8266/umm_malloc/umm_integrity.c
cores/esp8266/umm_malloc/umm_poison.c
cores/esp8266/umm_malloc/umm_info.c
cores/esp8266/umm_malloc/umm_local.c

[TD-er]

Well I set the warning as a C-only warning, so the reported warnings are whenever the compiler is acting as a C-compiler and processing the code.

Lately the WiFi issues with different stability per build do occur less often, so the move to C++ code probably also has a positive effect.
But on the ESP32 it really is a problem with really bad wifi in one build and perfectly fine in another build with no clearly related code changes. (e.g. moving a log line into comments or enable/disable a plugin)
And on the ESP32 I do see lots and lots of these warnings.

In the esp8266/Arduino code base I use there are only 3 occurrences of these warnings.

[earlephilhower]

Fair enough. But I can pretty much guarantee that the warning about not having void for the param list instead of ()has 0 effect on code generation and any seen instability. Honestly, I think your time is better spent looking elsewhere. If you'd like to do a PR with fixes for the 3 spots where Arduino 8266 has an issue, we'd definitely look at it!

[TD-er]

Well, I've already spent a lot of time on the build issues, and @Jason2866 is convinced it is related, so I am ready to try anything.

Will provide a PR maybe tomorrow, but I am in the middle of moving to a new place so it is all a bit chaotic right now.

[igrr]

For what it's worth, we did a -Wstrict-prototypes cleanup in the ESP32 SDK (after v4.0-dev). As a result, we did find a place or two where the function declared with an empty argument list was called with a non-zero amount of arguments. However nothing major and certainly no effect on the functionality.

[TD-er]

@igrr Was any of those few occurrences related to WiFi/network code?

[devyte]

@TD-er @Jason2866 what @earlephilhower and @igrr explained is correct. Code stability doesn't depend on changing something like a function prototype, or in general adding/removing code. Trying to analyze or trying to find a workaround from that angle is a waste of time.
What happens is that the code has some instability for whatever reason (illegal mem access, dangling ptr, garbage, etc), and it causes some specific weird behavior. Then if you change something in the build, the observed behavior will change, sometimes radically.
Any investigation should be focused on checking typical sources of instability: missing null terms in char strings, off-by-1 or similar, buffer overflows, ptr use/write after free or uninitialized, unsafe critical sections, etc.
If you would like to discuss this further, please let's do so elsewhere like gitter, just @ me.

About the specific issue reported here: I think we currently only have umm and eboot as C code, and nothing else, so my inclination is to just close. However, I'll leave it open for now to track double checking the warnings you reported in case they warrant a clean up.

[TD-er]

Any investigation should be focused on checking typical sources of instability: missing null terms in char strings, off-by-1 or similar, buffer overflows, ptr use/write after free or uninitialized, unsafe critical sections, etc.

Well those were the ones I did check for in my own code and are indeed the most obvious ones for these kind of build issues.
But like I said, it was suggested as a fix, and to be honest I don't have much experience from the C department. But it sounds like something that can cause some hard to track build issues and the changes needed for getting rid of these warnings are really trivial.
On the other hand, I expect code with such build issues would crash and not be really unstable with WiFi.

[earlephilhower]

I added -Wstrict-prototypes to platform.txt and built a bunch of examples and there were no warnings emitted, at all, even for ones that built files listed in your log:

$ git diff
diff --git a/platform.txt b/platform.txt
index 678be1ba..aedb2315 100644
--- a/platform.txt
+++ b/platform.txt
@@ -53,7 +53,7 @@ compiler.libc.path={runtime.platform.path}/tools/sdk/libc/xtensa-lx106-elf
 compiler.cpreprocessor.flags=-D__ets__ -DICACHE_FLASH -U__STRICT_ANSI__ "-I{compiler.sdk.path}/include" "-I{compiler.sdk.path}/{build.lwip_include}" "-I{compiler.libc.path}/include" "-I{build.path}/core"
 
 compiler.c.cmd=xtensa-lx106-elf-gcc
-compiler.c.flags=-c {compiler.warning_flags} -Os -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 {build.exception_flags} {build.sslflags}
+compiler.c.flags=-c {compiler.warning_flags} -Os -g -Wpointer-arith -Wno-implicit-function-declaration -Wstrict-prototypes -Wl,-EL -fno-inline-functions -nostdlib -mlongcalls -mtext-section-literals -falign-functions=4 -MMD -std=gnu99 -ffunction-sections -fdata-sections {build.exception_flags} {build.sslflags}
 
 compiler.S.cmd=xtensa-lx106-elf-gcc
 compiler.S.flags=-c -g -x assembler-with-cpp -MMD -mlongcalls

If you look at your dump, you'll see the files are cpp extension, but are still being built with -Wstrict-prototypes. Which should give an error (g++ already has this enabled, always, for C++). But it wasn't in your logs, so I looked around and found this:

https://stackoverflow.com/questions/8106258/cc1plus-warning-command-line-option-wstrict-prototypes-is-valid-for-ada-c-o

Looks like PIO is compiling C++ files with gcc and not g++ (allowing it to use -Wstrict-proto). Sorry, looks like a PIO issue as Arduino never does that so we can't even reproduce such a thing. You should look into the PIO build process...I'm not sure why it's doing things the way it is, but it seems strange.

Closing as nothing to do on the Arduino 8266 side, and we've had chime-ins from us and igrr that the lack of void as the empty param is not an issue.

[TD-er]

Looks like PIO is compiling C++ files with gcc and not g++ (allowing it to use -Wstrict-proto). Sorry, looks like a PIO issue as Arduino never does that so we can't even reproduce such a thing. You should look into the PIO build process...I'm not sure why it's doing things the way it is, but it seems strange.

So just to get this right, PlatformIO is using a different compiler compared to ArduinoIDE?

[earlephilhower]

That's what it looks like, but I can't say for sure since I could never get it working right for me. I suggest you do a clean build from scratch with dumped command lines to see exactly what it's calling.

[TD-er]

But what if it is indeed compiling using gcc, could these warnings be valid?

[earlephilhower]

No. In C++ when a function proto has fn() it is === fn(void). These few spots are not the cause of your instability, even if PIO is using a different compile method.

[devyte]

I think PIO is using the same compiler (at least I fervently hope so), just a different build system and/or different compile flags than our standard Arduino way.