Skip to content

Fix Clang -Wconditional-uninitialized warning#59

Merged
fanquake merged 1 commit into
bitcoin-core:bitcoin-forkfrom
hebasto:260203-uninitialized
May 18, 2026
Merged

Fix Clang -Wconditional-uninitialized warning#59
fanquake merged 1 commit into
bitcoin-core:bitcoin-forkfrom
hebasto:260203-uninitialized

Conversation

@hebasto

@hebasto hebasto commented Feb 3, 2026

Copy link
Copy Markdown
Member

Steps to reproduce the warning on Ubuntu 25.10 using Clang 22.0:

  1. Apply a diff to workaround a build issue:
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -12,7 +12,7 @@ set(CMAKE_C_STANDARD_REQUIRED OFF)
 set(CMAKE_C_EXTENSIONS OFF)
 
 # This project requires C++11.
-set(CMAKE_CXX_STANDARD 11)
+set(CMAKE_CXX_STANDARD 20)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
  1. Configure:
$ cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_CXX_FLAGS=-Wconditional-uninitialized -DLEVELDB_BUILD_BENCHMARKS=ON
  1. Build:
$ cmake --build build
[23/161] Building CXX object CMakeFiles/leveldb.dir/db/version_set.cc.o
FAILED: CMakeFiles/leveldb.dir/db/version_set.cc.o 
/usr/bin/clang++ -DLEVELDB_COMPILE_LIBRARY -DLEVELDB_PLATFORM_POSIX=1 -I/home/hebasto/dev/leveldb/build/include -I/home/hebasto/dev/leveldb/. -I/home/hebasto/dev/leveldb/include -Wconditional-uninitialized -fno-exceptions -fno-rtti -std=c++20 -Werror -Wthread-safety -MD -MT CMakeFiles/leveldb.dir/db/version_set.cc.o -MF CMakeFiles/leveldb.dir/db/version_set.cc.o.d -o CMakeFiles/leveldb.dir/db/version_set.cc.o -c /home/hebasto/dev/leveldb/db/version_set.cc
/home/hebasto/dev/leveldb/db/version_set.cc:1016:55: error: variable 'manifest_size' may be uninitialized when used here [-Werror,-Wconditional-uninitialized]
 1016 |   descriptor_log_ = new log::Writer(descriptor_file_, manifest_size);
      |                                                       ^~~~~~~~~~~~~
/home/hebasto/dev/leveldb/db/version_set.cc:997:25: note: initialize the variable 'manifest_size' to silence this warning
  997 |   uint64_t manifest_size;
      |                         ^
      |                          = 0
1 error generated.
[40/161] Building CXX object CMakeFiles/leveldb.dir/util/env_posix.cc.o
ninja: build stopped: subcommand failed.
hebasto@TS-P340:~/dev/leveldb

This eliminates the need for the following code:

else()
  try_append_cxx_flags("-Wconditional-uninitialized" TARGET nowarn_leveldb_interface SKIP_LINK
    IF_CHECK_PASSED "-Wno-conditional-uninitialized"
  )

@hebasto hebasto mentioned this pull request Feb 3, 2026
Merged
maflcko
maflcko approved these changes Feb 6, 2026
View reviewed changes

@maflcko maflcko left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Comment thread db/corruption_test.cc Outdated
@maflcko

maflcko commented Feb 6, 2026

Copy link
Copy Markdown

Unrelated: Google is using clang to build their projects, so I wonder why they haven't fixed it yet upstream or if they are also using a workaround 🤔

reuse_logs isn't set by Bitcoin Core, so this is just refactoring dead code?

Seems fine, if the motivation is to remove build code.

@hebasto

hebasto commented Feb 6, 2026

Copy link
Copy Markdown
Member Author

reuse_logs isn't set by Bitcoin Core, so this is just refactoring dead code?

From the build log of the Bitcoin Core patched to enable the warning:

[55/626] Building CXX object src/CMakeFiles/leveldb.dir/leveldb/db/version_set.cc.o
../src/leveldb/db/version_set.cc:1016:55: warning: variable 'manifest_size' may be uninitialized when used here [-Wconditional-uninitialized]
 1016 |   descriptor_log_ = new log::Writer(descriptor_file_, manifest_size);
      |                                                       ^~~~~~~~~~~~~
../src/leveldb/db/version_set.cc:997:25: note: initialize the variable 'manifest_size' to silence this warning
  997 |   uint64_t manifest_size;
      |                         ^
      |                          = 0
1 warning generated.

@fanquake

fanquake commented Feb 6, 2026

Copy link
Copy Markdown
Member

Yes the compiler warns, but Core doesn't set reuse_logs (which defaults to false), so the code you are changing here, can never be hit, because we always return early out of ReuseManifest.

@hebasto hebasto force-pushed the 260203-uninitialized branch from 8c70878 to 0711e6d Compare February 6, 2026 15:37
@maflcko

maflcko commented Feb 6, 2026

Copy link
Copy Markdown

lgtm ACK 0711e6d

@fanquake fanquake merged commit 68740f5 into bitcoin-core:bitcoin-fork May 18, 2026
@hebasto hebasto deleted the 260203-uninitialized branch May 18, 2026 10:41
@fanquake fanquake mentioned this pull request May 18, 2026
Merged
fanquake added a commit to fanquake/bitcoin that referenced this pull request May 28, 2026
@fanquake
Squashed 'src/leveldb/' changes from ab6c84e6f3..a7f9bdc611
58cdb5c 
a7f9bdc611 Merge bitcoin-core/leveldb-subtree#52: Revert "Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems"
a2f531d2d0 Merge bitcoin-core/leveldb-subtree#60: util: use [[fallthrough]] directly
1a166221cf Merge bitcoin-core/leveldb-subtree#61: Disable seek compaction
6bfdb6093b Disable seek compaction
42a5f29aa9 util: use [[fallthrough]] directly
c274b50867 Merge bitcoin-core/leveldb-subtree#53: refactor: Delete unused `ScopedHandle:operator=(ScopedHandle&&)`
68740f586f Merge bitcoin-core/leveldb-subtree#59: Fix Clang `-Wconditional-uninitialized` warning
70142e186b Merge bitcoin-core/leveldb-subtree#55: build: Require C++17
d123cf5a83 Effectively, this change
31361bf339 Allow different C/C++ standards when this is used as a subproject.
0711e6d082 Fix Clang `-Wconditional-uninitialized` warning
85665f9547 refactor: Delete unused `ScopedHandle:operator=(ScopedHandle&&)`
fd8f69657e Revert "Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems"

git-subtree-dir: src/leveldb
git-subtree-split: a7f9bdc6114fe6eeb848fda2980fe61d86ff2045
fanquake added a commit to bitcoin/bitcoin that referenced this pull request May 28, 2026
@fanquake
Merge #35313: Bump leveldb subtree
a34dbc8 
a9ac680 build: remove FALLTHROUGH_INTENDED from leveldb.cmake (fanquake)
4d58c32 build: remove -Wno-conditional-uninitialized from leveldb build (fanquake)
58cdb5c Squashed 'src/leveldb/' changes from ab6c84e6f3..a7f9bdc611 (fanquake)

Pull request description:

  Includes:
  * bitcoin-core/leveldb-subtree#52
  * bitcoin-core/leveldb-subtree#53
  * bitcoin-core/leveldb-subtree#55
  * bitcoin-core/leveldb-subtree#59
  * bitcoin-core/leveldb-subtree#60
  * bitcoin-core/leveldb-subtree#61

ACKs for top commit:
  hebasto:
    ACK a9ac680.
  sedited:
    ACK a9ac680
  andrewtoth:
    ACK a9ac680

Tree-SHA512: dc80a0e5eabd63866b395681935ba47bd3f67292049f2cce77f6bcf9cdd6f3bb9bcf2d87ae836a7af5b3f07fe21a1885697cbae3eb930900e396c2588910e12a
mraksoll4 added a commit to Bitwebissss/bitweb-ver1 that referenced this pull request Jun 3, 2026
@mraksoll4
Squashed 'src/leveldb/' changes from aba469ad6a..8e89537cef
cd91f4a 
8e89537cef Merge bitcoin-core/leveldb-subtree#62: ci: Add LevelDB test workflow
47abe23ff2 ci: run LevelDB tests in GitHub Actions
a7f9bdc611 Merge bitcoin-core/leveldb-subtree#52: Revert "Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems"
a2f531d2d0 Merge bitcoin-core/leveldb-subtree#60: util: use [[fallthrough]] directly
1a166221cf Merge bitcoin-core/leveldb-subtree#61: Disable seek compaction
6bfdb6093b Disable seek compaction
42a5f29aa9 util: use [[fallthrough]] directly
c274b50867 Merge bitcoin-core/leveldb-subtree#53: refactor: Delete unused `ScopedHandle:operator=(ScopedHandle&&)`
68740f586f Merge bitcoin-core/leveldb-subtree#59: Fix Clang `-Wconditional-uninitialized` warning
70142e186b Merge bitcoin-core/leveldb-subtree#55: build: Require C++17
d123cf5a83 Effectively, this change
31361bf339 Allow different C/C++ standards when this is used as a subproject.
0711e6d082 Fix Clang `-Wconditional-uninitialized` warning
ab6c84e6f3 Merge bitcoin-core/leveldb-subtree#58: Initialize file_size to 0 to avoid UB
ad9b1c9893 Initialize file_size to 0 to avoid UB
cad64b151d Merge bitcoin-core/leveldb-subtree#57: doc: fix typos
157ed16be9 doc: fix typos
85665f9547 refactor: Delete unused `ScopedHandle:operator=(ScopedHandle&&)`
fd8f69657e Revert "Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems"

git-subtree-dir: src/leveldb
git-subtree-split: 8e89537cef1144b6b75df83ac53239b79d6e2617
mraksoll4 added a commit to bitweb-project/bitweb that referenced this pull request Jun 4, 2026
@mraksoll4
Squashed 'src/leveldb/' changes from aba469ad6a..8e89537cef
0f9e115 
8e89537cef Merge bitcoin-core/leveldb-subtree#62: ci: Add LevelDB test workflow
47abe23ff2 ci: run LevelDB tests in GitHub Actions
a7f9bdc611 Merge bitcoin-core/leveldb-subtree#52: Revert "Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems"
a2f531d2d0 Merge bitcoin-core/leveldb-subtree#60: util: use [[fallthrough]] directly
1a166221cf Merge bitcoin-core/leveldb-subtree#61: Disable seek compaction
6bfdb6093b Disable seek compaction
42a5f29aa9 util: use [[fallthrough]] directly
c274b50867 Merge bitcoin-core/leveldb-subtree#53: refactor: Delete unused `ScopedHandle:operator=(ScopedHandle&&)`
68740f586f Merge bitcoin-core/leveldb-subtree#59: Fix Clang `-Wconditional-uninitialized` warning
70142e186b Merge bitcoin-core/leveldb-subtree#55: build: Require C++17
d123cf5a83 Effectively, this change
31361bf339 Allow different C/C++ standards when this is used as a subproject.
0711e6d082 Fix Clang `-Wconditional-uninitialized` warning
ab6c84e6f3 Merge bitcoin-core/leveldb-subtree#58: Initialize file_size to 0 to avoid UB
ad9b1c9893 Initialize file_size to 0 to avoid UB
cad64b151d Merge bitcoin-core/leveldb-subtree#57: doc: fix typos
157ed16be9 doc: fix typos
85665f9547 refactor: Delete unused `ScopedHandle:operator=(ScopedHandle&&)`
fd8f69657e Revert "Increase maximum read-only mmap()s used from 1000 to 4096 on 64-bit systems"

git-subtree-dir: src/leveldb
git-subtree-split: 8e89537cef1144b6b75df83ac53239b79d6e2617
Kino1994 pushed a commit to Kino1994/bitcoin-full-history that referenced this pull request Jun 28, 2026
@fanquake
Merge bitcoin/bitcoin#35313: Bump leveldb subtree
6043b59 
0a6aeed build: remove FALLTHROUGH_INTENDED from leveldb.cmake (fanquake)
9b7550e build: remove -Wno-conditional-uninitialized from leveldb build (fanquake)
58cdb5c Squashed 'src/leveldb/' changes from ab6c84e6f3..a7f9bdc611 (fanquake)

Pull request description:

  Includes:
  * bitcoin-core/leveldb-subtree#52
  * bitcoin-core/leveldb-subtree#53
  * bitcoin-core/leveldb-subtree#55
  * bitcoin-core/leveldb-subtree#59
  * bitcoin-core/leveldb-subtree#60
  * bitcoin-core/leveldb-subtree#61

ACKs for top commit:
  hebasto:
    ACK 0a6aeed.
  sedited:
    ACK 0a6aeed
  andrewtoth:
    ACK 0a6aeed

Tree-SHA512: dc80a0e5eabd63866b395681935ba47bd3f67292049f2cce77f6bcf9cdd6f3bb9bcf2d87ae836a7af5b3f07fe21a1885697cbae3eb930900e396c2588910e12a
BigcoinBGC pushed a commit to BigcoinBGC/bigcoin that referenced this pull request Jun 30, 2026
@fanquake
Merge bitcoin/bitcoin#35313: Bump leveldb subtree
a60c44d 
b27d612 build: remove FALLTHROUGH_INTENDED from leveldb.cmake (fanquake)
f2b0e14 build: remove -Wno-conditional-uninitialized from leveldb build (fanquake)
58cdb5c Squashed 'src/leveldb/' changes from ab6c84e6f3..a7f9bdc611 (fanquake)

Pull request description:

  Includes:
  * bitcoin-core/leveldb-subtree#52
  * bitcoin-core/leveldb-subtree#53
  * bitcoin-core/leveldb-subtree#55
  * bitcoin-core/leveldb-subtree#59
  * bitcoin-core/leveldb-subtree#60
  * bitcoin-core/leveldb-subtree#61

ACKs for top commit:
  hebasto:
    ACK b27d612.
  sedited:
    ACK b27d612
  andrewtoth:
    ACK b27d612

Tree-SHA512: dc80a0e5eabd63866b395681935ba47bd3f67292049f2cce77f6bcf9cdd6f3bb9bcf2d87ae836a7af5b3f07fe21a1885697cbae3eb930900e396c2588910e12a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@maflcko maflcko maflcko approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hebasto @maflcko @fanquake