extent-layer: enforce that block map hash regions are dense

A block map is an offset index of regions; within a region the hash
vector is dense, index k being the block at m_begin + k *
BLOCK_SIZE_SUMS.  Unreachable and zero blocks are not holes inside a
region, they are ranges the packed regions do not cover.  Every
consumer depends on that: tree_slice_block_map converts a byte offset
to a vector index, visible_hashes advances one block per hash and
publishes an address for each, and the matcher's extension loop walks
two regions in lockstep by index.

Neither end enforced it.  The producer built a data region's vector
with a conditional push, so a data block without a hash would be
skipped rather than represented, shifting every later hash down one
block.  The slicer then clamped its upper bound to the vector's actual
size, which turns a short vector into a short result instead of an
error -- and a short vector does not drop hashes, it moves them, so the
clamp converts a detectable inconsistency into a silent wrong answer
that surfaces much later as a FIDEDUPERANGE rejection.

Make the invariant explicit at both ends: the producer throws if a data
block has no hash and checks the finished vector against the region's
block count, and the slicer throws rather than clamping when the
computed subrange would run past the end.  Both name the extent, so a
violation identifies itself instead of propagating.

No violation has been observed yet -- in the current producer a data
block always receives a hash in the same statement that sets its state.
These are traps for an invariant that was being relied on without being
stated.

Assisted-by: Claude-Code:claude-opus-5

diff --git a/src/bees-extent-layer.cc b/src/bees-extent-layer.cc
index 91f5a102c5f1811401df1267239b654f9d69de25..c9be1bbe6b0ea3747c43774bd19babbd288c8623 100644 (file)
--- a/src/bees-extent-layer.cc
+++ b/src/bees-extent-layer.cc
@@ -841,15 +841,29 @@ BeesBtrfsExtentLayer::block_map_fetch(BeesBtrfsExtentNode &node,
                        .m_state = state,
                };
                if (state == BeesTreeBlockState::data) {
+                       // A region's hash vector is dense: index k is the block at
+                       // m_begin + k * BLOCK_SIZE_SUMS, with no gaps.  Every
+                       // consumer relies on that -- tree_slice_block_map converts
+                       // a byte offset to an index, visible_hashes advances one
+                       // block per hash, and the matcher walks two regions in
+                       // lockstep by index.  Skipping a block here instead of
+                       // placing an entry would shift every later hash down one
+                       // block and silently mismatch, so a data block without a
+                       // hash is a contradiction rather than something to step
+                       // over: unreachable blocks belong in their own regions,
+                       // which the packed ones do not cover.
                        MmapVector<BeesHash> hashes;
                        for (size_t k = i; k < j; ++k) {
-                               if (blocks[k].hash) {
-                                       hashes.push_back(*blocks[k].hash);
-                               }
-                       }
-                       if (!hashes.empty()) {
-                               region.m_hashes = std::move(hashes);
+                               THROW_CHECK2(runtime_error, to_hex(node.m_bytenr), k,
+                                       blocks[k].hash.has_value());
+                               hashes.push_back(*blocks[k].hash);
                        }
+                       const auto want = ranged_cast<size_t>(
+                               (region_end - region_begin + BLOCK_SIZE_SUMS - 1)
+                               / BLOCK_SIZE_SUMS);
+                       THROW_CHECK3(runtime_error, to_hex(node.m_bytenr),
+                               hashes.size(), want, hashes.size() == want);
+                       region.m_hashes = std::move(hashes);
                }
                result.m_regions.push_back(std::move(region));
                i = j;

diff --git a/src/bees-extent-tree.cc b/src/bees-extent-tree.cc
index fa3cbc92a0052b0b3aa297f87ce171aa3491e4ca..32c95f347daf848bcb91f45e0c1bd8fdde7335c0 100644 (file)
--- a/src/bees-extent-tree.cc
+++ b/src/bees-extent-tree.cc
@@ -139,7 +139,17 @@ tree_slice_block_map(const BeesTreeBlockMap &full, uint64_t begin, uint64_t end)
                                        (clipped_begin - region.m_begin) / BLOCK_SIZE_SUMS);
                                const auto count = ranged_cast<size_t>(
                                        (clipped_end - clipped_begin + BLOCK_SIZE_SUMS - 1) / BLOCK_SIZE_SUMS);
-                               const auto last = min(first + count, region.m_hashes.size());
+                               // The source region is dense, so first + count must be
+                               // within it.  Clamping instead would silently return a
+                               // short vector, and every consumer indexes by block
+                               // position -- a short vector shifts hashes rather than
+                               // dropping them.  Fail here, where the extent is still
+                               // identifiable, instead of thousands of operations later
+                               // as a kernel byte-compare rejection.
+                               THROW_CHECK3(runtime_error, first, count,
+                                       region.m_hashes.size(),
+                                       first + count <= region.m_hashes.size());
+                               const auto last = first + count;
                                clipped.m_hashes = MmapVector<BeesHash>(
                                        region.m_hashes.begin() + ranged_cast<ptrdiff_t>(first),
                                        region.m_hashes.begin() + ranged_cast<ptrdiff_t>(last));