stacked_borrow now has an item module, and its own FrameExtra. These
serve to protect the implementation of Item (which is a bunch of
bit-packing tricks) from the primary logic of Stacked Borrows, and the
FrameExtra we have separates Stacked Borrows more cleanly from the
interpreter itself.
The new strategy for checking protectors also makes some subtle
performance tradeoffs, so they are now documented in Stack::item_popped
because that function primarily benefits from them, and it also touches
every aspect of them.
Also separating the actual CallId that is protecting a Tag from the Tag
makes it inconvienent to reproduce exactly the same protector errors, so
this also takes the opportunity to use some slightly cleaner English in
those errors. We need to make some change, might as well make it good.
Previously, Item was a struct of a NonZeroU64, an Option which was
usually unset or irrelevant, and a 4-variant enum. So collectively, the
size of an Item was 24 bytes, but only 8 bytes were used for the most
part.
So this takes advantage of the fact that it is probably impossible to
exhaust the total space of SbTags, and steals 3 bits from it to pack the
whole struct into a single u64. This bit-packing means that we reduce
peak memory usage when Miri goes memory-bound by ~3x. We also get CPU
performance improvements of varying size, because not only are we simply
accessing less memory, we can now compare a Vec<Item> using a memcmp
because it does not have any padding.
Print spans where tags are created and invalidated
5225225 called this "automatic tag tracking" and I think that may be a reasonable description, but I would like to kill tag tracking as a primary use of Miri if possible. Tag tracking isn't always possible; for example if the UB is only detected with isolation off and the failing tag is made unstable by removing isolation. (also it's bad UX to run the tool twice)
This is just one of the things we can do with https://github.com/rust-lang/miri/pull/2024
The memory usage of this is _shockingly_ low, I think because the memory usage of Miri is driven by allocations where each byte ends up with its own very large stack. The memory usage in this change is linear with the number of tags, not tags * bytes. If memory usage gets out of control we can cap the number of events we save per allocation, from experience we tend to only use the most recent few in diagnostics but of course there's no guarantee of that so if we can manage to keep everything that would be best.
In many cases now I can tell exactly what these codebases are doing wrong just from the new outputs here, which I think is extremely cool.
New helps generated with plain old `cargo miri test` on `rust-argon2` v1.0.0:
```
test argon2::tests::single_thread_verification_multi_lane_hash ... error: Undefined Behavior: trying to reborrow <1485898> for Unique permission at alloc110523[0x0], but that tag does not exist in the borrow stack for this location
--> /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/mem/manually_drop.rs:89:9
|
89 | slot.value
| ^^^^^^^^^^
| |
| trying to reborrow <1485898> for Unique permission at alloc110523[0x0], but that tag does not exist in the borrow stack for this location
| this error occurs as part of a reborrow at alloc110523[0x0..0x20]
|
= help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <1485898> was created by a retag at offsets [0x0..0x20]
--> src/memory.rs:42:13
|
42 | vec.push(unsafe { &mut (*ptr) });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: <1485898> was later invalidated at offsets [0x0..0x20]
--> src/memory.rs:42:31
|
42 | vec.push(unsafe { &mut (*ptr) });
| ^^^^^^^^^^^
```
And with `-Zmiri-tag-raw-pointers` on `slab` v0.4.5
```
error: Undefined Behavior: trying to reborrow <2915> for Unique permission at alloc1418[0x0], but that tag does not exist in the borrow stack for this location
--> /tmp/slab-0.4.5/src/lib.rs:835:16
|
835 | match (&mut *ptr1, &mut *ptr2) {
| ^^^^^^^^^^
| |
| trying to reborrow <2915> for Unique permission at alloc1418[0x0], but that tag does not exist in the borrow stack for this location
| this error occurs as part of a reborrow at alloc1418[0x0..0x10]
|
= help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <2915> was created by a retag at offsets [0x0..0x10]
--> /tmp/slab-0.4.5/src/lib.rs:833:20
|
833 | let ptr1 = self.entries.get_unchecked_mut(key1) as *mut Entry<T>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: <2915> was later invalidated at offsets [0x0..0x20]
--> /tmp/slab-0.4.5/src/lib.rs:834:20
|
834 | let ptr2 = self.entries.get_unchecked_mut(key2) as *mut Entry<T>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
And without raw pointer tagging, `cargo miri test` on `half` v1.8.2
```
error: Undefined Behavior: trying to reborrow <untagged> for Unique permission at alloc1340[0x0], but that tag only grants SharedReadOnly permission for this location
--> /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/slice/raw.rs:141:9
|
141 | &mut *ptr::slice_from_raw_parts_mut(data, len)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| trying to reborrow <untagged> for Unique permission at alloc1340[0x0], but that tag only grants SharedReadOnly permission for this location
| this error occurs as part of a reborrow at alloc1340[0x0..0x6]
|
= help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: tag was most recently created at offsets [0x0..0x6]
--> /tmp/half-1.8.2/src/slice.rs:309:22
|
309 | let length = self.len();
| ^^^^^^^^^^
help: this tag was also created here at offsets [0x0..0x6]
--> /tmp/half-1.8.2/src/slice.rs:308:23
|
308 | let pointer = self.as_ptr() as *mut u16;
| ^^^^^^^^^^^^^
```
The second suggestion is close to guesswork, but from experience it tends to be correct (as in, it tends to locate the pointer the user wanted) more often that it doesn't.
* Store the local crates in an Rc<[CrateNum]>
* Move all the allocation history into Stacks
* Clean up the implementation of get_logs_relevant_to a bit
Ralf Jung