search

cloudflare / quiche

🥧 Savoury implementation of the QUIC transport protocol and HTTP/3
https://docs.quic.tech/quiche/
BSD 2-Clause "Simplified" License
9.45k stars 718 forks source link

FAILED: crypto/fipsmodule/CMakeFiles/fipsmodule.dir/bcm.c.o #589

Closed candrews closed 4 years ago

candrews commented 4 years ago

Building quiche 0.5.0, I'm getting this failure:

FAILED: crypto/fipsmodule/CMakeFiles/fipsmodule.dir/bcm.c.o 
/usr/lib/ccache/bin/x86_64-pc-linux-gnu-gcc -DBORINGSSL_DISPATCH_TEST -DBORINGSSL_HAVE_LIBUNWIND -DBORINGSSL_IMPLEMENTATION -DOPENSSL_NO_ASM -I/var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/third_party/googletest/include -I/var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/../include -I/var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/../../include  -DNDEBUG -O2 -march=native -pipe -fuse-linker-plugin -flto -Wl,-flto -ftree-vectorize -falign-functions=32 -fgraphite-identity -floop-nest-optimize -fno-common -fno-lto -fno-use-linker-plugin -fPIC -Werror -Wformat=2 -Wsign-compare -Wmissing-field-initializers -Wwrite-strings -Wvla -ggdb -Wall -fvisibility=hidden -fno-common -Wno-free-nonheap-object -Wimplicit-fallthrough -Wmissing-prototypes -Wold-style-definition -Wstrict-prototypes -Wshadow -std=c11 -D_XOPEN_SOURCE=700 -MD -MT crypto/fipsmodule/CMakeFiles/fipsmodule.dir/bcm.c.o -MF crypto/fipsmodule/CMakeFiles/fipsmodule.dir/bcm.c.o.d -o crypto/fipsmodule/CMakeFiles/fipsmodule.dir/bcm.c.o -c /var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/bcm.c
In file included from /var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/bcm.c:31:
/var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/ec/simple.c: In function ‘ec_affine_jacobian_equal’:
/var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/../internal.h:363:38: error: ‘({anonymous})’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  363 |   return constant_time_msb_w(~a & (a - 1));
      |                                   ~~~^~~~
In file included from /var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/bcm.c:76:
/var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/ec/simple.c:287:5: note: ‘({anonymous})’ was declared here
  287 | int ec_affine_jacobian_equal(const EC_GROUP *group, const EC_AFFINE *a,
      |     ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.
ghedo commented 4 years ago

Which C compiler and version are you using?

candrews commented 4 years ago

Which C compiler and version are you using?

$ gcc --version
gcc (Gentoo 10.1.0-r2 p3) 10.1.0
ghedo commented 4 years ago

That's weird... I can't reproduce with the GCC 10 provided by Debian, even after enabling -Wmaybe-uninitialized manually.

 % gcc --version
gcc (Debian 10.1.0-4) 10.1.0

not sure if there are any differences there.

candrews commented 4 years ago

I have CFLAGS="-fno-common" (the default in GCC 10) and the failure occurs.

If I use CFLAGS="-fcommon" the build succeed. I'm guessing Debian is overriding the GCC default configuration.

So this is a bug and it should be fixed as it is manifested when using the default GCC configuration. See https://wiki.gentoo.org/wiki/Gcc_10_porting_notes/fno_common too

ghedo commented 4 years ago

Hmmm, I still can't reproduce :/

Can you please check whether you see the failure when building BoringSSL on its own? Under deps/boringssl/ you can run something like cmake . && make -j$(nproc).

And if you can reproduce it, can you also check with the latest BoringSSL version from https://github.com/google/boringssl/ ?

candrews commented 4 years ago

I think this might be a compiler bug that happens with certain optimizations... The simplest CFLAGS I've come up with that reproduce this issue are -O2 -floop-nest-optimize

Here's the error output in that case:

FAILED: crypto/fipsmodule/CMakeFiles/fipsmodule.dir/bcm.c.o 
/usr/lib/ccache/bin/x86_64-pc-linux-gnu-gcc -DBORINGSSL_DISPATCH_TEST -DBORINGSSL_HAVE_LIBUNWIND -DBORINGSSL_IMPLEMENTATION -DOPENSSL_NO_ASM -I/var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/third_party/googletest/include -I/var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/../include -I/var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/../../include  -DNDEBUG -O2 -floop-nest-optimize -fPIC -Werror -Wformat=2 -Wsign-compare -Wmissing-field-initializers -Wwrite-strings -Wvla -ggdb -Wall -fvisibility=hidden -fno-common -Wno-free-nonheap-object -Wimplicit-fallthrough -Wmissing-prototypes -Wold-style-definition -Wstrict-prototypes -Wshadow -std=c11 -D_XOPEN_SOURCE=700 -MD -MT crypto/fipsmodule/CMakeFiles/fipsmodule.dir/bcm.c.o -MF crypto/fipsmodule/CMakeFiles/fipsmodule.dir/bcm.c.o.d -o crypto/fipsmodule/CMakeFiles/fipsmodule.dir/bcm.c.o -c /var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/bcm.c
In file included from /var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/bcm.c:31:
/var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/ec/simple.c: In function ‘ec_affine_jacobian_equal’:
/var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/../internal.h:363:38: error: ‘({anonymous})’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  363 |   return constant_time_msb_w(~a & (a - 1));
      |                                   ~~~^~~~
In file included from /var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/bcm.c:76:
/var/tmp/portage/net-libs/quiche-0.5.0/work/quiche-0.5.0/deps/boringssl/crypto/fipsmodule/ec/simple.c:287:5: note: ‘({anonymous})’ was declared here
  287 | int ec_affine_jacobian_equal(const EC_GROUP *group, const EC_AFFINE *a,
      |     ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.

I'm eager to learn if this information allows you to reproduce the issue on Debian.

If so, perhaps this issue could be reported to the boringssl project and/or the gcc project?

ghedo commented 4 years ago

It looks like there are quite a few tickets for GCC related to maybe-uninitialized warnings, not sure if any of those are related though: https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=SUSPENDED&bug_status=WAITING&bug_status=REOPENED&field0-0-0=product&field0-0-1=component&field0-0-2=alias&field0-0-3=short_desc&field0-0-4=status_whiteboard&field0-0-5=content&no_redirect=1&order=bug_id%20DESC&query_format=advanced&type0-0-0=substring&type0-0-1=substring&type0-0-2=substring&type0-0-3=substring&type0-0-4=substring&type0-0-5=matches&value0-0-0=maybe-uninitialized&value0-0-1=maybe-uninitialized&value0-0-2=maybe-uninitialized&value0-0-3=maybe-uninitialized&value0-0-4=maybe-uninitialized&value0-0-5=%22maybe-uninitialized%22

I think for now it might be a good idea to simply disable the warning when building BoringSSL.

ghedo commented 4 years ago

But of course adding -Wno-maybe-uninitialized breaks building with Clang, and there doesn't seem to be any way to detect the compiler when using cmake.rs, so I made https://boringssl-review.googlesource.com/c/boringssl/+/42184 to add a build option to BoringSSL so we can disable -Werror completely.

ghedo commented 4 years ago

Apparently the master-with-bazel branch already does that... @candrews can you try to build from the branch from https://github.com/cloudflare/quiche/pull/593 and see if it fixes the problem for you?