style: clean up and add SAFETY comments

Author: morrisonlevi

profiling/src/profiling/stack_walking.rs

@@ -89,7 +89,7 @@ unsafe fn extract_file_and_line(execute_data: &zend_execute_data) -> (Option<Thi
 mod detail {
     use super::*;
     use crate::string_set::StringSet;
-    use datadog_thin_str::ThinStr;
+    use datadog_thin_str::{ThinHeader, ThinStr};
     use log::{debug, trace};
     use std::cell::RefCell;
     use std::ops::Deref;
@@ -103,34 +103,37 @@ mod detail {
         string_set: &'a mut StringSet,
     }
 
+    #[repr(usize)]
+    enum CacheSlot {
+        Function = 0,
+        Filename = 1,
+    }
+
     impl<'a> StringCache<'a> {
         /// Makes a copy of the string in the cache slot. If there isn't a
         /// string in the slot currently, then create one by calling the
         /// provided function, store it in the string cache and cache slot,
         /// and return it.
-        fn get_or_insert<F>(&mut self, slot: usize, f: F) -> Option<ThinString>
+        fn get_or_insert<F>(&mut self, slot: CacheSlot, f: F) -> Option<ThinString>
         where
             F: FnOnce() -> Option<ThinString>,
         {
-            debug_assert!(slot < self.cache_slots.len());
-            let cached = unsafe { self.cache_slots.get_unchecked_mut(slot) };
+            // SAFETY: the slot is in-bounds from CacheSlot -> usize conv.
+            let cached = unsafe { self.cache_slots.get_unchecked_mut(slot as usize) };
 
-            let ptr = *cached as *mut u8;
+            let ptr = *cached as *mut ThinHeader;
             match NonNull::new(ptr) {
                 Some(non_null) => {
-                    // SAFETY: transmuting ThinStr from its repr.
-                    let thin_str: ThinStr = unsafe { core::mem::transmute(non_null) };
                     // SAFETY: the string set is only reset between requests,
-                    // so this ThinStr points into the same string set that
-                    // created it.
-                    let str = unsafe { self.string_set.get_thin_str(thin_str) };
-                    Some(ThinString::from_str_in(str, Global))
+                    // so this ThinStr points into the same string set arena
+                    // that created it.
+                    let thin_str: ThinStr = unsafe { ThinStr::from_raw(non_null) };
+                    Some(ThinString::from_str_in(thin_str.deref(), Global))
                 }
                 None => {
                     let string = f()?;
                     let thin_str = self.string_set.insert(&string);
-                    // SAFETY: transmuting ThinStr into its repr.
-                    let non_null: NonNull<u8> = unsafe { core::mem::transmute(thin_str) };
+                    let non_null = thin_str.header_ptr();
                     *cached = non_null.as_ptr() as usize;
                     Some(string)
                 }
@@ -321,15 +324,16 @@ mod detail {
         func: &zend_function,
         string_cache: &mut StringCache,
     ) -> Option<ThinString> {
-        let fname = string_cache.get_or_insert(0, || extract_function_name(func))?;
+        let fname =
+            string_cache.get_or_insert(CacheSlot::Function, || extract_function_name(func))?;
         Some(ThinString::from_str_in(&fname, Global))
     }
 
     unsafe fn handle_file_cache_slot(
         execute_data: &zend_execute_data,
         string_cache: &mut StringCache,
     ) -> (Option<ThinString>, u32) {
-        let option = string_cache.get_or_insert(1, || -> Option<ThinString> {
+        let option = string_cache.get_or_insert(CacheSlot::Filename, || -> Option<ThinString> {
             unsafe {
                 // Safety: if we have cache slots, we definitely have a func.
                 let func = &*execute_data.func;

profiling/src/string_set.rs

@@ -1,5 +1,4 @@
 use core::hash;
-use core::ops::Deref;
 use datadog_alloc::{ChainAllocator, VirtualAllocator};
 use datadog_thin_str::*;
 
@@ -103,16 +102,4 @@ impl StringSet {
     pub fn arena_used_bytes(&self) -> usize {
         self.arena.used_bytes()
     }
-
-    /// Creates a `&str` from the `thin_str`, binding it to the lifetime of
-    /// the set.
-    ///
-    /// # Safety
-    /// The `thin_str` must live in this string set.
-    #[inline]
-    pub unsafe fn get_thin_str(&self, thin_str: ThinStr) -> &str {
-        // todo: debug_assert it exists in the memory region?
-        // SAFETY: see function's safety conditions.
-        unsafe { core::mem::transmute(thin_str.deref()) }
-    }
 }

thin-str/src/thin_str.rs

@@ -48,6 +48,13 @@ impl ThinStr<'_> {
         // SAFETY: ConstStorage objects are valid Storage objects.
         unsafe { Self::from_raw(ptr) }
     }
+
+    /// Gets the header pointer for this string. Note that it is safe to get
+    /// the pointer, but unsafe to do much with it other than to pass it back
+    /// to [Self::from_raw].
+    pub const fn header_ptr(&self) -> ptr::NonNull<ThinHeader> {
+        self.ptr
+    }
 }
 
 impl From<usize> for ThinHeader {