Remove 419 unsafe blocks from bun_sys, uWS, the HTTP client, the shell, the package manager, bun_jsc, the Web APIs, node:fs and the bundler

docs/project/bindgen.mdx

@@ -154,7 +154,7 @@ and `&mut self` for its object, because nothing can invalidate either while it
 runs.
 
 The compiler holds a body to its capability as far as this API goes. The older API has
-other ways to a `&JSGlobalObject` (`VirtualMachine::get().global()`, a field of the struct), and a
+other ways to a `&JSGlobalObject` (`VirtualMachine::get().global()`, `heap.bun_vm().global()`, a field of the struct), and a
 body that takes one of those to run JavaScript has stepped around the check.
 
 Much of the older API takes a `&JSGlobalObject` only to allocate or to throw. A `"heap"`

src/ast/ast_memory_allocator.rs

@@ -3,6 +3,7 @@ use core::ptr;
 
 use bun_alloc::Arena;
 use bun_alloc::ast_alloc::{self, AstAllocState};
+use bun_ptr::{BackRef, Mut};
 
 use crate::expr;
 use crate::stmt;
@@ -64,7 +65,6 @@ pub struct ASTMemoryAllocator {
     ast_state: Option<Box<AstAllocState>>,
     /// `true` while `ast_state` is installed in the `AST_ALLOC` thread-local.
     ast_pushed: bool,
-    previous: *mut ASTMemoryAllocator,
     previous_logger: *const Arena,
     /// The `AST_ALLOC` occupant displaced by `push()`, restored by `pop()`.
     previous_ast_state: Option<Box<AstAllocState>>,
@@ -78,7 +78,6 @@ impl Default for ASTMemoryAllocator {
             arena_dirty: false,
             ast_state: None,
             ast_pushed: false,
-            previous: ptr::null_mut(),
             previous_logger: ptr::null(),
             previous_ast_state: None,
         }
@@ -138,7 +137,6 @@ impl ASTMemoryAllocator {
             arena_dirty: false,
             ast_state: None,
             ast_pushed: false,
-            previous: ptr::null_mut(),
             previous_logger: ptr::null(),
             previous_ast_state: None,
         }
@@ -221,10 +219,9 @@ impl ASTMemoryAllocator {
             }
         }
         self.arena_dirty = true;
-        self.previous = ptr::null_mut();
         let mut ast_scope = Scope {
             current: Some(self),
-            previous: Some(stmt::data::Store::memory_allocator()),
+            previous: None,
             previous_logger: ptr::null(),
             previous_ast_state: None,
             entered: false,
@@ -272,8 +269,9 @@ impl ASTMemoryAllocator {
         // directly into it across many modules with no intervening `reset()`).
         self.arena_dirty = true;
         let arena: *const Arena = self.arena_raw();
-        stmt::data::Store::set_memory_allocator(std::ptr::from_mut::<Self>(self));
-        expr::data::Store::set_memory_allocator(std::ptr::from_mut::<Self>(self));
+        let this = Some(BackRef::new_mut(self));
+        stmt::data::Store::set_memory_allocator(this);
+        expr::data::Store::set_memory_allocator(this);
         let spill = self.arena().heap_ptr();
         if !self.ast_pushed {
             // Capture the outer override only on the first (un-popped) push so
@@ -298,10 +296,8 @@ impl ASTMemoryAllocator {
     }
 
     pub fn pop(&mut self) {
-        let prev = self.previous;
-        debug_assert!(prev != std::ptr::from_mut::<Self>(self));
-        stmt::data::Store::set_memory_allocator(prev);
-        expr::data::Store::set_memory_allocator(prev);
+        stmt::data::Store::set_memory_allocator(None);
+        expr::data::Store::set_memory_allocator(None);
         crate::set_data_store_override(self.previous_logger);
         if self.ast_pushed {
             // Take the state back; its contents stay alive until the owner
@@ -313,7 +309,6 @@ impl ASTMemoryAllocator {
                 "ASTMemoryAllocator::pop: the pushed AstAllocState was uninstalled by someone else"
             );
         }
-        self.previous = ptr::null_mut();
         self.previous_logger = ptr::null();
     }
 
@@ -327,7 +322,7 @@ impl ASTMemoryAllocator {
 
 pub struct Scope<'a> {
     current: Option<&'a mut ASTMemoryAllocator>,
-    previous: Option<*mut ASTMemoryAllocator>,
+    previous: Option<BackRef<ASTMemoryAllocator, Mut>>,
     previous_logger: *const Arena,
     /// The `AST_ALLOC` occupant displaced by `enter()`, restored by `exit()`.
     previous_ast_state: Option<Box<AstAllocState>>,
@@ -351,15 +346,15 @@ impl<'a> Default for Scope<'a> {
 impl<'a> Scope<'a> {
     pub fn enter(&mut self) {
         debug_assert!(
-            expr::data::Store::memory_allocator() == stmt::data::Store::memory_allocator()
+            expr::data::Store::MEMORY_ALLOCATOR.get() == stmt::data::Store::MEMORY_ALLOCATOR.get()
         );
         debug_assert!(!self.entered);
 
-        self.previous = Some(expr::data::Store::memory_allocator());
+        self.previous = expr::data::Store::MEMORY_ALLOCATOR.get();
         self.previous_logger = crate::data_store_override();
         self.entered = true;
 
-        let (current, arena): (*mut ASTMemoryAllocator, *const Arena) = match &mut self.current {
+        let (current, arena) = match &mut self.current {
             Some(r) => {
                 debug_assert!(
                     !r.ast_pushed,
@@ -371,21 +366,21 @@ impl<'a> Scope<'a> {
                 state.set_spill_heap(r.arena().heap_ptr());
                 self.previous_ast_state = ast_alloc::swap_state(Some(state));
                 r.ast_pushed = true;
-                (std::ptr::from_mut::<ASTMemoryAllocator>(*r), arena)
+                (Some(BackRef::new_mut(*r)), arena)
             }
             None => {
                 // Block-store scope with no `ASTMemoryAllocator`: detach
                 // `AstAlloc` to the global-mimalloc fallback.
                 self.previous_ast_state = ast_alloc::swap_state(None);
-                (ptr::null_mut(), ptr::null())
+                (None, ptr::null())
             }
         };
 
         expr::data::Store::set_memory_allocator(current);
         stmt::data::Store::set_memory_allocator(current);
         crate::set_data_store_override(arena);
 
-        if current.is_null() {
+        if current.is_none() {
             stmt::data::Store::begin();
             expr::data::Store::begin();
         }
@@ -396,7 +391,7 @@ impl<'a> Scope<'a> {
             return;
         }
         self.entered = false;
-        let prev = self.previous.unwrap_or(ptr::null_mut());
+        let prev = self.previous;
         expr::data::Store::set_memory_allocator(prev);
         stmt::data::Store::set_memory_allocator(prev);
         crate::set_data_store_override(self.previous_logger);

src/ast/import_record.rs

@@ -10,6 +10,7 @@ use bun_paths::fs::Path;
 // `bun_ast::import_record::{ImportKind, Index, Loader}` keep resolving.
 pub use crate::{ImportKind, Index, Loader};
 
+#[derive(Clone)]
 pub struct ImportRecord<'a> {
     pub range: Range,
     pub path: Path<'a>,

src/ast/lib.rs

@@ -1232,8 +1232,7 @@ pub enum Level {
     Warn,    // 3
     Err,     // 4
 }
-// SAFETY: `#[repr(i8)]`, five variants, no payload — 1 byte, no padding.
-bun_core::unsafe_impl_atom!(Level);
+bun_core::impl_no_uninit!(Level);
 
 impl Level {
     pub fn at_least(self, other: Level) -> bool {
@@ -3069,6 +3068,27 @@ mod range_of_identifier_tests {
     }
 }
 
+#[cfg(test)]
+mod level_tests {
+    use super::*;
+
+    #[test]
+    fn every_level_survives_the_atomic_cell() {
+        let cell = bun_core::AtomicCell::new(Level::Warn);
+        assert_eq!(cell.load(), Level::Warn);
+        for level in [
+            Level::Verbose,
+            Level::Debug,
+            Level::Info,
+            Level::Warn,
+            Level::Err,
+        ] {
+            cell.store(level);
+            assert_eq!(cell.load(), level);
+        }
+    }
+}
+
 #[cfg(test)]
 mod location_tests {
     use super::*;

src/ast/new_store.rs

@@ -354,7 +354,7 @@ macro_rules! thread_local_ast_store {
             use ::core::cell::Cell;
             type Backing = $Backing;
 
-            // `#[thread_local]` (bare `__thread` slot) — `memory_allocator()` is
+            // `#[thread_local]` (bare `__thread` slot) — `MEMORY_ALLOCATOR` is
             // read on every node `alloc` (the hottest TLS in the parser), and
             // the `thread_local!` macro's `LocalKey` wrapper showed up in
             // next-lint profiles. All three are `Cell<ptr|bool>` (no destructor,
@@ -391,28 +391,26 @@ macro_rules! thread_local_ast_store {
                 unsafe { INSTANCE.get().as_mut() }
             }
             #[inline]
-            pub fn memory_allocator() -> *mut $crate::ASTMemoryAllocator {
-                MEMORY_ALLOCATOR
-                    .get()
-                    .map_or(::core::ptr::null_mut(), ::bun_ptr::BackRef::as_ptr)
+            pub fn has_memory_allocator() -> bool {
+                MEMORY_ALLOCATOR.get().is_some()
             }
             #[inline]
-            pub(crate) fn set_memory_allocator(p: *mut $crate::ASTMemoryAllocator) {
-                MEMORY_ALLOCATOR.set(::core::ptr::NonNull::new(p).map(|nn|
-                    // SAFETY: `p` is the caller's live allocator with write provenance.
-                    unsafe { ::bun_ptr::BackRef::from_raw_mut(nn.as_ptr()) }));
+            pub(crate) fn set_memory_allocator(
+                p: Option<::bun_ptr::BackRef<$crate::ASTMemoryAllocator, ::bun_ptr::Mut>>,
+            ) {
+                MEMORY_ALLOCATOR.set(p);
             }
 
             pub fn create() {
-                if !instance().is_null() || !memory_allocator().is_null() {
+                if !instance().is_null() || has_memory_allocator() {
                     return;
                 }
                 INSTANCE.set(Backing::init());
             }
 
             /// create || reset
             pub(crate) fn begin() {
-                if !memory_allocator().is_null() {
+                if has_memory_allocator() {
                     return;
                 }
                 match instance_mut() {
@@ -426,7 +424,7 @@ macro_rules! thread_local_ast_store {
             }
 
             pub fn reset() {
-                if DISABLE_RESET.get() || !memory_allocator().is_null() {
+                if DISABLE_RESET.get() || has_memory_allocator() {
                     return;
                 }
                 // Caller contract — instance is set when reset() is called.
@@ -448,7 +446,7 @@ macro_rules! thread_local_ast_store {
             }
 
             pub fn deinit() {
-                if instance().is_null() || !memory_allocator().is_null() {
+                if instance().is_null() || has_memory_allocator() {
                     return;
                 }
                 // SAFETY: checked non-null above; `destroy` frees the `Store`
@@ -460,7 +458,7 @@ macro_rules! thread_local_ast_store {
             #[inline]
             pub(crate) fn assert() {
                 if cfg!(debug_assertions) {
-                    if instance().is_null() && memory_allocator().is_null() {
+                    if instance().is_null() && !has_memory_allocator() {
                         unreachable!("Store must be init'd");
                     }
                 }

src/ast/nodes.rs

@@ -997,7 +997,7 @@ pub type DependencyList = bun_alloc::AstVec<Dependency>;
 /// splitting.
 pub struct Part {
     pub stmts: StoreSlice<Stmt>,
-    pub scopes: StoreSlice<*mut Scope>, // TODO: &'bump mut [&'bump mut Scope]
+    pub scopes: StoreSlice<StoreRef<Scope>>, // TODO: &'bump mut [&'bump mut Scope]
 
     /// Each is an index into the file-level import record list
     pub import_record_indices: PartImportRecordIndices,

src/ast/symbol.rs

@@ -100,7 +100,8 @@ pub struct Symbol {
     /// compile-time error like we do for real import items. This status lets us
     /// avoid this. We also need to be able to replace such import items with
     /// undefined, which this status is also used for.
-    pub import_item_status: ImportItemStatus,
+    /// `Cell`: the linker, whose graph is shared across threads, sets it in a serial step.
+    pub import_item_status: Cell<ImportItemStatus>,
 
     /// Packed boolean state — see [`SymbolFlags`].
     pub flags: SymbolFlags,
@@ -177,7 +178,7 @@ impl Default for Symbol {
             chunk_index: AtomicU32::new(INVALID_CHUNK_INDEX),
             nested_scope_slot: INVALID_NESTED_SCOPE_SLOT,
             kind: Kind::Other,
-            import_item_status: ImportItemStatus::None,
+            import_item_status: Cell::new(ImportItemStatus::None),
             flags: SymbolFlags::empty(),
         }
     }
@@ -244,7 +245,7 @@ impl Symbol {
     /// that a hoisting merge links has no import item status.
     #[inline]
     pub fn is_bound_import_item(&self) -> bool {
-        self.import_item_status != ImportItemStatus::None && self.has_link()
+        self.import_item_status.get() != ImportItemStatus::None && self.has_link()
     }
 }
 

src/base64/lib.rs

@@ -592,8 +592,7 @@ pub mod zig_base64 {
             let mut dest_idx: usize = 0;
             let mut leftover_idx: Option<usize> = None;
             for (src_idx, &c) in source.iter().enumerate() {
-                // SAFETY: `c: u8` so `c as usize` is in 0..=255, and `char_to_index` is `[u8; 256]`.
-                let d = unsafe { *self.char_to_index.get_unchecked(c as usize) };
+                let d = self.char_to_index[c as usize];
                 if d == Self::INVALID_CHAR {
                     if self.pad_char.is_none() || c != self.pad_char.unwrap() {
                         return Err(Error::InvalidCharacter);

src/bun_alloc/MimallocArena.rs

@@ -565,28 +565,18 @@ impl MimallocArena {
     pub fn allocated_bytes(&self) -> usize {
         extern "C" fn visit(
             _heap: *const mimalloc::Heap,
-            area: *const mimalloc::mi_heap_area_t,
+            area: &mimalloc::mi_heap_area_t,
             _block: *mut c_void,
             _block_size: usize,
-            arg: *mut c_void,
+            total: &mut usize,
         ) -> bool {
-            // SAFETY: mimalloc passes a valid `area` for each heap area when
-            // `visit_all_blocks == false`; `arg` is the `&mut usize` we passed.
-            unsafe {
-                let total = &mut *arg.cast::<usize>();
-                *total += (*area).used.saturating_mul((*area).full_block_size);
-            }
+            *total += area.used.saturating_mul(area.full_block_size);
             true
         }
         let mut total: usize = 0;
         // SAFETY: `self.heap` is live; `visit` upholds the callback contract.
         unsafe {
-            mimalloc::mi_heap_visit_blocks(
-                self.heap_ptr(),
-                false,
-                Some(visit),
-                (&raw mut total).cast(),
-            );
+            mimalloc::mi_heap_visit_blocks(self.heap_ptr(), false, Some(visit), &mut total);
         }
         total
     }