Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,32 @@ if (stat != Status::OK) return false; // do not skip this
- **Debug+ASan** (the default top-level Debug build) is the right mode for correctness work — most TPC-C bugs we have caught (use-after-free in `get_and_update_*`, the `cast_to<Order>` assertion, the gcRecord UAF) showed up there first and were invisible under pure Release.
- **Release** is for benchmark numbers only. CI builds Release without sanitizer (`-DENABLE_SANITIZER=OFF`) — it does not run binaries, just verifies they compile.

### Compiler-version mismatch with CI

CI runs on `ubuntu-latest` (currently Ubuntu 24.04 → **GCC 13**). The default devcontainer ships **GCC 11**. The two compilers disagree on `-Wmaybe-uninitialized` (and likely other flow-sensitive warnings): GCC 13 catches false-positive-prone cases that GCC 11 lets through.

When working on the phased `-Werror` cleanup (#43) — or anything else that promotes a warning to error — **verify on GCC 13 locally before pushing**. Three rounds of CI red on PR #44 (`-Wmaybe-uninitialized`) were avoidable by doing this once. Options:

```sh
# Option 1: PPA (one-time setup, fastest iteration afterwards)
sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test
sudo apt-get update && sudo apt-get install -y gcc-13 g++-13
CC=gcc-13 CXX=g++-13 cmake -S . -B build-gcc13 -DCMAKE_BUILD_TYPE=Release -DENABLE_SANITIZER=OFF
cmake --build build-gcc13 -j
```

```sh
# Option 2: Docker (no host changes; slower because deps re-install each run)
docker run --rm -v "$PWD":/ccbench -w /ccbench ubuntu:24.04 bash -c '
apt-get update && apt-get install -y --no-install-recommends \
$(cat build_tools/ubuntu.deps) build-essential pkg-config && \
cmake -S . -B build-gcc13 -DCMAKE_BUILD_TYPE=Release -DENABLE_SANITIZER=OFF && \
cmake --build build-gcc13 -j
'
```

Pushing a `-Werror=<flag>` promotion without a GCC 13 build first counts as "didn't actually verify."

## Repository layout

- [include/](include/) — shared headers (atomics, rwlock, zipf, masstree wrapper, etc.)
Expand Down
7 changes: 6 additions & 1 deletion cc/cicada/transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,12 @@ bool TxExecutor::validation() {
if ((*itr).op_ == OpType::INSERT) {
continue;
}
Version *expected(nullptr), *ver, *pre_ver;
// pre_ver is only read on the else branch below, which is reachable
// only after the while-loop has assigned to it — but GCC 13 cannot
// prove that across the (op != RMW && op != DELETE && !WRITE_LATEST_ONLY
// && ver != expected) condition. Initialize to silence -Wmaybe-uninitialized
// without changing runtime behavior.
Version *expected(nullptr), *ver, *pre_ver = nullptr;
for (;;) {
if ((*itr).op_ == OpType::RMW || (*itr).op_ == OpType::DELETE || WRITE_LATEST_ONLY) {
ver = expected = (*itr).rcdptr_->ldAcqLatest();
Expand Down
7 changes: 6 additions & 1 deletion cc/mocc/transaction.cc
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,12 @@ Status TxExecutor::delete_record(Storage s, std::string_view key) {

void TxExecutor::lock(Tuple *tuple, bool mode) {
unsigned int vioctr = 0;
Tuple* threshold;
// Sentinel "no violation found yet" (max pointer). The CLL_ scan
// below overwrites this on the first violation; the explicit
// `if (vioctr == 0) threshold = (Tuple*)-1` afterwards confirms the
// default. Initializing here keeps GCC 13's -Wmaybe-uninitialized
// happy without changing runtime semantics.
Tuple* threshold = (Tuple*)-1;
bool upgrade = false;

#ifdef RWLOCK
Expand Down
5 changes: 4 additions & 1 deletion cc/oze/include/transaction.hh
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,10 @@ public:
uint32_t max = expected & 0xffffffff;
uint32_t left = *reinterpret_cast<const uint32_t*>(left_key.data());
uint32_t right = *reinterpret_cast<const uint32_t*>(right_key.data());
uint64_t updated;
// Must start at 0: the |= branches below only write half the
// word each (upper 32b in one, lower 32b in the other), so
// any uninitialized garbage would leak into the result.
uint64_t updated = 0;
if (max < right) {
updated |= static_cast<uint64_t>(right) & 0xffffffff;
} else {
Expand Down
12 changes: 10 additions & 2 deletions cc/silo/include/log.hh
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,17 @@ public:
std::string_view key_;
char val_[VAL_SIZE];

LogRecord() : tid_(0), key_("") {}
// computeChkSum() below sums every int-sized chunk of *this*, including
// any trailing struct padding after val_, so both constructors zero
// the entire object first to make those reads well-defined.
LogRecord() {
memset(this, 0, sizeof(LogRecord));
}

LogRecord(uint64_t tid, std::string_view key, char *val) : tid_(tid), key_(key) {
LogRecord(uint64_t tid, std::string_view key, char *val) {
memset(this, 0, sizeof(LogRecord));
tid_ = tid;
key_ = key;
memcpy(this->val_, val, VAL_SIZE);
}

Expand Down
12 changes: 7 additions & 5 deletions cmake/ProtocolHelpers.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,13 @@ function(ccbench_add_protocol name)
${_universal_defs}
${_extra_defs})

# NOTE: deliberately do NOT call set_compile_options(${target}) here.
# The old per-protocol CMakeLists never enabled -Wall -Wextra -Werror,
# and the existing source tree has accumulated warnings under those
# flags. Turning Werror on here would be a separate cleanup task —
# this issue is just about declarative target wiring.
# Phased -Werror cleanup (see #43). The full -Wall -Wextra -Werror via
# set_compile_options() is still gated on Phase 5 — we promote one
# warning class to error at a time as the existing offenders get
# fixed. Append the next entry here when its source-level cleanup
# PR lands.
target_compile_options(${target} PRIVATE
-Werror=maybe-uninitialized)

set_property(TARGET ${target} PROPERTY CCBENCH_PROTOCOL "${name}")
set_property(TARGET ${target} PROPERTY CCBENCH_WORKLOAD "${wl}")
Expand Down
Loading