From 046d7a47f4ebe85ef172df8fd03c3b26559df28e Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Fri, 7 Jun 2024 09:28:16 +1000 Subject: [PATCH] document all unsafe blocks --- core/src/ctxt.rs | 14 ++++++++++---- core/src/path.rs | 2 +- core/src/props.rs | 1 + core/src/runtime.rs | 12 ++++++++---- core/src/str.rs | 6 +++++- emitter/file/Cargo.toml | 5 +++++ emitter/file/src/lib.rs | 5 ++++- emitter/otlp/src/client/http.rs | 8 ++++++++ src/frame.rs | 4 ++++ src/macro_hooks.rs | 1 + 10 files changed, 47 insertions(+), 11 deletions(-) diff --git a/core/src/ctxt.rs b/core/src/ctxt.rs index de025d2..4587d8f 100644 --- a/core/src/ctxt.rs +++ b/core/src/ctxt.rs @@ -235,11 +235,13 @@ mod internal { pub struct Slot(*const T, PhantomData<*mut fn()>); impl Slot { + // SAFETY: `Slot` must not outlive `&T` pub(super) unsafe fn new(v: &T) -> Slot { Slot(v as *const T, PhantomData) } pub(super) fn get(&self) -> &T { + // SAFETY: `Slot` must not outlive `&T`, as per `Slot::new` unsafe { &*self.0 } } } @@ -256,8 +258,8 @@ mod internal { #[cfg(feature = "alloc")] mod alloc_support { - use core::any::Any; use alloc::boxed::Box; + use core::any::Any; use crate::props::ErasedProps; @@ -275,7 +277,7 @@ mod alloc_support { use super::ErasedFrame; pub trait DispatchCtxt { - fn dispatch_with_current(&self, with: &mut dyn FnMut(ErasedCurrent)); + fn dispatch_with_current(&self, with: &mut dyn FnMut(&ErasedCurrent)); fn dispatch_open_root(&self, props: &dyn ErasedProps) -> ErasedFrame; fn dispatch_open_push(&self, props: &dyn ErasedProps) -> ErasedFrame; @@ -294,6 +296,7 @@ mod alloc_support { ); impl ErasedCurrent { + // SAFETY: `ErasedCurrent` must not outlive `&v` pub(super) unsafe fn new<'a>(v: &'a impl Props) -> Self { let v: &'a dyn ErasedProps = v; let v: &'a (dyn ErasedProps + 'static) = @@ -303,6 +306,7 @@ mod alloc_support { } pub(super) fn get<'a>(&'a self) -> &'a (dyn ErasedProps + 'a) { + // SAFETY: `ErasedCurrent` must not outlive `&v`, as per `ErasedCurrent::new` unsafe { &*self.0 } } } @@ -344,8 +348,10 @@ mod alloc_support { where C::Frame: Send + 'static, { - fn dispatch_with_current(&self, with: &mut dyn FnMut(internal::ErasedCurrent)) { - self.with_current(move |props| with(unsafe { internal::ErasedCurrent::new(&props) })) + fn dispatch_with_current(&self, with: &mut dyn FnMut(&internal::ErasedCurrent)) { + // SAFETY: The borrow passed to `with` is arbitarily short, so `ErasedCurrent::get` + // cannot outlive `props` + self.with_current(move |props| with(&unsafe { internal::ErasedCurrent::new(&props) })) } fn dispatch_open_root(&self, props: &dyn ErasedProps) -> ErasedFrame { diff --git a/core/src/path.rs b/core/src/path.rs index 7e70db1..54a1b04 100644 --- a/core/src/path.rs +++ b/core/src/path.rs @@ -242,7 +242,7 @@ impl<'a> serde::Serialize for Path<'a> { #[cfg(feature = "alloc")] mod alloc_support { - use alloc::{boxed::Box, borrow::Cow}; + use alloc::{borrow::Cow, boxed::Box}; use super::*; diff --git a/core/src/props.rs b/core/src/props.rs index 01e495a..df62792 100644 --- a/core/src/props.rs +++ b/core/src/props.rs @@ -297,6 +297,7 @@ mod alloc_support { impl Dedup

{ pub(super) fn new<'a>(props: &'a P) -> &'a Dedup

{ + // SAFETY: `Dedup

` and `P` have the same ABI unsafe { &*(props as *const P as *const Dedup

) } } } diff --git a/core/src/runtime.rs b/core/src/runtime.rs index cba699d..b68e16c 100644 --- a/core/src/runtime.rs +++ b/core/src/runtime.rs @@ -487,8 +487,8 @@ impl Rng for AssertInternal { #[cfg(feature = "std")] mod std_support { - use core::any::Any; use alloc::boxed::Box; + use core::any::Any; use std::sync::OnceLock; use crate::{ @@ -726,9 +726,13 @@ mod std_support { self.0 .get() - .map(|rt| unsafe { - &*(&rt.runtime as *const AmbientSyncRuntime as *const AmbientRuntime) - }) + .map(|rt| + // SAFETY: The borrow of `self` cannot outlive the components + // it contains. This block is converting `*const dyn T + Send + Sync` + // to `&'_ dyn T + Send + Sync` + unsafe { + &*(&rt.runtime as *const AmbientSyncRuntime as *const AmbientRuntime) + }) .unwrap_or(&EMPTY_AMBIENT_RUNTIME) } } diff --git a/core/src/str.rs b/core/src/str.rs index 4f33217..94100ca 100644 --- a/core/src/str.rs +++ b/core/src/str.rs @@ -176,6 +176,7 @@ impl<'k> Str<'k> { // NOTE: It's important here that the lifetime returned is not `'k` // If it was it would be possible to return a `&'static str` from // an owned value + // SAFETY: `self.value` is guaranteed to outlive the borrow of `self` unsafe { &(*self.value) } } @@ -339,7 +340,10 @@ impl<'k> serde::Serialize for Str<'k> { #[cfg(feature = "alloc")] mod alloc_support { - use alloc::{borrow::{Cow, ToOwned}, string::String}; + use alloc::{ + borrow::{Cow, ToOwned}, + string::String, + }; use super::*; diff --git a/emitter/file/Cargo.toml b/emitter/file/Cargo.toml index aef42d5..d1380d4 100644 --- a/emitter/file/Cargo.toml +++ b/emitter/file/Cargo.toml @@ -37,3 +37,8 @@ path = "../../batcher" [dependencies.rand] version = "0.8" + +[dev-dependencies.emit] +version = "0.11.0-alpha.1" +path = "../../" +features = ["implicit_rt"] diff --git a/emitter/file/src/lib.rs b/emitter/file/src/lib.rs index 14b8cdc..0f85eea 100644 --- a/emitter/file/src/lib.rs +++ b/emitter/file/src/lib.rs @@ -482,7 +482,10 @@ fn default_writer( ) -> io::Result<()> { use std::ops::ControlFlow; - use emit::{Props as _, well_known::{KEY_MSG, KEY_TPL, KEY_TS, KEY_TS_START}}; + use emit::{ + well_known::{KEY_MSG, KEY_TPL, KEY_TS, KEY_TS_START}, + Props as _, + }; struct EventValue<'a, P>(&'a emit::Event<'a, P>); diff --git a/emitter/otlp/src/client/http.rs b/emitter/otlp/src/client/http.rs index 306cca7..47dae1a 100644 --- a/emitter/otlp/src/client/http.rs +++ b/emitter/otlp/src/client/http.rs @@ -630,6 +630,7 @@ impl HttpResponse { type Output = Result; fn poll(self: Pin<&mut Self>, ctx: &mut task::Context<'_>) -> task::Poll { + // SAFETY: `self` does not use interior pinning let BufNext(incoming, body, trailer) = unsafe { Pin::get_unchecked_mut(self) }; match Pin::new(incoming).poll_frame(ctx) { @@ -675,13 +676,17 @@ impl hyper::rt::Read for HttpIo { cx: &mut Context<'_>, mut buf: hyper::rt::ReadBufCursor<'_>, ) -> Poll> { + // SAFETY: `io` inherits the pinning requirements of `self` let io = unsafe { self.map_unchecked_mut(|io| &mut io.0) }; + // SAFETY: `io` does not uninitialize any bytes let mut read_buf = tokio::io::ReadBuf::uninit(unsafe { buf.as_mut() }); match tokio::io::AsyncRead::poll_read(io, cx, &mut read_buf) { Poll::Ready(Ok(())) => { let read = read_buf.filled().len(); + + // SAFETY: The bytes being advanced have been initialized by `read_buf` unsafe { buf.advance(read) }; Poll::Ready(Ok(())) @@ -698,12 +703,14 @@ impl hyper::rt::Write for HttpIo { cx: &mut Context<'_>, buf: &[u8], ) -> Poll> { + // SAFETY: `io` inherits the pinning requirements of `self` let io = unsafe { self.map_unchecked_mut(|io| &mut io.0) }; tokio::io::AsyncWrite::poll_write(io, cx, buf) } fn poll_flush(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + // SAFETY: `io` inherits the pinning requirements of `self` let io = unsafe { self.map_unchecked_mut(|io| &mut io.0) }; tokio::io::AsyncWrite::poll_flush(io, cx) @@ -713,6 +720,7 @@ impl hyper::rt::Write for HttpIo { self: Pin<&mut Self>, cx: &mut Context<'_>, ) -> Poll> { + // SAFETY: `io` inherits the pinning requirements of `self` let io = unsafe { self.map_unchecked_mut(|io| &mut io.0) }; tokio::io::AsyncWrite::poll_shutdown(io, cx) diff --git a/src/frame.rs b/src/frame.rs index b72efff..6eaa6e3 100644 --- a/src/frame.rs +++ b/src/frame.rs @@ -132,6 +132,7 @@ impl<'a, C: Ctxt> Drop for EnterGuard<'a, C> { impl Drop for Frame { fn drop(&mut self) { + // SAFETY: We're being dropped, so won't access `scope` again self.ctxt .close(unsafe { mem::ManuallyDrop::take(&mut self.scope) }) } @@ -150,9 +151,12 @@ impl Future for FrameFuture { #[track_caller] fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + // SAFETY: The fields of `FrameFuture` remain pinned let unpinned = unsafe { Pin::get_unchecked_mut(self) }; let __guard = unpinned.frame.enter(); + + // SAFETY: `FrameFuture::future` is pinned unsafe { Pin::new_unchecked(&mut unpinned.future) }.poll(cx) } } diff --git a/src/macro_hooks.rs b/src/macro_hooks.rs index d89d9f2..832f244 100644 --- a/src/macro_hooks.rs +++ b/src/macro_hooks.rs @@ -627,6 +627,7 @@ impl __PrivateMacroProps<'static> { impl<'a> __PrivateMacroProps<'a> { pub fn new_ref<'b>(props: &'b [(Str<'a>, Option>)]) -> &'b Self { + // SAFETY: `__PrivateMacroProps` and the array have the same ABI unsafe { &*(props as *const [(Str<'a>, Option>)] as *const __PrivateMacroProps<'a>) }