search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.74k stars 6.56k forks source link

twister missing build warning in cpp files #78348

Closed fabiobaltieri closed 1 month ago

fabiobaltieri commented 1 month ago

Describe the bug

Hey folks, bumped into some cpp tests that are generating warnings, specifically

$ west build -p -b native_sim -T tests/subsys/rtio/rtio_i2c/rtio.i2c
...
zephyrproject/zephyr/include/zephyr/drivers/emul.h:117:49: warning: unnecessary parentheses in declaration of ‘__emulreg_DT_N_S_i2c_100_S_blocking_emul_80_bus’ [-Wparentheses]
...
$ west build -p -b native_sim -T tests/drivers/i2c/i2c_emul/drivers.i2c.emul.target_buf
...
/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/tests/drivers/i2c/i2c_emul/src/test_fowarding_common.cpp:75:40: warning: ‘data’ may be used uninitialized [-Wmaybe-uninitialized]

but the same do not seem to be caught by twister

$ ./scripts/twister -v --testsuite-root tests/drivers/i2c/i2c_emul
...
INFO    - 1/2 native_sim                tests/drivers/i2c/i2c_emul/drivers.i2c.emul.target_buf PASSED (native 0.020s)
INFO    - 2/2 native_sim                tests/drivers/i2c/i2c_emul/drivers.i2c.emul.target_pio PASSED (native 0.020s)
...
$ ./scripts/twister -v --testsuite-root tests/subsys/rtio/rtio_i2c
...
INFO    - 1/1 native_sim                tests/subsys/rtio/rtio_i2c/rtio.i2c                PASSED (native 0.034s)

The error shows up build.log, shouldn't the twister run fail?

To Reproduce The commands above.

Expected behavior Twister is unhappy.

Impact Confusion

Environment (please complete the following information): Linux, Zephyr SDK 0.16.8, cc544803811

fabiobaltieri commented 1 month ago

cc @teburd @yperess

nashif commented 1 month ago

interesting, warning-as-error does not work when building the test code with g++

[29/111] ccache /usr/lib64/ccache/g++ -DKERNEL -DK_HEAP_MEM_POOL_SIZE=0 -D__ZEPHYR__=1 -I/home/nashif/zephyrproject/zephyr/tests/subsys/rtio/rtio_i2c/include -I/home/nashif/zephyrproject/zephyr/build/zephyr/include/generated/zephyr -I/home/nashif/zephyrproject/zephyr/include -I/home/nashif/zephyrproject/zephyr/build/zephyr/include/generated -I/home/nashif/zephyrproject/zephyr/soc/native/inf_clock -I/home/nashif/zephyrproject/zephyr/boards/native/native_sim -I/home/nashif/zephyrproject/zephyr/scripts/native_simulator/common/src/include -I/home/nashif/zephyrproject/zephyr/scripts/native_simulator/native/src/include -I/home/nashif/zephyrproject/zephyr/subsys/rtio -I/home/nashif/zephyrproject/zephyr/subsys/testsuite/include -I/home/nashif/zephyrproject/zephyr/subsys/testsuite/coverage -I/home/nashif/zephyrproject/zephyr/subsys/testsuite/ztest/include -fno-strict-aliasing -Os -fcheck-new -std=c++17 -Wno-register -fno-exceptions -fno-rtti -imacros /home/nashif/zephyrproject/zephyr/build/zephyr/include/generated/zephyr/autoconf.h -fno-common -g -gdwarf-4 -fdiagnostics-color=always -Wall -Wformat -Wformat-security -Wno-format-zero-length -Wdouble-promotion -Wpointer-arith -Wexpansion-to-defined -Wno-unused-but-set-variable -fno-pic -fno-pie -fno-asynchronous-unwind-tables -fno-reorder-functions --param=min-pagesize=0 -fno-defer-pop -fmacro-prefix-map=/home/nashif/zephyrproject/zephyr/tests/subsys/rtio/rtio_i2c=CMAKE_SOURCE_DIR -fmacro-prefix-map=/home/nashif/zephyrproject/zephyr=ZEPHYR_BASE -fmacro-prefix-map=/home/nashif/zephyrproject=WEST_TOPDIR -ffunction-sections -fdata-sections -m32 -msse2 -mfpmath=sse -fvisibility=hidden -fno-freestanding -MD -MT CMakeFiles/app.dir/src/blocking_emul.cpp.obj -MF CMakeFiles/app.dir/src/blocking_emul.cpp.obj.d -o CMakeFiles/app.dir/src/blocking_emul.cpp.obj -c /home/nashif/zephyrproject/zephyr/tests/subsys/rtio/rtio_i2c/src/blocking_emul.cpp

but it does have -Werror elsewhere... Twister seems to be doing the right thing.

@tejlmand any idea?

teburd commented 1 month ago

This is a recent test @yperess added and should look at

yperess commented 1 month ago

Will send a fix tonight

fabiobaltieri commented 1 month ago

This is a recent test @yperess added and should look at

Sure but the point is not the specific warning per se, more like the fact that these are not getting detected by twister, assigning back to Stephanos for C++ area.

yperess commented 1 month ago

This is a recent test @yperess added and should look at

Sure but the point is not the specific warning per se, more like the fact that these are not getting detected by twister, assigning back to Stephanos for C++ area.

Are we sure that twister treats warnings as errors in C? I have a hard time imagining this is actually a C vs C++ issue but much more likely a "twister needs to inject a flag" issue

fabiobaltieri commented 1 month ago

Are we sure that twister treats warnings as errors in C? I have a hard time imagining this is actually a C vs C++ issue but much more likely a "twister needs to inject a flag" issue

Yes, C tests runs with -Werror but it does not seem to be the case for CPP ones, that one compiles with

[89/105] ccache /usr/bin/g++ -DKERNEL -DK_HEAP_MEM_POOL_SIZE=0 -DTC_RUNID=6ccffc5fc3209b4f1f54989656379f7e -D__ZEPHYR__=1 -I/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/tests/drivers/i2c/i2c_emul/include -I/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/twister-out/native_sim/tests/drivers/i2c/i2c_emul/drivers.i2c.emul.target_buf/zephyr/include/generated/zephyr -I/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/include -I/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/twister-out/native_sim/tests/drivers/i2c/i2c_emul/drivers.i2c.emul.target_buf/zephyr/include/generated -I/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/soc/native/inf_clock -I/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/boards/native/native_sim -I/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/scripts/native_simulator/common/src/include -I/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/scripts/native_simulator/native/src/include -I/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/subsys/testsuite/include -I/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/subsys/testsuite/coverage -I/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/subsys/testsuite/ztest/include -fno-strict-aliasing -Os -fcheck-new -std=c++17 -Wno-register -fno-exceptions -fno-rtti -imacros /usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/twister-out/native_sim/tests/drivers/i2c/i2c_emul/drivers.i2c.emul.target_buf/zephyr/include/generated/zephyr/autoconf.h -fno-common -g -gdwarf-4 -fdiagnostics-color=always -Wall -Wformat -Wformat-security -Wno-format-zero-length -Wdouble-promotion -Wpointer-arith -Wexpansion-to-defined -Wno-unused-but-set-variable -fno-pic -fno-pie -fno-asynchronous-unwind-tables -fno-reorder-functions --param=min-pagesize=0 -fno-defer-pop -fmacro-prefix-map=/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/tests/drivers/i2c/i2c_emul=CMAKE_SOURCE_DIR -fmacro-prefix-map=/usr/local/google/home/fabiobaltieri/zephyrproject/zephyr=ZEPHYR_BASE -fmacro-prefix-map=/usr/local/google/home/fabiobaltieri/zephyrproject=WEST_TOPDIR -ffunction-sections -fdata-sections -m32 -msse2 -mfpmath=sse -fvisibility=hidden -fno-freestanding -MD -MT CMakeFiles/app.dir/src/test_fowarding_common.cpp.obj -MF CMakeFiles/app.dir/src/test_fowarding_common.cpp.obj.d -o CMakeFiles/app.dir/src/test_fowarding_common.cpp.obj -c /usr/local/google/home/fabiobaltieri/zephyrproject/zephyr/tests/drivers/i2c/i2c_emul/src/test_fowarding_common.cpp

(guess it's the same that Anas pointed out) there's no -Werror in there so this appears to be a build system problem

fabiobaltieri commented 1 month ago

Think I found the missing bit https://github.com/zephyrproject-rtos/zephyr/pull/78419