This adds support for running the simulator under Miri to detect UB.
There are a few things to note about Miri and its limitations
- It has limited `libc` coverage, so it's not really possible to have
Miri help with `UringIO`/`UringFile` or `UnixIO`/`UnixFile`. That's a
big gap ☹️
- It **can** work for `GenericIO`/`GenericFile`, which only uses `std`
- It can't call external C libraries, so even using `sqlite` is out
(hence adding `--disable-integrity-check` to the simulator for Miri use)
- It runs on nightly, consequently there are a few new lints that don't
exist on turso's pinned version of rustc
Some questions I have about this MR
- I made `GenericFile::{lock_file,unlock_file}` noops so I could use
`GenericIO`. This isn't great, but if/when you update from Rust 1.88.0
to 1.89.0, `std::File::{lock,lock_shared,unlock}` will be stabilized and
available. Should I note that as a TODO or something?
- Previously, the sim runner shelled out to `git` to get stuff like the
current git hash and the repo directory. For Miri, that's out, and so is
`git2`. Unfortunately, `gix` is also out since it has a required
dependency that uses inline assembly, which Miri doesn't like. I wrote a
hacky shim that uses only std to look for `.git` and find the hash that
HEAD is pointing to. It doesn't deal with stuff like packed-refs or the
repo being a secondary one made with `git worktree`. I'm happy to
support that, but wanted to hear from maintainers before doing more
work.
Two UB occurrences I already found:
- `TursoRwLock::read` used `AtomicU64::compare_exchange_weak`, which is
(evidently) [allowed to spuriously fail](https://doc.rust-lang.org/std/s
ync/atomic/struct.AtomicU64.html#method.compare_exchange_weak) in
exchange for perf. Miri forces this behavior, which triggers trivial
read deadlocks even with zero readers/writers. I changed it to
`compare_exchange`, but I'm not an atomics expert.
- Uninitialized read in non-Unix
`core::storage::buffer_pool::arena::alloc`. This is a simple one,
resolved by using `std::alloc::alloc_zeroed` instead of
`std::alloc::alloc`
Moving forward, I'd be interested in potentially getting the tests to
run in Miri, too. `tokio` looks like a good example of a project with
partial coverage that runs it where they can. They have some extra test
config to allow as many as possible to run under Miri, with
appropriately scaled-down parameter values since Miri is super slow
Closes#3720
Added the ability for us to generate `Drop Index` queries in the
simulator. Most of the code is just boilerplate and some checks to make
sure we do not generate `Drop Index` when we have no indexes to drop
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Reviewed-by: bit-aloo (@Shourya742)
Closes#3713
gix doesn't work here, since while it's pure Rust, it has a
non-configurable dependency on crates using inline assembly, which Miri
does not support. This commit is a bit of a hack, and only works in
non-bare git repos without e.g packed-refs.
Adds `ALTER TABLE` to the simulator. Currently, there are no properties
that generate `ALTER TABLE`. The query is only generated in
`Property::Query` or in extension queries.
Conditions to generate `ALTER TABLE`:
- In differential testing, do not generate `ALTER COLUMN` as SQLite does
not support it.
- If there is only 1 column, or all columns are present in indexes, do
not generate a `DROP COLUMN` as it would be an error in the database
- if there are no tables, obviously do not generate `ALTER TABLE`
Some fixes:
- handle NULL generation in `GTValue` and `LTValue`, as we now have to
handle nulls due to `ADD COLUMN` adding cols with NULL
- correctly compare NULLs in `binary_compare`
Closes#3650
We currently return the exact same error from two different IdxDelete
paths. Improve the messages with context about what we're doing to make
this error more debuggable.
Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>
Closes#3699
Add `--busy-timeout` command-line option to turso-stress with a default
value of 5000 ms. This helps prevent spurious database busy errors
during concurrent stress testing and ensure that integrity checks are
not skipped because of concurrent writes.
Closes#3696
