ConflictSet::Impl::addWrites computes newestVersionFullPrecision + kNominalVersionWindow (ConflictSet.cpp:5094). When versions approach INT64_MAX, this addition overflows. With wrap-around semantics the comparison spuriously becomes true, so addWrites takes the "version window exceeded" path and calls eraseTree() + init(), destroying every entry whose version is still within the valid 2-billion-version read window. Subsequent reads that must return Conflict silently return Commit.
The docs only require versions to be >= 0 ("All versions must be >= 0"), so versions near INT64_MAX are valid input. The bug reproduces with the default build and with USE_64_BIT=1.
Location
ConflictSet.cpp, Impl::addWrites, line 5092-5097:
if(oldestExtantVersion<writeVersion-kMaxCorrectVersionWindow)[[unlikely]]{if(writeVersion>newestVersionFullPrecision+kNominalVersionWindow){// line 5094: overflow
eraseTree(rootParent->children[0],&writeContext);init(writeVersion-kNominalVersionWindow);}...
kNominalVersionWindow is int64_t(2e9) (line 97). newestVersionFullPrecision + kNominalVersionWindow overflows once newestVersionFullPrecision > INT64_MAX - 2e9. Everywhere else in this function the same comparisons are written in subtraction form (lines 5092, 5101, 5106, 5112), which cannot overflow for valid versions; line 5094 is the only addition-form comparison.
Reproduction
#include"ConflictSet.h"#include"Internal.h"// for ReferenceImpl (Internal.h)
#include<limits>#include<cstdio>usingnamespaceweaselab;intmain(){constint64_tIMAX=std::numeric_limits<int64_t>::max();constint64_tV0=IMAX-1000000000LL;// 1e9 below IMAX, well inside the 2e9 window
constint64_tV1=IMAX;ConflictSetcs(0);ReferenceImplref(0);ConflictSet::WriteRangew{(constuint8_t*)"key",3,(constuint8_t*)"",0};cs.addWrites(&w,1,V0);ref.addWrites(&w,1,V0);cs.addWrites(&w,0,V1);// count 0: only advance the version by 1e9 (< 2e9 window)
ref.addWrites(&w,0,V1);// Valid read: V0-1 >= oldestVersion (auto-advanced to V0 - 2e9) and <= newest.
// The write at V0 > V0-1 must Conflict.
ConflictSet::ReadRanger{(constuint8_t*)"key",3,(constuint8_t*)"",0,V0-1};ConflictSet::Resultgot,expected;cs.check(&r,&got,1);ref.check(&r,&expected,1);printf("read {key} @ V0-1: got=%s expected=%s\n",resultToStr(got),resultToStr(expected));}
Actual: got=commit expected=conflict — the write at V0 (only 1e9 old, i.e. still inside the valid read window) was erased.
Expected: conflict.
With UBSan (-fsanitize=undefined), the exact site is flagged:
ConflictSet.cpp:5094:53: runtime error: signed integer overflow: 9223372035854775807 + 2000000000 cannot be represented in type 'long int'
(9223372035854775807 = newestVersionFullPrecision = V0 in this sequence; + 2e9 overflows by 1e9.)
Mechanism
addWrites(V0) with V0 = INT64_MAX - 1e9 stores the write normally.
addWrites(count = 0, V1 = INT64_MAX) enters the [[unlikely]] branch (because oldestExtantVersion < V1 - kMaxCorrectVersionWindow, which is legitimately true for old oldestExtantVersion).
newestVersionFullPrecision + kNominalVersionWindow = V0 + 2e9 = INT64_MAX + 1e9 — signed overflow. With wrap-around it becomes a large negative value, so writeVersion > newestVersionFullPrecision + kNominalVersionWindow spuriously evaluates to true even though writeVersion - newestVersionFullPrecision = 1e9 < kNominalVersionWindow.
eraseTree() + init(V1 - kNominalVersionWindow) discard all extant entries, including writes with version >= V1 - 2e9 that reads at valid readVersion values must still see. (Side effect visible in metrics: getBytes() drops from 3458 to 3384; the erased-entry counters are not even incremented because init() re-constructs writeContext and zeroes accum.)
A read of the written key at V0 - 1 (valid: >= oldestVersion = V1 - 2e9) returns Commit instead of Conflict.
USE_64_BIT=1 behaves identically (the window logic at line 5094 is shared).
Impact
For a user whose version space approaches INT64_MAX (permitted by the documented contract: versions must merely be >= 0), a single addWrites call silently erases all conflict history while the version window says it must be preserved. Optimistic-concurrency checks then return Commit for reads that overlap recent writes, i.e. lost-update detection silently fails. Independently of the wrong results, the addition itself is undefined behavior (signed overflow), which the compiler is free to exploit in other ways.
Suggested fix
Rewrite the comparison in subtraction form, matching the style used at lines 5092/5101/5106/5112 (and safe for all valid inputs, since writeVersion >= 0):
I verified this one-line change: the reproduction above then returns conflict as expected, and the full ctest suite (8598 tests: corpus blackbox/ASan/TSan/valgrind, script tests, API smoke tests) passes at 100%.
## Summary
`ConflictSet::Impl::addWrites` computes `newestVersionFullPrecision + kNominalVersionWindow` (ConflictSet.cpp:5094). When versions approach `INT64_MAX`, this addition overflows. With wrap-around semantics the comparison spuriously becomes true, so `addWrites` takes the "version window exceeded" path and calls `eraseTree()` + `init()`, destroying every entry whose version is still within the valid 2-billion-version read window. Subsequent reads that must return `Conflict` silently return `Commit`.
The docs only require versions to be >= 0 ("All versions must be >= 0"), so versions near `INT64_MAX` are valid input. The bug reproduces with the default build and with `USE_64_BIT=1`.
## Location
ConflictSet.cpp, `Impl::addWrites`, line 5092-5097:
```cpp
if (oldestExtantVersion < writeVersion - kMaxCorrectVersionWindow) [[unlikely]] {
if (writeVersion > newestVersionFullPrecision + kNominalVersionWindow) { // line 5094: overflow
eraseTree(rootParent->children[0], &writeContext);
init(writeVersion - kNominalVersionWindow);
}
...
```
`kNominalVersionWindow` is `int64_t(2e9)` (line 97). `newestVersionFullPrecision + kNominalVersionWindow` overflows once `newestVersionFullPrecision > INT64_MAX - 2e9`. Everywhere else in this function the same comparisons are written in subtraction form (lines 5092, 5101, 5106, 5112), which cannot overflow for valid versions; line 5094 is the only addition-form comparison.
## Reproduction
```cpp
#include "ConflictSet.h"
#include "Internal.h" // for ReferenceImpl (Internal.h)
#include <limits>
#include <cstdio>
using namespace weaselab;
int main() {
const int64_t IMAX = std::numeric_limits<int64_t>::max();
const int64_t V0 = IMAX - 1000000000LL; // 1e9 below IMAX, well inside the 2e9 window
const int64_t V1 = IMAX;
ConflictSet cs(0);
ReferenceImpl ref(0);
ConflictSet::WriteRange w{(const uint8_t *)"key", 3, (const uint8_t *)"", 0};
cs.addWrites(&w, 1, V0);
ref.addWrites(&w, 1, V0);
cs.addWrites(&w, 0, V1); // count 0: only advance the version by 1e9 (< 2e9 window)
ref.addWrites(&w, 0, V1);
// Valid read: V0-1 >= oldestVersion (auto-advanced to V0 - 2e9) and <= newest.
// The write at V0 > V0-1 must Conflict.
ConflictSet::ReadRange r{(const uint8_t *)"key", 3, (const uint8_t *)"", 0, V0 - 1};
ConflictSet::Result got, expected;
cs.check(&r, &got, 1);
ref.check(&r, &expected, 1);
printf("read {key} @ V0-1: got=%s expected=%s\n", resultToStr(got), resultToStr(expected));
}
```
Actual: `got=commit expected=conflict` — the write at V0 (only 1e9 old, i.e. still inside the valid read window) was erased.
Expected: `conflict`.
With UBSan (`-fsanitize=undefined`), the exact site is flagged:
```
ConflictSet.cpp:5094:53: runtime error: signed integer overflow: 9223372035854775807 + 2000000000 cannot be represented in type 'long int'
```
(9223372035854775807 = `newestVersionFullPrecision` = V0 in this sequence; + 2e9 overflows by 1e9.)
## Mechanism
1. `addWrites(V0)` with `V0 = INT64_MAX - 1e9` stores the write normally.
2. `addWrites(count = 0, V1 = INT64_MAX)` enters the `[[unlikely]]` branch (because `oldestExtantVersion < V1 - kMaxCorrectVersionWindow`, which is legitimately true for old `oldestExtantVersion`).
3. `newestVersionFullPrecision + kNominalVersionWindow` = `V0 + 2e9` = `INT64_MAX + 1e9` — signed overflow. With wrap-around it becomes a large negative value, so `writeVersion > newestVersionFullPrecision + kNominalVersionWindow` spuriously evaluates to `true` even though `writeVersion - newestVersionFullPrecision = 1e9 < kNominalVersionWindow`.
4. `eraseTree()` + `init(V1 - kNominalVersionWindow)` discard **all** extant entries, including writes with version >= `V1 - 2e9` that reads at valid `readVersion` values must still see. (Side effect visible in metrics: `getBytes()` drops from 3458 to 3384; the erased-entry counters are not even incremented because `init()` re-constructs `writeContext` and zeroes `accum`.)
5. A read of the written key at `V0 - 1` (valid: `>= oldestVersion = V1 - 2e9`) returns `Commit` instead of `Conflict`.
`USE_64_BIT=1` behaves identically (the window logic at line 5094 is shared).
## Impact
For a user whose version space approaches `INT64_MAX` (permitted by the documented contract: versions must merely be >= 0), a single `addWrites` call silently erases all conflict history while the version window says it must be preserved. Optimistic-concurrency checks then return `Commit` for reads that overlap recent writes, i.e. lost-update detection silently fails. Independently of the wrong results, the addition itself is undefined behavior (signed overflow), which the compiler is free to exploit in other ways.
## Suggested fix
Rewrite the comparison in subtraction form, matching the style used at lines 5092/5101/5106/5112 (and safe for all valid inputs, since `writeVersion >= 0`):
```cpp
if (writeVersion - kNominalVersionWindow > newestVersionFullPrecision) {
```
I verified this one-line change: the reproduction above then returns `conflict` as expected, and the full ctest suite (8598 tests: corpus blackbox/ASan/TSan/valgrind, script tests, API smoke tests) passes at 100%.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
ConflictSet::Impl::addWritescomputesnewestVersionFullPrecision + kNominalVersionWindow(ConflictSet.cpp:5094). When versions approachINT64_MAX, this addition overflows. With wrap-around semantics the comparison spuriously becomes true, soaddWritestakes the "version window exceeded" path and callseraseTree()+init(), destroying every entry whose version is still within the valid 2-billion-version read window. Subsequent reads that must returnConflictsilently returnCommit.The docs only require versions to be >= 0 ("All versions must be >= 0"), so versions near
INT64_MAXare valid input. The bug reproduces with the default build and withUSE_64_BIT=1.Location
ConflictSet.cpp,
Impl::addWrites, line 5092-5097:kNominalVersionWindowisint64_t(2e9)(line 97).newestVersionFullPrecision + kNominalVersionWindowoverflows oncenewestVersionFullPrecision > INT64_MAX - 2e9. Everywhere else in this function the same comparisons are written in subtraction form (lines 5092, 5101, 5106, 5112), which cannot overflow for valid versions; line 5094 is the only addition-form comparison.Reproduction
Actual:
got=commit expected=conflict— the write at V0 (only 1e9 old, i.e. still inside the valid read window) was erased.Expected:
conflict.With UBSan (
-fsanitize=undefined), the exact site is flagged:(9223372035854775807 =
newestVersionFullPrecision= V0 in this sequence; + 2e9 overflows by 1e9.)Mechanism
addWrites(V0)withV0 = INT64_MAX - 1e9stores the write normally.addWrites(count = 0, V1 = INT64_MAX)enters the[[unlikely]]branch (becauseoldestExtantVersion < V1 - kMaxCorrectVersionWindow, which is legitimately true for oldoldestExtantVersion).newestVersionFullPrecision + kNominalVersionWindow=V0 + 2e9=INT64_MAX + 1e9— signed overflow. With wrap-around it becomes a large negative value, sowriteVersion > newestVersionFullPrecision + kNominalVersionWindowspuriously evaluates totrueeven thoughwriteVersion - newestVersionFullPrecision = 1e9 < kNominalVersionWindow.eraseTree()+init(V1 - kNominalVersionWindow)discard all extant entries, including writes with version >=V1 - 2e9that reads at validreadVersionvalues must still see. (Side effect visible in metrics:getBytes()drops from 3458 to 3384; the erased-entry counters are not even incremented becauseinit()re-constructswriteContextand zeroesaccum.)V0 - 1(valid:>= oldestVersion = V1 - 2e9) returnsCommitinstead ofConflict.USE_64_BIT=1behaves identically (the window logic at line 5094 is shared).Impact
For a user whose version space approaches
INT64_MAX(permitted by the documented contract: versions must merely be >= 0), a singleaddWritescall silently erases all conflict history while the version window says it must be preserved. Optimistic-concurrency checks then returnCommitfor reads that overlap recent writes, i.e. lost-update detection silently fails. Independently of the wrong results, the addition itself is undefined behavior (signed overflow), which the compiler is free to exploit in other ways.Suggested fix
Rewrite the comparison in subtraction form, matching the style used at lines 5092/5101/5106/5112 (and safe for all valid inputs, since
writeVersion >= 0):I verified this one-line change: the reproduction above then returns
conflictas expected, and the full ctest suite (8598 tests: corpus blackbox/ASan/TSan/valgrind, script tests, API smoke tests) passes at 100%.