Skip to content

Commit 588ba3f

Browse files
hbgitclaude
andcommitted
fix(btree): resolve all 8 Valgrind memcheck defects in BTreeTest
Valgrind (ctest -T memcheck) reported 8 error contexts in BTreeTest. Root causes, confirmed by reproducing locally under Valgrind: - B_TREE_CREATE never initialized currentLoadedPages, so CHECK_AND_RELEASE_PAGES branched on stack garbage (3 contexts of "conditional jump depends on uninitialised value") - B_TREE_PAGE_CREATE malloc'd pages but left rows[], references[] and padding uninitialized while DISK_WRITE persists the whole struct (1 context of "syscall write(buf) points to uninitialised bytes"); now uses calloc. The children-clearing loop also wrote one element past the array (the known BTree.c:276 off-by-one flagged by UBSan) - BTreeTest never freed the tree: TearDown was commented out (4 "definitely lost" contexts, up to 2.5 MB in AddMultipleItems); B_TREE_FREE verified sound under Valgrind before re-enabling it - Test rows are now zero-initialized since B_TREE_INSERT persists the full row (value union + padding) to disk Verified with the real GTest suite built against googletest 1.12.1: 4/4 tests pass and Valgrind reports 0 errors from 0 contexts with the CI memcheck flags and valgrind.supp. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 68e86f4 commit 588ba3f

2 files changed

Lines changed: 12 additions & 10 deletions

File tree

modules/backend/library/lib/BTree.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ B_TREE_ROW *B_TREE_SEARCH(B_TREE *btree, unsigned key) {
130130
B_TREE B_TREE_CREATE(const char *filename) {
131131
B_TREE bt;
132132
strncpy(bt.filename, filename, FUNCTION_MAX_LENGTH_NAME);
133+
bt.currentLoadedPages = 0;
133134
bt.root = B_TREE_PAGE_CREATE(&bt);
134135
bt.root->isLeaf = TRUE;
135136
return bt;
@@ -261,7 +262,9 @@ Bool B_TREE_SPLIT_CHILD(B_TREE *btree, B_TREE_PAGE *X, int index,
261262

262263
B_TREE_PAGE *B_TREE_PAGE_CREATE(B_TREE *btree) {
263264
btree->currentLoadedPages += 1;
264-
B_TREE_PAGE *btp = malloc(sizeof(B_TREE_PAGE));
265+
/* calloc: the whole page (rows, references, padding) is written to disk
266+
* by DISK_WRITE, so every byte must be initialized */
267+
B_TREE_PAGE *btp = calloc(1, sizeof(B_TREE_PAGE));
265268
if (btp == NULL) {
266269
return NULL;
267270
}
@@ -271,12 +274,9 @@ B_TREE_PAGE *B_TREE_PAGE_CREATE(B_TREE *btree) {
271274
btp->isLeaf = TRUE;
272275

273276
int i = 0;
274-
for (; i < B_TREE_MAP2CHECK_ORDER * 2 + 1; i++) {
275-
// btp->rows[i] = NULL;
277+
for (; i < B_TREE_MAP2CHECK_ORDER * 2; i++) {
276278
btp->children[i] = NULL;
277-
// btp->references[i] = -1;
278279
}
279-
// btp->references[i] = -1;
280280

281281
if (!DISK_WRITE(btree, btp)) {
282282
return NULL;

tests/unit/backend-library/BTreeTest.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class BTreeTest : public ::testing::Test {
99
system("rm -rf btree_test.bin");
1010
bt = B_TREE_CREATE("btree_test.bin");
1111
}
12-
// void TearDown() override { B_TREE_FREE(&bt); }
12+
void TearDown() override { B_TREE_FREE(&bt); }
1313
B_TREE bt;
1414
};
1515

@@ -18,13 +18,15 @@ TEST_F(BTreeTest, ConstructorShouldBeSettedRight) {
1818
}
1919

2020
TEST_F(BTreeTest, AddItem) {
21-
B_TREE_ROW row;
21+
// Zero-initialized: the whole row (value union + padding) is persisted
22+
// to disk by B_TREE_INSERT, so every byte must be defined.
23+
B_TREE_ROW row = {};
2224
row.index = 0;
2325
B_TREE_INSERT(&bt, &row);
2426
}
2527

2628
TEST_F(BTreeTest, AddItemShouldSetValue) {
27-
B_TREE_ROW row;
29+
B_TREE_ROW row = {};
2830
row.index = 0;
2931
B_TREE_INSERT(&bt, &row);
3032

@@ -35,8 +37,8 @@ TEST_F(BTreeTest, AddItemShouldSetValue) {
3537
}
3638

3739
TEST_F(BTreeTest, AddMultipleItems) {
38-
unsigned length = 10000;
39-
B_TREE_ROW row[length];
40+
const unsigned length = 10000;
41+
B_TREE_ROW row[length] = {};
4042
for (int i = 0; i < length; i++) {
4143
row[i].index = i + 1;
4244
;

0 commit comments

Comments
 (0)