From 23d1cc4b84ad6d966f5a686ac6a93daf0239c455 Mon Sep 17 00:00:00 2001
From: joboet <jonasboettiger@icloud.com>
Date: Wed, 26 Jun 2024 00:06:16 +0200
Subject: [PATCH] core: avoid `extern` types in formatting infrastructure

---
 library/core/src/fmt/rt.rs | 46 +++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/library/core/src/fmt/rt.rs b/library/core/src/fmt/rt.rs
index 92626feabf3..5836ce59ec1 100644
--- a/library/core/src/fmt/rt.rs
+++ b/library/core/src/fmt/rt.rs
@@ -5,6 +5,7 @@
 
 use super::*;
 use crate::hint::unreachable_unchecked;
+use crate::ptr::NonNull;
 
 #[lang = "format_placeholder"]
 #[derive(Copy, Clone)]
@@ -66,7 +67,13 @@ pub(super) enum Flag {
 
 #[derive(Copy, Clone)]
 enum ArgumentType<'a> {
-    Placeholder { value: &'a Opaque, formatter: fn(&Opaque, &mut Formatter<'_>) -> Result },
+    Placeholder {
+        // INVARIANT: if `formatter` had type `fn(&T, _) -> _` then `value`
+        // was derived from a `&T` with lifetime `'a`.
+        value: NonNull<()>,
+        formatter: unsafe fn(NonNull<()>, &mut Formatter<'_>) -> Result,
+        _lifetime: PhantomData<&'a ()>,
+    },
     Count(usize),
 }
 
@@ -90,21 +97,15 @@ pub struct Argument<'a> {
 impl<'a> Argument<'a> {
     #[inline(always)]
     fn new<'b, T>(x: &'b T, f: fn(&T, &mut Formatter<'_>) -> Result) -> Argument<'b> {
-        // SAFETY: `mem::transmute(x)` is safe because
-        //     1. `&'b T` keeps the lifetime it originated with `'b`
-        //              (so as to not have an unbounded lifetime)
-        //     2. `&'b T` and `&'b Opaque` have the same memory layout
-        //              (when `T` is `Sized`, as it is here)
-        // `mem::transmute(f)` is safe since `fn(&T, &mut Formatter<'_>) -> Result`
-        // and `fn(&Opaque, &mut Formatter<'_>) -> Result` have the same ABI
-        // (as long as `T` is `Sized`)
-        unsafe {
-            Argument {
-                ty: ArgumentType::Placeholder {
-                    formatter: mem::transmute(f),
-                    value: mem::transmute(x),
-                },
-            }
+        Argument {
+            // INVARIANT: this creates an `ArgumentType<'b>` from a `&'b T` and
+            // a `fn(&T, ...)`, so the invariant is maintained.
+            ty: ArgumentType::Placeholder {
+                value: NonNull::from(x).cast(),
+                // SAFETY: function pointers always have the same layout.
+                formatter: unsafe { mem::transmute(f) },
+                _lifetime: PhantomData,
+            },
         }
     }
 
@@ -162,7 +163,14 @@ pub fn from_usize(x: &usize) -> Argument<'_> {
     #[inline(always)]
     pub(super) unsafe fn fmt(&self, f: &mut Formatter<'_>) -> Result {
         match self.ty {
-            ArgumentType::Placeholder { formatter, value } => formatter(value, f),
+            // SAFETY:
+            // Because of the invariant that if `formatter` had the type
+            // `fn(&T, _) -> _` then `value` has type `&'b T` where `'b` is
+            // the lifetime of the `ArgumentType`, and because references
+            // and `NonNull` are ABI-compatible, this is completely equivalent
+            // to calling the original function passed to `new` with the
+            // original reference, which is sound.
+            ArgumentType::Placeholder { formatter, value, .. } => unsafe { formatter(value, f) },
             // SAFETY: the caller promised this.
             ArgumentType::Count(_) => unsafe { unreachable_unchecked() },
         }
@@ -208,7 +216,3 @@ pub unsafe fn new() -> Self {
         Self { _private: () }
     }
 }
-
-extern "C" {
-    type Opaque;
-}