Github/littlefs
1
Fork 0
Code Actions Activity

Added tests for nasty btree drop conditions and fixed related bug

Browse Source 
Thanks to lazy merging, our btree nodes can drop to zero weight at
pretty much any time. Unfortunately, we can't really represent non-root
zero weight btree nodes, so things break. (Though even if we could,
those nodes would become unreachable).

Previously we relied on fuzz testing to try to catch these cases, but
that turned out to be insufficient.

This adds explicit tests covering the cases where btree drops can occur,
thanks to the realy-big-attr trick used in similar mtree tests.

Sure enough this revealed a bug that can occur when we split a btree
node at the same time one of the siblings goes to zero weight. (Remember
splits carried out before playing attr-lists).

---

Fortunately this is pretty easy to fix. We can just reroute our split
code to the normal commit/compact recursion handling if one of our
siblings drops to zero, at the cost of some spaghetti.

xkcd.com/292 seems relevant here.
This commit is contained in:
Christopher Haster
2023-08-17 12:49:07 -05:00
parent df0ad072ea
commit 256488d4b4
2 changed files with 267 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+9
View File
@@ -4201,6 +4201,15 @@ static int lfsr_btree_commit(lfs_t *lfs, lfsr_btree_t *btree,
return err;
}
// did one of our siblings drop to zero? yes this can happen! revert
// to a normal commit in that case
if (rbyd_.weight == 0 || sibling.weight == 0) {
if (rbyd_.weight == 0) {
rbyd_ = sibling;
}
goto commit_recurse;
}
// lookup first name in sibling to use as the split name
//
// note we need to do this after playing out pending attrs in case
tests/test_btree.toml
+258
View File
@@ -2542,6 +2542,264 @@ code = '''
'''
# Some specific corner cases
[cases.test_btree_drop]
# this should large enough so only one entry can fit in a block
defines.SIZE = 'BLOCK_SIZE / 4'
defines.SIBLING = [0, 1]
in = 'lfs.c'
code = '''
lfs_t lfs;
lfs_init(&lfs, CFG) => 0;
// create free lookahead
memset(lfs.lookahead.buffer, 0, CFG->lookahead_size);
lfs.lookahead.start = 0;
lfs.lookahead.size = lfs_min(8*CFG->lookahead_size,
CFG->block_count);
lfs.lookahead.next = 0;
lfs_alloc_ack(&lfs);
// create a tree
lfsr_btree_t btree = LFSR_BTREE_NULL;
// force it to split
// the extra push here avoids trying to inline the big entry
lfsr_btree_push(&lfs, &btree, 0, LFSR_TAG_INLINED, 1,
LFSR_DATA("_", 1)) => 0;
uint8_t buf1[SIZE];
memset(buf1, 'a', SIZE);
uint8_t buf2[SIZE];
memset(buf2, 'b', SIZE);
lfsr_btree_split(&lfs, &btree, 0, LFSR_DATA_NULL,
LFSR_TAG_INLINED, 1, LFSR_DATA(buf1, SIZE),
LFSR_TAG_INLINED, 1, LFSR_DATA(buf2, SIZE)) => 0;
// force compaction
btree.u.r.rbyd.eoff = -1;
memset(buf2, 'b', SIZE);
lfsr_btree_set(&lfs, &btree, 1, LFSR_TAG_INLINED, 1,
LFSR_DATA(buf2, SIZE)) => 0;
assert(lfsr_btree_weight(&btree) == 2);
// now remove one entry, since this brings the rbyd down to zero,
// this should force one of the blocks to drop
lfsr_btree_pop(&lfs, &btree, SIBLING) => 0;
printf("btree: w%d 0x%x.%x\n",
btree.u.r.rbyd.weight,
btree.u.r.rbyd.block,
btree.u.r.rbyd.trunk);
assert(lfsr_btree_weight(&btree) == 1);
// check that our other entry is fine
lfsr_tag_t tag_;
lfs_size_t weight_;
lfsr_btree_get(&lfs, &btree, 0,
&tag_, &weight_, buf1, SIZE) => SIZE;
assert(tag_ == LFSR_TAG_INLINED);
assert(weight_ == 1);
assert(memcmp(buf1, (SIBLING ? "a" : "b"), 1) == 0);
// and check that our pop worked
lfsr_btree_get(&lfs, &btree, 1,
&tag_, &weight_, buf1, SIZE) => LFS_ERR_NOENT;
'''
[cases.test_btree_drop_compact]
# this should large enough so only one entry can fit in a block
defines.SIZE = 'BLOCK_SIZE / 4'
defines.SIBLING = [0, 1]
in = 'lfs.c'
code = '''
lfs_t lfs;
lfs_init(&lfs, CFG) => 0;
// create free lookahead
memset(lfs.lookahead.buffer, 0, CFG->lookahead_size);
lfs.lookahead.start = 0;
lfs.lookahead.size = lfs_min(8*CFG->lookahead_size,
CFG->block_count);
lfs.lookahead.next = 0;
lfs_alloc_ack(&lfs);
// create a tree
lfsr_btree_t btree = LFSR_BTREE_NULL;
// force it to split
// the extra push here avoids trying to inline the big entry
lfsr_btree_push(&lfs, &btree, 0, LFSR_TAG_INLINED, 1,
LFSR_DATA("_", 1)) => 0;
uint8_t buf1[SIZE];
memset(buf1, 'a', SIZE);
uint8_t buf2[SIZE];
memset(buf2, 'b', SIZE);
lfsr_btree_split(&lfs, &btree, 0, LFSR_DATA_NULL,
LFSR_TAG_INLINED, 1, LFSR_DATA(buf1, SIZE),
LFSR_TAG_INLINED, 1, LFSR_DATA(buf2, SIZE)) => 0;
// force compaction
btree.u.r.rbyd.eoff = -1;
memset(buf2, 'b', SIZE);
lfsr_btree_set(&lfs, &btree, 1, LFSR_TAG_INLINED, 1,
LFSR_DATA(buf2, SIZE)) => 0;
assert(lfsr_btree_weight(&btree) == 2);
// now remove one entry, since this brings the rbyd down this zero,
// this should force one of the blocks to drop
//
// do this while forcing a compaction
btree.u.r.rbyd.eoff = -1;
lfsr_btree_pop(&lfs, &btree, SIBLING) => 0;
printf("btree: w%d 0x%x.%x\n",
btree.u.r.rbyd.weight,
btree.u.r.rbyd.block,
btree.u.r.rbyd.trunk);
assert(lfsr_btree_weight(&btree) == 1);
// check that our other entry is fine
lfsr_tag_t tag_;
lfs_size_t weight_;
lfsr_btree_get(&lfs, &btree, 0,
&tag_, &weight_, buf1, SIZE) => SIZE;
assert(tag_ == LFSR_TAG_INLINED);
assert(weight_ == 1);
assert(memcmp(buf1, (SIBLING ? "a" : "b"), 1) == 0);
// and check that our pop worked
lfsr_btree_get(&lfs, &btree, 1,
&tag_, &weight_, buf1, SIZE) => LFS_ERR_NOENT;
'''
[cases.test_btree_drop_split]
# this should large enough so only one entry can fit in a block
defines.SIZE = 'BLOCK_SIZE / 4'
defines.SIBLING = [0, 1]
in = 'lfs.c'
code = '''
lfs_t lfs;
lfs_init(&lfs, CFG) => 0;
// create free lookahead
memset(lfs.lookahead.buffer, 0, CFG->lookahead_size);
lfs.lookahead.start = 0;
lfs.lookahead.size = lfs_min(8*CFG->lookahead_size,
CFG->block_count);
lfs.lookahead.next = 0;
lfs_alloc_ack(&lfs);
// create a tree
lfsr_btree_t btree = LFSR_BTREE_NULL;
// force it to split
// the extra push here avoids trying to inline the big entry
lfsr_btree_push(&lfs, &btree, 0, LFSR_TAG_INLINED, 1,
LFSR_DATA("_", 1)) => 0;
uint8_t buf1[SIZE];
memset(buf1, 'a', SIZE);
uint8_t buf2[SIZE];
memset(buf2, 'b', SIZE);
lfsr_btree_split(&lfs, &btree, 0, LFSR_DATA_NULL,
LFSR_TAG_INLINED, 1, LFSR_DATA(buf1, SIZE),
LFSR_TAG_INLINED, 1, LFSR_DATA(buf2, SIZE)) => 0;
// force compaction, causing a split, but while we're splitting,
// also remove an entry, bringing the split rbyd down to zero mid split
//
// messy, isn't it? this is why we need an explicit test
//
btree.u.r.rbyd.eoff = -1;
lfsr_btree_pop(&lfs, &btree, SIBLING) => 0;
printf("btree: w%d 0x%x.%x\n",
btree.u.r.rbyd.weight,
btree.u.r.rbyd.block,
btree.u.r.rbyd.trunk);
assert(lfsr_btree_weight(&btree) == 1);
// check that our other entry is fine
lfsr_tag_t tag_;
lfs_size_t weight_;
lfsr_btree_get(&lfs, &btree, 0,
&tag_, &weight_, buf1, SIZE) => SIZE;
assert(tag_ == LFSR_TAG_INLINED);
assert(weight_ == 1);
assert(memcmp(buf1, (SIBLING ? "a" : "b"), 1) == 0);
// and check that our pop worked
lfsr_btree_get(&lfs, &btree, 1,
&tag_, &weight_, buf1, SIZE) => LFS_ERR_NOENT;
'''
[cases.test_btree_drop_merge]
# this should large enough so only one entry can fit in a block
defines.SIZE = 'BLOCK_SIZE / 4'
defines.SIBLING = [0, 1]
in = 'lfs.c'
code = '''
lfs_t lfs;
lfs_init(&lfs, CFG) => 0;
// create free lookahead
memset(lfs.lookahead.buffer, 0, CFG->lookahead_size);
lfs.lookahead.start = 0;
lfs.lookahead.size = lfs_min(8*CFG->lookahead_size,
CFG->block_count);
lfs.lookahead.next = 0;
lfs_alloc_ack(&lfs);
// create a tree
lfsr_btree_t btree = LFSR_BTREE_NULL;
// force it to split
// the extra push here avoids trying to inline the big entry
lfsr_btree_push(&lfs, &btree, 0, LFSR_TAG_INLINED, 1,
LFSR_DATA("_", 1)) => 0;
uint8_t buf1[SIZE];
memset(buf1, 'a', SIZE);
uint8_t buf2[SIZE];
memset(buf2, 'b', SIZE);
lfsr_btree_split(&lfs, &btree, 0, LFSR_DATA_NULL,
LFSR_TAG_INLINED, 1, LFSR_DATA(buf1, SIZE),
LFSR_TAG_INLINED, 1, LFSR_DATA(buf2, SIZE)) => 0;
// force compaction
btree.u.r.rbyd.eoff = -1;
memset(buf2, 'b', SIZE);
lfsr_btree_set(&lfs, &btree, 1, LFSR_TAG_INLINED, 1,
LFSR_DATA(buf2, SIZE)) => 0;
assert(lfsr_btree_weight(&btree) == 2);
// now make both entries small so they should be merged if either compacts
lfsr_btree_set(&lfs, &btree, 0, LFSR_TAG_INLINED, 1,
LFSR_DATA("a", 1)) => 0;
lfsr_btree_set(&lfs, &btree, 1, LFSR_TAG_INLINED, 1,
LFSR_DATA("b", 1)) => 0;
// force compaction, while removing one entry, this drops the rbyd
// down to zero while also triggering a merge
btree.u.r.rbyd.eoff = -1;
lfsr_btree_pop(&lfs, &btree, SIBLING) => 0;
printf("btree: w%d 0x%x.%x\n",
btree.u.r.rbyd.weight,
btree.u.r.rbyd.block,
btree.u.r.rbyd.trunk);
assert(lfsr_btree_weight(&btree) == 1);
// check that our other entry is fine
lfsr_tag_t tag_;
lfs_size_t weight_;
lfsr_btree_get(&lfs, &btree, 0,
&tag_, &weight_, buf1, SIZE) => 1;
assert(tag_ == LFSR_TAG_INLINED);
assert(weight_ == 1);
assert(memcmp(buf1, (SIBLING ? "a" : "b"), 1) == 0);
// and check that our pop worked
lfsr_btree_get(&lfs, &btree, 1,
&tag_, &weight_, buf1, SIZE) => LFS_ERR_NOENT;
'''
# Some more general fuzz testing
[cases.test_btree_general_fuzz]
defines.N = [1, 2, 4, 8, 16, 32, 64, 128, 256, 512, 1024]