facebook / facebook/rocksdb

Uninitialized warnings when compiling in portable+lite mode

Open
#10,750 0 comments 0 reactions 0 assignees View on GitHub
Build up-for-grabs
Dominant language
C++
Stars
32.1k
Forks
6.9k
Avg merge
32m
Merged PRs (30d)
1

Description

This bug seems pretty straightforward: I downloaded and unpacked `rocksdb-7.5.3.tar.gz` and compiled it with GCC 11.2 on Ubuntu 22.0, specifically, `gcc version 11.2.0 (Ubuntu 11.2.0-19ubuntu1)`. I used the default static library build:
```sh
$ make static_lib
```
That copy compiled fine.

I used that for a while, but it's a very big build, and I wanted to strip out parts that I didn't need, and also make a binary that could run on other servers that I have, so I ran this:
```sh
$ make clean
$ PORTABLE=1 USE_SSE=0 LITE=1 make static_lib
```
However, that crashes partway through the build:
```
db/db_impl/db_impl.cc: In member function ‘virtual bool rocksdb::DBImpl::GetProperty(rocksdb::ColumnFamilyHandle*, const rocksdb::Slice&, std::string*)’:
db/db_impl/db_impl.cc:3579:30: error: ‘int_value’ may be used uninitialized [-Werror=maybe-uninitialized]
3579 | *value = std::to_string(int_value);
| ~~~~~~~~~~~~~~^~~~~~~~~~~
db/db_impl/db_impl.cc:3575:14: note: ‘int_value’ declared here
3575 | uint64_t int_value;
| ^~~~~~~~~
db/db_impl/db_impl.cc: In member function ‘virtual bool rocksdb::DBImpl::GetAggregatedIntProperty(const rocksdb::Slice&, uint64_t*)’:
db/db_impl/db_impl.cc:3714:13: error: ‘value’ may be used uninitialized [-Werror=maybe-uninitialized]
3714 | sum += value;
| ~~~~^~~~~~~~
db/db_impl/db_impl.cc:3705:14: note: ‘value’ declared here
3705 | uint64_t value;
| ^~~~~
cc1plus: all warnings being treated as errors
make: *** [Makefile:2449: db/db_impl/db_impl.o] Error 1
```
I'm actually a little surprised at this error — not because it's an error, but because looking at the code, it actually looks like the compile should fail _every_ time, not just fail when `ROCKSDB_LITE` is defined:
```
uint64_t int_value;
bool ret_value =
GetIntPropertyInternal(cfd, *property_info, false, &int_value);
if (ret_value) {
*value = std::to_string(int_value);
}
return ret_value;
```
It looks like the compiler is right to be complaining there that `int_value` could be uninitialized by the time `to_string()` is called; there's no proof that `GetIntPropertyInternal()` will have populated it. The other error has a similar structure, a call to `GetIntPropertyInternal()` for a `value` variable that isn't guaranteed to be initialized.

Not sure what else to say here, other than that it looks like there's a bug.

Contributor guide

Open the contributing guide

Research direction

Start with db/db_impl/db_impl.cc and inspect the GetProperty and GetAggregatedIntProperty paths shown in the report. Reproduce the issue with PORTABLE=1 USE_SSE=0 LITE=1 make static_lib, then verify the portable lite build completes without the reported maybe-uninitialized errors.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, linux
Domain
build-system, databases
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.