Compile error with Compiler options - Optimization Level (Release (-Os))

[gigijoe]

Hello

I found that compile fail with Compiler options - Optimization Level (Release (-Os))

AR build/main/libmain.a
CC build/nghttp/nghttp2/lib/nghttp2_session.o
/home/gigijoe/ESP32/esp-idf/components/nghttp/nghttp2/lib/nghttp2_session.c: In function 'nghttp2_session_get_remote_settings':
/home/gigijoe/ESP32/esp-idf/components/nghttp/nghttp2/lib/nghttp2_session.c:7132:1: error: control reaches end of non-void function [-Werror=return-type]
}
^
/home/gigijoe/ESP32/esp-idf/components/nghttp/nghttp2/lib/nghttp2_session.c: In function 'nghttp2_session_get_local_settings':
/home/gigijoe/ESP32/esp-idf/components/nghttp/nghttp2/lib/nghttp2_session.c:7152:1: error: control reaches end of non-void function [-Werror=return-type]
}
^
cc1: some warnings being treated as errors


The errors are easy to fix but ever time I updte submodule of SDK the errors come back. Can anyone help to fix it on github

diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c
index 1c060f1..bf00706 100644
--- a/lib/nghttp2_session.c
+++ b/lib/nghttp2_session.c
@@ -7129,6 +7129,7 @@ uint32_t nghttp2_session_get_remote_settings(nghttp2_session *session,
}

assert(0);
+return 0;
}

uint32_t nghttp2_session_get_local_settings(nghttp2_session *session,
@@ -7149,6 +7150,7 @@ uint32_t nghttp2_session_get_local_settings(nghttp2_session *session,
}

assert(0);
+return 0;
}

static int nghttp2_session_upgrade_internal(nghttp2_session *session,

[ESP_Sprite]

I seem to remember that we ran into and solved these issues a while ago... are you sure you're on the latest master? If not, can you do a git pull and a git submodule update and see if you still get this?

[gigijoe]

I double confirm that the problem exist for me
Please correct me if I did anything wrong

gigijoe@gigijoe-i5-2500k:~/ESP32/esp-idf$ git pull
Already up-to-date.
gigijoe@gigijoe-i5-2500k:~/ESP32/esp-idf$ git submodule update --init
Submodule 'components/aws_iot/aws-iot-device-sdk-embedded-C' () registered for path 'components/aws_iot/aws-iot-device-sdk-embedded-C'
Submodule 'components/bt/lib' () registered for path 'components/bt/lib'
Submodule 'components/coap/libcoap' () registered for path 'components/coap/libcoap'
Submodule 'components/esp32/lib' () registered for path 'components/esp32/lib'
Submodule 'components/esptool_py/esptool' () registered for path 'components/esptool_py/esptool'
Submodule 'components/libsodium/libsodium' () registered for path 'components/libsodium/libsodium'
Submodule 'components/micro-ecc/micro-ecc' () registered for path 'components/micro-ecc/micro-ecc'
Submodule 'components/nghttp/nghttp2' () registered for path 'components/nghttp/nghttp2'
gigijoe@gigijoe-i5-2500k:~/ESP32/esp-idf$ cd ../SGM9200

gigijoe@gigijoe-i5-2500k:~/ESP32/SGM9200$ make clean
RM component_project_vars.mk
RM libbootloader_support.a src/bootloader_flash.o src/bootloader_random.o src/bootloader_sha.o src/efuse.o src/esp_image_format.o src/flash_encrypt.o src/flash_partitions.o src/secure_boot.o src/secure_boot_signatures.o src/bootloader_flash.d src/bootloader_random.d src/bootloader_sha.d src/efuse.d src/esp_image_format.d src/flash_encrypt.d src/flash_partitions.d src/secure_boot.d src/secure_boot_signatures.d component_project_vars.mk
...
...

gigijoe@gigijoe-i5-2500k:~/ESP32/SGM9200$ make
make[1]: Entering directory `/home/gigijoe/ESP32/esp-idf/tools/kconfig'
cc -DCURSES_LOC="<ncurses.h>" -DLOCALE -c -o mconf.o mconf.c
flex -L -P zconf -o zconf.lex.c zconf.l
zconf.l:255: warning, -s option given but default rule can be matched
bison -t -l -p zconf -o zconf.tab.c zconf.y
sed -E "s/\\x0D$//" zconf.gperf | gperf -t --output-file zconf.hash.c -a -C -E -g -k '1,3,$' -p -t
cc -DCURSES_LOC="<ncurses.h>" -DLOCALE -c -o zconf.tab.o zconf.tab.c
lxdialog/check-lxdialog.sh -check cc -DCURSES_LOC="<ncurses.h>" -DLOCALE -lncurses -ltinfo
cc -DCURSES_LOC="<ncurses.h>" -DLOCALE -c -o lxdialog/checklist.o lxdialog/checklist.c
...
...

CC build/nghttp/nghttp2/lib/nghttp2_queue.o
CC build/nghttp/nghttp2/lib/nghttp2_rcbuf.o
CC build/nghttp/nghttp2/lib/nghttp2_session.o
/home/gigijoe/ESP32/esp-idf/components/nghttp/nghttp2/lib/nghttp2_session.c: In function 'nghttp2_session_get_remote_settings':
/home/gigijoe/ESP32/esp-idf/components/nghttp/nghttp2/lib/nghttp2_session.c:7132:1: error: control reaches end of non-void function [-Werror=return-type]
}
^
/home/gigijoe/ESP32/esp-idf/components/nghttp/nghttp2/lib/nghttp2_session.c: In function 'nghttp2_session_get_local_settings':
/home/gigijoe/ESP32/esp-idf/components/nghttp/nghttp2/lib/nghttp2_session.c:7152:1: error: control reaches end of non-void function [-Werror=return-type]
}
^
cc1: some warnings being treated as errors
make[1]: *** [nghttp2/lib/nghttp2_session.o] Error 1
make: *** [component-nghttp-build] Error 2


Thank you

[Lucas.Hutchinson]

Hi all,

We have run into this issue before.

It seems to be related to (kind of recent) setting

Code: Select all

Copmpiler options --> Assertion level

When this value is set to Disabled (an hence disables the asserts at the bottom of the offending functions), those two functions in the nghttp lib warn that there is a case which they wont return 0, which because of the convert all warnings to errors compile option, results in a compilation error.

Temporarily to get around this we have enabled assertions. However it would be good if the project will still compile when assertions are disabled.

[ESP_Angus]

Thanks for reporting this folks. I actually submitted a PR about this to the nghttp2 project and they fixed it in nghttp2 v1.24. We had that version in IDF already but it looks like an unrelated commit accidentally rolled it back to v1.22 back in June. We will get this fixed ASAP.

(BTW, where possible I strongly recommend enabling silent assertions rather than disabling assertions where there is any possibility of debugging. Assertions catch a wide variety of errors that can be extremely hard to debug otherwise.)

[gigijoe]

Angus

Thank you

[Lucas.Hutchinson]

Hi Angus,

Thanks for the reply.
I look forward to seeing this change making its way back into the github master.

Thanks for the suggestion re: assersions.
So to confirm, you suggest:

• For debugging: Enable all assertions
• For testing (some debugging): enable silent assertions
• For release: disable assersions (also add -DNDEBUG)

[ESP_Angus]

• For debugging: Enable all assertions
• For testing (some debugging): enable silent assertions
• For release: disable assersions (also add -DNDEBUG)

More or less, yes.

Regarding "release", I would suggest that keeping "silent assertions" in this case is also worth considering. Usually if an assertion fails things are fairly broken internally, and continuing may have worse side effects than just resetting and starting again (device gets "stuck" in some state, corrupt data gets sent or received over the network, etc.).

This also gives you the benefit that you can test with 100% identical binary code to the code you release. Which is a nice reassurance.

The upside to disabling assertions entirely is you gain some small amount of performance, and the code size goes down further compared to "silent" assertions ("silent" saves code size from not needing to compile in all the separate assertion error messages, but not having to test assertions at all saves the most code size).

You'll have to decide what trade-off is most suitable for your particular case.