diff --git a/CHANGELOG.md b/CHANGELOG.md index d750e8292..9618a0637 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +# Unreleased + +**Fixes** +- Unship "Support for DWARFv5 embedded source code extension ([#849](https://github.com/getsentry/symbolic/pull/849))". + Unfortunately the check for whether an elf file contains embedded sources is prohibitively expensive in terms of memory. + ## 12.11.1 **Fixes** diff --git a/symbolic-debuginfo/src/base.rs b/symbolic-debuginfo/src/base.rs index 01d981aa7..256ff2f59 100644 --- a/symbolic-debuginfo/src/base.rs +++ b/symbolic-debuginfo/src/base.rs @@ -450,25 +450,13 @@ pub struct FileInfo<'data> { name: Cow<'data, [u8]>, /// Path to the file. dir: Cow<'data, [u8]>, - /// Source code - source: Option>, } impl<'data> FileInfo<'data> { /// Creates a `FileInfo` with a given directory and the file name. #[cfg(feature = "dwarf")] pub fn new(dir: Cow<'data, [u8]>, name: Cow<'data, [u8]>) -> Self { - Self::with_source(dir, name, None) - } - - /// Creates a `FileInfo` with a given directory, the file name, and optional source code. - #[cfg(feature = "dwarf")] - pub fn with_source( - dir: Cow<'data, [u8]>, - name: Cow<'data, [u8]>, - source: Option>, - ) -> Self { - FileInfo { name, dir, source } + FileInfo { name, dir } } /// Creates a `FileInfo` from a joined path by trying to split it. @@ -482,7 +470,6 @@ impl<'data> FileInfo<'data> { Some(dir) => Cow::Borrowed(dir), None => Cow::default(), }, - source: None, } } @@ -498,7 +485,6 @@ impl<'data> FileInfo<'data> { Some(dir) => Cow::Owned(dir.to_vec()), None => Cow::default(), }, - source: None, } } @@ -507,7 +493,6 @@ impl<'data> FileInfo<'data> { FileInfo { name: Cow::Borrowed(name), dir: Cow::default(), - source: None, } } @@ -521,11 +506,6 @@ impl<'data> FileInfo<'data> { from_utf8_cow_lossy(&self.dir) } - /// The embedded source code as an UTF-8 string. - pub fn source_str(&self) -> Option> { - self.source.as_ref().map(from_utf8_cow_lossy) - } - /// The full path to the file, relative to the compilation directory. pub fn path_str(&self) -> String { let joined = join_path(&self.dir_str(), &self.name_str()); diff --git a/symbolic-debuginfo/src/dwarf.rs b/symbolic-debuginfo/src/dwarf.rs index 3153e13cf..e9d32922a 100644 --- a/symbolic-debuginfo/src/dwarf.rs +++ b/symbolic-debuginfo/src/dwarf.rs @@ -10,7 +10,7 @@ //! [`PeObject`]: ../pe/struct.PeObject.html use std::borrow::Cow; -use std::collections::{BTreeSet, HashMap}; +use std::collections::BTreeSet; use std::error::Error; use std::fmt; use std::marker::PhantomData; @@ -692,7 +692,7 @@ impl<'d, 'a> DwarfUnit<'d, 'a> { line_program: &LineNumberProgramHeader<'d>, file: &LineProgramFileEntry<'d>, ) -> FileInfo<'d> { - FileInfo::with_source( + FileInfo::new( Cow::Borrowed(resolve_byte_name( self.bcsymbolmap, file.directory(line_program) @@ -703,14 +703,6 @@ impl<'d, 'a> DwarfUnit<'d, 'a> { self.bcsymbolmap, self.inner.slice_value(file.path_name()).unwrap_or_default(), )), - file.source().and_then(|source| { - let unit_ref = self.inner.unit.unit_ref(self.inner.info); - match unit_ref.attr_string(source) { - Ok(source) if source.is_empty() => None, - Err(_) => None, - Ok(source) => Some(Cow::Borrowed(source.slice())), - } - }), ) } @@ -1303,9 +1295,6 @@ impl std::iter::FusedIterator for DwarfUnitIterator<'_> {} pub struct DwarfDebugSession<'data> { cell: SelfCell>, DwarfInfo<'data>>, bcsymbolmap: Option>>, - // We store the "lookup path" for each entry here to avoid lifetime issues w.r.t - // HashMap<_, SourceFileDescriptor<'data>>, as we can't construct this in a method call. - sources_path_to_file_idx: OnceCell>, } impl<'data> DwarfDebugSession<'data> { @@ -1327,7 +1316,6 @@ impl<'data> DwarfDebugSession<'data> { Ok(DwarfDebugSession { cell, bcsymbolmap: None, - sources_path_to_file_idx: OnceCell::default(), }) } @@ -1360,33 +1348,11 @@ impl<'data> DwarfDebugSession<'data> { } /// See [DebugSession::source_by_path] for more information. - /// This lookup returns entries that match a given [FileEntry::abs_path_str]. - /// - /// Note that this does not load additional sources from disk and only works with sources - /// embedded directly in the debug information (DW_LNCT_LLVM_source). pub fn source_by_path( &self, - path: &str, + _path: &str, ) -> Result>, DwarfError> { - // Construct / fetch a lookup table to avoid scanning and comparing each file's path in this - // operation: - let sources = self.sources_path_to_file_idx.get_or_init(|| { - let mut res = HashMap::new(); - for (i, file) in self.files().enumerate() { - if let Ok(file) = file { - if file.source_str().is_some() { - res.insert(file.abs_path_str(), i); - } - } - } - res - }); - - Ok(sources.get(path).map(|&idx| { - // These unwraps hold by construction above - let file = self.files().nth(idx).unwrap().unwrap(); - SourceFileDescriptor::new_embedded(file.source_str().unwrap(), None) - })) + Ok(None) } } diff --git a/symbolic-debuginfo/src/elf.rs b/symbolic-debuginfo/src/elf.rs index 266c69b88..6ddb2f298 100644 --- a/symbolic-debuginfo/src/elf.rs +++ b/symbolic-debuginfo/src/elf.rs @@ -537,14 +537,7 @@ impl<'data> ElfObject<'data> { /// Determines whether this object contains embedded source. pub fn has_sources(&self) -> bool { - // Note: It'd be great to be able to determine this without iterating over all file entries. - // Unfortunately, though, this seems to be the only correct implementation. - match self.debug_session() { - Ok(session) => session - .files() - .any(|f| f.is_ok_and(|f| f.source_str().is_some())), - Err(_) => false, - } + false } /// Determines whether this object is malformed and was only partially parsed diff --git a/symbolic-debuginfo/src/wasm.rs b/symbolic-debuginfo/src/wasm.rs index 583bf0a03..1454f2f49 100644 --- a/symbolic-debuginfo/src/wasm.rs +++ b/symbolic-debuginfo/src/wasm.rs @@ -131,12 +131,7 @@ impl<'data> WasmObject<'data> { /// Determines whether this object contains embedded source. pub fn has_sources(&self) -> bool { - match self.debug_session() { - Ok(session) => session - .files() - .any(|f| f.is_ok_and(|f| f.source_str().is_some())), - Err(_) => false, - } + false } /// Determines whether this object is malformed and was only partially parsed diff --git a/symbolic-debuginfo/tests/test_objects.rs b/symbolic-debuginfo/tests/test_objects.rs index c98ee9976..6372f0a18 100644 --- a/symbolic-debuginfo/tests/test_objects.rs +++ b/symbolic-debuginfo/tests/test_objects.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, env, ffi::CString, fmt, io::BufWriter}; +use std::{env, ffi::CString, fmt, io::BufWriter}; use symbolic_common::ByteView; use symbolic_debuginfo::{ @@ -871,8 +871,6 @@ fn test_wasm_symbols() -> Result<(), Error> { Some("bda18fd85d4a4eb893022d6bfad846b1".into()) ); - assert!(!object.has_sources()); - let symbols = object.symbol_map(); insta::assert_debug_snapshot!("wasm_symbols", SymbolsDebug(&symbols)); @@ -895,35 +893,3 @@ fn test_wasm_line_program() -> Result<(), Error> { Ok(()) } - -#[test] -fn test_wasm_has_sources() -> Result<(), Error> { - let view = ByteView::open(fixture("wasm/embed-source.wasm"))?; - let object = Object::parse(&view)?; - - assert!(object.has_sources()); - - let session = object.debug_session()?; - - let files_with_source = session - .files() - .filter_map(|x| { - let file_entry = x.ok()?; - let source = file_entry.source_str()?; - Some((file_entry.abs_path_str(), source)) - }) - .collect::>(); - - insta::assert_debug_snapshot!(files_with_source, @r###" - { - "/tmp/foo.c": "int main(void) { return 42; }\n", - } - "###); - - assert!(session.source_by_path("foo/bar.c").unwrap().is_none()); - assert!(session.source_by_path("foo.c").unwrap().is_none()); - // source_by_path matches the absolute file path - assert!(session.source_by_path("/tmp/foo.c").unwrap().is_some()); - - Ok(()) -} diff --git a/symbolic-symcache/src/lookup.rs b/symbolic-symcache/src/lookup.rs index 9c1a4f4a9..e7635d750 100644 --- a/symbolic-symcache/src/lookup.rs +++ b/symbolic-symcache/src/lookup.rs @@ -228,8 +228,9 @@ impl<'data> Iterator for Functions<'data> { } } - function.inspect(|_f| { + function.map(|f| { self.function_idx += 1; + f }) } } @@ -265,8 +266,9 @@ impl<'data> Iterator for Files<'data> { type Item = File<'data>; fn next(&mut self) -> Option { - self.cache.get_file(self.file_idx).inspect(|_f| { + self.cache.get_file(self.file_idx).map(|f| { self.file_idx += 1; + f }) } } diff --git a/symbolic-testutils/fixtures/wasm/embed-source.wasm b/symbolic-testutils/fixtures/wasm/embed-source.wasm deleted file mode 100755 index dfe3186e0..000000000 Binary files a/symbolic-testutils/fixtures/wasm/embed-source.wasm and /dev/null differ