prysmaticlabs / hashtree

SHA256 library highly optimized for Merkle tree computations
MIT License
27 stars 7 forks source link

build warning/error: hashtree.c:45:11: error: 'b' may be used uninitialized #20

Closed olegrok closed 9 months ago

olegrok commented 9 months ago
hashtree-x86_64-unknown-linux-musl> hashtree.c: In function 'hashtree_detect':
hashtree-x86_64-unknown-linux-musl> hashtree.c:45:11: error: 'b' may be used uninitialized [8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wmaybe-uninitialized-Werror=maybe-uninitialized8;;]
hashtree-x86_64-unknown-linux-musl>    45 |     if (b & bit_SHA) {
hashtree-x86_64-unknown-linux-musl>       |           ^
hashtree-x86_64-unknown-linux-musl> hashtree.c:42:17: note: 'b' was declared here
hashtree-x86_64-unknown-linux-musl>    42 |     uint32_t a, b, c, d;
hashtree-x86_64-unknown-linux-musl>       |                 ^
hashtree-x86_64-unknown-linux-musl> hashtree.c:58:11: error: 'c' may be used uninitialized [8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wmaybe-uninitialized-Werror=maybe-uninitialized8;;]
hashtree-x86_64-unknown-linux-musl>    58 |     if (c & bit_AVX) {
hashtree-x86_64-unknown-linux-musl>       |           ^
hashtree-x86_64-unknown-linux-musl> hashtree.c:42:20: note: 'c' was declared here
hashtree-x86_64-unknown-linux-musl>    42 |     uint32_t a, b, c, d;
hashtree-x86_64-unknown-linux-musl>       |                    ^
hashtree-x86_64-unknown-linux-musl> cc1: all warnings being treated as errors
hashtree-x86_64-unknown-linux-musl> make[1]: *** [<builtin>: hashtree.o] Error 1
hashtree-x86_64-unknown-linux-musl> make[1]: Leaving directory '/build/source/src'
hashtree-x86_64-unknown-linux-musl> make: *** [Makefile:2: libhashtree] Error 2
error: builder for '/nix/store/f9d8y8arcxa3dhds15j3s9ddxdn6c6fr-hashtree-x86_64-unknown-linux-musl.drv' failed with exit code 2;
       last 10 log lines:
       > hashtree.c:58:11: error: 'c' may be used uninitialized [8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wmaybe-uninitialized-Werror=maybe-uninitialized8;;]
       >    58 |     if (c & bit_AVX) {
       >       |           ^
       > hashtree.c:42:20: note: 'c' was declared here
       >    42 |     uint32_t a, b, c, d;
       >       |                    ^
       > cc1: all warnings being treated as errors
       > make[1]: *** [<builtin>: hashtree.o] Error 1
       > make[1]: Leaving directory '/build/source/src'
       > make: *** [Makefile:2: libhashtree] Error 2
potuz commented 9 months ago

What's the compiler here and the system? this is a non-issue AFAICS

olegrok commented 9 months ago

What's the compiler here and the system? this is a non-issue AFAICS

GCC 13.2.0. It's easy to suppress this warning but it's better to fix it to avoid missing something useful.

potuz commented 9 months ago

Well I don't know what is there to fix, I can make the compiler shut up, but __get_cpuid_count is guaranteed to have initialized these values. At any rate, here is GCC

$ gcc --version
gcc (GCC) 13.2.1 20230801
...
$ make
make -C src
make[1]: Entering directory '/home/heluani/Documents/code/ethereum/hashtree/src'
cc -g -fpic   -c -o sha256_shani.o sha256_shani.S
cc -g -fpic   -c -o sha256_avx_x16.o sha256_avx_x16.S
cc -g -fpic   -c -o sha256_avx_x8.o sha256_avx_x8.S
cc -g -fpic   -c -o sha256_avx_x4.o sha256_avx_x4.S
cc -g -fpic   -c -o sha256_avx_x1.o sha256_avx_x1.S
cc -g -fpic   -c -o sha256_sse_x1.o sha256_sse_x1.S
cc -g -Wall -Werror   -c -o hashtree.o hashtree.c
ar rcs libhashtree.a sha256_shani.o sha256_avx_x16.o sha256_avx_x8.o sha256_avx_x4.o sha256_avx_x1.o sha256_sse_x1.o hashtree.o
make[1]: Leaving directory '/path/hashtree/src'
olegrok commented 9 months ago

I use musl (not glibc). Maybe it matters. To suppress you can initialize a, b, c, d with zeroes. It won't change anything but just makes compiler happy.

potuz commented 9 months ago

I understand how to make the compiler shut-up, just being picky about adding inconsequential lines :smile:

Surely the problem is musl's handling of the built-in function that doesn't have the glibc annotation that it is innitializing those variables. I'll add the initialization to shut musl libc down.

olegrok commented 9 months ago

@potuz Thanks for your patch! Module code is built successfully but tests still fail. I prepared a patch to fix that https://github.com/prysmaticlabs/hashtree/pull/22