opus icon indicating copy to clipboard operation
opus copied to clipboard

Uninitialized warning for `quant_LTP_gains.c`: `res_nrg_Q15`

Open constant-flow opened this issue 4 years ago • 4 comments

When compiling opus for ESP32 this file has some maybe uninitialized portion throwing warnings, for me treated as errors:

...
-MF esp-idf/opus/CMakeFiles/__idf_opus.dir/opus/silk/quant_LTP_gains.c.obj.d -o esp-idf/opus/CMakeFiles/__idf_opus.dir/opus/silk/quant_LTP_gains.c.obj   -c ../components/opus/opus/silk/quant_LTP_gains.c
../components/opus/opus/silk/quant_LTP_gains.c: In function 'silk_quant_LTP_gains':
../components/opus/opus/silk/quant_LTP_gains.c:127:21: error: 'res_nrg_Q15' may be used uninitialized in this function [-Werror=maybe-uninitialized]
         res_nrg_Q15 = silk_RSHIFT32( res_nrg_Q15, 2 );
cc1: some warnings being treated as errors
...

Setting it to zero worked as a workaround for me.

Wish: Please make sure this is set to the correct initial value. Cheers for your work 👍

constant-flow avatar Oct 18 '20 09:10 constant-flow

I don't see it, can you give example how it may be used uninitialized?

xnorpx avatar Oct 19 '20 00:10 xnorpx

I didn't dive deeper into the implementation of opus, but after a short look, it looks to me like the compiler is super anxious 😄 . To me it doesn't look like it's not initialised, when it reaches the mentioned section.

To reproduce (req. snapclient, esp-idf, esp-adf) check out this project and follow my comment.

As mentioned in the comment, setting it initially to 0 did the deal for me, but dunno why the compiler asks an init. 🤷

Cheers, Conni

constant-flow avatar Oct 19 '20 06:10 constant-flow

There is several of these instances that other compilers also could not identify them correctly. (MSVC)

What it boils down to is if the owners think it's a valuable warning to remove false positives as well and add it as official warning to Opus "-Wmaybe-uninitialized"

https://github.com/xiph/opus/blob/034c1b61a250457649d788bbf983b3f0fb63f02e/configure.ac#L882

@mark4o @rillian

xnorpx avatar Oct 19 '20 15:10 xnorpx

@constant-flow:

You may find a fix for snapclient in this commit https://github.com/jorgenkraghjakobsen/snapclient/pull/45/commits/1a0a36dba9aa9b442d27e9406932e4d09f2f0126 .

The solution I chose was adding -Os to the CFLAGS, since this makes GCC take a closer look at data- and control-flow paths, which makes it notice that the variable is indeed never used uninitialized.

The other option is to add -Wno-maybe-uninitialized to avoid the warning which would then become an error due to -Werror.

While it is nice to avoid false compiler warnings, this may be more the domain of build tools and how the compiler is invoked, than the actual source. Maybe this can be closed.

nerilex avatar Jan 29 '23 22:01 nerilex