Skip to content

Commit b9a25dd

Browse files
committed
colblk: add more bound checks
Add invariants-only bound checking, especially before accessing `UnsafeOffset`s. We also change to using `At2()` instead of two `At()` calls in a few places.
1 parent b494d9b commit b9a25dd

File tree

6 files changed

+41
-7
lines changed

6 files changed

+41
-7
lines changed

internal/invariants/off.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,7 @@ func (*Value[V]) Get() V {
4949

5050
// Set the value; no-op in non-invariant builds.
5151
func (*Value[V]) Set(v V) {}
52+
53+
// CheckBounds panics if the index is not in the range [0, n). No-op in
54+
// non-invariant builds.
55+
func CheckBounds(i int, n int) {}

internal/invariants/on.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66

77
package invariants
88

9-
import "math/rand/v2"
9+
import (
10+
"fmt"
11+
"math/rand/v2"
12+
)
1013

1114
// Sometimes returns true percent% of the time if invariants are Enabled (i.e.
1215
// we were built with the "invariants" or "race" build tags). Otherwise, always
@@ -72,3 +75,11 @@ func (v *Value[V]) Get() V {
7275
func (v *Value[V]) Set(inner V) {
7376
v.v = inner
7477
}
78+
79+
// CheckBounds panics if the index is not in the range [0, n). No-op in
80+
// non-invariant builds.
81+
func CheckBounds(i int, n int) {
82+
if i < 0 || i >= n {
83+
panic(fmt.Sprintf("index %d out of bounds [0, %d)", i, n))
84+
}
85+
}

sstable/colblk/bitmap.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ func (b Bitmap) At(i int) bool {
6969
// zero bitmap case.
7070
return false
7171
}
72+
invariants.CheckBounds(i, b.bitCount)
7273
val := b.data.At(int(uint(i) >> 6)) // aka At(i/64)
7374
return val&(1<<(uint(i)&63)) != 0
7475
}
@@ -96,6 +97,10 @@ func (b Bitmap) SeekSetBitGE(i int) int {
9697
// on the summary word to get the index of which word has a set bit, if any.
9798
summaryTableOffset, summaryTableEnd := b.summaryTableBounds()
9899
summaryWordIdx := summaryTableOffset + wordIdx>>6
100+
if invariants.Enabled {
101+
sz := bitmapRequiredSize(b.bitCount)
102+
invariants.CheckBounds(summaryTableEnd-1, sz)
103+
}
99104
summaryNext := nextBitInWord(b.data.At(summaryWordIdx), uint(wordIdx%64)+1)
100105
// If [summaryNext] == 64, then there are no set bits in any of the earlier
101106
// words represented by the summary word at [summaryWordIdx]. In that case,
@@ -171,11 +176,11 @@ func (b Bitmap) SeekSetBitLE(i int) int {
171176
// in the bitmap. The i parameter must be in [0, bitCount). Returns the number
172177
// of bits represented by the bitmap if no next bit is unset.
173178
func (b Bitmap) SeekUnsetBitGE(i int) int {
179+
invariants.CheckBounds(i, b.bitCount)
174180
if b.data.ptr == nil {
175181
// Zero bitmap case.
176182
return i
177183
}
178-
179184
wordIdx := i >> 6 // i/64
180185
// If the there's a bit ≥ i unset in the same word, return it.
181186
if next := nextBitInWord(^b.data.At(wordIdx), uint(i)&63); next < 64 {
@@ -199,6 +204,7 @@ func (b Bitmap) SeekUnsetBitGE(i int) int {
199204
// bitmap. The i parameter must be in [0, bitCount). Returns -1 if no previous
200205
// bit is unset.
201206
func (b Bitmap) SeekUnsetBitLE(i int) int {
207+
invariants.CheckBounds(i, b.bitCount)
202208
if b.data.ptr == nil {
203209
// Zero bitmap case.
204210
return i

sstable/colblk/data_block.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1336,6 +1336,7 @@ func (i *DataBlockIter) Next() *base.InternalKV {
13361336
i.kv.K.SetSeqNum(base.SeqNum(n))
13371337
}
13381338
}
1339+
invariants.CheckBounds(i.row, i.d.values.slices)
13391340
// Inline i.d.values.At(row).
13401341
v := i.d.values.slice(i.d.values.offsets.At2(i.row))
13411342
if i.d.isValueExternal.At(i.row) {
@@ -1504,9 +1505,10 @@ func (i *DataBlockIter) decodeRow() *base.InternalKV {
15041505
i.kv.K.SetSeqNum(base.SeqNum(n))
15051506
}
15061507
}
1508+
invariants.CheckBounds(i.row, i.d.values.slices)
15071509
// Inline i.d.values.At(row).
1508-
startOffset := i.d.values.offsets.At(i.row)
1509-
v := unsafe.Slice((*byte)(i.d.values.ptr(startOffset)), i.d.values.offsets.At(i.row+1)-startOffset)
1510+
v := i.d.values.slice(i.d.values.offsets.At2(i.row))
1511+
invariants.CheckBounds(i.row, i.d.values.slices)
15101512
if i.d.isValueExternal.At(i.row) {
15111513
i.kv.V = i.getLazyValuer.GetInternalValueForPrefixAndValueHandle(v)
15121514
} else {

sstable/colblk/prefix_bytes.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,7 @@ func (i *PrefixBytesIter) Init(maxKeyLength int, syntheticPrefix block.Synthetic
282282
func (b *PrefixBytes) SetAt(it *PrefixBytesIter, i int) {
283283
// Determine the offset and length of the bundle prefix.
284284
bundleOffsetIndex := b.bundleOffsetIndexForRow(i)
285+
invariants.CheckBounds(bundleOffsetIndex, b.rawBytes.slices)
285286
bundleOffsetStart, bundleOffsetEnd := b.rawBytes.offsets.At2(bundleOffsetIndex)
286287
bundlePrefixLen := bundleOffsetEnd - bundleOffsetStart
287288

@@ -329,6 +330,7 @@ func (b *PrefixBytes) SetNext(it *PrefixBytesIter) {
329330
// If the next row is in the same bundle, we can take a fast path of only
330331
// updating the per-row suffix.
331332
if it.offsetIndex < it.nextBundleOffsetIndex {
333+
invariants.CheckBounds(it.offsetIndex, b.rawBytes.slices)
332334
rowSuffixStart, rowSuffixEnd := b.rawBytes.offsets.At2(it.offsetIndex)
333335
rowSuffixLen := rowSuffixEnd - rowSuffixStart
334336
if rowSuffixLen == 0 {
@@ -351,6 +353,7 @@ func (b *PrefixBytes) SetNext(it *PrefixBytesIter) {
351353
// The offsetIndex is currently pointing to the start of the new bundle
352354
// prefix. Increment it to point at the start of the new row suffix.
353355
it.offsetIndex++
356+
invariants.CheckBounds(it.offsetIndex, b.rawBytes.slices)
354357
rowSuffixStart, rowSuffixEnd := b.rawBytes.offsets.At2(it.offsetIndex)
355358
rowSuffixLen := rowSuffixEnd - rowSuffixStart
356359

@@ -391,15 +394,17 @@ func (b *PrefixBytes) SharedPrefix() []byte {
391394
// prefix for the column. The returned slice should not be mutated.
392395
func (b *PrefixBytes) RowBundlePrefix(row int) []byte {
393396
i := b.bundleOffsetIndexForRow(row)
394-
return b.rawBytes.slice(b.rawBytes.offsets.At(i), b.rawBytes.offsets.At(i+1))
397+
invariants.CheckBounds(i, b.rawBytes.slices)
398+
return b.rawBytes.slice(b.rawBytes.offsets.At2(i))
395399
}
396400

397401
// BundlePrefix returns the prefix of the i-th bundle in the column. The
398402
// provided i must be in the range [0, BundleCount()). The returned slice should
399403
// not be mutated.
400404
func (b *PrefixBytes) BundlePrefix(i int) []byte {
401405
j := b.offsetIndexByBundleIndex(i)
402-
return b.rawBytes.slice(b.rawBytes.offsets.At(j), b.rawBytes.offsets.At(j+1))
406+
invariants.CheckBounds(j, b.rawBytes.slices)
407+
return b.rawBytes.slice(b.rawBytes.offsets.At2(j))
403408
}
404409

405410
// RowSuffix returns a []byte of the suffix unique to the row. A row's full key
@@ -415,6 +420,7 @@ func (b *PrefixBytes) RowSuffix(row int) []byte {
415420
// accounting for duplicate keys. It takes the index of the row, and the value
416421
// of rowSuffixIndex(row).
417422
func (b *PrefixBytes) rowSuffixOffsets(row, i int) (low uint32, high uint32) {
423+
invariants.CheckBounds(i, b.rawBytes.slices)
418424
// Retrieve the low and high offsets indicating the start and end of the
419425
// row's suffix slice.
420426
low, high = b.rawBytes.offsets.At2(i)
@@ -493,6 +499,7 @@ func (b *PrefixBytes) Search(k []byte) (rowIndex int, isEqual bool) {
493499
// offset(j) offset(j+1) offset(j+2)
494500
//
495501
j := b.offsetIndexByBundleIndex(h)
502+
invariants.CheckBounds(j+1, b.rawBytes.slices)
496503
bundleFirstKey := b.rawBytes.slice(b.rawBytes.offsets.At(j), b.rawBytes.offsets.At(j+2))
497504
c = bytes.Compare(k, bundleFirstKey)
498505
switch {
@@ -516,7 +523,8 @@ func (b *PrefixBytes) Search(k []byte) (rowIndex int, isEqual bool) {
516523
// among them, but if the seek key doesn't share the previous bundle's
517524
// prefix there's no need.
518525
j := b.offsetIndexByBundleIndex(bi - 1)
519-
bundlePrefix := b.rawBytes.slice(b.rawBytes.offsets.At(j), b.rawBytes.offsets.At(j+1))
526+
invariants.CheckBounds(j, b.rawBytes.slices)
527+
bundlePrefix := b.rawBytes.slice(b.rawBytes.offsets.At2(j))
520528

521529
// The row we are looking for might still be in the previous bundle even
522530
// though the seek key is greater than the first key. This is possible only
@@ -553,6 +561,7 @@ func (b *PrefixBytes) Search(k []byte) (rowIndex int, isEqual bool) {
553561
// The beginning of the zero-indexed i-th key of the bundle is at
554562
// offset(j+i+1).
555563
//
564+
invariants.CheckBounds(j+h+1, b.rawBytes.slices)
556565
hStart, hEnd := b.rawBytes.offsets.At2(j + h + 1)
557566
// There's a complication with duplicate keys. When keys are repeated,
558567
// the PrefixBytes encoding avoids re-encoding the duplicate key,

sstable/colblk/raw_bytes.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"unsafe"
1212

1313
"github.com/cockroachdb/pebble/internal/binfmt"
14+
"github.com/cockroachdb/pebble/internal/invariants"
1415
"github.com/cockroachdb/pebble/internal/treeprinter"
1516
)
1617

@@ -117,6 +118,7 @@ func (b *RawBytes) slice(start, end uint32) []byte {
117118

118119
// At returns the []byte at index i. The returned slice should not be mutated.
119120
func (b RawBytes) At(i int) []byte {
121+
invariants.CheckBounds(i, b.slices)
120122
return b.slice(b.offsets.At2(i))
121123
}
122124

0 commit comments

Comments
 (0)