Github/littlefs
1
Fork 0
Code Actions Activity

Expanded fuzz testing a bit, found/fixed an mid neighbor update bug

Browse Source 
This bug was just overlooked in testing the mtree, fortunately dir
fuzzing found it. Though since this depends on neighboring mdirs, it
probably would have been found quicker with smaller block sizes. At the
moment I am only testing on NOR-liked geometry (4KiB blocks).

The fix is easy, we can use the difference in the mtree size to
determine if a split or drop happened in mdir commit, since at most one
of these can happen on any mdir commit.

Also added an explicit test for mid updates when splitting and dropping.
This commit is contained in:
Christopher Haster
2023-07-07 16:31:52 -05:00
parent 039bdf91b4
commit cb1319c9e6
4 changed files with 237 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+22 -9
View File
@@ -4966,14 +4966,14 @@ static inline int lfsr_mtree_isinlined(lfs_t *lfs) {
return lfsr_btree_weight(&lfs->mtree) == 0;
}
static inline lfs_ssize_t lfsr_mtree_weight(lfs_t *lfs) {
static inline lfs_size_t lfsr_mtree_weight(lfs_t *lfs) {
return lfsr_btree_weight(&lfs->mtree);
}
static int lfsr_mtree_lookup(lfs_t *lfs, lfs_ssize_t mid, lfsr_mdir_t *mdir_) {
// TODO should we really allow -1=>mroot lookup?
LFS_ASSERT(mid >= -1);
LFS_ASSERT(mid < lfsr_mtree_weight(lfs));
LFS_ASSERT(mid < (lfs_ssize_t)lfsr_mtree_weight(lfs));
// looking up mroot?
if (mid < 0) {
@@ -5649,17 +5649,18 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid,
}
// success?? update in-device state
lfs->mroot = mroot_;
lfs->mtree = mtree_;
// update any opened mdirs
for (lfsr_openedmdir_t *opened = lfs->opened;
opened;
opened = opened->next) {
if (opened->mdir.mid == mdir->mid
// avoid double-updating our current mdir
&& &opened->mdir != mdir) {
LFS_ASSERT(opened->rid <= (lfs_ssize_t)opened->mdir.rbyd.weight);
// avoid double-updating our current mdir
if (&opened->mdir == mdir) {
continue;
}
if (opened->mdir.mid == mdir->mid) {
LFS_ASSERT(opened->rid < (lfs_ssize_t)opened->mdir.rbyd.weight);
LFS_ASSERT(opened->rid != -1);
// first play out any attrs that change our rid
@@ -5681,12 +5682,24 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid,
} else if ((lfs_size_t)opened->rid < mdir_.rbyd.weight) {
opened->mdir = mdir_;
} else {
LFS_ASSERT(lfsr_btree_weight(&mtree_)
!= lfsr_mtree_weight(lfs));
opened->rid = opened->rid - mdir_.rbyd.weight;
opened->mdir = msibling_;
}
// update mid if we had a split or drop
} else if (opened->mdir.mid > mdir->mid
&& lfsr_btree_weight(&mtree_) != lfsr_mtree_weight(lfs)) {
opened->mdir.mid += lfsr_btree_weight(&mtree_)
- lfsr_mtree_weight(lfs);
}
}
// update our mroot and mtree
lfs->mroot = mroot_;
lfs->mtree = mtree_;
// update mdir to follow requested rid
lfs_ssize_t rid_ = *rid;
LFS_ASSERT(rid_ <= (lfs_ssize_t)mdir->rbyd.weight);
@@ -6904,7 +6917,7 @@ int lfsr_dir_read(lfs_t *lfs, lfsr_dir_t *dir, struct lfs_info *info) {
if (rid >= (lfs_ssize_t)dir->mdir.mdir.rbyd.weight) {
// out of mdirs?
lfs_ssize_t mid = dir->mdir.mdir.mid + 1;
if (mid >= lfsr_mtree_weight(lfs)) {
if (mid >= (lfs_ssize_t)lfsr_mtree_weight(lfs)) {
return LFS_ERR_NOENT;
}
tests/t3_mtree.toml
+202 -15
View File
@@ -426,7 +426,7 @@ code = '''
// try looking up each entry
lfs_size_t i = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -451,7 +451,7 @@ code = '''
// try looking up each entry
i = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -535,7 +535,7 @@ code = '''
lfs_size_t count_ = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -564,7 +564,7 @@ code = '''
count_ = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -1306,7 +1306,7 @@ code = '''
// try looking up each entry
lfs_size_t i = N - REMAINING;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -1331,7 +1331,7 @@ code = '''
// try looking up each entry
i = N - REMAINING;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -1387,7 +1387,7 @@ code = '''
// try looking up each entry
lfs_size_t i = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -1517,7 +1517,7 @@ code = '''
lfs_size_t count_ = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -1550,7 +1550,7 @@ code = '''
count_ = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -2629,7 +2629,7 @@ code = '''
lfs_size_t count_ = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -2662,7 +2662,7 @@ code = '''
count_ = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -3224,6 +3224,193 @@ code = '''
lfsr_unmount(&lfs) => 0;
'''
[cases.t3_mtree_neighbor_mid_split]
# this should be set so only one entry can fit in a metadata block
defines.SIZE = 'BLOCK_SIZE / 4'
# make it so blocks relocate every two compacts
defines.BLOCK_CYCLES = 2
in = 'lfs.c'
code = '''
const char *alphas = "abcdefghijklmnopqrstuvwxyz";
lfs_t lfs;
lfsr_format(&lfs, cfg) => 0;
lfsr_mount(&lfs, cfg) => 0;
lfs_alloc_ack(&lfs);
// remove root dstart for now
lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS(
LFSR_ATTR(0, UNR, -1, NULL, 0))) => 0;
//// create a situation where we have 3 mdirs in our tree
// first force mroot to uninlined+split
uint8_t buffer[SIZE];
memset(buffer, alphas[0 % 26], SIZE);
lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS(
LFSR_ATTR(0, INLINED, +1, buffer, SIZE))) => 0;
memset(buffer, alphas[1 % 26], SIZE);
lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS(
LFSR_ATTR(1, INLINED, +1, buffer, SIZE))) => 0;
// force mroot to compact
lfs.mroot.rbyd.off = BLOCK_SIZE;
lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, NULL, 0) => 0;
// we should now have 2 mdirs
assert(lfsr_mtree_weight(&lfs) == 2);
// now force one of our siblings to split
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, 1, &mdir) => 0;
assert(mdir.rbyd.weight == 1);
memset(buffer, alphas[2 % 26], SIZE);
lfsr_mdir_commit(&lfs, &mdir, &(lfs_ssize_t){1}, LFSR_ATTRS(
LFSR_ATTR(1, INLINED, +1, buffer, SIZE))) => 0;
// force mdir to compact
mdir.rbyd.off = BLOCK_SIZE;
lfsr_mdir_commit(&lfs, &mdir, &(lfs_ssize_t){-1}, NULL, 0) => 0;
// we should now have 3 mdirs
assert(lfsr_mtree_weight(&lfs) == 3);
//// Now test splitting updates mids correctly
// setup our neighbors
lfsr_openedmdir_t left_neighbor;
lfsr_mtree_lookup(&lfs, 0, &left_neighbor.mdir) => 0;
assert(left_neighbor.mdir.rbyd.weight == 1);
left_neighbor.rid = 0;
lfsr_openedmdir_t right_neighbor;
lfsr_mtree_lookup(&lfs, 2, &right_neighbor.mdir) => 0;
assert(right_neighbor.mdir.rbyd.weight == 1);
right_neighbor.rid = 0;
lfsr_mdir_addopened(&lfs, &left_neighbor);
lfsr_mdir_addopened(&lfs, &right_neighbor);
// cause middle mdir to split
lfsr_mtree_lookup(&lfs, 1, &mdir) => 0;
assert(mdir.rbyd.weight == 1);
memset(buffer, alphas[3 % 26], SIZE);
lfsr_mdir_commit(&lfs, &mdir, &(lfs_ssize_t){1}, LFSR_ATTRS(
LFSR_ATTR(1, INLINED, +1, buffer, SIZE))) => 0;
// force mdir to compact
mdir.rbyd.off = BLOCK_SIZE;
lfsr_mdir_commit(&lfs, &mdir, &(lfs_ssize_t){-1}, NULL, 0) => 0;
// we should now have 4 mdirs
assert(lfsr_mtree_weight(&lfs) == 4);
// assert that our neighbors were updated correctly
assert(left_neighbor.rid == 0);
assert(left_neighbor.mdir.mid == 0);
lfsr_mtree_lookup(&lfs, 0, &mdir) => 0;
assert(memcmp(&left_neighbor.mdir, &mdir, sizeof(lfsr_mdir_t)) == 0);
assert(right_neighbor.rid == 0);
assert(right_neighbor.mdir.mid == 3);
lfsr_mtree_lookup(&lfs, 3, &mdir) => 0;
assert(memcmp(&right_neighbor.mdir, &mdir, sizeof(lfsr_mdir_t)) == 0);
lfsr_mdir_removeopened(&lfs, &left_neighbor);
lfsr_mdir_removeopened(&lfs, &right_neighbor);
lfsr_unmount(&lfs) => 0;
'''
[cases.t3_mtree_neighbor_mid_drop]
# this should be set so only one entry can fit in a metadata block
defines.SIZE = 'BLOCK_SIZE / 4'
# make it so blocks relocate every two compacts
defines.BLOCK_CYCLES = 2
in = 'lfs.c'
code = '''
const char *alphas = "abcdefghijklmnopqrstuvwxyz";
lfs_t lfs;
lfsr_format(&lfs, cfg) => 0;
lfsr_mount(&lfs, cfg) => 0;
lfs_alloc_ack(&lfs);
// remove root dstart for now
lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS(
LFSR_ATTR(0, UNR, -1, NULL, 0))) => 0;
//// create a situation where we have 3 mdirs in our tree
// first force mroot to uninlined+split
uint8_t buffer[SIZE];
memset(buffer, alphas[0 % 26], SIZE);
lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS(
LFSR_ATTR(0, INLINED, +1, buffer, SIZE))) => 0;
memset(buffer, alphas[1 % 26], SIZE);
lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, LFSR_ATTRS(
LFSR_ATTR(1, INLINED, +1, buffer, SIZE))) => 0;
// force mroot to compact
lfs.mroot.rbyd.off = BLOCK_SIZE;
lfsr_mdir_commit(&lfs, &lfs.mroot, &(lfs_ssize_t){-1}, NULL, 0) => 0;
// we should now have 2 mdirs
assert(lfsr_mtree_weight(&lfs) == 2);
// now force one of our siblings to split
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, 1, &mdir) => 0;
assert(mdir.rbyd.weight == 1);
memset(buffer, alphas[2 % 26], SIZE);
lfsr_mdir_commit(&lfs, &mdir, &(lfs_ssize_t){1}, LFSR_ATTRS(
LFSR_ATTR(1, INLINED, +1, buffer, SIZE))) => 0;
// force mdir to compact
mdir.rbyd.off = BLOCK_SIZE;
lfsr_mdir_commit(&lfs, &mdir, &(lfs_ssize_t){-1}, NULL, 0) => 0;
// we should now have 3 mdirs
assert(lfsr_mtree_weight(&lfs) == 3);
//// Now test dropping updates mids correctly
// setup our neighbors
lfsr_openedmdir_t left_neighbor;
lfsr_mtree_lookup(&lfs, 0, &left_neighbor.mdir) => 0;
assert(left_neighbor.mdir.rbyd.weight == 1);
left_neighbor.rid = 0;
lfsr_openedmdir_t right_neighbor;
lfsr_mtree_lookup(&lfs, 2, &right_neighbor.mdir) => 0;
assert(right_neighbor.mdir.rbyd.weight == 1);
right_neighbor.rid = 0;
lfsr_mdir_addopened(&lfs, &left_neighbor);
lfsr_mdir_addopened(&lfs, &right_neighbor);
// cause middle mdir to drop
lfsr_mtree_lookup(&lfs, 1, &mdir) => 0;
assert(mdir.rbyd.weight == 1);
lfsr_mdir_commit(&lfs, &mdir, &(lfs_ssize_t){0}, LFSR_ATTRS(
LFSR_ATTR(0, UNR, -1, NULL, 0))) => 0;
// we should now have 2 mdirs
assert(lfsr_mtree_weight(&lfs) == 2);
// assert that our neighbors were updated correctly
assert(left_neighbor.rid == 0);
assert(left_neighbor.mdir.mid == 0);
lfsr_mtree_lookup(&lfs, 0, &mdir) => 0;
assert(memcmp(&left_neighbor.mdir, &mdir, sizeof(lfsr_mdir_t)) == 0);
assert(right_neighbor.rid == 0);
assert(right_neighbor.mdir.mid == 1);
lfsr_mtree_lookup(&lfs, 1, &mdir) => 0;
assert(memcmp(&right_neighbor.mdir, &mdir, sizeof(lfsr_mdir_t)) == 0);
lfsr_mdir_removeopened(&lfs, &left_neighbor);
lfsr_mdir_removeopened(&lfs, &right_neighbor);
lfsr_unmount(&lfs) => 0;
'''
## mtree traversal ##
@@ -3749,7 +3936,7 @@ code = '''
// try looking up each entry
lfs_size_t i = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -3831,7 +4018,7 @@ code = '''
// try looking up each entry
i = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -3915,7 +4102,7 @@ code = '''
lfs_size_t count_ = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
@@ -4002,7 +4189,7 @@ code = '''
count_ = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
tests/t4_alloc.toml
+1 -1
View File
@@ -142,7 +142,7 @@ code = '''
// test that all of our metadata entries are still there
lfs_size_t i = 0;
for (lfs_ssize_t mid = (lfsr_mtree_isinlined(&lfs) ? -1 : 0);
mid < lfsr_mtree_weight(&lfs);
mid < (lfs_ssize_t)lfsr_mtree_weight(&lfs);
mid++) {
lfsr_mdir_t mdir;
lfsr_mtree_lookup(&lfs, mid, &mdir) => 0;
tests/t5_dirs.toml
+12 -4
View File
@@ -547,6 +547,9 @@ code = '''
[cases.t5_dirs_mkdir_fuzz]
defines.N = [1, 2, 4, 8, 16, 32, 64, 128, 256, 512]
# 0 => do this test in the root dir
# 1 => do this test in a directory named "parent"
defines.PARENT = [0, 1]
defines.SAMPLES = 10
# -1 => all pseudo-random seeds
# n => reproduce a specific seed
@@ -561,6 +564,9 @@ code = '''
lfs_t lfs;
lfsr_format(&lfs, cfg) => 0;
lfsr_mount(&lfs, cfg) => 0;
if (PARENT) {
lfsr_mkdir(&lfs, "parent") => 0;
}
// set up a simulation to compare against
lfs_size_t *sim = malloc(N*sizeof(lfs_size_t));
@@ -590,7 +596,7 @@ code = '''
// create a directory here
char name[256];
sprintf(name, "dir%04d", x);
sprintf(name, "%s/dir%04d", (PARENT ? "parent" : ""), x);
lfsr_mkdir(&lfs, name) => 0;
next:;
}
@@ -598,15 +604,17 @@ code = '''
// test that our directories match our simulation
for (lfs_size_t j = 0; j < sim_size; j++) {
char name[256];
sprintf(name, "dir%04d", sim[j]);
sprintf(name, "%s/dir%04d", (PARENT ? "parent" : ""), sim[j]);
struct lfs_info info;
lfsr_stat(&lfs, name, &info) => 0;
assert(strcmp(info.name, name) == 0);
char name2[256];
sprintf(name2, "dir%04d", sim[j]);
assert(strcmp(info.name, name2) == 0);
assert(info.type == LFS_TYPE_DIR);
}
lfsr_dir_t dir;
lfsr_dir_open(&lfs, &dir, "/") => 0;
lfsr_dir_open(&lfs, &dir, (PARENT ? "parent" : "/")) => 0;
struct lfs_info info;
lfsr_dir_read(&lfs, &dir, &info) => 0;
assert(strcmp(info.name, ".") == 0);