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

Calling a value returning function that doesn't actually return anything crashes the program #236

Closed
RandomHacks-Git opened this issue Jul 4, 2021 · 3 comments · Fixed by #237

Comments

@RandomHacks-Git
Copy link

I just spent a few hours trying to figure out what was wrong with my code as it was behaving very strangely (a for loop wouldn't increment) altough it was working just fine with the oficial core as well as with other microcontrollers.

I found I had defined a value returning function with several ifs and returns but the part the code was following didn't actually have a return in the end and therefore the pico was crashing. Here is a very simple example that replicates the issue in case my explanation isn't clear enough. Notice how printTest( ) doesn't return anything but is defined as an int returning function. I can see how a lot of people could get stuck with this issue as it isn't really easy to debug if the code is large.

int i;

void setup() {
  Serial.begin(115200);
}

void loop() {
  printTest();
  delay(500);
}

int printTest(){
  Serial.println(i);
  i++;
}
@earlephilhower
Copy link
Owner

Good catch. That is a programming error and according to the C spec is "undefined behavior" so crashing is acceptable there.

I was going to port in the following patch from the ESP8266, but completely forgot. Thanks for reminding me!
esp8266/Arduino#8165

@RandomHacks-Git
Copy link
Author

Yeah it definitely isn't a mistake I usually do but this completely escaped me for a few hours until I finally realised what was going on😅
A patch would be awesome.

earlephilhower added a commit that referenced this issue Jul 4, 2021
Fixes #236

Compiler warning flags were completely ignored/missed in platform.txt.
Add them, as normal, and include -Werror=return-type because GCC will
produce crashing apps when a function return value is missing.
@earlephilhower
Copy link
Owner

Not only did I need to add the patch, it turns out I had forgot to add any of the warning options to the actual compile options. D'oh!

No matter what the warning options, your sample will fail to build with the following report using the PR:

Compiling sketch...
/home/earle/Arduino/hardware/pico/rp2040/system/arm-none-eabi/bin/arm-none-eabi-g++ -c -Werror=return-type -I/home/earle/Arduino/hardware/pico/rp2040/tools/libpico -DCFG_TUSB_MCU=OPT_MCU_RP2040 -DUSB_VID=0x2e8a -DUSB_PID=0x000a "-DUSB_MANUFACTURER=\"Raspberry Pi\"" "-DUSB_PRODUCT=\"Pico\"" -Os -march=armv6-m -mcpu=cortex-m0plus -mthumb -ffunction-sections -fdata-sections -fno-exceptions -iprefix/home/earle/Arduino/hardware/pico/rp2040/ @/home/earle/Arduino/hardware/pico/rp2040/lib/platform_inc.txt -fno-rtti -std=gnu++17 -g -DSERIALUSB_PID=0x000a -DF_CPU=125000000L -DARDUINO=10815 -DARDUINO_RASPBERRY_PI_PICO "-DBOARD_NAME=\"RASPBERRY_PI_PICO\"" -DARDUINO_ARCH_RP2040 -I/home/earle/Arduino/hardware/pico/rp2040/cores/rp2040 -I/home/earle/Arduino/hardware/pico/rp2040/variants/rpipico /tmp/arduino_build_863393/sketch/sketch_jul04a.ino.cpp -o /tmp/arduino_build_863393/sketch/sketch_jul04a.ino.cpp.o
/home/earle/Arduino/sketch_jul04a/sketch_jul04a.ino: In function 'int printTest()':
sketch_jul04a:15:1: error: no return statement in function returning non-void [-Werror=return-type]
   15 | }
      | ^
cc1plus: some warnings being treated as errors
exit status 1

earlephilhower added a commit that referenced this issue Jul 4, 2021
Fixes #236

Compiler warning flags were completely ignored/missed in platform.txt.
Add them, as normal, and include -Werror=return-type because GCC will
produce crashing apps when a function return value is missing.
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 a pull request may close this issue.

2 participants