Improve API.

samply-api/src/lib.rs

@@ -15,7 +15,7 @@
 //!
 //! ```rust
 //! use samply_api::samply_symbols::{
-//!     DwoRef, FileContents, FileAndPathHelper, FileAndPathHelperResult, OptionallySendFuture,
+//!     FileContents, FileAndPathHelper, FileAndPathHelperResult, OptionallySendFuture,
 //!     CandidatePathInfo, FileLocation, LibraryInfo, SymbolManager,
 //! };
 //! use samply_api::samply_symbols::debugid::{CodeId, DebugId};
@@ -130,8 +130,8 @@
 //!         Some(Self(self.0.with_extension("symindex")))
 //!     }
 //!
-//!     fn location_for_dwo(&self, dwo_ref: &DwoRef) -> Option<Self> {
-//!         Some(Self(std::path::Path::new(&dwo_ref.path).into()))
+//!     fn location_for_dwo(&self, _comp_dir: &str, path: &str) -> Option<Self> {
+//!         Some(Self(std::path::Path::new(path).into()))
 //!     }
 //! }
 //! ```

samply-api/src/source/mod.rs

@@ -68,26 +68,19 @@ impl<'a, H: FileAndPathHelper> SourceApi<'a, H> {
         };
         let symbol_map = self.symbol_manager.load_symbol_map(&info).await?;
         let debug_file_location = symbol_map.debug_file_location().clone();
-        let frames = match symbol_map.lookup_relative_address(*module_offset) {
-            Some(address_info) => address_info.frames,
-            None => FramesLookupResult::Unavailable,
-        };
+        let frames = symbol_map
+            .lookup_relative_address(*module_offset)
+            .and_then(|address_info| address_info.frames);
 
         let frames = match frames {
-            FramesLookupResult::Available(frames) => frames,
-            FramesLookupResult::External(address) => {
+            Some(FramesLookupResult::Available(frames)) => frames,
+            Some(FramesLookupResult::External(address)) => {
                 match symbol_map.lookup_external(&address).await {
                     Some(frames) => frames,
                     None => return Err(SourceError::NoDebugInfo),
                 }
             }
-            FramesLookupResult::Unavailable => return Err(SourceError::NoDebugInfo),
-            FramesLookupResult::NeedDwo { svma, .. } => {
-                match symbol_map.lookup_frames_async(svma).await {
-                    Some(frames) => frames,
-                    None => return Err(SourceError::NoDebugInfo),
-                }
-            }
+            None => return Err(SourceError::NoDebugInfo),
         };
 
         // Find the SourceFilePath whose "api file path" matches the requested file.

samply-api/src/symbolicate/mod.rs

@@ -74,7 +74,6 @@ impl<'a, H: FileAndPathHelper> SymbolicateApi<'a, H> {
 
         let mut symbolication_result = LookedUpAddresses::for_addresses(&addresses);
         let mut external_addresses = Vec::new();
-        let mut external_addresses_dwo = Vec::new();
 
         // Do the synchronous work first, and accumulate external_addresses which need
         // to be handled asynchronously. This allows us to group async file loads by
@@ -98,16 +97,13 @@ impl<'a, H: FileAndPathHelper> SymbolicateApi<'a, H> {
                     address_info.symbol.size,
                 );
                 match address_info.frames {
-                    FramesLookupResult::Available(frames) => {
+                    Some(FramesLookupResult::Available(frames)) => {
                         symbolication_result.add_address_debug_info(address, frames)
                     }
-                    FramesLookupResult::External(ext_address) => {
+                    Some(FramesLookupResult::External(ext_address)) => {
                         external_addresses.push((address, ext_address));
                     }
-                    FramesLookupResult::Unavailable => {}
-                    FramesLookupResult::NeedDwo { svma, dwo_ref, .. } => {
-                        external_addresses_dwo.push((address, svma, dwo_ref));
-                    }
+                    None => {}
                 }
             }
         }
@@ -117,20 +113,13 @@ impl<'a, H: FileAndPathHelper> SymbolicateApi<'a, H> {
         // external addresses by ExternalFileAddressRef before we do the lookup,
         // in order to get the best hit rate in lookup_external.
         external_addresses.sort_unstable_by(|(_, a), (_, b)| a.cmp(b));
-        external_addresses_dwo.sort_unstable_by(|(_, _, a), (_, _, b)| a.cmp(b));
 
         for (address, ext_address) in external_addresses {
             if let Some(frames) = symbol_map.lookup_external(&ext_address).await {
                 symbolication_result.add_address_debug_info(address, frames);
             }
         }
 
-        for (address, svma, _) in external_addresses_dwo {
-            if let Some(frames) = symbol_map.lookup_frames_async(svma).await {
-                symbolication_result.add_address_debug_info(address, frames);
-            }
-        }
-
         Ok(symbolication_result)
     }
 }

samply-api/tests/integration_tests/main.rs

@@ -2,7 +2,6 @@ use assert_json_diff::assert_json_eq;
 pub use samply_api::debugid::DebugId;
 use samply_api::samply_symbols;
 use samply_api::Api;
-use samply_symbols::DwoRef;
 use samply_symbols::{
     CandidatePathInfo, FileAndPathHelper, FileAndPathHelperResult, FileLocation, LibraryInfo,
     SymbolManager,
@@ -197,7 +196,7 @@ impl FileLocation for FileLocationType {
         Some(Self(self.0.with_extension("symindex")))
     }
 
-    fn location_for_dwo(&self, _dwo_ref: &DwoRef) -> Option<Self> {
+    fn location_for_dwo(&self, _comp_dir: &str, _path: &str) -> Option<Self> {
         None // TODO
     }
 }

samply-symbols/src/breakpad/symbol_map.rs

@@ -257,7 +257,7 @@ impl<'a, T: FileContents> SymbolMapTrait for BreakpadSymbolMapInner<'a, T> {
                         }),
                         name: info.name.to_string(),
                     },
-                    frames: FramesLookupResult::Unavailable,
+                    frames: None,
                 })
             }
             BreakpadSymbolType::Func(func) => {
@@ -307,7 +307,7 @@ impl<'a, T: FileContents> SymbolMapTrait for BreakpadSymbolMapInner<'a, T> {
                         size: Some(info.size),
                         name: info.name.to_string(),
                     },
-                    frames: FramesLookupResult::Available(frames),
+                    frames: Some(FramesLookupResult::Available(frames)),
                 })
             }
         }
@@ -395,7 +395,7 @@ mod test {
         assert_eq!(lookup_result.symbol.size, Some(0xaa));
 
         let frames = match lookup_result.frames {
-            FramesLookupResult::Available(frames) => frames,
+            Some(FramesLookupResult::Available(frames)) => frames,
             _ => panic!("Frames should be available"),
         };
         assert_eq!(frames.len(), 4);

samply-symbols/src/external_file.rs

@@ -8,48 +8,44 @@ use crate::dwarf::{get_frames, Addr2lineContextData};
 use crate::error::Error;
 use crate::path_mapper::PathMapper;
 use crate::shared::{
-    ExternalFileAddressInFileRef, ExternalFileRef, FileAndPathHelper, FileContents,
-    FileContentsWrapper, FileLocation, FrameDebugInfo,
+    ExternalFileAddressInFileRef, FileAndPathHelper, FileContents, FileContentsWrapper,
+    FrameDebugInfo,
 };
 
 pub async fn load_external_file<H>(
     helper: &H,
-    original_file_location: &H::FL,
-    external_file_ref: &ExternalFileRef,
+    external_file_location: H::FL,
+    external_file_path: &str,
 ) -> Result<ExternalFileSymbolMap<H::F>, Error>
 where
     H: FileAndPathHelper,
 {
     let file = helper
-        .load_file(
-            original_file_location
-                .location_for_external_object_file(&external_file_ref.file_name)
-                .ok_or(Error::FileLocationRefusedExternalObjectLocation)?,
-        )
+        .load_file(external_file_location)
         .await
-        .map_err(|e| Error::HelperErrorDuringOpenFile(external_file_ref.file_name.clone(), e))?;
-    let symbol_map = ExternalFileSymbolMap::new(&external_file_ref.file_name, file)?;
+        .map_err(|e| Error::HelperErrorDuringOpenFile(external_file_path.to_string(), e))?;
+    let symbol_map = ExternalFileSymbolMap::new(external_file_path, file)?;
     Ok(symbol_map)
 }
 
 struct ExternalFileOuter<F: FileContents> {
-    name: String,
+    file_path: String,
     file_contents: FileContentsWrapper<F>,
     addr2line_context_data: Addr2lineContextData,
 }
 
 impl<F: FileContents> ExternalFileOuter<F> {
-    pub fn new(file_name: &str, file: F) -> Self {
+    pub fn new(file_path: &str, file: F) -> Self {
         let file_contents = FileContentsWrapper::new(file);
         Self {
-            name: file_name.to_owned(),
+            file_path: file_path.to_owned(),
             file_contents,
             addr2line_context_data: Addr2lineContextData::new(),
         }
     }
 
-    pub fn name(&self) -> &str {
-        &self.name
+    pub fn file_path(&self) -> &str {
+        &self.file_path
     }
 
     fn make_member_context(
@@ -74,9 +70,9 @@ impl<F: FileContents> ExternalFileOuter<F> {
         let symbol_addresses = object_file
             .symbols()
             .filter_map(|symbol| {
-                let name = symbol.name_bytes().ok()?;
+                let file_path = symbol.name_bytes().ok()?;
                 let address = symbol.address();
-                Some((name, address))
+                Some((file_path, address))
             })
             .collect();
         let member_context = ExternalFileMemberContext {
@@ -102,8 +98,8 @@ impl<F: FileContents> ExternalFileOuter<F> {
                 let mut member_ranges = HashMap::new();
                 for member in archive.members() {
                     let member = member.map_err(Error::ParseErrorInExternalArchive)?;
-                    let name = member.name().to_owned();
-                    member_ranges.insert(name, member.file_range());
+                    let file_path = member.name().to_owned();
+                    member_ranges.insert(file_path, member.file_range());
                 }
                 ExternalFileMemberContexts::Archive {
                     member_ranges,
@@ -127,7 +123,7 @@ impl<F: FileContents> ExternalFileOuter<F> {
 
 enum ExternalFileMemberContexts<'a> {
     SingleObject(ExternalFileMemberContext<'a>),
-    /// member name -> context
+    /// member file_path -> context
     Archive {
         member_ranges: HashMap<Vec<u8>, (u64, u64)>,
         contexts: Mutex<HashMap<String, ExternalFileMemberContext<'a>>>,
@@ -201,7 +197,8 @@ impl<'a, F: FileContents> ExternalFileInnerTrait for ExternalFileInner<'a, F> {
             | (
                 ExternalFileMemberContexts::Archive { .. },
                 ExternalFileAddressInFileRef::MachoOsoObject { .. },
-            ) => None,
+            )
+            | (_, ExternalFileAddressInFileRef::ElfDwo { .. }) => None,
         }
     }
 }
@@ -229,8 +226,8 @@ pub struct ExternalFileSymbolMap<F: FileContents + 'static>(
 );
 
 impl<F: FileContents + 'static> ExternalFileSymbolMap<F> {
-    fn new(file_name: &str, file: F) -> Result<Self, Error> {
-        let outer = ExternalFileOuter::new(file_name, file);
+    pub fn new(file_path: &str, file: F) -> Result<Self, Error> {
+        let outer = ExternalFileOuter::new(file_path, file);
         let inner = Yoke::try_attach_to_cart(
             Box::new(outer),
             |outer| -> Result<ExternalFileInnerWrapper<'_>, Error> {
@@ -243,15 +240,8 @@ impl<F: FileContents + 'static> ExternalFileSymbolMap<F> {
 
     /// The string which identifies this external file. This is usually an absolute
     /// path.
-    pub fn name(&self) -> &str {
-        self.0.backing_cart().name()
-    }
-
-    /// Checks whether `external_file_ref` refers to this external file.
-    ///
-    /// Used to avoid repeated loading of the same external file.
-    pub fn is_same_file(&self, external_file_ref: &ExternalFileRef) -> bool {
-        self.name() == external_file_ref.file_name
+    pub fn file_path(&self) -> &str {
+        self.0.backing_cart().file_path()
     }
 
     /// Look up the debug info for the given [`ExternalFileAddressInFileRef`].

samply-symbols/src/jitdump.rs

@@ -278,25 +278,17 @@ impl<'a, T: FileContents> JitDumpSymbolMapInner<'a, T> {
         let name_bytes = cache.get_function_name(index)?;
         let name = String::from_utf8_lossy(name_bytes).into_owned();
         let debug_info = cache.get_debug_info(index);
-        let frames = match debug_info {
-            Some(debug_info) => {
-                let lookup_avma = debug_info.code_addr + offset_relative_to_symbol;
-                match debug_info.lookup(lookup_avma) {
-                    Some(entry) => {
-                        let file_path =
-                            String::from_utf8_lossy(&entry.file_path.as_slice()).into_owned();
-                        let frame = FrameDebugInfo {
-                            function: Some(name.clone()),
-                            file_path: Some(SourceFilePath::new(file_path, None)),
-                            line_number: Some(entry.line),
-                        };
-                        FramesLookupResult::Available(vec![frame])
-                    }
-                    None => FramesLookupResult::Unavailable,
-                }
-            }
-            None => FramesLookupResult::Unavailable,
-        };
+        let frames = debug_info.and_then(|debug_info| {
+            let lookup_avma = debug_info.code_addr + offset_relative_to_symbol;
+            let entry = debug_info.lookup(lookup_avma)?;
+            let file_path = String::from_utf8_lossy(&entry.file_path.as_slice()).into_owned();
+            let frame = FrameDebugInfo {
+                function: Some(name.clone()),
+                file_path: Some(SourceFilePath::new(file_path, None)),
+                line_number: Some(entry.line),
+            };
+            Some(FramesLookupResult::Available(vec![frame]))
+        });
         Some(AddressInfo {
             symbol: SymbolInfo {
                 address: symbol_address,

samply-symbols/src/lib.rs

@@ -95,31 +95,21 @@
 //!             println!("0x1f98f: {}", address_info.symbol.name);
 //!
 //!             // See if we have debug info (file name + line, and inlined frames):
-//!             match address_info.frames {
-//!                 FramesLookupResult::Available(frames) => {
-//!                     println!("Debug info:");
-//!                     for frame in frames {
-//!                         println!(
-//!                             " - {:?} ({:?}:{:?})",
-//!                             frame.function, frame.file_path, frame.line_number
-//!                         );
-//!                     }
+//!             let frames = match address_info.frames {
+//!                 Some(FramesLookupResult::Available(frames)) => Some(frames),
+//!                 Some(FramesLookupResult::External(external)) => {
+//!                     symbol_map.lookup_external(&external).await
 //!                 }
-//!                 FramesLookupResult::External(ext_address) => {
-//!                     // Debug info is located in a different file.
-//!                     if let Some(frames) =
-//!                         symbol_manager.lookup_external(&symbol_map.debug_file_location(), &ext_address).await
-//!                     {
-//!                         println!("Debug info:");
-//!                         for frame in frames {
-//!                             println!(
-//!                                 " - {:?} ({:?}:{:?})",
-//!                                 frame.function, frame.file_path, frame.line_number
-//!                             );
-//!                         }
-//!                     }
+//!                 None => None,
+//!             };
+//!             if let Some(frames) = frames {
+//!                 println!("Debug info:");
+//!                 for frame in frames {
+//!                     println!(
+//!                         " - {:?} ({:?}:{:?})",
+//!                         frame.function, frame.file_path, frame.line_number
+//!                     );
 //!                 }
-//!                 FramesLookupResult::Unavailable => {}
 //!             }
 //!         }
 //!         None => {
@@ -208,6 +198,10 @@
 //!     fn location_for_breakpad_symindex(&self) -> Option<Self> {
 //!         Some(Self(self.0.with_extension("symindex")))
 //!     }
+//!
+//!     fn location_for_dwo(&self, _comp_dir: &str, path: &str) -> Option<Self> {
+//!         Some(Self(path.into()))
+//!     }
 //! }
 //! ```
 
@@ -257,7 +251,7 @@ pub use crate::jitdump::debug_id_and_code_id_for_jitdump;
 pub use crate::macho::FatArchiveMember;
 pub use crate::mapped_path::MappedPath;
 pub use crate::shared::{
-    relative_address_base, AddressInfo, CandidatePathInfo, CodeId, DwoRef, ElfBuildId,
+    relative_address_base, AddressInfo, CandidatePathInfo, CodeId, ElfBuildId,
     ExternalFileAddressInFileRef, ExternalFileAddressRef, ExternalFileRef, FileAndPathHelper,
     FileAndPathHelperError, FileAndPathHelperResult, FileContents, FileContentsWrapper,
     FileLocation, FrameDebugInfo, FramesLookupResult, LibraryInfo, MultiArchDisambiguator,
@@ -381,9 +375,12 @@ where
     pub async fn load_external_file(
         &self,
         debug_file_location: &H::FL,
-        external_file_ref: &ExternalFileRef,
+        external_file_path: &str,
     ) -> Result<ExternalFileSymbolMap<H::F>, Error> {
-        external_file::load_external_file(&*self.helper, debug_file_location, external_file_ref)
+        let external_file_location = debug_file_location
+            .location_for_external_object_file(external_file_path)
+            .ok_or(Error::FileLocationRefusedExternalObjectLocation)?;
+        external_file::load_external_file(&*self.helper, external_file_location, external_file_path)
             .await
     }