Open sgraham opened 1 year ago
I just discovered a bug in hashmap.c, which I found quite surprising as I'd been using it for quite a while and on some pretty large codebases!
I think the logic for TOMBSTONE is incorrect which can manifest with #undef when used by the preprocessor.
TOMBSTONE
#undef
In particular:
delete
put
rehash()
assert()
I think the only easy solution is to never re-use TOMBSTONEs when doing a get_or_insert_entry().
get_or_insert_entry()
I put the test data that discovered this at https://github.com/rui314/chibicc/pull/134 for reference (run ./chibicc -hashmap-test).
./chibicc -hashmap-test
Ah, I see the problem. That's indeed a bug, and it's hard to believe I haven't noticed before. Thank you for pointing it out!
Actually, you have! #53
Oops, I should have done some more searching first I guess!
I just discovered a bug in hashmap.c, which I found quite surprising as I'd been using it for quite a while and on some pretty large codebases!
I think the logic for
TOMBSTONE
is incorrect which can manifest with#undef
when used by the preprocessor.In particular:
delete
d, so its key is marked with aTOMBSTONE
put
: during probing, theTOMBSTONE
from B's deletion will be found (https://github.com/rui314/chibicc/blob/main/hashmap.c#L92 ), and so there will be a duplicate of A added as the later entry will not be found.rehash()
this will eventuallyassert()
, but in the interim, incorrect values would be retrieved.I think the only easy solution is to never re-use
TOMBSTONE
s when doing aget_or_insert_entry()
.I put the test data that discovered this at https://github.com/rui314/chibicc/pull/134 for reference (run
./chibicc -hashmap-test
).