feat(variables): Begin reading minidump memory for variables

Cargo.lock

@@ -4976,6 +4976,7 @@ dependencies = [
  "minidump-unwind",
  "moka",
  "regex",
+ "scroll 0.12.0",
  "sentry",
  "serde",
  "serde_json",

Cargo.toml

@@ -109,6 +109,7 @@ rayon = "1.10.0"
 regex = "1.5.5"
 reqwest = "0.12.15"
 rustls = { version = "0.23.31", features = ["ring"] }
+scroll = "0.12.0"
 sentry = { version = "0.42.0", default-features = false, features = [
     # default features, except `release-health` is disabled
     "backtrace",

crates/symbolicator-native/Cargo.toml

@@ -17,6 +17,7 @@ minidump-processor = { workspace = true }
 minidump-unwind = { workspace = true }
 moka = { workspace = true }
 regex = { workspace = true }
+scroll = { workspace = true }
 sentry = { workspace = true }
 serde = { workspace = true }
 serde_json = { workspace = true }

crates/symbolicator-native/src/interface.rs

@@ -19,6 +19,7 @@ use symbolicator_service::utils::hex::HexValue;
 use symbolicator_sources::SourceConfig;
 use thiserror::Error;
 
+use crate::memory::MemoryAccess;
 pub use crate::metrics::StacktraceOrigin;
 
 #[derive(Debug, Clone)]
@@ -68,6 +69,8 @@ pub struct SymbolicateStacktraces {
     pub frame_order: FrameOrder,
     /// Whether we extract variables.
     pub extract_variables: bool,
+    /// The program memory, if it is available.
+    pub memory: Option<Arc<dyn MemoryAccess>>,
 }
 
 /// Location of an attachment file, such as a minidump.

crates/symbolicator-native/src/lib.rs

@@ -1,5 +1,6 @@
 mod caches;
 pub mod interface;
+mod memory;
 mod metrics;
 mod symbolication;
 

crates/symbolicator-native/src/memory.rs

@@ -0,0 +1,49 @@
+use minidump::Minidump;
+use scroll::ctx::{SizeWith, TryFromCtx};
+use symbolic::common::ByteView;
+
+/// Unified memory access for crash dumps.
+pub trait MemoryAccess: std::fmt::Debug + Send + Sync {
+    /// Attempts to lookup a memory range at the specified `addr` with the specified size.
+    fn get_memory_at_address(&self, addr: u64, size: usize) -> Option<&'_ [u8]>;
+
+    /// The endianness of the crash.
+    fn endian(&self) -> scroll::Endian;
+}
+
+impl MemoryAccess for Minidump<'static, ByteView<'static>> {
+    fn get_memory_at_address(&self, addr: u64, size: usize) -> Option<&'_ [u8]> {
+        let memory = self.get_memory()?;
+        let memory = memory.memory_at_address(addr)?;
+
+        let start = addr.checked_sub(memory.base_address())? as usize;
+        let end = start.checked_add(size)?;
+
+        match memory {
+            minidump::UnifiedMemory::Memory(region) => region.bytes.get(start..end),
+            minidump::UnifiedMemory::Memory64(region) => region.bytes.get(start..end),
+        }
+    }
+
+    fn endian(&self) -> scroll::Endian {
+        self.endian
+    }
+}
+
+/// Extension trait for [`MemoryAccess`].
+pub trait MemoryAccessExt: MemoryAccess {
+    /// Helper which access the memory of a dump and converts the memory to the specified type.
+    fn get_value_at_address<T>(&self, addr: u64) -> Option<T>
+    where
+        T: SizeWith<scroll::Endian>,
+        for<'a> T: TryFromCtx<'a, scroll::Endian>,
+    {
+        let endian = self.endian();
+        let size = T::size_with(&endian);
+        let memory = self.get_memory_at_address(addr, size)?;
+
+        T::try_from_ctx(memory, endian).ok().map(|(value, _)| value)
+    }
+}
+
+impl<T: MemoryAccess + ?Sized> MemoryAccessExt for T {}

crates/symbolicator-native/src/symbolication/apple.rs

@@ -89,6 +89,7 @@ impl SymbolicationActor {
             rewrite_first_module: Default::default(),
             frame_order: FrameOrder::CalleeFirst,
             extract_variables,
+            memory: None,
         };
 
         let mut system_info = SystemInfo {

crates/symbolicator-native/src/symbolication/native.rs

@@ -2,13 +2,16 @@ use std::collections::BTreeMap;
 
 use minidump::CpuContext;
 use symbolic::common::{CpuFamily, InstructionInfo, Language, split_path};
-use symbolic::symcache::{SourceLocation, SymCache, Type, Variable, VariableLocation};
+use symbolic::symcache::{
+    SourceLocation, SymCache, Type, TypeSize, VariableLocation, VariableLocationInfo,
+};
 use symbolicator_service::metric;
 use symbolicator_service::utils::hex::HexValue;
 
 use crate::interface::{
     AdjustInstructionAddr, FrameStatus, RawFrame, Registers, Signal, SymbolicatedFrame,
 };
+use crate::memory::{MemoryAccess, MemoryAccessExt};
 
 use super::demangle::DemangleCache;
 use super::module_lookup::CacheLookupResult;
@@ -20,7 +23,7 @@ pub fn symbolicate_native_frame(
     relative_addr: u64,
     frame: &RawFrame,
     index: usize,
-    extract_variables: bool,
+    memory: Option<&dyn MemoryAccess>,
 ) -> Result<Vec<SymbolicatedFrame>, FrameStatus> {
     tracing::trace!("Symbolicating {:#x}", relative_addr);
     let mut rv = vec![];
@@ -53,8 +56,8 @@ pub fn symbolicate_native_frame(
         };
 
         let mut vars = None;
-        if extract_variables {
-            vars = do_extract_variables(&source_location, symcache, &frame.registers);
+        if let Some(memory) = memory {
+            vars = do_extract_variables(&source_location, symcache, &frame.registers, memory);
         }
 
         rv.push(SymbolicatedFrame {
@@ -167,6 +170,7 @@ fn do_extract_variables<'data, 'cache>(
     source_location: &SourceLocation<'data, 'cache>,
     cache: &SymCache<'cache>,
     registers: &Registers,
+    memory: &dyn MemoryAccess,
 ) -> Option<BTreeMap<String, serde_json::Value>> {
     let mut result = BTreeMap::new();
 
@@ -178,7 +182,9 @@ fn do_extract_variables<'data, 'cache>(
         let mut ty = String::new();
         resolve_type_name(&mut ty, cache, variable.ty(), 0);
 
-        let value = resolve_variable_value(cache, registers, &variable);
+        let value = variable
+            .locations()
+            .find_map(|loc| resolve_variable_value(cache, registers, memory, loc, variable.ty()));
 
         // This doesn't handle name collisions currently.
         result.insert(
@@ -199,24 +205,59 @@ fn do_extract_variables<'data, 'cache>(
 fn resolve_variable_value(
     cache: &SymCache<'_>,
     registers: &Registers,
-    variable: &Variable<'_, '_>,
-) -> Option<HexValue> {
-    variable.locations().find_map(|location| {
-        let VariableLocation::Register { id } = location.location else {
-            return None;
-        };
+    memory: &dyn MemoryAccess,
+    location: VariableLocationInfo,
+    ty: Option<Type<'_>>,
+) -> Option<String> {
+    let TypeSize::Bytes(size) = match ty? {
+        Type::Primitive(ty) => ty.size(),
+        Type::Pointer(ty) => ty.size(),
+        _ => return None,
+    };
 
-        // Temporary hack, `symbolic` will need an abstraction over registers, which allows
-        // mapping register names to the gimli register ids.
-        match cache.arch().cpu_family() {
-            CpuFamily::Amd64 => minidump::format::CONTEXT_AMD64::REGISTERS,
-            CpuFamily::Arm64 => minidump::format::CONTEXT_ARM64::REGISTERS,
-            _ => &[],
+    match location.location {
+        VariableLocation::Register { id } => {
+            // Temporary hack, `symbolic` will need an abstraction over registers, which allows
+            // mapping register names to the gimli register ids.
+            match cache.arch().cpu_family() {
+                CpuFamily::Amd64 => minidump::format::CONTEXT_AMD64::REGISTERS,
+                CpuFamily::Arm64 => minidump::format::CONTEXT_ARM64::REGISTERS,
+                _ => &[],
+            }
+            .get(id as usize)
+            .and_then(|&reg| registers.get(reg))
+            .map(|v| v.to_string())
         }
-        .get(id as usize)
-        .and_then(|&reg| registers.get(reg))
-        .copied()
-    })
+        VariableLocation::FrameOffset { offset } => {
+            let &HexValue(frame_base) = match cache.arch().cpu_family() {
+                CpuFamily::Amd64 => Some("rbp"),
+                CpuFamily::Arm64 => Some("fp"),
+                _ => None,
+            }
+            .and_then(|reg| registers.get(reg))?;
+
+            let addr = u64::try_from(i64::try_from(frame_base).ok()? + offset).ok()?;
+
+            // This obviously will need to be changed to consider the variable type.
+            match size {
+                1 => memory
+                    .get_value_at_address::<u8>(addr)
+                    .map(|v| HexValue(v.into()).to_string()),
+                2 => memory
+                    .get_value_at_address::<u16>(addr)
+                    .map(|v| HexValue(v.into()).to_string()),
+                4 => memory
+                    .get_value_at_address::<u32>(addr)
+                    .map(|v| HexValue(v.into()).to_string()),
+                8 => memory
+                    .get_value_at_address::<u64>(addr)
+                    .map(|v| HexValue(v).to_string()),
+                s => memory
+                    .get_memory_at_address(addr, s as usize)
+                    .map(|s| format!("{s:?}")),
+            }
+        }
+    }
 }
 
 fn resolve_type_name(

crates/symbolicator-native/src/symbolication/process_minidump.rs

@@ -631,6 +631,8 @@ impl SymbolicationActor {
         metric!(distribution("minidump.upload.size") = len as f64);
 
         let bv = ByteView::map_file(minidump_file)?;
+        // let minidump = SelfCell::try_new(bv, |bv| Minidump::read(unsafe { &*bv }))?;
+
         let minidump = Minidump::read(bv)?;
 
         let StackWalkMinidumpResult {
@@ -672,6 +674,7 @@ impl SymbolicationActor {
             rewrite_first_module,
             frame_order: FrameOrder::CalleeFirst,
             extract_variables: request.extract_variables,
+            memory: Some(Arc::new(minidump)),
         };
 
         Ok((request, minidump_state))

crates/symbolicator-native/src/symbolication/symbolicate.rs

@@ -18,6 +18,7 @@ use crate::interface::{
     FrameTrust, RawFrame, RawStacktrace, Registers, Signal, SymbolicateStacktraces,
     SymbolicatedFrame,
 };
+use crate::memory::MemoryAccess;
 use crate::metrics::{StacktraceMetrics, record_symbolication_metrics};
 
 use super::demangle::DemangleCache;
@@ -110,6 +111,7 @@ impl SymbolicationActor {
             rewrite_first_module,
             frame_order,
             extract_variables,
+            memory,
         } = request;
 
         if frame_order == FrameOrder::CallerFirst {
@@ -142,7 +144,7 @@ impl SymbolicationActor {
                     &module_lookup,
                     &mut metrics,
                     signal,
-                    extract_variables,
+                    memory.as_deref().filter(|_| extract_variables),
                 )
             })
             .collect();
@@ -178,7 +180,7 @@ fn symbolicate_stacktrace(
     caches: &ModuleLookup,
     metrics: &mut StacktraceMetrics,
     signal: Option<Signal>,
-    extract_variables: bool,
+    memory: Option<&dyn MemoryAccess>,
 ) -> CompleteStacktrace {
     let default_adjustment = AdjustInstructionAddr::default_for_thread(&thread);
     let mut symbolicated_frames = vec![];
@@ -194,7 +196,7 @@ fn symbolicate_stacktrace(
             &mut frame,
             index,
             adjustment,
-            extract_variables,
+            memory,
         ) {
             Ok(frames) => {
                 if matches!(frame.trust, FrameTrust::Scan) {
@@ -319,7 +321,7 @@ fn symbolicate_frame(
     frame: &mut RawFrame,
     index: usize,
     adjustment: AdjustInstructionAddr,
-    extract_variables: bool,
+    memory: Option<&dyn MemoryAccess>,
 ) -> Result<Vec<SymbolicatedFrame>, FrameStatus> {
     let lookup_result = caches
         .lookup_cache(frame.instruction_addr.0, frame.addr_mode)
@@ -347,7 +349,7 @@ fn symbolicate_frame(
                 relative_addr,
                 frame,
                 index,
-                extract_variables,
+                memory,
             )
         }
         Ok(CacheFileEntry::PortablePdbCache(ppdb_cache)) => {

crates/symbolicator-native/tests/integration/utils.rs

@@ -64,6 +64,7 @@ pub fn make_symbolication_request(
         rewrite_first_module: Default::default(),
         frame_order: FrameOrder::CalleeFirst,
         extract_variables: false,
+        memory: None,
     }
 }
 

crates/symbolicator-stress/src/workloads.rs

@@ -91,6 +91,7 @@ pub fn prepare_payload(
                 rewrite_first_module: Default::default(),
                 frame_order: FrameOrder::CallerFirst,
                 extract_variables: true,
+                memory: None,
             })
         }
         Payload::Js { source, event } => {

crates/symbolicator/src/endpoints/symbolicate.rs

@@ -79,6 +79,7 @@ pub async fn symbolicate_frames(
             rewrite_first_module: Default::default(),
             frame_order: body.options.frame_order,
             extract_variables: body.options.extract_variables,
+            memory: None,
         },
         body.options,
     )?;

crates/symbolicator/src/service.rs

@@ -636,6 +636,7 @@ mod tests {
             rewrite_first_module: Default::default(),
             frame_order: FrameOrder::CalleeFirst,
             extract_variables: false,
+            memory: None,
         };
 
         let request_id = service
@@ -681,6 +682,7 @@ mod tests {
             rewrite_first_module: Default::default(),
             frame_order: FrameOrder::CalleeFirst,
             extract_variables: false,
+            memory: None,
         }
     }
 

crates/symbolicli/src/event.rs

@@ -154,6 +154,7 @@ pub fn create_native_symbolication_request(
         // "callee first"
         frame_order: FrameOrder::CalleeFirst,
         extract_variables,
+        memory: None,
     })
 }