protocolbuffers / protocolbuffers/protobuf

Protobuf's constant initialization is not actually constant on Windows

Open
#10,159 20 comments 5 reactions 2 assignees View on GitHub

@sbenzaquen is already working on this.

Since Sep 1, 2022.

c++ help wanted wait for user action windows
Dominant language
C++
Stars
72k
Forks
16.3k
Avg merge
1d 17h
Merged PRs (30d)
140

Description

What version of protobuf and what language are you using?
Version: main
Language: C++

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

What runtime / compiler are you using (e.g., python version or gcc version)
clang-cl from Visual Studio 2019

What did you do?
Steps to reproduce the behavior:

  1. Install Visual Studio 2019
  2. In the installer, include "C++ Clang tools for Windows"
  3. git clone https://github.com/protocolbuffers/protobuf.git
  4. cd protobuf
  5. git submodule update --init --recursive
  6. cmake -B build-shared -D BUILD_SHARED_LIBS=1 -T ClangCL
  7. cmake --build build-shared --parallel 16

What did you expect to see
The build should succeed

What did you see instead?
The build fails with errors like:

...\protobuf\src\google\protobuf\compiler\plugin.pb.cc(75,31): message : required by 'require_constant_initializatio
n' attribute here [...\protobuf\build-shared\libprotoc.vcxproj]
...\protobuf\src\google/protobuf/port_def.inc(656,30): message : expanded from macro 'PROTOBUF_CONSTINIT' [...\pr
otobuf\build-shared\libprotoc.vcxproj]
...\protobuf\src\google\protobuf\compiler\plugin.pb.cc(91,125): error : variable does not have a constant initialize
r [...\protobuf\build-shared\libprotoc.vcxproj]

Anything else we should know about your project / environment
This was an issue updating protobuf in Chromium. We've applied a workaround by patching out the PROTOBUF_CONSTINIT in some configurations, but the root problem is that protobuf's new constant initialization was not actually cross-platform, and relies on assumption that aren't actually universally true.

While, for the repro steps, I used clang-cl, this is only because protobuf has a pre-C++20 version of PROTOBUF_CONSTINIT for Clang, [[clang::require_constant_initialization]]. When protobuf moves to C++20, I expect it will have the same problem on MSVC.

This happens because the protobuf constant initialization assumes it can reference &fixed_address_empty_string. fixed_address_empty_string lives in the protobuf dll, so when referenced from another dll or exe, is __declspec(dllimport). But pointers to dllimport variables require a static initializer on Windows. See https://godbolt.org/z/5rn4xWrhE. As I understand it, Windows does not have a suitable relocation to express this.

As a result, the new constant initialization actually requires a per-message static initializer, which trips PROTOBUF_CONSTINIT and fails to build.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.