Mercurial > hg
changeset 50682:2cc5de261d76
rust-hg-core: move from `ouroboros` to `self_cell`
`ouroboros` has a fundamental soundness problem that, while not applicable
today, could become applicable given new compiler optimizations.ยน
`self_cell` is a crate that accomplishes a lot of the same things that
`ouroboros` did while remaining sound (that is, unless a new soundness issue
is discovered) by not assuming as much about the memory layout of the program.
`self_cell` has been scrutinized heavily in the past few months by very
competent people, some from the compiler team and has shown no weaknesses
for a while, with a 1.0 stable release coming out a couple months ago.
Our internal API is exactly the same, this is just an implementation detail.
To reiterate, no actual soundness issue was found with our use of `ouroboros`,
but there might be evolutions of `rustc` (or even a future separate compiler)
that could generate unsound code.
[1] https://github.com/joshua-maros/ouroboros/issues/88
author | Raphaël Gomès <rgomes@octobus.net> |
---|---|
date | Mon, 12 Jun 2023 23:41:28 +0200 |
parents | 47b44d80d836 |
children | d39ac3468ad4 |
files | rust/Cargo.lock rust/hg-core/Cargo.toml rust/hg-core/src/dirstate_tree/owning.rs |
diffstat | 3 files changed, 31 insertions(+), 73 deletions(-) [+] |
line wrap: on
line diff
--- a/rust/Cargo.lock Fri Jun 02 15:12:05 2023 +0200 +++ b/rust/Cargo.lock Mon Jun 12 23:41:28 2023 +0200 @@ -3,12 +3,6 @@ version = 3 [[package]] -name = "Inflector" -version = "0.11.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fe438c63458706e03479442743baae6c88256498e6431708f6dfc520a26515d3" - -[[package]] name = "adler" version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -35,12 +29,6 @@ ] [[package]] -name = "aliasable" -version = "0.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "250f629c0161ad8107cf89319e990051fae62832fd343083bea452d93e2205fd" - -[[package]] name = "android_system_properties" version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -539,7 +527,6 @@ "logging_timer", "memmap2", "once_cell", - "ouroboros", "pretty_assertions", "rand 0.8.5", "rand_distr", @@ -547,6 +534,7 @@ "rayon", "regex", "same-file", + "self_cell", "sha-1 0.10.0", "tempfile", "thread_local", @@ -809,29 +797,6 @@ checksum = "7b5bf27447411e9ee3ff51186bf7a08e16c341efdde93f4d823e8844429bed7e" [[package]] -name = "ouroboros" -version = "0.15.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dfbb50b356159620db6ac971c6d5c9ab788c9cc38a6f49619fca2a27acb062ca" -dependencies = [ - "aliasable", - "ouroboros_macro", -] - -[[package]] -name = "ouroboros_macro" -version = "0.15.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4a0d9d1a6191c4f391f87219d1ea42b23f09ee84d64763cd05ee6ea88d9f384d" -dependencies = [ - "Inflector", - "proc-macro-error", - "proc-macro2", - "quote", - "syn", -] - -[[package]] name = "output_vt100" version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -1130,6 +1095,12 @@ checksum = "9c8132065adcfd6e02db789d9285a0deb2f3fcb04002865ab67d5fb103533898" [[package]] +name = "self_cell" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4a3926e239738d36060909ffe6f511502f92149a45a1fade7fe031cb2d33e88b" + +[[package]] name = "semver" version = "1.0.14" source = "registry+https://github.com/rust-lang/crates.io-index"
--- a/rust/hg-core/Cargo.toml Fri Jun 02 15:12:05 2023 +0200 +++ b/rust/hg-core/Cargo.toml Mon Jun 12 23:41:28 2023 +0200 @@ -20,12 +20,12 @@ lazy_static = "1.4.0" libc = "0.2.137" logging_timer = "1.1.0" -ouroboros = "0.15.5" rand = "0.8.5" rand_pcg = "0.3.1" rand_distr = "0.4.3" rayon = "1.7.0" regex = "1.7.0" +self_cell = "1.0" sha-1 = "0.10.0" twox-hash = "1.6.3" same-file = "1.0.6"
--- a/rust/hg-core/src/dirstate_tree/owning.rs Fri Jun 02 15:12:05 2023 +0200 +++ b/rust/hg-core/src/dirstate_tree/owning.rs Mon Jun 12 23:41:28 2023 +0200 @@ -1,19 +1,18 @@ use crate::{DirstateError, DirstateParents}; use super::dirstate_map::DirstateMap; +use self_cell::self_cell; use std::ops::Deref; -use ouroboros::self_referencing; - -/// Keep a `DirstateMap<'on_disk>` next to the `on_disk` buffer that it -/// borrows. -#[self_referencing] -pub struct OwningDirstateMap { - on_disk: Box<dyn Deref<Target = [u8]> + Send>, - #[borrows(on_disk)] - #[covariant] - map: DirstateMap<'this>, -} +self_cell!( + /// Keep a `DirstateMap<'owner>` next to the `owner` buffer that it + /// borrows. + pub struct OwningDirstateMap { + owner: Box<dyn Deref<Target = [u8]> + Send>, + #[covariant] + dependent: DirstateMap, + } +); impl OwningDirstateMap { pub fn new_empty<OnDisk>(on_disk: OnDisk) -> Self @@ -22,11 +21,7 @@ { let on_disk = Box::new(on_disk); - OwningDirstateMapBuilder { - on_disk, - map_builder: |bytes| DirstateMap::empty(bytes), - } - .build() + OwningDirstateMap::new(on_disk, |bytes| DirstateMap::empty(bytes)) } pub fn new_v1<OnDisk>( @@ -40,16 +35,12 @@ let mut parents = DirstateParents::NULL; Ok(( - OwningDirstateMapTryBuilder { - on_disk, - map_builder: |bytes| { - DirstateMap::new_v1(bytes, identity).map(|(dmap, p)| { - parents = p.unwrap_or(DirstateParents::NULL); - dmap - }) - }, - } - .try_build()?, + OwningDirstateMap::try_new(on_disk, |bytes| { + DirstateMap::new_v1(bytes, identity).map(|(dmap, p)| { + parents = p.unwrap_or(DirstateParents::NULL); + dmap + }) + })?, parents, )) } @@ -66,28 +57,24 @@ { let on_disk = Box::new(on_disk); - OwningDirstateMapTryBuilder { - on_disk, - map_builder: |bytes| { - DirstateMap::new_v2(bytes, data_size, metadata, uuid, identity) - }, - } - .try_build() + OwningDirstateMap::try_new(on_disk, |bytes| { + DirstateMap::new_v2(bytes, data_size, metadata, uuid, identity) + }) } pub fn with_dmap_mut<R>( &mut self, f: impl FnOnce(&mut DirstateMap) -> R, ) -> R { - self.with_map_mut(f) + self.with_dependent_mut(|_owner, dmap| f(dmap)) } pub fn get_map(&self) -> &DirstateMap { - self.borrow_map() + self.borrow_dependent() } pub fn on_disk(&self) -> &[u8] { - self.borrow_on_disk() + self.borrow_owner() } pub fn old_uuid(&self) -> Option<&[u8]> {