From 2b6a933538621ced69273d478b30f184b1605a06 Mon Sep 17 00:00:00 2001 From: Charles Hall Date: Sun, 12 May 2024 19:10:31 -0700 Subject: [PATCH] enable `undocumented_unsafe_blocks` lint There was only one unsafe block (thankfully) but it also had no docs. I did some reading and found out this in fact safe, but only for cursed reasons, and documented them. Also, the name of the type was misleading, as the entire point is the aliasing, and `Box` is already non-aliasing. --- Cargo.toml | 1 + src/database/abstraction/sqlite.rs | 29 ++++++++++++++++++++++------- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 46c5c8f5..29b73a30 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,6 +57,7 @@ string_to_string = "warn" suspicious_xor_used_as_pow = "warn" tests_outside_test_module = "warn" try_err = "warn" +undocumented_unsafe_blocks = "warn" unnecessary_safety_comment = "warn" unnecessary_safety_doc = "warn" unnecessary_self_imports = "warn" diff --git a/src/database/abstraction/sqlite.rs b/src/database/abstraction/sqlite.rs index 1ec56c59..51490442 100644 --- a/src/database/abstraction/sqlite.rs +++ b/src/database/abstraction/sqlite.rs @@ -19,7 +19,7 @@ thread_local! { struct PreparedStatementIterator<'a> { pub(crate) iterator: Box + 'a>, - pub(crate) _statement_ref: NonAliasingBox>, + pub(crate) _statement_ref: AliasableBox>, } impl Iterator for PreparedStatementIterator<'_> { @@ -30,10 +30,25 @@ impl Iterator for PreparedStatementIterator<'_> { } } -struct NonAliasingBox(*mut T); -impl Drop for NonAliasingBox { +struct AliasableBox(*mut T); +impl Drop for AliasableBox { fn drop(&mut self) { - drop(unsafe { Box::from_raw(self.0) }); + // SAFETY: This is cursed and relies on non-local reasoning. + // + // In order for this to be safe: + // + // * All aliased references to this value must have been dropped first, + // for example by coming after its referrers in struct fields, because + // struct fields are automatically dropped in order from top to bottom + // in the absence of an explicit Drop impl. Otherwise, the referrers + // may read into deallocated memory. + // * This type must not be copyable or cloneable. Otherwise, double-free + // can occur. + // + // These conditions are met, but again, note that changing safe code in + // this module can result in unsoundness if any of these constraints are + // violated. + unsafe { drop(Box::from_raw(self.0)) } } } @@ -171,7 +186,7 @@ impl SqliteTable { .unwrap(), )); - let statement_ref = NonAliasingBox(statement); + let statement_ref = AliasableBox(statement); //let name = self.name.clone(); @@ -270,7 +285,7 @@ impl KvTree for SqliteTable { .unwrap(), )); - let statement_ref = NonAliasingBox(statement); + let statement_ref = AliasableBox(statement); let iterator = Box::new( statement @@ -292,7 +307,7 @@ impl KvTree for SqliteTable { .unwrap(), )); - let statement_ref = NonAliasingBox(statement); + let statement_ref = AliasableBox(statement); let iterator = Box::new( statement