Backfill missing DebugId from CFI Cache

crates/symbolicator-service/src/services/cficaches.rs

@@ -4,10 +4,12 @@ use std::sync::Arc;
 
 use futures::future::BoxFuture;
 use minidump_unwind::SymbolFile;
+use sentry::types::DebugId;
 use tempfile::NamedTempFile;
 
 use symbolic::cfi::CfiCache;
 use symbolic::common::ByteView;
+use symbolic::debuginfo::breakpad::BreakpadModuleRecord;
 use symbolicator_sources::{FileType, ObjectId, ObjectType, SourceConfig};
 
 use crate::caching::{
@@ -22,10 +24,15 @@ use crate::utils::sentry::ConfigureScope;
 use super::caches::versions::CFICACHE_VERSIONS;
 use super::derived::{derive_from_object_handle, DerivedCache};
 
-type CfiItem = (u32, Option<Arc<SymbolFile>>);
+type CfiItem = Option<Arc<(SymbolFile, Option<CfiModuleInfo>)>>;
+
+pub struct CfiModuleInfo {
+    pub debug_id: DebugId,
+    pub debug_file: String,
+}
 
 #[tracing::instrument(skip_all)]
-fn parse_cfi_cache(bytes: ByteView<'static>) -> CacheEntry<CfiItem> {
+fn parse_cfi_cache(bytes: ByteView<'static>) -> CacheEntry<(u32, CfiItem)> {
     let weight = bytes.len().try_into().unwrap_or(u32::MAX);
     // NOTE: we estimate the in-memory structures to be ~8x as heavy in memory as on disk
     let weight = weight.saturating_mul(8);
@@ -39,7 +46,23 @@ fn parse_cfi_cache(bytes: ByteView<'static>) -> CacheEntry<CfiItem> {
     let symbol_file =
         SymbolFile::from_bytes(cfi_cache.as_slice()).map_err(CacheError::from_std_error)?;
 
-    Ok((weight, Some(Arc::new(symbol_file))))
+    let first_line = cfi_cache
+        .as_slice()
+        .split(|b| *b == b'\n')
+        .next()
+        .unwrap_or_default();
+    let cfi_module = BreakpadModuleRecord::parse(first_line)
+        .ok()
+        .and_then(|module| {
+            let debug_id = module.id.parse().ok()?;
+            let debug_file = module.name.into();
+            Some(CfiModuleInfo {
+                debug_id,
+                debug_file,
+            })
+        });
+
+    Ok((weight, Some(Arc::new((symbol_file, cfi_module)))))
 }
 
 #[derive(Clone, Debug)]
@@ -79,7 +102,7 @@ async fn compute_cficache(
 }
 
 impl CacheItemRequest for FetchCfiCacheInternal {
-    type Item = CfiItem;
+    type Item = (u32, CfiItem);
 
     const VERSIONS: CacheVersions = CFICACHE_VERSIONS;
 
@@ -109,7 +132,7 @@ pub struct FetchCfiCache {
     pub scope: Scope,
 }
 
-pub type FetchedCfiCache = DerivedCache<Option<Arc<SymbolFile>>>;
+pub type FetchedCfiCache = DerivedCache<CfiItem>;
 
 impl CfiCacheActor {
     /// Fetches the CFI cache file for a given code module.

crates/symbolicator-service/src/services/symbolication/process_minidump.rs

@@ -20,7 +20,7 @@ use tempfile::TempPath;
 use symbolic::common::{Arch, ByteView, CodeId, DebugId};
 use symbolicator_sources::{ObjectId, ObjectType, SourceConfig};
 
-use crate::services::cficaches::{CfiCacheActor, FetchCfiCache, FetchedCfiCache};
+use crate::services::cficaches::{CfiCacheActor, CfiModuleInfo, FetchCfiCache, FetchedCfiCache};
 use crate::services::minidump::parse_stacktraces_from_minidump;
 use crate::services::module_lookup::object_file_status_from_cache_entry;
 use crate::types::{
@@ -230,7 +230,7 @@ impl SymbolProvider for SymbolicatorSymbolProvider {
         walker: &mut (dyn FrameWalker + Send),
     ) -> Option<()> {
         let cfi_module = self.load_cfi_module(module).await;
-        cfi_module.cache.ok()??.walk_frame(module, walker)
+        cfi_module.cache.ok()??.0.walk_frame(module, walker)
     }
 
     async fn get_file_path(
@@ -355,6 +355,15 @@ async fn stackwalk(
                     // NOTE: minidump stackwalking is the first thing that happens to a request,
                     // hence the current candidate list is empty.
                     obj_info.candidates = cfi_module.candidates;
+
+                    // if the debug_id/file is empty, it might be possible to
+                    // backfill that using the reference in the executable file.
+                    if let Ok(Some(cfi_item)) = &cfi_module.cache {
+                        if let Some(cfi_module_info) = &cfi_item.1 {
+                            maybe_backfill_debugid(&mut obj_info.raw, cfi_module_info);
+                        }
+                    }
+
                     object_file_status_from_cache_entry(&cfi_module.cache)
                 }
             });
@@ -376,6 +385,15 @@ async fn stackwalk(
     })
 }
 
+fn maybe_backfill_debugid(info: &mut RawObjectInfo, cfi_module: &CfiModuleInfo) {
+    if info.debug_id.is_none() {
+        info.debug_id = Some(cfi_module.debug_id.to_string());
+    }
+    if info.debug_file.is_none() {
+        info.debug_file = Some(cfi_module.debug_file.clone());
+    }
+}
+
 impl SymbolicationActor {
     /// Saves the given `minidump_file` in the diagnostics cache if configured to do so.
     fn maybe_persist_minidump(&self, minidump_file: TempPath) {