Added explicit test over unreachable tag holes
Unreachable tag holes, null tags that _should_ be unreachable but
actually are reachable, are an unfortunate quirk to our alt tag
encoding. Because we only have an altgt, not altge, our "unreachable"
tag ends up encoded with an altgt 0, an alt, which you may notice, does
not guarantee unreachability.
Fortunately, tag 0, the null tag, should intentionally be unused. So as
long as we never lookup tag 0, nothing should break.
If you do lookup tag 0, you end up with spurious null tags, which can
complicate things.
The solution here is a tag_ = max(tag, 1) in lfsr_rbyd_lookupnext.
---
One interesting thing to note, as I was writing these tests I discovered
that setting tag=max(tag,1) in lfsr_rbyd_appendattr had no effect.
appendattr needs zip the rbyd tree to keep everything connected during
range removals, so tag=0/tag=1 both end up with the same tree.
So might as well drop the tag=max(tag,1) in lfsr_rbyd_appendattr.
A side effect of this, both before and after this commit, is that any
null tag holes created during range removals sort of stick around until
the next compaction.
---
Why altgt and not altge? altgt is the inverse of altle, requiring only
a single bit flip to flip between the two. And trust me, it would be
much more costly to make altle/altgt flips more complicated than a bit
flip.
---
Why altgt/altle and not altge/altlt? This is because our rbyds are
right-leaning, that is, lookups always find the requested rid+tag, or
the next smallest rid+tag.
Consider a simple tree:
<5
.----'|
>=2 |
|'-. |
1 2 5
What should lookup(3) return? If we are right-leaning, the answer
_should_ be 5. But we need to take the <5 branch to determine if there
is a hidden 3 or 4 in that subtree.
altgt/altle does not have that problem:
<=2
.----'|
>1 |
|'-. |
1 2 5
It might seem like you can workaround this by conservatively using the
neighbor +1 as the alt target, but this runs into tag overflow problems.
UATTR(0xff)+1 (0x057f+1) becomes UATTR(0x100) (0x0580) which is not
allowed due to reserving bit 7 for future subtype extensions.
Maybe you can workaround this workaround by using (tag+0x81)&~0x80
anywhere you need to increment (including lookupnext/iteration calls!),
but this becomes a bit of a mess. And there are still concerns about
overflows at the 0x77f boundary and 0xf7f boundary.
This commit is contained in:
Christopher Haster
@@ -13071,3 +13071,266 @@ code = '''
|
||||
free(backup_block);
|
||||
}
|
||||
'''
|
||||
|
||||
|
||||
# Some very specific cases we want to cover
|
||||
|
||||
# One downside of having only altgt tags (not altge) is that we can end
|
||||
# up with an awkward null tag in our rbyd. Need to test we handle this
|
||||
# correctly.
|
||||
[cases.test_rbyd_unreachable_hole]
|
||||
in = 'lfs.c'
|
||||
code = '''
|
||||
lfs_t lfs;
|
||||
lfs_init(&lfs, CFG) => 0;
|
||||
|
||||
lfsr_rbyd_t init_rbyd = {
|
||||
.blocks[0] = 0,
|
||||
.eoff = 0,
|
||||
.cksum = 0,
|
||||
.trunk = 0,
|
||||
.weight = 0,
|
||||
};
|
||||
lfsr_rbyd_t rbyd;
|
||||
|
||||
// create a null tag hole
|
||||
rbyd = init_rbyd;
|
||||
lfsr_rbyd_commit(&lfs, &rbyd, LFSR_ATTRS(
|
||||
LFSR_ATTR(-1, UATTR(0), 0, BUF("\xaa\xaa\xaa\xaa", 4)),
|
||||
LFSR_ATTR(-1, UATTR(1), 0, BUF("\xbb\xbb\xbb\xbb", 4)),
|
||||
LFSR_ATTR(-1, RM(UATTR(0)), 0, NULL()))) => 0;
|
||||
assert(rbyd.weight == 0);
|
||||
|
||||
// can we still access things?
|
||||
lfs_ssize_t rid_;
|
||||
lfsr_tag_t tag_;
|
||||
lfs_size_t weight_;
|
||||
lfsr_data_t data_;
|
||||
lfsr_rbyd_lookupnext(&lfs, &rbyd, -1, 0,
|
||||
&rid_, &tag_, &weight_, &data_) => 0;
|
||||
assert(rid_ == -1);
|
||||
assert(tag_ == LFSR_TAG_UATTR(1));
|
||||
assert(weight_ == 0);
|
||||
assert(lfsr_data_size(&data_) == 4);
|
||||
uint8_t rbuf[32];
|
||||
lfsr_data_read(&lfs, &data_, rbuf, 32) => 4;
|
||||
assert(memcmp(rbuf, "\xbb\xbb\xbb\xbb", 4) == 0);
|
||||
|
||||
lfsr_rbyd_lookupnext(&lfs, &rbyd, -1, LFSR_TAG_UATTR(1)+1,
|
||||
&rid_, &tag_, &weight_, &data_) => LFS_ERR_NOENT;
|
||||
'''
|
||||
|
||||
[cases.test_rbyd_unreachable_hole_rm]
|
||||
in = 'lfs.c'
|
||||
code = '''
|
||||
lfs_t lfs;
|
||||
lfs_init(&lfs, CFG) => 0;
|
||||
|
||||
lfsr_rbyd_t init_rbyd = {
|
||||
.blocks[0] = 0,
|
||||
.eoff = 0,
|
||||
.cksum = 0,
|
||||
.trunk = 0,
|
||||
.weight = 0,
|
||||
};
|
||||
lfsr_rbyd_t rbyd;
|
||||
|
||||
// create a null tag hole
|
||||
rbyd = init_rbyd;
|
||||
lfsr_rbyd_commit(&lfs, &rbyd, LFSR_ATTRS(
|
||||
LFSR_ATTR(-1, UATTR(0), 0, BUF("\xaa\xaa\xaa\xaa", 4)),
|
||||
LFSR_ATTR(-1, UATTR(1), 0, BUF("\xbb\xbb\xbb\xbb", 4)),
|
||||
LFSR_ATTR(-1, UATTR(2), 0, BUF("\xcc\xcc\xcc\xcc", 4)),
|
||||
LFSR_ATTR(-1, RM(UATTR(0)), 0, NULL()))) => 0;
|
||||
assert(rbyd.weight == 0);
|
||||
|
||||
// remove a neighbor to the hole
|
||||
lfsr_rbyd_commit(&lfs, &rbyd, LFSR_ATTRS(
|
||||
LFSR_ATTR(-1, RM(UATTR(1)), 0, NULL()))) => 0;
|
||||
assert(rbyd.weight == 0);
|
||||
|
||||
// can we still access things?
|
||||
lfs_ssize_t rid_;
|
||||
lfsr_tag_t tag_;
|
||||
lfs_size_t weight_;
|
||||
lfsr_data_t data_;
|
||||
lfsr_rbyd_lookupnext(&lfs, &rbyd, -1, 0,
|
||||
&rid_, &tag_, &weight_, &data_) => 0;
|
||||
assert(rid_ == -1);
|
||||
assert(tag_ == LFSR_TAG_UATTR(2));
|
||||
assert(weight_ == 0);
|
||||
assert(lfsr_data_size(&data_) == 4);
|
||||
uint8_t rbuf[32];
|
||||
lfsr_data_read(&lfs, &data_, rbuf, 32) => 4;
|
||||
assert(memcmp(rbuf, "\xcc\xcc\xcc\xcc", 4) == 0);
|
||||
|
||||
lfsr_rbyd_lookupnext(&lfs, &rbyd, -1, LFSR_TAG_UATTR(2)+1,
|
||||
&rid_, &tag_, &weight_, &data_) => LFS_ERR_NOENT;
|
||||
'''
|
||||
|
||||
[cases.test_rbyd_unreachable_hole_delete]
|
||||
in = 'lfs.c'
|
||||
code = '''
|
||||
lfs_t lfs;
|
||||
lfs_init(&lfs, CFG) => 0;
|
||||
|
||||
lfsr_rbyd_t init_rbyd = {
|
||||
.blocks[0] = 0,
|
||||
.eoff = 0,
|
||||
.cksum = 0,
|
||||
.trunk = 0,
|
||||
.weight = 0,
|
||||
};
|
||||
lfsr_rbyd_t rbyd;
|
||||
|
||||
// create a null tag hole
|
||||
rbyd = init_rbyd;
|
||||
lfsr_rbyd_commit(&lfs, &rbyd, LFSR_ATTRS(
|
||||
LFSR_ATTR(-1, UATTR(0), 0, BUF("\xaa\xaa\xaa\xaa", 4)),
|
||||
LFSR_ATTR(0, REG, +1, BUF("\xbb\xbb\xbb\xbb", 4)),
|
||||
LFSR_ATTR(1, REG, +1, BUF("\xcc\xcc\xcc\xcc", 4)),
|
||||
LFSR_ATTR(-1, RM(UATTR(0)), 0, NULL()))) => 0;
|
||||
assert(rbyd.weight == 2);
|
||||
|
||||
// delete a neighbor to the hole
|
||||
lfsr_rbyd_commit(&lfs, &rbyd, LFSR_ATTRS(
|
||||
LFSR_ATTR(0, RM, -1, NULL()))) => 0;
|
||||
assert(rbyd.weight == 1);
|
||||
|
||||
// can we still access things?
|
||||
lfs_ssize_t rid_;
|
||||
lfsr_tag_t tag_;
|
||||
lfs_size_t weight_;
|
||||
lfsr_data_t data_;
|
||||
lfsr_rbyd_lookupnext(&lfs, &rbyd, -1, 0,
|
||||
&rid_, &tag_, &weight_, &data_) => 0;
|
||||
assert(rid_ == 0);
|
||||
assert(tag_ == LFSR_TAG_REG);
|
||||
assert(weight_ == 1);
|
||||
assert(lfsr_data_size(&data_) == 4);
|
||||
uint8_t rbuf[32];
|
||||
lfsr_data_read(&lfs, &data_, rbuf, 32) => 4;
|
||||
assert(memcmp(rbuf, "\xcc\xcc\xcc\xcc", 4) == 0);
|
||||
|
||||
lfsr_rbyd_lookupnext(&lfs, &rbyd, 0, LFSR_TAG_REG+1,
|
||||
&rid_, &tag_, &weight_, &data_) => LFS_ERR_NOENT;
|
||||
'''
|
||||
|
||||
[cases.test_rbyd_unreachable_hole_subwide]
|
||||
in = 'lfs.c'
|
||||
code = '''
|
||||
lfs_t lfs;
|
||||
lfs_init(&lfs, CFG) => 0;
|
||||
|
||||
lfsr_rbyd_t init_rbyd = {
|
||||
.blocks[0] = 0,
|
||||
.eoff = 0,
|
||||
.cksum = 0,
|
||||
.trunk = 0,
|
||||
.weight = 0,
|
||||
};
|
||||
lfsr_rbyd_t rbyd;
|
||||
|
||||
// create a null tag hole
|
||||
rbyd = init_rbyd;
|
||||
lfsr_rbyd_commit(&lfs, &rbyd, LFSR_ATTRS(
|
||||
LFSR_ATTR(-1, UATTR(0), 0, BUF("\xaa\xaa\xaa\xaa", 4)),
|
||||
LFSR_ATTR(-1, UATTR(1), 0, BUF("\xbb\xbb\xbb\xbb", 4)),
|
||||
LFSR_ATTR(-1, SATTR(0), 0, BUF("\xdd\xdd\xdd\xdd", 4)),
|
||||
LFSR_ATTR(-1, RM(UATTR(0)), 0, NULL()))) => 0;
|
||||
assert(rbyd.weight == 0);
|
||||
|
||||
// subwide replace a neighbor to the hole
|
||||
lfsr_rbyd_commit(&lfs, &rbyd, LFSR_ATTRS(
|
||||
LFSR_ATTR(-1,
|
||||
SUBWIDE(UATTR(2)), 0,
|
||||
BUF("\xcc\xcc\xcc\xcc", 4)))) => 0;
|
||||
assert(rbyd.weight == 0);
|
||||
|
||||
// can we still access things?
|
||||
lfs_ssize_t rid_;
|
||||
lfsr_tag_t tag_;
|
||||
lfs_size_t weight_;
|
||||
lfsr_data_t data_;
|
||||
lfsr_rbyd_lookupnext(&lfs, &rbyd, -1, 0,
|
||||
&rid_, &tag_, &weight_, &data_) => 0;
|
||||
assert(rid_ == -1);
|
||||
assert(tag_ == LFSR_TAG_UATTR(2));
|
||||
assert(weight_ == 0);
|
||||
assert(lfsr_data_size(&data_) == 4);
|
||||
uint8_t rbuf[32];
|
||||
lfsr_data_read(&lfs, &data_, rbuf, 32) => 4;
|
||||
assert(memcmp(rbuf, "\xcc\xcc\xcc\xcc", 4) == 0);
|
||||
|
||||
lfsr_rbyd_lookupnext(&lfs, &rbyd, -1, LFSR_TAG_UATTR(2)+1,
|
||||
&rid_, &tag_, &weight_, &data_) => 0;
|
||||
assert(rid_ == -1);
|
||||
assert(tag_ == LFSR_TAG_SATTR(0));
|
||||
assert(weight_ == 0);
|
||||
assert(lfsr_data_size(&data_) == 4);
|
||||
lfsr_data_read(&lfs, &data_, rbuf, 32) => 4;
|
||||
assert(memcmp(rbuf, "\xdd\xdd\xdd\xdd", 4) == 0);
|
||||
|
||||
lfsr_rbyd_lookupnext(&lfs, &rbyd, -1, LFSR_TAG_SATTR(0)+1,
|
||||
&rid_, &tag_, &weight_, &data_) => LFS_ERR_NOENT;
|
||||
'''
|
||||
|
||||
[cases.test_rbyd_unreachable_hole_supwide]
|
||||
in = 'lfs.c'
|
||||
code = '''
|
||||
lfs_t lfs;
|
||||
lfs_init(&lfs, CFG) => 0;
|
||||
|
||||
lfsr_rbyd_t init_rbyd = {
|
||||
.blocks[0] = 0,
|
||||
.eoff = 0,
|
||||
.cksum = 0,
|
||||
.trunk = 0,
|
||||
.weight = 0,
|
||||
};
|
||||
lfsr_rbyd_t rbyd;
|
||||
|
||||
// create a null tag hole
|
||||
rbyd = init_rbyd;
|
||||
lfsr_rbyd_commit(&lfs, &rbyd, LFSR_ATTRS(
|
||||
LFSR_ATTR(-1, UATTR(0), 0, BUF("\xaa\xaa\xaa\xaa", 4)),
|
||||
LFSR_ATTR(-1, UATTR(1), 0, BUF("\xbb\xbb\xbb\xbb", 4)),
|
||||
LFSR_ATTR(0, REG, +1, BUF("\xdd\xdd\xdd\xdd", 4)),
|
||||
LFSR_ATTR(-1, RM(UATTR(0)), 0, NULL()))) => 0;
|
||||
assert(rbyd.weight == 1);
|
||||
|
||||
// supwide replace a neighbor to the hole
|
||||
lfsr_rbyd_commit(&lfs, &rbyd, LFSR_ATTRS(
|
||||
LFSR_ATTR(-1,
|
||||
SUPWIDE(UATTR(2)), 0,
|
||||
BUF("\xcc\xcc\xcc\xcc", 4)))) => 0;
|
||||
assert(rbyd.weight == 1);
|
||||
|
||||
// can we still access things?
|
||||
lfs_ssize_t rid_;
|
||||
lfsr_tag_t tag_;
|
||||
lfs_size_t weight_;
|
||||
lfsr_data_t data_;
|
||||
lfsr_rbyd_lookupnext(&lfs, &rbyd, -1, 0,
|
||||
&rid_, &tag_, &weight_, &data_) => 0;
|
||||
assert(rid_ == -1);
|
||||
assert(tag_ == LFSR_TAG_UATTR(2));
|
||||
assert(weight_ == 0);
|
||||
assert(lfsr_data_size(&data_) == 4);
|
||||
uint8_t rbuf[32];
|
||||
lfsr_data_read(&lfs, &data_, rbuf, 32) => 4;
|
||||
assert(memcmp(rbuf, "\xcc\xcc\xcc\xcc", 4) == 0);
|
||||
|
||||
lfsr_rbyd_lookupnext(&lfs, &rbyd, -1, LFSR_TAG_UATTR(2)+1,
|
||||
&rid_, &tag_, &weight_, &data_) => 0;
|
||||
assert(rid_ == 0);
|
||||
assert(tag_ == LFSR_TAG_REG);
|
||||
assert(weight_ == 1);
|
||||
assert(lfsr_data_size(&data_) == 4);
|
||||
lfsr_data_read(&lfs, &data_, rbuf, 32) => 4;
|
||||
assert(memcmp(rbuf, "\xdd\xdd\xdd\xdd", 4) == 0);
|
||||
|
||||
lfsr_rbyd_lookupnext(&lfs, &rbyd, 0, LFSR_TAG_REG+1,
|
||||
&rid_, &tag_, &weight_, &data_) => LFS_ERR_NOENT;
|
||||
'''
|
||||
|
||||
|
||||
Reference in New Issue
Block a user