From 331710a16f49e2c9951fb70de499efe3636a0e3c Mon Sep 17 00:00:00 2001 From: Huang-Huang Bao Date: Tue, 19 Sep 2023 01:59:51 +0800 Subject: [PATCH] Use self_cell for creating self-referential struct Remove indigenous unsafe self-referential implemention which has a lesser chance to be audited. --- Cargo.toml | 2 +- src/font/mod.rs | 145 +++++++++++++++++------------------------------- 2 files changed, 51 insertions(+), 96 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a939103..6de8ca1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,6 @@ rust-version = "1.65" fontdb = { version = "0.14.1", default-features = false } libm = "0.2.7" log = "0.4.20" -aliasable = "0.1.3" rustybuzz = { version = "0.10.0", default-features = false, features = ["libm"] } swash = { version = "0.1.8", optional = true } syntect = { version = "5.1.0", optional = true } @@ -24,6 +23,7 @@ unicode-segmentation = "1.10.1" rangemap = "1.3.0" hashbrown = { version = "0.14.0", optional = true, default-features = false } rustc-hash = { version = "1.1.0", default-features = false } +self_cell = "1.0.1" [dependencies.unicode-bidi] version = "0.3.13" diff --git a/src/font/mod.rs b/src/font/mod.rs index af3c590..5be76c3 100644 --- a/src/font/mod.rs +++ b/src/font/mod.rs @@ -1,116 +1,68 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 pub(crate) mod fallback; -use alloc::boxed::Box; +use core::fmt; + use alloc::sync::Arc; +use rustybuzz::Face as RustybuzzFace; +use self_cell::self_cell; + pub use self::system::*; mod system; -pub use font_inner::Font; +self_cell!( + struct OwnedFace { + owner: Arc + Send + Sync>, -/// Encapsulates the self-referencing `Font` struct to ensure all field accesses have to go through -/// safe methods. -mod font_inner { - use super::*; - use aliasable::boxed::AliasableBox; - use core::fmt; + #[covariant] + dependent: RustybuzzFace, + } +); - /// A font - // - // # Safety invariant - // - // `data` must never have a mutable reference taken, nor be modified during the lifetime of - // this `Font`. - pub struct Font { - #[cfg(feature = "swash")] - swash: (u32, swash::CacheKey), - rustybuzz: rustybuzz::Face<'static>, - // Note: This field must be after rustybuzz to ensure that it is dropped later. Otherwise - // there would be a dangling reference when dropping rustybuzz. - data: aliasable::boxed::AliasableBox + Send + Sync>>, - id: fontdb::ID, +/// A font +pub struct Font { + #[cfg(feature = "swash")] + swash: (u32, swash::CacheKey), + rustybuzz: OwnedFace, + data: Arc + Send + Sync>, + id: fontdb::ID, +} + +impl fmt::Debug for Font { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Font") + .field("id", &self.id) + .finish_non_exhaustive() + } +} + +impl Font { + pub fn id(&self) -> fontdb::ID { + self.id } - impl fmt::Debug for Font { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Font") - .field("id", &self.id) - .finish_non_exhaustive() - } + pub fn data(&self) -> &[u8] { + (*self.data).as_ref() } - pub(super) struct FontTryBuilder< - RustybuzzBuilder: for<'this> FnOnce( - &'this Arc + Send + Sync>, - ) -> Option>, - > { - pub(super) id: fontdb::ID, - pub(super) data: Arc + Send + Sync>, - pub(super) rustybuzz_builder: RustybuzzBuilder, - #[cfg(feature = "swash")] - pub(super) swash: (u32, swash::CacheKey), - } - impl< - RustybuzzBuilder: for<'this> FnOnce( - &'this Arc + Send + Sync>, - ) -> Option>, - > FontTryBuilder - { - pub(super) fn try_build(self) -> Option { - unsafe fn change_lifetime<'old, 'new: 'old, T: 'new>(data: &'old T) -> &'new T { - &*(data as *const _) - } - - let data: AliasableBox + Send + Sync>> = - AliasableBox::from_unique(Box::new(self.data)); - - // Safety: We use AliasableBox to allow the references in rustybuzz::Face to alias with - // the data stored behind the AliasableBox. In addition the entire public interface of - // Font ensures that no mutable reference is given to data. And finally we use - // for<'this> for the rustybuzz_builder to ensure it can't leak a reference. Combined - // this ensures that it is sound to produce a self-referential type. - let rustybuzz = (self.rustybuzz_builder)(unsafe { change_lifetime(&*data) })?; - - Some(Font { - id: self.id, - data, - rustybuzz, - #[cfg(feature = "swash")] - swash: self.swash, - }) - } + pub fn rustybuzz(&self) -> &RustybuzzFace<'_> { + self.rustybuzz.borrow_dependent() } - impl Font { - pub fn id(&self) -> fontdb::ID { - self.id - } - - pub fn data(&self) -> &[u8] { - // Safety: This only gives an immutable access to `data`. - (**self.data).as_ref() - } - - pub fn rustybuzz(&self) -> &rustybuzz::Face<'_> { - &self.rustybuzz - } - - #[cfg(feature = "swash")] - pub fn as_swash(&self) -> swash::FontRef<'_> { - let swash = &self.swash; - swash::FontRef { - data: self.data(), - offset: swash.0, - key: swash.1, - } + #[cfg(feature = "swash")] + pub fn as_swash(&self) -> swash::FontRef<'_> { + let swash = &self.swash; + swash::FontRef { + data: self.data(), + offset: swash.0, + key: swash.1, } } } impl Font { pub fn new(info: &fontdb::FaceInfo) -> Option { - #[allow(unused_variables)] let data = match &info.source { fontdb::Source::Binary(data) => Arc::clone(data), #[cfg(feature = "std")] @@ -121,16 +73,19 @@ impl Font { #[cfg(feature = "std")] fontdb::Source::SharedFile(_path, data) => Arc::clone(data), }; - font_inner::FontTryBuilder { + + Some(Self { id: info.id, #[cfg(feature = "swash")] swash: { let swash = swash::FontRef::from_index((*data).as_ref(), info.index as usize)?; (swash.offset, swash.key) }, + rustybuzz: OwnedFace::try_new(Arc::clone(&data), |data| { + RustybuzzFace::from_slice((**data).as_ref(), info.index).ok_or(()) + }) + .ok()?, data, - rustybuzz_builder: |data| rustybuzz::Face::from_slice((**data).as_ref(), info.index), - } - .try_build() + }) } }