-Werror by default can break downstream Bazel builds

[coryan]

What version of protobuf and what language are you using?

Version: main and v24.x
Language: C++

What operating system (Linux, Windows, ...) and version?

Linux, but probably all of them.

What runtime / compiler are you using (e.g., python version or gcc version)

In this case GCC 13.2:

gcc (Debian 13.2.0-4) 13.2.0
Copyright (C) 2023 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

What did you do?

1. I was compiling Protobuf with GCC 13 and Address Sanitizer, as part of a different project (googleapis/google-cloud-cpp)
2. This project uses slightly different options in its asan configuration, and the build breaks.
3. To reproduce, you can apply this patch to the Protobuf source:

diff --git a/.bazelrc b/.bazelrc
index fb29fe10d..39e7120e0 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -4,7 +4,7 @@ build:dbg --compilation_mode=dbg
 
 build:opt --compilation_mode=opt
 
-build:san-common --config=dbg --strip=never --copt=-O0 --copt=-fno-omit-frame-pointer
+build:san-common --config=dbg --strip=never --copt=-Og --copt=-fno-omit-frame-pointer
 
 build:asan --config=san-common --copt=-fsanitize=address --linkopt=-fsanitize=address
 build:asan --copt=-DADDRESS_SANITIZER=1

4. And then compile using

bazel build --config asan //:protobuf

What did you expect to see

A successful build.

What did you see instead?

The build fails with:

ERROR: /usr/local/google/home/coryan/protobuf/src/google/protobuf/io/BUILD.bazel:11:11: Compiling src/google/protobuf/io/coded_stream.cc failed: (Exit 1): gcc failed: error executing command (from target //src/google/protobuf/io:io) /usr/bin/gcc -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -g '-std=c++14' -MD -MF ... (remaining 40 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
src/google/protobuf/io/coded_stream.cc: In member function 'int64_t google::protobuf::io::CodedInputStream::ReadVarint32Fallback(uint32_t)':
src/google/protobuf/io/coded_stream.cc:525:12: error: 'temp' may be used uninitialized [-Werror=maybe-uninitialized]
  525 |     return temp;
      |            ^~~~
src/google/protobuf/io/coded_stream.cc:520:14: note: 'temp' was declared here
  520 |     uint32_t temp;
      |              ^~~~
cc1plus: all warnings being treated as errors

Anything else we should know about your project / environment

You know where to find me if needed.

An opinion

Someday I will write "-Werror by default considered harmful". It is a good idea to turn this flag on development builds, I thank you for doing so, but it is not a good idea to turn it on for downstream builds too. The downstream builds may have slightly different compilers, or compiler flags, and the build would break.

https://github.com/protocolbuffers/protobuf/blob/24.x/build_defs/cpp_opts.bzl#L24

[traversaro]

An opinion

Someday I will write "-Werror by default considered harmful". It is a good idea to turn this flag on development builds, I thank you for doing so, but it is not a good idea to turn it on for downstream builds too. The downstream builds may have slightly different compilers, or compiler flags, and the build would break.

I think this point is explained with a lot of detail in the "Don't create spurious errors" section of Robert Schumacher's Talk from CppCon2019 “Don't Package Your Libraries, Write Packagable Libraries! (Part 2)”: https://youtu.be/_5weX5mx8hc?si=boacaiOG-pw_yG23&t=322 .

[github-actions]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago.

[coryan]

I think this was fixed in #14752 ? @mkruskal-google thoughts?

[vvviktor]

@coryan No, -Werror flag still exists in build_defs/cpp_opts.bzl

[hgminh95]

I got the same issue too. Can confirm that merging #14752 should fix it.

[github-actions]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please add a comment.

This issue is labeled inactive because the last activity was over 90 days ago. This issue will be closed and archived after 14 additional days without activity.