From c1ffa98b2a4893b5da2a4dfb9488a74221e14c4f Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 9 Feb 2023 17:21:23 +0100 Subject: [PATCH 01/19] refactor: change source_by_path to return SourceCode enum --- symbolic-debuginfo/src/base.rs | 18 ++++++++++++---- symbolic-debuginfo/src/breakpad.rs | 8 +++----- symbolic-debuginfo/src/dwarf.rs | 8 +++----- symbolic-debuginfo/src/object.rs | 11 +++++----- symbolic-debuginfo/src/pdb.rs | 8 +++----- symbolic-debuginfo/src/ppdb.rs | 10 ++++----- symbolic-debuginfo/src/sourcebundle.rs | 26 +++++++++++++----------- symbolic-debuginfo/tests/test_objects.rs | 8 +++++--- 8 files changed, 51 insertions(+), 46 deletions(-) diff --git a/symbolic-debuginfo/src/base.rs b/symbolic-debuginfo/src/base.rs index fba9a9644..f77a4e3ff 100644 --- a/symbolic-debuginfo/src/base.rs +++ b/symbolic-debuginfo/src/base.rs @@ -670,6 +670,16 @@ impl fmt::Debug for Function<'_> { /// A dynamically dispatched iterator over items with the given lifetime. pub type DynIterator<'a, T> = Box + 'a>; +/// Represents a source file referenced by a debug information object file. +#[derive(Debug, Clone)] +pub enum SourceCode<'a> { + /// Verbatim source code/file contents. + Content(Cow<'a, str>), + + /// Url (usually HTTP/S) where the content can be fetched from. + Url(Cow<'a, str>), +} + /// A stateful session for interfacing with debug information. /// /// Debug sessions can be obtained via [`ObjectLike::debug_session`]. Since computing a session may @@ -716,10 +726,10 @@ pub trait DebugSession<'session> { /// Returns an iterator over all source files referenced by this debug file. fn files(&'session self) -> Self::FileIterator; - /// Looks up a file's source contents by its full canonicalized path. - /// - /// The given path must be canonicalized. - fn source_by_path(&self, path: &str) -> Result>, Self::Error>; + /// Looks up a file's source by its full canonicalized path. + /// Returns either source contents, if it was embedded, or a source link. + /// TODO also change to `&mut self` when breaking the API - see https://github.com/getsentry/symbolic/issues/736 + fn source_by_path(&self, path: &str) -> Result>, Self::Error>; } /// An object containing debug information. diff --git a/symbolic-debuginfo/src/breakpad.rs b/symbolic-debuginfo/src/breakpad.rs index ed843ad75..a782b47a9 100644 --- a/symbolic-debuginfo/src/breakpad.rs +++ b/symbolic-debuginfo/src/breakpad.rs @@ -1276,10 +1276,8 @@ impl<'data> BreakpadDebugSession<'data> { } } - /// Looks up a file's source contents by its full canonicalized path. - /// - /// The given path must be canonicalized. - pub fn source_by_path(&self, _path: &str) -> Result>, BreakpadError> { + /// See [DebugSession::source_by_path] for more information. + pub fn source_by_path(&self, _path: &str) -> Result>, BreakpadError> { Ok(None) } } @@ -1297,7 +1295,7 @@ impl<'data, 'session> DebugSession<'session> for BreakpadDebugSession<'data> { self.files() } - fn source_by_path(&self, path: &str) -> Result>, Self::Error> { + fn source_by_path(&self, path: &str) -> Result>, BreakpadError> { self.source_by_path(path) } } diff --git a/symbolic-debuginfo/src/dwarf.rs b/symbolic-debuginfo/src/dwarf.rs index 30d0ffc5e..17ab0e165 100644 --- a/symbolic-debuginfo/src/dwarf.rs +++ b/symbolic-debuginfo/src/dwarf.rs @@ -1331,10 +1331,8 @@ impl<'data> DwarfDebugSession<'data> { } } - /// Looks up a file's source contents by its full canonicalized path. - /// - /// The given path must be canonicalized. - pub fn source_by_path(&self, _path: &str) -> Result>, DwarfError> { + /// See [DebugSession::source_by_path] for more information. + pub fn source_by_path(&self, _path: &str) -> Result>, DwarfError> { Ok(None) } } @@ -1352,7 +1350,7 @@ impl<'data, 'session> DebugSession<'session> for DwarfDebugSession<'data> { self.files() } - fn source_by_path(&self, path: &str) -> Result>, Self::Error> { + fn source_by_path(&self, path: &str) -> Result>, Self::Error> { self.source_by_path(path) } } diff --git a/symbolic-debuginfo/src/object.rs b/symbolic-debuginfo/src/object.rs index 7c98c0894..0755bba13 100644 --- a/symbolic-debuginfo/src/object.rs +++ b/symbolic-debuginfo/src/object.rs @@ -1,6 +1,5 @@ //! Generic wrappers over various object file formats. -use std::borrow::Cow; use std::error::Error; use std::fmt; @@ -483,10 +482,10 @@ impl<'d> ObjectDebugSession<'d> { } } - /// Looks up a file's source contents by its full canonicalized path. - /// - /// The given path must be canonicalized. - pub fn source_by_path(&self, path: &str) -> Result>, ObjectError> { + /// Looks up a file's source by its full canonicalized path. + /// Returns either source contents, if it was embedded, or a source link. + /// TODO also change to `&mut self` when breaking the API - see https://github.com/getsentry/symbolic/issues/736 + pub fn source_by_path(&self, path: &str) -> Result>, ObjectError> { match *self { ObjectDebugSession::Breakpad(ref s) => { s.source_by_path(path).map_err(ObjectError::transparent) @@ -520,7 +519,7 @@ impl<'session> DebugSession<'session> for ObjectDebugSession<'_> { self.files() } - fn source_by_path(&self, path: &str) -> Result>, Self::Error> { + fn source_by_path(&self, path: &str) -> Result>, Self::Error> { self.source_by_path(path) } } diff --git a/symbolic-debuginfo/src/pdb.rs b/symbolic-debuginfo/src/pdb.rs index f7b864646..bc906915e 100644 --- a/symbolic-debuginfo/src/pdb.rs +++ b/symbolic-debuginfo/src/pdb.rs @@ -637,10 +637,8 @@ impl<'d> PdbDebugSession<'d> { } } - /// Looks up a file's source contents by its full canonicalized path. - /// - /// The given path must be canonicalized. - pub fn source_by_path(&self, _path: &str) -> Result>, PdbError> { + /// See [DebugSession::source_by_path] for more information. + pub fn source_by_path(&self, _path: &str) -> Result>, PdbError> { Ok(None) } } @@ -658,7 +656,7 @@ impl<'session> DebugSession<'session> for PdbDebugSession<'_> { self.files() } - fn source_by_path(&self, path: &str) -> Result>, Self::Error> { + fn source_by_path(&self, path: &str) -> Result>, Self::Error> { self.source_by_path(path) } } diff --git a/symbolic-debuginfo/src/ppdb.rs b/symbolic-debuginfo/src/ppdb.rs index 852f275de..9d5e919c8 100644 --- a/symbolic-debuginfo/src/ppdb.rs +++ b/symbolic-debuginfo/src/ppdb.rs @@ -169,15 +169,13 @@ impl<'data> PortablePdbDebugSession<'data> { PortablePdbFileIterator::new(&self.ppdb) } - /// Looks up a file's source contents by its full canonicalized path. - /// - /// The given path must be canonicalized. - pub fn source_by_path(&self, path: &str) -> Result>, FormatError> { + /// See [DebugSession::source_by_path] for more information. + pub fn source_by_path(&self, path: &str) -> Result>, FormatError> { match self.sources.get(path) { None => Ok(None), Some(source) => source .get_contents() - .map(|bytes| Some(from_utf8_cow_lossy(&bytes))), + .map(|bytes| Some(SourceCode::Content(from_utf8_cow_lossy(&bytes)))), } } } @@ -195,7 +193,7 @@ impl<'data, 'session> DebugSession<'session> for PortablePdbDebugSession<'data> self.files() } - fn source_by_path(&self, path: &str) -> Result>, Self::Error> { + fn source_by_path(&self, path: &str) -> Result>, Self::Error> { self.source_by_path(path) } } diff --git a/symbolic-debuginfo/src/sourcebundle.rs b/symbolic-debuginfo/src/sourcebundle.rs index 8f045b08d..94aa4a5c3 100644 --- a/symbolic-debuginfo/src/sourcebundle.rs +++ b/symbolic-debuginfo/src/sourcebundle.rs @@ -650,17 +650,15 @@ impl<'data> SourceBundleDebugSession<'data> { Ok(Some(source_content)) } - /// Looks up a file's source contents by its full canonicalized path. - /// - /// The given path must be canonicalized. - pub fn source_by_path(&self, path: &str) -> Result>, SourceBundleError> { + /// See [DebugSession::source_by_path] for more information. + pub fn source_by_path(&self, path: &str) -> Result>, SourceBundleError> { let zip_path = match self.zip_path_by_source_path(path) { Some(zip_path) => zip_path, None => return Ok(None), }; - self.source_by_zip_path(zip_path) - .map(|opt| opt.map(Cow::Owned)) + let content = self.source_by_zip_path(zip_path)?; + Ok(content.map(|opt| SourceCode::Content(Cow::Owned(opt)))) } } @@ -677,7 +675,7 @@ impl<'data, 'session> DebugSession<'session> for SourceBundleDebugSession<'data> self.files() } - fn source_by_path(&self, path: &str) -> Result>, Self::Error> { + fn source_by_path(&self, path: &str) -> Result>, Self::Error> { self.source_by_path(path) } } @@ -1152,11 +1150,15 @@ mod tests { .flatten() .flat_map(|f| { let path = f.abs_path_str(); - session - .source_by_path(&path) - .ok() - .flatten() - .map(|c| (path, c.into_owned())) + session.source_by_path(&path).ok().flatten().map(|source| { + ( + path, + match source { + SourceCode::Content(text) => text.into_owned(), + SourceCode::Url(_) => panic!(), + }, + ) + }) }) .collect(); diff --git a/symbolic-debuginfo/tests/test_objects.rs b/symbolic-debuginfo/tests/test_objects.rs index a1fb70670..b7639d37e 100644 --- a/symbolic-debuginfo/tests/test_objects.rs +++ b/symbolic-debuginfo/tests/test_objects.rs @@ -2,7 +2,7 @@ use std::{env, ffi::CString, fmt, io::BufWriter}; use symbolic_common::ByteView; use symbolic_debuginfo::{ - elf::ElfObject, pe::PeObject, FileEntry, Function, LineInfo, Object, SymbolMap, + elf::ElfObject, pe::PeObject, FileEntry, Function, LineInfo, Object, SourceCode, SymbolMap, }; use symbolic_testutils::fixture; @@ -776,8 +776,10 @@ fn test_ppdb_source_by_path() -> Result<(), Error> { "C:\\dev\\sentry-dotnet\\samples\\Sentry.Samples.Console.Basic\\Program.cs", ) .unwrap(); - let source_text = source.unwrap(); - assert_eq!(source_text.len(), 204); + match source.unwrap() { + SourceCode::Content(text) => assert_eq!(text.len(), 204), + SourceCode::Url(_) => panic!(), + } } Ok(()) From 9932e80c4844642618719ae069283ce7b024225b Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 9 Feb 2023 21:36:12 +0100 Subject: [PATCH 02/19] feat: ppdb source link mapping --- symbolic-debuginfo/src/base.rs | 2 +- symbolic-ppdb/Cargo.toml | 1 + symbolic-ppdb/src/format/mod.rs | 31 ++++++ symbolic-ppdb/src/format/sourcelinks.rs | 103 ++++++++++++++++++ symbolic-ppdb/tests/test_ppdb.rs | 34 ++++++ ...Extensions.Logging.EventLog-sourcelink.pdb | Bin 0 -> 8648 bytes 6 files changed, 170 insertions(+), 1 deletion(-) create mode 100644 symbolic-ppdb/src/format/sourcelinks.rs create mode 100644 symbolic-testutils/fixtures/windows/Microsoft.Extensions.Logging.EventLog-sourcelink.pdb diff --git a/symbolic-debuginfo/src/base.rs b/symbolic-debuginfo/src/base.rs index f77a4e3ff..a7baf60d5 100644 --- a/symbolic-debuginfo/src/base.rs +++ b/symbolic-debuginfo/src/base.rs @@ -676,7 +676,7 @@ pub enum SourceCode<'a> { /// Verbatim source code/file contents. Content(Cow<'a, str>), - /// Url (usually HTTP/S) where the content can be fetched from. + /// Url (usually https) where the content can be fetched from. Url(Cow<'a, str>), } diff --git a/symbolic-ppdb/Cargo.toml b/symbolic-ppdb/Cargo.toml index 3e62d0edf..9b29b6f06 100644 --- a/symbolic-ppdb/Cargo.toml +++ b/symbolic-ppdb/Cargo.toml @@ -26,6 +26,7 @@ watto = { version = "0.1.0", features = ["writer", "strings"] } thiserror = "1.0.31" uuid = "1.0.0" flate2 = { version ="1.0.13", default-features = false, features = [ "rust_backend" ] } +serde_json = { version = "1.0.40" } [dev-dependencies] symbolic-debuginfo = { path = "../symbolic-debuginfo" } diff --git a/symbolic-ppdb/src/format/mod.rs b/symbolic-ppdb/src/format/mod.rs index 6021304c7..deea49d02 100644 --- a/symbolic-ppdb/src/format/mod.rs +++ b/symbolic-ppdb/src/format/mod.rs @@ -1,6 +1,7 @@ mod metadata; mod raw; mod sequence_points; +mod sourcelinks; mod streams; mod utils; @@ -15,6 +16,7 @@ use symbolic_common::{DebugId, Language, Uuid}; use metadata::{ CustomDebugInformation, CustomDebugInformationIterator, MetadataStream, Table, TableType, }; +use sourcelinks::SourceLinkMappings; use streams::{BlobStream, GuidStream, PdbStream, StringStream, UsStream}; /// The kind of a [`FormatError`]. @@ -101,6 +103,9 @@ pub enum FormatErrorKind { /// Tried to read contents of a blob in an unknown format. #[error("invalid blob format {0}")] InvalidBlobFormat(u32), + /// Failed to parse Source Link JSON + #[error("invalid source link JSON")] + InvalidSourceLinkJson, } /// An error encountered while parsing a [`PortablePdb`] file. @@ -160,6 +165,8 @@ pub struct PortablePdb<'data> { blob_stream: Option>, /// The file's #GUID stream, if it exists. guid_stream: Option>, + /// Source link mappings + source_link_mappings: SourceLinkMappings, } impl fmt::Debug for PortablePdb<'_> { @@ -229,6 +236,7 @@ impl<'data> PortablePdb<'data> { us_stream: None, blob_stream: None, guid_stream: None, + source_link_mappings: SourceLinkMappings::empty(), }; let mut metadata_stream = None; @@ -281,6 +289,19 @@ impl<'data> PortablePdb<'data> { )?) } + // Read source link mappings. + // https://github.com/dotnet/runtime/blob/main/docs/design/specs/PortablePdb-Metadata.md#source-link-c-and-vb-compilers + const SOURCE_LINK_KIND: Uuid = uuid::uuid!("CC110556-A091-4D38-9FEC-25AB9A351A6A"); + for cdi in CustomDebugInformationIterator::new(&result, SOURCE_LINK_KIND)? { + let cdi = cdi?; + // Note: only handle module #1 (do we actually handle multiple modules in any way??) + if let (metadata::CustomDebugInformationTag::Module, 1) = (cdi.tag, cdi.value) { + let json = String::from_utf8_lossy(result.get_blob(cdi.blob)?); + result.source_link_mappings.add_mappings(&json)?; + } + } + result.source_link_mappings.sort(); + Ok(result) } @@ -363,6 +384,16 @@ impl<'data> PortablePdb<'data> { pub fn get_embedded_sources(&self) -> Result, FormatError> { EmbeddedSourceIterator::new(self) } + + /// Tries to resolve given document as a source link (URL). + /// Make sure to try [Self::get_embedded_sources] first when looking for a source file, because + /// function may return a link that actually doesn't exist (e.g. file is in .gitignore). + /// In that case, it's usually the case that the file is embedded in the PPDB instead. + pub fn get_source_link(&self, document: &Document) -> Option> { + self.source_link_mappings + .resolve(&document.name) + .map(Cow::Owned) + } } /// Represents a source file that is referenced by this PDB. diff --git a/symbolic-ppdb/src/format/sourcelinks.rs b/symbolic-ppdb/src/format/sourcelinks.rs new file mode 100644 index 000000000..11db10dba --- /dev/null +++ b/symbolic-ppdb/src/format/sourcelinks.rs @@ -0,0 +1,103 @@ +use std::cmp::Ordering; + +use crate::{FormatError, FormatErrorKind}; + +/// See https://github.com/dotnet/designs/blob/main/accepted/2020/diagnostics/source-link.md#source-link-json-schema +#[derive(Clone)] +pub(crate) struct SourceLinkMappings { + rules: Vec, + sorted: bool, +} + +#[derive(Clone)] +struct Rule { + pattern: Pattern, + url: String, +} + +#[derive(Clone)] +enum Pattern { + Exact(String), + Prefix(String), +} + +impl SourceLinkMappings { + pub fn empty() -> Self { + SourceLinkMappings { + rules: Vec::new(), + sorted: false, + } + } + + pub fn add_mappings(&mut self, json: &str) -> Result<(), FormatError> { + use serde_json::*; + let json: Value = serde_json::from_str(json) + .map_err(|e| FormatError::new(FormatErrorKind::InvalidSourceLinkJson, e))?; + + let docs = json + .get("documents") + .and_then(|v| v.as_object()) + .ok_or_else(Self::err)?; + + self.rules.reserve(docs.len()); + for doc in docs.iter() { + /* + Each document is defined by a file path and a URL. Original source file paths are compared + case-insensitively to documents and the resulting URL is used to download source. The document + may contain an asterisk to represent a wildcard in order to match anything in the asterisk's + location. The rules for the asterisk are as follows: + 1. The only acceptable wildcard is one and only one '*', which if present will be replaced by a relative path. + 2. If the file path does not contain a *, the URL cannot contain a * and if the file path contains a * the URL must contain a *. + 3. If the file path contains a *, it must be the final character. + 4. If the URL contains a *, it may be anywhere in the URL. + */ + let key = doc.0; + let url = doc.1.as_str().ok_or_else(Self::err)?.into(); + let pattern = if key.ends_with('*') { + Pattern::Prefix(key[..(key.len() - 1)].into()) + } else { + Pattern::Exact(key.into()) + }; + self.rules.push(Rule { pattern, url }) + } + Ok(()) + } + + fn err() -> FormatError { + FormatError { + kind: FormatErrorKind::InvalidSourceLinkJson, + source: None, + } + } + + /// Sort internal rules. This must be called before [Self::resolve]. + pub fn sort(&mut self) { + // Put Exact matches first, then sort by the Prefix length, longest to shortest. + self.rules.sort_unstable_by(|a, b| match &a.pattern { + Pattern::Exact(_) => Ordering::Less, + Pattern::Prefix(a) => match &b.pattern { + Pattern::Exact(_) => Ordering::Greater, + Pattern::Prefix(b) => b.len().cmp(&a.len()), + }, + }); + self.sorted = true; + } + + /// Resolve the path to a URL. + pub fn resolve(&self, path: &str) -> Option { + // Must be sorted first so we can return on the first match, which is guaranteed to be the most specific. + if !self.sorted { + return None; + } + + for rule in &self.rules { + if match &rule.pattern { + Pattern::Exact(value) => value == path, + Pattern::Prefix(value) => path.starts_with(value), + } { + return Some(rule.url.clone()); + } + } + None + } +} diff --git a/symbolic-ppdb/tests/test_ppdb.rs b/symbolic-ppdb/tests/test_ppdb.rs index 9988bb7e1..5f892200b 100644 --- a/symbolic-ppdb/tests/test_ppdb.rs +++ b/symbolic-ppdb/tests/test_ppdb.rs @@ -1,3 +1,5 @@ +use std::path::Path; + use symbolic_debuginfo::pe::PeObject; use symbolic_ppdb::{EmbeddedSource, PortablePdb}; use symbolic_testutils::fixture; @@ -156,3 +158,35 @@ fn test_pe_embedded_ppdb_with_sources() { "Sentry.Samples.Console.Basic.AssemblyInfo.cs", ); } + +#[test] +fn test_source_links() { + // Source: https://nuget.info/api/MsdlProxy?symbolKey=microsoft.extensions.logging.eventlog.pdb/37e9e8a61a8e404eb93c6902e277ff55FFFFFFFF/microsoft.extensions.logging.eventlog.pdb + let buf = std::fs::read(fixture( + "windows/Microsoft.Extensions.Logging.EventLog-sourcelink.pdb", + )) + .unwrap(); + + let ppdb = PortablePdb::parse(&buf).unwrap(); + + // Firstly, let's assert for the sources that are embedded in the PPDB itself because they're not on GitHub. + let embedded_sources = ppdb + .get_embedded_sources() + .unwrap() + .map(|src| { + Path::new(src.unwrap().get_path()) + .file_name() + .unwrap() + .to_string_lossy() + .to_string() + }) + .collect::>(); + assert_eq!( + embedded_sources, + vec![ + ".NETFramework,Version=v4.6.2.AssemblyAttributes.cs", + "Microsoft.Extensions.Logging.EventLog.AssemblyInfo.cs", + "LibraryImports.g.cs" + ] + ); +} diff --git a/symbolic-testutils/fixtures/windows/Microsoft.Extensions.Logging.EventLog-sourcelink.pdb b/symbolic-testutils/fixtures/windows/Microsoft.Extensions.Logging.EventLog-sourcelink.pdb new file mode 100644 index 0000000000000000000000000000000000000000..732fb01d3b0d39dad5d1269fa4aaf8f2fa69a9b2 GIT binary patch literal 8648 zcma)B2V7Iv_rDp0Fajh%L_i5pKm-DW4GuQ4DMQfuS2A885E9G)qoT;DqwZa;qje9g zpaSCjIjT7Nx7b>%wZ*EnT4%Ltwd(KOmqee`-|zE-&o}3td&WI?oqOMXi3J&nG(#Y%D;}=z2x1#2{=EUzEqM-j$`?~0=X6afeM#Uaq*jU*i zGO9S>eE`9LGC&T%5`g`&00&Gw7wABMKY$K^sz`gRgIkAV!oYtZH#Qi3X)HiZ2-{=- zqW~O$r->GWYAIMO0=f$5MWDX`?MI^_PZ}NN0v!c(CD2VkF9W>+bSRyMvgmX)5$H^y zhk>328pWWY5ezz-0(2hGpMgFADq_-55|fU013e0~%YufwE$FC?%|wUUxJB>(vqoP! zSR+du{yTphWXI(oSAZuV2oMg4C-pce4W0`CqXA_A9bhtG5nwBzjcbRx0H*;xfS&-j z0Z#yIo*l9WxC4R#QGnrqG(aKG21x)~z(l|_zzo1Vz*4{}z8T9(;8uzc`Uc@BX{T{=k}5F`pBL7$Z;d6{`nMnKalmK z@HiHiKKlJBa=fNoDLnS=)<@sH4?lpSkNsmG)Ob87ayG~WeR$74ycdPH2Yv58{yw0{ zu|FRQkM(`~===5I2T^!z?@zJE{NO%(0ENfx3#8cN@eJz23n@JIhjSUVKX9@&^H?5A z;jupvg~#$R3XkRC6dubXkkuQ1IKD`V9P3BHj5OCb0QhJMk7JFY@Hh{~LUuIkV|$#F z%{1 z+)kzNcx=)r{8QjZ0FPHN{@Vvg2j0TO4+1^|X1tll{hLYQ@p{Ul@ObaYrtpry=TLY7 z@VUUVO#ZRIJb;2nI9)G;oQ{_?$kNwLi`VABw@8u;X-UjN!>mnUc z2IFIEs$Xs5@r=fO9R9!6^Qc)oy>mb(QhO*jXuVbQhXl3 z)`*MuU_3|gDZ&DPTgCw37SK?%BhLJIcH&bM4NZ2$D-UNMVA|x@{2&zu&Q?3=8xpTgtRRPjSG|>4aRYSL#w~2$e2fhN=jyR$N9Pty-^< z5;@{30#%8nzqE{$oga%cM74V}`R}&hY zg3y{B7Z7DtP!rPX#2Q_?N=8gTT0AcJ&;FeMj83j&;`C2y?V6Wu=RcEtIQ%Qi^W6Cb z=y2{j`sZ%Nv+R5os?^;rvv#I(|9o|a@A7)=R)_5APu1zt#_-bpI^m)%mnS68NwpVk z>e_XFqy_iPg0<1vYwq}LTM^+}VR7fxo^!JsGnQUTl{D?kyvR<=o}#iEleOd0X@{-~ z9Za2Z46R0LrBp~XVhsfCldX_y)LOM%Crp{3BUD<2TBQ|2E6d=RD>OIR!~9T2Xi~*e zomyj{XiK&1U_@hBav4#A?$CsL;`>+hbyqG1eEQ9NS1U%dQ+7&Og$b*?(@$WceLlLA7g!)JMl z`80;0vcu_wOE2HM%VYL^d3WIL$Z5MDmM^Z|RZ}!(X06AVp|nQO?((n`$@Mo+y7+0`@Ki}w)2YLos-m9P6`gU zKL0Fu);GK_pZ>UIpF{IgLy)>^^6S7e+uDz98J%En%}P>NRjF0hZ^zKmG~(_#dZiK< z0Q5O{gOJptN!YpJj+_zT0>+hB$oMll}=Gb*puU+t}Y-nH3})A?GG|o6^8VxYPCj} zpo3REi5|?PS_fmb_ZSUZ8!Z|+jROSthg=gq9QW9xvSifZ);5jiYLk^sSxM;mjVs0; ztQ%JSg_H3`F5K8t2Z&{f*s%n7HBnG8bC; zb`DQ}<~ryz{}$dbkDI!!Q(wAABcy3>vmQxk@S2AJo%57pom{P{`j0Lxnz&8w0-I`u zYuWPCo4Qx*`O&#x+1%H?`QgV?e|vml$I%YU>8+=%AnhsSkR`MXwWPu_pHLEFEx}R| zy6{MmNSKpSn5u!KTdUSo2AXOgS`#je6pDliS}jo}Q5xQf0MDEzS(A#}q-mPiG))k; z@@=2u5I z1T-&fnDI%4MZ*8pe13dR=ZL!z%t!ODr{4P6ZF1q5uRjYbaN5~BZF_vDA18g=ay!%ERQs4EH;QdOUY}YMe{-FieaX>P@t0zE zJQQEir(Bo0vD5l*i#mJm=9+Z9 zH+_+=lH>I7PA@b!2`1mM+#_1HdGnI_Kh{sFY#R{w+q}Sr}vf+pUjrZ&GxWlvVVRyUO1Ov&wVWLBT7$d8N3_;z+k-OppY!h$-dC2wGB<)xpa z@eJD${#WC^Ykt1DRFo$5iN8mDWs%tJI-;b?p)+goh2;*%N_O@yc3qw&{OMbe$K zQ?w6C{^jx0sjugkoSIQ(c(v);t(kJ4ua~@<;`d~1vbA!u z-I={#+5TDb)$I!Ha_=)GLFI#@!;-wtURzOpI_}!ck*UOWk8#`#=^B^Ug*Wz|T|ae5 zQu>pdxdH3WcaK=Iq-cb~T&Y|9F_@;GZ{=BU_i z?YDV~h-+J3_O`Ftxx43r&B?$Qf44*py?C$2onvr&RwJ>xZ&UH;T*pC==Lc4M+_gJW zUbnq`R%`6YI!`~kks{3ANg0FcEOPizczXAI}>>0<_;@Q?uFs| zPIs;PdC=PlZ_2pBcQS1np&~S5I8->kQ>+!@oXH_mFPAYx5C5y3*7oizSy@ul;pLxH za(bT2XHo92KE_8#bbsHW;9$WJv0kSRDlymS&-w^@^S3 z6+A9@uve^CxlUKDjSUXgh--yq3SGHgqSq1{sTwwDl};#CR|U(|I>=nX8snA`EDMQ= zk%vS@5aAIrkysoV5)vgAi6l{>lJE#|NOV+0bVNjqJURrt*VQ$+)EKiXyw`%SYK4-} z1euG%h2g@GpfI6G6dn~L42=vBg%MIhERu#x2x)kfNFEbGghoq6;+XJgnLI`=4U>e5 z32B5JDaEQXJwCxEd7BQgL2_^%BvnI@C^QPG^}1@kE~paD2q@X05?3jtum>5k6sk%> zX6j?4u7*8OF;T38?2k0YBpoCpB>J+TYNcL_s!h8AN>CY+s`bd!18?Gw5{5wh8mTZ8 zH$fDJn-CrrA`KUbrIHXr5)vLADvyYkg#sHIB@)ZS!Xv{X!w3-}lfG%f8*kMbqDDc~ zB9&=dLn$h}yWz7t9L!N+enL`8V17zgN*vhL}*`|2vOgX$bS_4!zhw!gqkVm?nJ2)lD$DCB^L*5 zoP<^=Q!3&0=x)#49jG2|8Ok3KzOqH}^JIgS?e?kX->#>(Cpcj;QBAa>R7q=;e>-mE+&*#{4?*o^WnYW%S~3_ALJ6-mO#I^gR`sKTMgEZ1Z3J7(*CY zYVip+b^Lz%6qFP3MI#r;{?{jWrR}@IbJj8KZe3Y|dWx8mckQMnB(!)3zMwYQhOl=&sj+Hq>H zbrGa7eF$WT3@|Qei5jwRh^;h|Qs!%7053|1f@4c751N*SQZhy(Xn>q6%J3 zO;ufwtd*crI(n2cs(z2QvQtxF`xEv0>%ol(v-1HQ3w4EQLkm88ysfY zpm|49nq%*z`CeGJ&}0fPPlQ;8d%sYut<+Lq@r}cAv;w9b#Ghj~bARr-A*u6zD1AKS zliz2U>_>`~dZJKYtu%+a0=E}>X?^$UU1==$^aIxCt_b}PNov&Kx+_$eqX8C{WcouE`VcPs&e4#biGX~#b1xH)4;8IGCi1fSST8RbYzj(4{L=_f+7!8O%_e@#dl$W_4A<26_sKpvVI9LT%^PIgv|Qo z-h7~m_1mxplWmFI=x|>~qZ@E>R(+?rm`BG~?sS9E>X5gQ9`VLq5z7NDcWev_WW9V} z91728I219R$>+3W{A@!$%Tw_)`N@o@D7HjA=Ao*$##LT8PG0>?qXoXTeLFt7e-CT0%R;W$Ix%&jrU>Fq;?JIF)^lU!(|k;IWC zHrz;JND}{nk@#bO!vetHKx(@~{BXHvKaLHmU_w9IGZE#K`QLZNu=i&9_idkV{n*eN zw$c_}rY&O9xcElX(-&&?qQfm{4HNNk^|;hK5=!|@7(ib-d{}@|JPO_%h@Qqo4DdlV z%86_utv?jQUFbj#CLT_Sq>K}YO&slwCb)>kC-v+(5n;e72LcCOVXB>jD|5zECKx{r zpu?SPp*37$k7mQID)hdqX~86^l}h8GKr1BA+X^Y~pKHm+R8&Yz(AnZ9FM3E@Y)f10 zLBav z06Ow9j)xQ99#2)h`+Mhs>7c-%EwRN@EC%;?Fq2I#RdQ$s*~5iAQ;&NjE|}v0_u{b; z9492iV*(ip>v#-Ms#uec1OBT8Zki;{}NQSVO@MX`(33J>* zPW~3~Eg*>FhBE2!HKhRmK<;4U=5TN}U?Dypb}`F<=imSbD@)utEL6f|+u5k4%y6CMhZg&z@2r9?5;8d)=C@Cc)YqAXD| cvo-|L>vG^b18K~&4q_kyb1Gb4GsmNU1EfEmBme*a literal 0 HcmV?d00001 From f78f5af8e3e915b27d79916b6a829b88779fd913 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Feb 2023 14:05:17 +0100 Subject: [PATCH 03/19] source link resolve() and tests --- symbolic-debuginfo/src/ppdb.rs | 49 +++++++--- symbolic-debuginfo/tests/test_objects.rs | 31 ++++++ symbolic-ppdb/src/format/sourcelinks.rs | 114 +++++++++++++++++++++-- symbolic-ppdb/src/lib.rs | 2 +- symbolic-ppdb/tests/test_ppdb.rs | 8 ++ 5 files changed, 183 insertions(+), 21 deletions(-) diff --git a/symbolic-debuginfo/src/ppdb.rs b/symbolic-debuginfo/src/ppdb.rs index 9d5e919c8..9c72a3b71 100644 --- a/symbolic-debuginfo/src/ppdb.rs +++ b/symbolic-debuginfo/src/ppdb.rs @@ -4,9 +4,10 @@ use std::collections::HashMap; use std::fmt; use std::iter; +use lazycell::LazyCell; use symbolic_common::{Arch, CodeId, DebugId}; use symbolic_ppdb::EmbeddedSource; -use symbolic_ppdb::{FormatError, PortablePdb}; +use symbolic_ppdb::{Document, FormatError, PortablePdb}; use crate::base::*; @@ -141,24 +142,44 @@ impl fmt::Debug for PortablePdbObject<'_> { /// A debug session for a Portable PDB object. pub struct PortablePdbDebugSession<'data> { ppdb: PortablePdb<'data>, - sources: HashMap>, + sources: LazyCell>>, +} + +#[derive(Debug, Clone)] +enum PPDBSource<'data> { + Embedded(EmbeddedSource<'data>), + Link(Document), } impl<'data> PortablePdbDebugSession<'data> { fn new(ppdb: &'_ PortablePdb<'data>) -> Result { - let mut sources: HashMap> = HashMap::new(); - for source in ppdb.get_embedded_sources()? { - match source { - Ok(source) => sources.insert(source.get_path().into(), source), - Err(e) => return Err(e), - }; - } Ok(PortablePdbDebugSession { ppdb: ppdb.clone(), - sources, + sources: LazyCell::new(), }) } + fn init_sources(&self) -> HashMap> { + let count = self.ppdb.get_documents_count().unwrap_or(0); + let mut result = HashMap::with_capacity(count); + + if let Ok(iter) = self.ppdb.get_embedded_sources() { + for source in iter.flatten() { + result.insert(source.get_path().to_string(), PPDBSource::Embedded(source)); + } + }; + + for i in 0..count { + if let Ok(doc) = self.ppdb.get_document(i + 1) { + if !result.contains_key(&doc.name) { + result.insert(doc.name.clone(), PPDBSource::Link(doc)); + } + } + } + + result + } + /// Returns an iterator over all functions in this debug file. pub fn functions(&self) -> PortablePdbFunctionIterator<'_> { iter::empty() @@ -171,11 +192,15 @@ impl<'data> PortablePdbDebugSession<'data> { /// See [DebugSession::source_by_path] for more information. pub fn source_by_path(&self, path: &str) -> Result>, FormatError> { - match self.sources.get(path) { + let sources = self.sources.borrow_with(|| self.init_sources()); + match sources.get(path) { None => Ok(None), - Some(source) => source + Some(PPDBSource::Embedded(source)) => source .get_contents() .map(|bytes| Some(SourceCode::Content(from_utf8_cow_lossy(&bytes)))), + Some(PPDBSource::Link(document)) => { + Ok(self.ppdb.get_source_link(document).map(SourceCode::Url)) + } } } } diff --git a/symbolic-debuginfo/tests/test_objects.rs b/symbolic-debuginfo/tests/test_objects.rs index b7639d37e..4c5f44c3f 100644 --- a/symbolic-debuginfo/tests/test_objects.rs +++ b/symbolic-debuginfo/tests/test_objects.rs @@ -785,6 +785,37 @@ fn test_ppdb_source_by_path() -> Result<(), Error> { Ok(()) } +#[test] +fn test_ppdb_source_links() -> Result<(), Error> { + let view = ByteView::open(fixture( + "windows/Microsoft.Extensions.Logging.EventLog-sourcelink.pdb", + ))?; + let object = Object::parse(&view)?; + let session = object.debug_session()?; + + let known_embedded_sources = vec![ + ".NETFramework,Version=v4.6.2.AssemblyAttributes.cs", + "Microsoft.Extensions.Logging.EventLog.AssemblyInfo.cs", + "LibraryImports.g.cs", + ]; + + for file in session.files() { + let file = file.unwrap(); + + match session.source_by_path(&file.path_str()).unwrap().unwrap() { + SourceCode::Content(text) => { + assert!(known_embedded_sources.contains(&file.name_str().as_ref())); + assert!(!text.is_empty()); + } + SourceCode::Url(url) => { + // testing this is simple because there's just one prefix rule in this PPDB. + assert_eq!(url, format!("https://raw.githubusercontent.com/dotnet/runtime/d099f075e45d2aa6007a22b71b45a08758559f80/{}", &file.path_str()[3..])); + } + } + } + Ok(()) +} + #[test] fn test_wasm_symbols() -> Result<(), Error> { let view = ByteView::open(fixture("wasm/simple.wasm"))?; diff --git a/symbolic-ppdb/src/format/sourcelinks.rs b/symbolic-ppdb/src/format/sourcelinks.rs index 11db10dba..a953222fc 100644 --- a/symbolic-ppdb/src/format/sourcelinks.rs +++ b/symbolic-ppdb/src/format/sourcelinks.rs @@ -51,14 +51,15 @@ impl SourceLinkMappings { 3. If the file path contains a *, it must be the final character. 4. If the URL contains a *, it may be anywhere in the URL. */ - let key = doc.0; + let key = doc.0.to_lowercase(); let url = doc.1.as_str().ok_or_else(Self::err)?.into(); let pattern = if key.ends_with('*') { Pattern::Prefix(key[..(key.len() - 1)].into()) } else { - Pattern::Exact(key.into()) + Pattern::Exact(key) }; - self.rules.push(Rule { pattern, url }) + self.rules.push(Rule { pattern, url }); + self.sorted = false; } Ok(()) } @@ -90,14 +91,111 @@ impl SourceLinkMappings { return None; } + // Note: this is currently quite simple, just pick the first match. If we needed to improve + // performance in the future because we encounter PDBs with too many items, we can do a + // prefix binary search, for example. + let path_lower = path.to_lowercase(); for rule in &self.rules { - if match &rule.pattern { - Pattern::Exact(value) => value == path, - Pattern::Prefix(value) => path.starts_with(value), - } { - return Some(rule.url.clone()); + match &rule.pattern { + Pattern::Exact(value) => { + if value == &path_lower { + return Some(rule.url.clone()); + } + } + Pattern::Prefix(value) => { + if path_lower.starts_with(value) { + let replacement = path + .get(value.len()..) + .map(|v| v.replace('\\', "/")) + .unwrap_or_default(); + return Some(rule.url.replace('*', &replacement)); + } + } } } None } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_invalid_json() { + let mut mappings = SourceLinkMappings::empty(); + assert!(mappings.add_mappings("").is_err()); + assert!(mappings.add_mappings("foo").is_err()); + assert!(mappings.add_mappings("{\"docs\": {\"k\": \"v\"}}").is_err()); + assert!(mappings + .add_mappings("{\"documents\": [\"k\", \"v\"]}") + .is_err()); + assert_eq!(mappings.rules.len(), 0); + } + + #[test] + fn test_mapping() { + let mut mappings = SourceLinkMappings::empty(); + mappings + .add_mappings( + r#" + { + "documents": { + "C:\\src\\*": "http://MyDefaultDomain.com/src/*", + "C:\\src\\fOO\\*": "http://MyFooDomain.com/src/*", + "C:\\src\\foo\\specific.txt": "http://MySpecificFoodDomain.com/src/specific.txt", + "C:\\src\\bar\\*": "http://MyBarDomain.com/src/*" + } + } + "# + ).unwrap(); + + assert_eq!(mappings.rules.len(), 4); + assert!(!mappings.sorted); + mappings.sort(); + assert!(mappings.sorted); + + mappings + .add_mappings( + r#" + { + "documents": { + "C:\\src\\file.txt": "https://example.com/file.txt" + } + } + "#, + ) + .unwrap(); + + assert_eq!(mappings.rules.len(), 5); + assert!(!mappings.sorted); + mappings.sort(); + assert!(mappings.sorted); + + // In this example: + // All files under directory bar will map to a relative URL beginning with http://MyBarDomain.com/src/. + // All files under directory foo will map to a relative URL beginning with http://MyFooDomain.com/src/ EXCEPT foo/specific.txt which will map to http://MySpecificFoodDomain.com/src/specific.txt. + // All other files anywhere under the src directory will map to a relative url beginning with http://MyDefaultDomain.com/src/. + assert!(mappings.resolve("c:\\other\\path").is_none()); + assert_eq!( + mappings.resolve("c:\\src\\bAr\\foo\\FiLe.txt").unwrap(), + "http://MyBarDomain.com/src/foo/FiLe.txt" + ); + assert_eq!( + mappings.resolve("c:\\src\\foo\\FiLe.txt").unwrap(), + "http://MyFooDomain.com/src/FiLe.txt" + ); + assert_eq!( + mappings.resolve("c:\\src\\foo\\SpEcIfIc.txt").unwrap(), + "http://MySpecificFoodDomain.com/src/specific.txt" + ); + assert_eq!( + mappings.resolve("c:\\src\\other\\path").unwrap(), + "http://MyDefaultDomain.com/src/other/path" + ); + assert_eq!( + mappings.resolve("c:\\src\\other\\path").unwrap(), + "http://MyDefaultDomain.com/src/other/path" + ); + } +} diff --git a/symbolic-ppdb/src/lib.rs b/symbolic-ppdb/src/lib.rs index 4b8408d89..d7ddc8179 100644 --- a/symbolic-ppdb/src/lib.rs +++ b/symbolic-ppdb/src/lib.rs @@ -60,4 +60,4 @@ mod format; pub use cache::lookup::LineInfo; pub use cache::writer::PortablePdbCacheConverter; pub use cache::{CacheError, CacheErrorKind, PortablePdbCache}; -pub use format::{EmbeddedSource, FormatError, FormatErrorKind, PortablePdb}; +pub use format::{Document, EmbeddedSource, FormatError, FormatErrorKind, PortablePdb}; diff --git a/symbolic-ppdb/tests/test_ppdb.rs b/symbolic-ppdb/tests/test_ppdb.rs index 5f892200b..f3cc6607c 100644 --- a/symbolic-ppdb/tests/test_ppdb.rs +++ b/symbolic-ppdb/tests/test_ppdb.rs @@ -189,4 +189,12 @@ fn test_source_links() { "LibraryImports.g.cs" ] ); + + for i in 0..ppdb.get_documents_count().unwrap() { + let doc = ppdb.get_document(i + 1).unwrap(); + let url = ppdb.get_source_link(&doc).unwrap(); + + // testing this is simple because there's just one prefix rule in this PPDB. + assert_eq!(url, format!("https://raw.githubusercontent.com/dotnet/runtime/d099f075e45d2aa6007a22b71b45a08758559f80/{}", &doc.name[3..])); + } } From 5d4f336c7ff381dcc38e07acc99f3a26d8bd64c9 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Feb 2023 15:20:03 +0100 Subject: [PATCH 04/19] chore: remove todo --- symbolic-debuginfo/src/base.rs | 1 - symbolic-debuginfo/src/object.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/symbolic-debuginfo/src/base.rs b/symbolic-debuginfo/src/base.rs index a7baf60d5..6f632d7f2 100644 --- a/symbolic-debuginfo/src/base.rs +++ b/symbolic-debuginfo/src/base.rs @@ -728,7 +728,6 @@ pub trait DebugSession<'session> { /// Looks up a file's source by its full canonicalized path. /// Returns either source contents, if it was embedded, or a source link. - /// TODO also change to `&mut self` when breaking the API - see https://github.com/getsentry/symbolic/issues/736 fn source_by_path(&self, path: &str) -> Result>, Self::Error>; } diff --git a/symbolic-debuginfo/src/object.rs b/symbolic-debuginfo/src/object.rs index 0755bba13..c9419248d 100644 --- a/symbolic-debuginfo/src/object.rs +++ b/symbolic-debuginfo/src/object.rs @@ -484,7 +484,6 @@ impl<'d> ObjectDebugSession<'d> { /// Looks up a file's source by its full canonicalized path. /// Returns either source contents, if it was embedded, or a source link. - /// TODO also change to `&mut self` when breaking the API - see https://github.com/getsentry/symbolic/issues/736 pub fn source_by_path(&self, path: &str) -> Result>, ObjectError> { match *self { ObjectDebugSession::Breakpad(ref s) => { From 5e2e6b6aa6f59ea2e0a0009925ccb569410ad805 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Feb 2023 15:35:25 +0100 Subject: [PATCH 05/19] chore: minor changes --- symbolic-debuginfo/src/breakpad.rs | 2 +- symbolic-debuginfo/tests/test_objects.rs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/symbolic-debuginfo/src/breakpad.rs b/symbolic-debuginfo/src/breakpad.rs index a782b47a9..0e6ccb2ab 100644 --- a/symbolic-debuginfo/src/breakpad.rs +++ b/symbolic-debuginfo/src/breakpad.rs @@ -1295,7 +1295,7 @@ impl<'data, 'session> DebugSession<'session> for BreakpadDebugSession<'data> { self.files() } - fn source_by_path(&self, path: &str) -> Result>, BreakpadError> { + fn source_by_path(&self, path: &str) -> Result>, Self::Error> { self.source_by_path(path) } } diff --git a/symbolic-debuginfo/tests/test_objects.rs b/symbolic-debuginfo/tests/test_objects.rs index 4c5f44c3f..678fba54d 100644 --- a/symbolic-debuginfo/tests/test_objects.rs +++ b/symbolic-debuginfo/tests/test_objects.rs @@ -813,6 +813,8 @@ fn test_ppdb_source_links() -> Result<(), Error> { } } } + + assert!(session.source_by_path("c:/non/existent/path.cs")?.is_none()); Ok(()) } From aa72c59de5519835772acc9b4ab70558fb748523 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Feb 2023 15:36:47 +0100 Subject: [PATCH 06/19] chore: update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec54c3538..6ccf4cf98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +**Breaking changes**: + +- Change `DebugSession::source_by_path()` to return `SourceCode` enum with either file content or a URL to fetch it from. ([#758](https://github.com/getsentry/symbolic/pull/758)) + **Fixes**: - Make sure to parse `PortablePdb` streams in the correct order. ([#760](https://github.com/getsentry/symbolic/pull/760)) From cdac4abf9eed53e4719ab9ce3a78802811375b59 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 10 Feb 2023 15:49:52 +0100 Subject: [PATCH 07/19] chore: fixup docs link --- symbolic-ppdb/src/format/sourcelinks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/symbolic-ppdb/src/format/sourcelinks.rs b/symbolic-ppdb/src/format/sourcelinks.rs index a953222fc..5e3e17435 100644 --- a/symbolic-ppdb/src/format/sourcelinks.rs +++ b/symbolic-ppdb/src/format/sourcelinks.rs @@ -2,7 +2,7 @@ use std::cmp::Ordering; use crate::{FormatError, FormatErrorKind}; -/// See https://github.com/dotnet/designs/blob/main/accepted/2020/diagnostics/source-link.md#source-link-json-schema +/// See [Source Link PPDB docs](https://github.com/dotnet/designs/blob/main/accepted/2020/diagnostics/source-link.md#source-link-json-schema). #[derive(Clone)] pub(crate) struct SourceLinkMappings { rules: Vec, From ce4bf588ecb8daea246a3a7ba3898b47c2490753 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos <6349682+vaind@users.noreply.github.com> Date: Sat, 11 Feb 2023 13:31:26 +0100 Subject: [PATCH 08/19] Apply suggestions from code review Co-authored-by: Sebastian Zivota --- symbolic-debuginfo/src/ppdb.rs | 4 ++-- symbolic-debuginfo/src/sourcebundle.rs | 11 ++++------- symbolic-ppdb/src/format/sourcelinks.rs | 4 ++-- symbolic-ppdb/tests/test_ppdb.rs | 4 ++-- 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/symbolic-debuginfo/src/ppdb.rs b/symbolic-debuginfo/src/ppdb.rs index 9c72a3b71..e7b9362a0 100644 --- a/symbolic-debuginfo/src/ppdb.rs +++ b/symbolic-debuginfo/src/ppdb.rs @@ -169,8 +169,8 @@ impl<'data> PortablePdbDebugSession<'data> { } }; - for i in 0..count { - if let Ok(doc) = self.ppdb.get_document(i + 1) { + for i in 1..count + 1 { + if let Ok(doc) = self.ppdb.get_document(i) { if !result.contains_key(&doc.name) { result.insert(doc.name.clone(), PPDBSource::Link(doc)); } diff --git a/symbolic-debuginfo/src/sourcebundle.rs b/symbolic-debuginfo/src/sourcebundle.rs index 94aa4a5c3..ffc053021 100644 --- a/symbolic-debuginfo/src/sourcebundle.rs +++ b/symbolic-debuginfo/src/sourcebundle.rs @@ -1151,13 +1151,10 @@ mod tests { .flat_map(|f| { let path = f.abs_path_str(); session.source_by_path(&path).ok().flatten().map(|source| { - ( - path, - match source { - SourceCode::Content(text) => text.into_owned(), - SourceCode::Url(_) => panic!(), - }, - ) + let SourceCode::Content(text) = source else { + unreachable!(); + }; + (path, text.into_owned()) }) }) .collect(); diff --git a/symbolic-ppdb/src/format/sourcelinks.rs b/symbolic-ppdb/src/format/sourcelinks.rs index 5e3e17435..352e7aadc 100644 --- a/symbolic-ppdb/src/format/sourcelinks.rs +++ b/symbolic-ppdb/src/format/sourcelinks.rs @@ -53,8 +53,8 @@ impl SourceLinkMappings { */ let key = doc.0.to_lowercase(); let url = doc.1.as_str().ok_or_else(Self::err)?.into(); - let pattern = if key.ends_with('*') { - Pattern::Prefix(key[..(key.len() - 1)].into()) + let pattern = if let Some(prefix) = key.strip_suffix('*') { + Pattern::Prefix(prefix.into()) } else { Pattern::Exact(key) }; diff --git a/symbolic-ppdb/tests/test_ppdb.rs b/symbolic-ppdb/tests/test_ppdb.rs index f3cc6607c..cb55a9e67 100644 --- a/symbolic-ppdb/tests/test_ppdb.rs +++ b/symbolic-ppdb/tests/test_ppdb.rs @@ -190,8 +190,8 @@ fn test_source_links() { ] ); - for i in 0..ppdb.get_documents_count().unwrap() { - let doc = ppdb.get_document(i + 1).unwrap(); + for i in 1..ppdb.get_documents_count().unwrap() + 1 { + let doc = ppdb.get_document(i).unwrap(); let url = ppdb.get_source_link(&doc).unwrap(); // testing this is simple because there's just one prefix rule in this PPDB. From f7c0a0977a5341828235b418f385bca65f758234 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Sat, 11 Feb 2023 13:50:55 +0100 Subject: [PATCH 09/19] refactor SourceLinkMappings to build sorted --- symbolic-ppdb/src/format/mod.rs | 6 ++- symbolic-ppdb/src/format/sourcelinks.rs | 53 +++++++++---------------- 2 files changed, 23 insertions(+), 36 deletions(-) diff --git a/symbolic-ppdb/src/format/mod.rs b/symbolic-ppdb/src/format/mod.rs index deea49d02..c752b7b1c 100644 --- a/symbolic-ppdb/src/format/mod.rs +++ b/symbolic-ppdb/src/format/mod.rs @@ -292,15 +292,17 @@ impl<'data> PortablePdb<'data> { // Read source link mappings. // https://github.com/dotnet/runtime/blob/main/docs/design/specs/PortablePdb-Metadata.md#source-link-c-and-vb-compilers const SOURCE_LINK_KIND: Uuid = uuid::uuid!("CC110556-A091-4D38-9FEC-25AB9A351A6A"); + let mut source_link_mappings = Vec::new(); for cdi in CustomDebugInformationIterator::new(&result, SOURCE_LINK_KIND)? { let cdi = cdi?; // Note: only handle module #1 (do we actually handle multiple modules in any way??) if let (metadata::CustomDebugInformationTag::Module, 1) = (cdi.tag, cdi.value) { let json = String::from_utf8_lossy(result.get_blob(cdi.blob)?); - result.source_link_mappings.add_mappings(&json)?; + source_link_mappings.push(json); } } - result.source_link_mappings.sort(); + result.source_link_mappings = + SourceLinkMappings::new(source_link_mappings.iter().map(|v| v.as_ref()))?; Ok(result) } diff --git a/symbolic-ppdb/src/format/sourcelinks.rs b/symbolic-ppdb/src/format/sourcelinks.rs index 352e7aadc..56e387734 100644 --- a/symbolic-ppdb/src/format/sourcelinks.rs +++ b/symbolic-ppdb/src/format/sourcelinks.rs @@ -6,7 +6,6 @@ use crate::{FormatError, FormatErrorKind}; #[derive(Clone)] pub(crate) struct SourceLinkMappings { rules: Vec, - sorted: bool, } #[derive(Clone)] @@ -23,13 +22,22 @@ enum Pattern { impl SourceLinkMappings { pub fn empty() -> Self { - SourceLinkMappings { - rules: Vec::new(), - sorted: false, + SourceLinkMappings { rules: Vec::new() } + } + + pub fn new<'a, I>(jsons: I) -> Result + where + I: Iterator, + { + let mut result = Self { rules: Vec::new() }; + for json in jsons { + result.add_mappings(json)?; } + result.sort(); + Ok(result) } - pub fn add_mappings(&mut self, json: &str) -> Result<(), FormatError> { + fn add_mappings(&mut self, json: &str) -> Result<(), FormatError> { use serde_json::*; let json: Value = serde_json::from_str(json) .map_err(|e| FormatError::new(FormatErrorKind::InvalidSourceLinkJson, e))?; @@ -59,7 +67,6 @@ impl SourceLinkMappings { Pattern::Exact(key) }; self.rules.push(Rule { pattern, url }); - self.sorted = false; } Ok(()) } @@ -72,7 +79,7 @@ impl SourceLinkMappings { } /// Sort internal rules. This must be called before [Self::resolve]. - pub fn sort(&mut self) { + fn sort(&mut self) { // Put Exact matches first, then sort by the Prefix length, longest to shortest. self.rules.sort_unstable_by(|a, b| match &a.pattern { Pattern::Exact(_) => Ordering::Less, @@ -81,16 +88,10 @@ impl SourceLinkMappings { Pattern::Prefix(b) => b.len().cmp(&a.len()), }, }); - self.sorted = true; } /// Resolve the path to a URL. pub fn resolve(&self, path: &str) -> Option { - // Must be sorted first so we can return on the first match, which is guaranteed to be the most specific. - if !self.sorted { - return None; - } - // Note: this is currently quite simple, just pick the first match. If we needed to improve // performance in the future because we encounter PDBs with too many items, we can do a // prefix binary search, for example. @@ -135,10 +136,8 @@ mod tests { #[test] fn test_mapping() { - let mut mappings = SourceLinkMappings::empty(); - mappings - .add_mappings( - r#" + let mappings = SourceLinkMappings::new( + [r#" { "documents": { "C:\\src\\*": "http://MyDefaultDomain.com/src/*", @@ -147,30 +146,16 @@ mod tests { "C:\\src\\bar\\*": "http://MyBarDomain.com/src/*" } } - "# - ).unwrap(); - - assert_eq!(mappings.rules.len(), 4); - assert!(!mappings.sorted); - mappings.sort(); - assert!(mappings.sorted); - - mappings - .add_mappings( - r#" + "#, r#" { "documents": { "C:\\src\\file.txt": "https://example.com/file.txt" } } - "#, - ) - .unwrap(); + "#].iter().copied() + ).unwrap(); assert_eq!(mappings.rules.len(), 5); - assert!(!mappings.sorted); - mappings.sort(); - assert!(mappings.sorted); // In this example: // All files under directory bar will map to a relative URL beginning with http://MyBarDomain.com/src/. From 71f008cc43d2f3ba7eaef9252447c87aaf80e39d Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Sat, 11 Feb 2023 14:05:25 +0100 Subject: [PATCH 10/19] formatting --- symbolic-debuginfo/src/sourcebundle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/symbolic-debuginfo/src/sourcebundle.rs b/symbolic-debuginfo/src/sourcebundle.rs index ffc053021..9291f7f97 100644 --- a/symbolic-debuginfo/src/sourcebundle.rs +++ b/symbolic-debuginfo/src/sourcebundle.rs @@ -1151,7 +1151,7 @@ mod tests { .flat_map(|f| { let path = f.abs_path_str(); session.source_by_path(&path).ok().flatten().map(|source| { - let SourceCode::Content(text) = source else { + let SourceCode::Content(text) = source else { unreachable!(); }; (path, text.into_owned()) From 18c8ac058b7d7f59fedd5d6d7eb1252e57a63a83 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos <6349682+vaind@users.noreply.github.com> Date: Mon, 13 Feb 2023 12:27:49 +0100 Subject: [PATCH 11/19] Update symbolic-ppdb/src/format/sourcelinks.rs Co-authored-by: Sebastian Zivota --- symbolic-ppdb/src/format/sourcelinks.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/symbolic-ppdb/src/format/sourcelinks.rs b/symbolic-ppdb/src/format/sourcelinks.rs index 56e387734..98b2131b6 100644 --- a/symbolic-ppdb/src/format/sourcelinks.rs +++ b/symbolic-ppdb/src/format/sourcelinks.rs @@ -106,9 +106,8 @@ impl SourceLinkMappings { Pattern::Prefix(value) => { if path_lower.starts_with(value) { let replacement = path - .get(value.len()..) - .map(|v| v.replace('\\', "/")) - .unwrap_or_default(); + .get(value.len()..).unwrap_or_default() + .replace('\\', "/"); return Some(rule.url.replace('*', &replacement)); } } From 6f45b8fa0419e40dffd9a75878453f32b2756c88 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos <6349682+vaind@users.noreply.github.com> Date: Mon, 13 Feb 2023 12:35:52 +0100 Subject: [PATCH 12/19] Update symbolic-ppdb/src/format/sourcelinks.rs --- symbolic-ppdb/src/format/sourcelinks.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/symbolic-ppdb/src/format/sourcelinks.rs b/symbolic-ppdb/src/format/sourcelinks.rs index 98b2131b6..27e9d1a9a 100644 --- a/symbolic-ppdb/src/format/sourcelinks.rs +++ b/symbolic-ppdb/src/format/sourcelinks.rs @@ -106,7 +106,8 @@ impl SourceLinkMappings { Pattern::Prefix(value) => { if path_lower.starts_with(value) { let replacement = path - .get(value.len()..).unwrap_or_default() + .get(value.len()..) + .unwrap_or_default() .replace('\\', "/"); return Some(rule.url.replace('*', &replacement)); } From adf4cfe872d30228ce1914f804572d23feb4f875 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 14 Feb 2023 10:28:17 +0100 Subject: [PATCH 13/19] chore: review changes --- symbolic-ppdb/src/format/mod.rs | 8 ++--- symbolic-ppdb/src/format/sourcelinks.rs | 48 ++++++++++++++----------- 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/symbolic-ppdb/src/format/mod.rs b/symbolic-ppdb/src/format/mod.rs index c752b7b1c..1674a32f9 100644 --- a/symbolic-ppdb/src/format/mod.rs +++ b/symbolic-ppdb/src/format/mod.rs @@ -236,7 +236,7 @@ impl<'data> PortablePdb<'data> { us_stream: None, blob_stream: None, guid_stream: None, - source_link_mappings: SourceLinkMappings::empty(), + source_link_mappings: SourceLinkMappings::default(), }; let mut metadata_stream = None; @@ -297,12 +297,10 @@ impl<'data> PortablePdb<'data> { let cdi = cdi?; // Note: only handle module #1 (do we actually handle multiple modules in any way??) if let (metadata::CustomDebugInformationTag::Module, 1) = (cdi.tag, cdi.value) { - let json = String::from_utf8_lossy(result.get_blob(cdi.blob)?); - source_link_mappings.push(json); + source_link_mappings.push(result.get_blob(cdi.blob)?); } } - result.source_link_mappings = - SourceLinkMappings::new(source_link_mappings.iter().map(|v| v.as_ref()))?; + result.source_link_mappings = SourceLinkMappings::new(source_link_mappings)?; Ok(result) } diff --git a/symbolic-ppdb/src/format/sourcelinks.rs b/symbolic-ppdb/src/format/sourcelinks.rs index 27e9d1a9a..f81bf27c0 100644 --- a/symbolic-ppdb/src/format/sourcelinks.rs +++ b/symbolic-ppdb/src/format/sourcelinks.rs @@ -3,7 +3,7 @@ use std::cmp::Ordering; use crate::{FormatError, FormatErrorKind}; /// See [Source Link PPDB docs](https://github.com/dotnet/designs/blob/main/accepted/2020/diagnostics/source-link.md#source-link-json-schema). -#[derive(Clone)] +#[derive(Default, Clone)] pub(crate) struct SourceLinkMappings { rules: Vec, } @@ -21,14 +21,7 @@ enum Pattern { } impl SourceLinkMappings { - pub fn empty() -> Self { - SourceLinkMappings { rules: Vec::new() } - } - - pub fn new<'a, I>(jsons: I) -> Result - where - I: Iterator, - { + pub fn new(jsons: Vec<&[u8]>) -> Result { let mut result = Self { rules: Vec::new() }; for json in jsons { result.add_mappings(json)?; @@ -37,9 +30,9 @@ impl SourceLinkMappings { Ok(result) } - fn add_mappings(&mut self, json: &str) -> Result<(), FormatError> { + fn add_mappings(&mut self, json: &[u8]) -> Result<(), FormatError> { use serde_json::*; - let json: Value = serde_json::from_str(json) + let json: Value = serde_json::from_slice(json) .map_err(|e| FormatError::new(FormatErrorKind::InvalidSourceLinkJson, e))?; let docs = json @@ -48,7 +41,7 @@ impl SourceLinkMappings { .ok_or_else(Self::err)?; self.rules.reserve(docs.len()); - for doc in docs.iter() { + for (key, url) in docs.iter() { /* Each document is defined by a file path and a URL. Original source file paths are compared case-insensitively to documents and the resulting URL is used to download source. The document @@ -59,8 +52,8 @@ impl SourceLinkMappings { 3. If the file path contains a *, it must be the final character. 4. If the URL contains a *, it may be anywhere in the URL. */ - let key = doc.0.to_lowercase(); - let url = doc.1.as_str().ok_or_else(Self::err)?.into(); + let key = key.to_lowercase(); + let url = url.as_str().ok_or_else(Self::err)?.into(); let pattern = if let Some(prefix) = key.strip_suffix('*') { Pattern::Prefix(prefix.into()) } else { @@ -124,12 +117,14 @@ mod tests { #[test] fn test_invalid_json() { - let mut mappings = SourceLinkMappings::empty(); - assert!(mappings.add_mappings("").is_err()); - assert!(mappings.add_mappings("foo").is_err()); - assert!(mappings.add_mappings("{\"docs\": {\"k\": \"v\"}}").is_err()); + let mut mappings = SourceLinkMappings::default(); + assert!(mappings.add_mappings("".as_bytes()).is_err()); + assert!(mappings.add_mappings("foo".as_bytes()).is_err()); assert!(mappings - .add_mappings("{\"documents\": [\"k\", \"v\"]}") + .add_mappings("{\"docs\": {\"k\": \"v\"}}".as_bytes()) + .is_err()); + assert!(mappings + .add_mappings("{\"documents\": [\"k\", \"v\"]}".as_bytes()) .is_err()); assert_eq!(mappings.rules.len(), 0); } @@ -152,16 +147,23 @@ mod tests { "C:\\src\\file.txt": "https://example.com/file.txt" } } - "#].iter().copied() + "#, r#" + { + "documents": { + "/home/user/src/*": "https://linux.com/*" + } + } + "#].map(|v| v.as_bytes()).to_vec() ).unwrap(); - assert_eq!(mappings.rules.len(), 5); + assert_eq!(mappings.rules.len(), 6); // In this example: // All files under directory bar will map to a relative URL beginning with http://MyBarDomain.com/src/. // All files under directory foo will map to a relative URL beginning with http://MyFooDomain.com/src/ EXCEPT foo/specific.txt which will map to http://MySpecificFoodDomain.com/src/specific.txt. // All other files anywhere under the src directory will map to a relative url beginning with http://MyDefaultDomain.com/src/. assert!(mappings.resolve("c:\\other\\path").is_none()); + assert!(mappings.resolve("/home/path").is_none()); assert_eq!( mappings.resolve("c:\\src\\bAr\\foo\\FiLe.txt").unwrap(), "http://MyBarDomain.com/src/foo/FiLe.txt" @@ -182,5 +184,9 @@ mod tests { mappings.resolve("c:\\src\\other\\path").unwrap(), "http://MyDefaultDomain.com/src/other/path" ); + assert_eq!( + mappings.resolve("/home/user/src/Path/TO/file.txt").unwrap(), + "https://linux.com/Path/TO/file.txt" + ); } } From 6aa003a7cc66e371452e6775e203c8fd48d3bcbb Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 14 Feb 2023 13:01:44 +0100 Subject: [PATCH 14/19] chore: review changes --- symbolic-ppdb/src/format/sourcelinks.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/symbolic-ppdb/src/format/sourcelinks.rs b/symbolic-ppdb/src/format/sourcelinks.rs index f81bf27c0..8fb6a8de9 100644 --- a/symbolic-ppdb/src/format/sourcelinks.rs +++ b/symbolic-ppdb/src/format/sourcelinks.rs @@ -118,13 +118,13 @@ mod tests { #[test] fn test_invalid_json() { let mut mappings = SourceLinkMappings::default(); - assert!(mappings.add_mappings("".as_bytes()).is_err()); - assert!(mappings.add_mappings("foo".as_bytes()).is_err()); + assert!(mappings.add_mappings(b"").is_err()); + assert!(mappings.add_mappings(b"foo").is_err()); assert!(mappings - .add_mappings("{\"docs\": {\"k\": \"v\"}}".as_bytes()) + .add_mappings(b"{\"docs\": {\"k\": \"v\"}}") .is_err()); assert!(mappings - .add_mappings("{\"documents\": [\"k\", \"v\"]}".as_bytes()) + .add_mappings(b"{\"documents\": [\"k\", \"v\"]}") .is_err()); assert_eq!(mappings.rules.len(), 0); } @@ -132,7 +132,7 @@ mod tests { #[test] fn test_mapping() { let mappings = SourceLinkMappings::new( - [r#" + vec![br#" { "documents": { "C:\\src\\*": "http://MyDefaultDomain.com/src/*", @@ -141,19 +141,19 @@ mod tests { "C:\\src\\bar\\*": "http://MyBarDomain.com/src/*" } } - "#, r#" + "#, br#" { "documents": { "C:\\src\\file.txt": "https://example.com/file.txt" } } - "#, r#" + "#, br#" { "documents": { "/home/user/src/*": "https://linux.com/*" } } - "#].map(|v| v.as_bytes()).to_vec() + "#] ).unwrap(); assert_eq!(mappings.rules.len(), 6); From 2e13de8e8bbbdde4b2d4c860338041223bb6ea58 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 15 Feb 2023 19:56:24 +0100 Subject: [PATCH 15/19] chore: update imports --- symbolic-ppdb/src/format/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/symbolic-ppdb/src/format/mod.rs b/symbolic-ppdb/src/format/mod.rs index 1674a32f9..70dda07f9 100644 --- a/symbolic-ppdb/src/format/mod.rs +++ b/symbolic-ppdb/src/format/mod.rs @@ -14,7 +14,8 @@ use watto::Pod; use symbolic_common::{DebugId, Language, Uuid}; use metadata::{ - CustomDebugInformation, CustomDebugInformationIterator, MetadataStream, Table, TableType, + CustomDebugInformation, CustomDebugInformationIterator, CustomDebugInformationTag, + MetadataStream, Table, TableType, }; use sourcelinks::SourceLinkMappings; use streams::{BlobStream, GuidStream, PdbStream, StringStream, UsStream}; @@ -296,7 +297,7 @@ impl<'data> PortablePdb<'data> { for cdi in CustomDebugInformationIterator::new(&result, SOURCE_LINK_KIND)? { let cdi = cdi?; // Note: only handle module #1 (do we actually handle multiple modules in any way??) - if let (metadata::CustomDebugInformationTag::Module, 1) = (cdi.tag, cdi.value) { + if let (CustomDebugInformationTag::Module, 1) = (cdi.tag, cdi.value) { source_link_mappings.push(result.get_blob(cdi.blob)?); } } @@ -439,7 +440,7 @@ impl<'object, 'data> Iterator for EmbeddedSourceIterator<'object, 'data> { match row { Err(e) => return Some(Err(e)), Ok(info) => { - if let metadata::CustomDebugInformationTag::Document = info.tag { + if let CustomDebugInformationTag::Document = info.tag { return Some(self.get_source(info)); } } From f84cf69568b8378a407a320806b710631f1b8393 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 17 Feb 2023 10:53:00 +0100 Subject: [PATCH 16/19] tmp --- symbolic-ppdb/tests/test_ppdb.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/symbolic-ppdb/tests/test_ppdb.rs b/symbolic-ppdb/tests/test_ppdb.rs index cb55a9e67..1ff378a82 100644 --- a/symbolic-ppdb/tests/test_ppdb.rs +++ b/symbolic-ppdb/tests/test_ppdb.rs @@ -161,11 +161,7 @@ fn test_pe_embedded_ppdb_with_sources() { #[test] fn test_source_links() { - // Source: https://nuget.info/api/MsdlProxy?symbolKey=microsoft.extensions.logging.eventlog.pdb/37e9e8a61a8e404eb93c6902e277ff55FFFFFFFF/microsoft.extensions.logging.eventlog.pdb - let buf = std::fs::read(fixture( - "windows/Microsoft.Extensions.Logging.EventLog-sourcelink.pdb", - )) - .unwrap(); + let buf = std::fs::read(fixture("ppdb-sourcelink-sample/ppdb-sourcelink-sample.pdb")).unwrap(); let ppdb = PortablePdb::parse(&buf).unwrap(); @@ -190,6 +186,8 @@ fn test_source_links() { ] ); + let src_prefix = ""; + for i in 1..ppdb.get_documents_count().unwrap() + 1 { let doc = ppdb.get_document(i).unwrap(); let url = ppdb.get_source_link(&doc).unwrap(); From 675126308dea4fc7d37a0ccb62a3d024af3686a3 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 17 Feb 2023 11:00:50 +0100 Subject: [PATCH 17/19] test: switch source-link tests to a custom project --- symbolic-debuginfo/tests/test_objects.rs | 19 +++++++++++------- symbolic-ppdb/tests/test_ppdb.rs | 13 ++++++------ .../ppdb-sourcelink-sample.dll | Bin 4096 -> 4096 bytes .../ppdb-sourcelink-sample.pdb | Bin 7732 -> 8232 bytes .../src/ppdb-sourcelink-sample.csproj | 1 + ...Extensions.Logging.EventLog-sourcelink.pdb | Bin 8648 -> 0 bytes 6 files changed, 20 insertions(+), 13 deletions(-) delete mode 100644 symbolic-testutils/fixtures/windows/Microsoft.Extensions.Logging.EventLog-sourcelink.pdb diff --git a/symbolic-debuginfo/tests/test_objects.rs b/symbolic-debuginfo/tests/test_objects.rs index 678fba54d..27b3e7306 100644 --- a/symbolic-debuginfo/tests/test_objects.rs +++ b/symbolic-debuginfo/tests/test_objects.rs @@ -787,18 +787,19 @@ fn test_ppdb_source_by_path() -> Result<(), Error> { #[test] fn test_ppdb_source_links() -> Result<(), Error> { - let view = ByteView::open(fixture( - "windows/Microsoft.Extensions.Logging.EventLog-sourcelink.pdb", - ))?; + let view = ByteView::open(fixture("ppdb-sourcelink-sample/ppdb-sourcelink-sample.pdb"))?; let object = Object::parse(&view)?; let session = object.debug_session()?; let known_embedded_sources = vec![ - ".NETFramework,Version=v4.6.2.AssemblyAttributes.cs", - "Microsoft.Extensions.Logging.EventLog.AssemblyInfo.cs", - "LibraryImports.g.cs", + ".NETStandard,Version=v2.0.AssemblyAttributes.cs", + "ppdb-sourcelink-sample.AssemblyInfo.cs", ]; + // Testing this is simple because there's just one prefix rule in this PPDB. + let src_prefix = "C:\\dev\\symbolic\\"; + let url_prefix = "https://raw.githubusercontent.com/getsentry/symbolic/9f7ceefc29da4c45bc802751916dbb3ea72bf08f/"; + for file in session.files() { let file = file.unwrap(); @@ -809,7 +810,11 @@ fn test_ppdb_source_links() -> Result<(), Error> { } SourceCode::Url(url) => { // testing this is simple because there's just one prefix rule in this PPDB. - assert_eq!(url, format!("https://raw.githubusercontent.com/dotnet/runtime/d099f075e45d2aa6007a22b71b45a08758559f80/{}", &file.path_str()[3..])); + let expected = file + .path_str() + .replace(src_prefix, url_prefix) + .replace('\\', "/"); + assert_eq!(url, expected); } } } diff --git a/symbolic-ppdb/tests/test_ppdb.rs b/symbolic-ppdb/tests/test_ppdb.rs index 1ff378a82..3c14ed7e4 100644 --- a/symbolic-ppdb/tests/test_ppdb.rs +++ b/symbolic-ppdb/tests/test_ppdb.rs @@ -180,19 +180,20 @@ fn test_source_links() { assert_eq!( embedded_sources, vec![ - ".NETFramework,Version=v4.6.2.AssemblyAttributes.cs", - "Microsoft.Extensions.Logging.EventLog.AssemblyInfo.cs", - "LibraryImports.g.cs" + ".NETStandard,Version=v2.0.AssemblyAttributes.cs", + "ppdb-sourcelink-sample.AssemblyInfo.cs" ] ); - let src_prefix = ""; + // Testing this is simple because there's just one prefix rule in this PPDB. + let src_prefix = "C:\\dev\\symbolic\\"; + let url_prefix = "https://raw.githubusercontent.com/getsentry/symbolic/9f7ceefc29da4c45bc802751916dbb3ea72bf08f/"; for i in 1..ppdb.get_documents_count().unwrap() + 1 { let doc = ppdb.get_document(i).unwrap(); let url = ppdb.get_source_link(&doc).unwrap(); - // testing this is simple because there's just one prefix rule in this PPDB. - assert_eq!(url, format!("https://raw.githubusercontent.com/dotnet/runtime/d099f075e45d2aa6007a22b71b45a08758559f80/{}", &doc.name[3..])); + let expected = doc.name.replace(src_prefix, url_prefix).replace('\\', "/"); + assert_eq!(url, expected); } } diff --git a/symbolic-testutils/fixtures/ppdb-sourcelink-sample/ppdb-sourcelink-sample.dll b/symbolic-testutils/fixtures/ppdb-sourcelink-sample/ppdb-sourcelink-sample.dll index 14c2d2acee834d57aa7a49a0fcb9030afae17a2a..824204f4a0c34337b12b58c95300a55b90f085aa 100644 GIT binary patch delta 402 zcmZorXi%8Y!E#0O+U|)xGK}{ot_)}Fnas#o&-i3=Bal3>nUN`(MPPSZ$A7**(Z0JB zmEKH>D%YP}!gj~jGR-_WH8m~S$TB6-B-z9?DcQon$lTP>($Fj=DaklB(cCC0&A=j! z0SeM8l;$xo`UWro`BNwNvdgoksxdI6PTt6_>~L<)o-<4DRJ*Nnh}6(|-hGY{sK(hU zCMC5jrnoXUDL*GO8P3oxNi8lZEy>I&j!Da`C@C#UEsmKS$04Q=Q(TlBlb@6o6O@{h znpm6~lb2djT#}fVl30{tq-Ow9q@)YAiA@h`6UbFU=65=Cj|vvHy^c5D0(27$oSVFs zLmC(cCplahCp$2TZ06&NWA?XXNMkT(NM=X{!ZaXl#9+yg!jQ;d0%V&om@*^*X$uAe yptw1ZX9$!vWH1A&NCHA*pq@maxDk+@22^9gkT!WYuP@UBv(18h?-?f=umb>EEqyxx delta 440 zcmZorXi%8Y!ScxG*rJI&GK`-lt_){fFqx6Dp7F=zMj&}*Gb2+ni-6264e?HMN!MQQ z!X2zpIo*>>*zVXG7?>Ly8m3tq7+6|bq?%f$nxvSUq?njlB&VfWrX-r17$l{oCMB9O zKtXhX-7E%1-vA~cf92#}c6ruHH3o*t$s5^~>mR$xyL{x{;IP21W}l1TorR1*HTt2& zsYS*5xrrqu`p)?&sru=uCB>+vNoG#5ep+TlNoi4P zv3@~8N|J7IerZv1YEEWewr+7^Zb43}esNKpR1^b-)>aNz#?5?O zam@Y(38pfBf~{TCLk{rXaWO}W(H!Qfk40wCP88qa!f$BI?z?&%<2rR3=e@M8^br&Frez6 z3}ry_6XRqvMlAv9@Gd^J%sjU(Yr8hM34Z-IIf2oToypmXIVH7h@@hseF3uQ677jrb zT^EnZOiVfT{4j29Lt{f{!wdx`-{4JMv+kvJ6g6DEW7#DqEpS*|_j9;C+N1HedU3?Tj|Jy< zR`O}{-0|M@WWvhknz?ERwY3+@*-Vc7x7gxB=;e^s(EP_mPVY}{snT$Gwa+zHWU-iz zw(}#8bMEGH9c*(|?pd3Bd2=`S@S70n8EP|El=+^`ur9g#CVEPL>x^{cA3vj?zL#p( zQkwE1Gok0fTfg&}pP1ApUt|=Uyo*Ty&2x>j6c!XO%YKk#G~NEj5qH;`o&RTFe!J*e z(66aV{^mTXi!7NMlZAlszIL*&H?yHY+k5W6{LcPeCEM3t+ZNRlwW2~`iEfnu|Kv9* z(r47XCvAPR-+Z@M=Er6(U%#0%ZQiFCO;(w^cwN{O6@!D9X8XC#s@JLV%9l#s8x4+jE`c@HcUU!@zVlzOsktT`xxN48(YsaheLJroy0PaZ+g1HNn>R`nDINU%SVaHF z(T0zK+wEn(9NT+(%EsbDiSs+(dC2Jf&hU*Ysy}jQ-IDMUmwWfO=Fa++SAO8txB3X% zd((oJe#lw6JaR_s&Z9c&ZyQ%HkaSxpTFcPTQLU7cpIn-onpaY+WL2%?Y!wp&OrbF` z#g(~9`8k=%F)@=(m@V}!)6A1oQ`3@-EK?Fql1)sLk}V93%uNj~4b4)Ll8jRm&5e@M z3@p<0wUlaWn}jFNVOBIY(latLGc?vSG|>jCPBAmFG)*xzO-)JyGL6lFx=mAzjLlLE YQcW$*4U>#g(vrHD=OTD#VRV4qtcepsf|!AFsSJz^MGQ;~_ki># zAZ^9S$l%Aw1myJsO<(}h%s>n@5D2)zBuH$53=@#84wTDhR0q0^i%Fe9)S_aR+{BU+7U%qwRL=C&lH%07lA_AVTNu4~dGr|=*?9z|Rm>g2qbGAP z<(LX1i8M@ZnBFkIVV#1VrOQvXTVn2&(NleOk}U4)oji8yAjit+mxo34a-G-KPIhBf zugBEcD6H_}YIEgOqX(<*`0g_O{qo4f_QXz)S!uFOr!&&f>I*HW@l$|xx*D7MnqFG?)eOV2FHD4lG> zs-kaTU~X(^m}Y5UU}Vq#{IoR(&pl4x#Xkd&61lxPYxqPDh4XmUTR zqLHzlk+Ffffu6axsim1oieaKfs)bRCd7@dWk$IAFlCfb@vSEr@s) netstandard2.0 + true diff --git a/symbolic-testutils/fixtures/windows/Microsoft.Extensions.Logging.EventLog-sourcelink.pdb b/symbolic-testutils/fixtures/windows/Microsoft.Extensions.Logging.EventLog-sourcelink.pdb deleted file mode 100644 index 732fb01d3b0d39dad5d1269fa4aaf8f2fa69a9b2..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 8648 zcma)B2V7Iv_rDp0Fajh%L_i5pKm-DW4GuQ4DMQfuS2A885E9G)qoT;DqwZa;qje9g zpaSCjIjT7Nx7b>%wZ*EnT4%Ltwd(KOmqee`-|zE-&o}3td&WI?oqOMXi3J&nG(#Y%D;}=z2x1#2{=EUzEqM-j$`?~0=X6afeM#Uaq*jU*i zGO9S>eE`9LGC&T%5`g`&00&Gw7wABMKY$K^sz`gRgIkAV!oYtZH#Qi3X)HiZ2-{=- zqW~O$r->GWYAIMO0=f$5MWDX`?MI^_PZ}NN0v!c(CD2VkF9W>+bSRyMvgmX)5$H^y zhk>328pWWY5ezz-0(2hGpMgFADq_-55|fU013e0~%YufwE$FC?%|wUUxJB>(vqoP! zSR+du{yTphWXI(oSAZuV2oMg4C-pce4W0`CqXA_A9bhtG5nwBzjcbRx0H*;xfS&-j z0Z#yIo*l9WxC4R#QGnrqG(aKG21x)~z(l|_zzo1Vz*4{}z8T9(;8uzc`Uc@BX{T{=k}5F`pBL7$Z;d6{`nMnKalmK z@HiHiKKlJBa=fNoDLnS=)<@sH4?lpSkNsmG)Ob87ayG~WeR$74ycdPH2Yv58{yw0{ zu|FRQkM(`~===5I2T^!z?@zJE{NO%(0ENfx3#8cN@eJz23n@JIhjSUVKX9@&^H?5A z;jupvg~#$R3XkRC6dubXkkuQ1IKD`V9P3BHj5OCb0QhJMk7JFY@Hh{~LUuIkV|$#F z%{1 z+)kzNcx=)r{8QjZ0FPHN{@Vvg2j0TO4+1^|X1tll{hLYQ@p{Ul@ObaYrtpry=TLY7 z@VUUVO#ZRIJb;2nI9)G;oQ{_?$kNwLi`VABw@8u;X-UjN!>mnUc z2IFIEs$Xs5@r=fO9R9!6^Qc)oy>mb(QhO*jXuVbQhXl3 z)`*MuU_3|gDZ&DPTgCw37SK?%BhLJIcH&bM4NZ2$D-UNMVA|x@{2&zu&Q?3=8xpTgtRRPjSG|>4aRYSL#w~2$e2fhN=jyR$N9Pty-^< z5;@{30#%8nzqE{$oga%cM74V}`R}&hY zg3y{B7Z7DtP!rPX#2Q_?N=8gTT0AcJ&;FeMj83j&;`C2y?V6Wu=RcEtIQ%Qi^W6Cb z=y2{j`sZ%Nv+R5os?^;rvv#I(|9o|a@A7)=R)_5APu1zt#_-bpI^m)%mnS68NwpVk z>e_XFqy_iPg0<1vYwq}LTM^+}VR7fxo^!JsGnQUTl{D?kyvR<=o}#iEleOd0X@{-~ z9Za2Z46R0LrBp~XVhsfCldX_y)LOM%Crp{3BUD<2TBQ|2E6d=RD>OIR!~9T2Xi~*e zomyj{XiK&1U_@hBav4#A?$CsL;`>+hbyqG1eEQ9NS1U%dQ+7&Og$b*?(@$WceLlLA7g!)JMl z`80;0vcu_wOE2HM%VYL^d3WIL$Z5MDmM^Z|RZ}!(X06AVp|nQO?((n`$@Mo+y7+0`@Ki}w)2YLos-m9P6`gU zKL0Fu);GK_pZ>UIpF{IgLy)>^^6S7e+uDz98J%En%}P>NRjF0hZ^zKmG~(_#dZiK< z0Q5O{gOJptN!YpJj+_zT0>+hB$oMll}=Gb*puU+t}Y-nH3})A?GG|o6^8VxYPCj} zpo3REi5|?PS_fmb_ZSUZ8!Z|+jROSthg=gq9QW9xvSifZ);5jiYLk^sSxM;mjVs0; ztQ%JSg_H3`F5K8t2Z&{f*s%n7HBnG8bC; zb`DQ}<~ryz{}$dbkDI!!Q(wAABcy3>vmQxk@S2AJo%57pom{P{`j0Lxnz&8w0-I`u zYuWPCo4Qx*`O&#x+1%H?`QgV?e|vml$I%YU>8+=%AnhsSkR`MXwWPu_pHLEFEx}R| zy6{MmNSKpSn5u!KTdUSo2AXOgS`#je6pDliS}jo}Q5xQf0MDEzS(A#}q-mPiG))k; z@@=2u5I z1T-&fnDI%4MZ*8pe13dR=ZL!z%t!ODr{4P6ZF1q5uRjYbaN5~BZF_vDA18g=ay!%ERQs4EH;QdOUY}YMe{-FieaX>P@t0zE zJQQEir(Bo0vD5l*i#mJm=9+Z9 zH+_+=lH>I7PA@b!2`1mM+#_1HdGnI_Kh{sFY#R{w+q}Sr}vf+pUjrZ&GxWlvVVRyUO1Ov&wVWLBT7$d8N3_;z+k-OppY!h$-dC2wGB<)xpa z@eJD${#WC^Ykt1DRFo$5iN8mDWs%tJI-;b?p)+goh2;*%N_O@yc3qw&{OMbe$K zQ?w6C{^jx0sjugkoSIQ(c(v);t(kJ4ua~@<;`d~1vbA!u z-I={#+5TDb)$I!Ha_=)GLFI#@!;-wtURzOpI_}!ck*UOWk8#`#=^B^Ug*Wz|T|ae5 zQu>pdxdH3WcaK=Iq-cb~T&Y|9F_@;GZ{=BU_i z?YDV~h-+J3_O`Ftxx43r&B?$Qf44*py?C$2onvr&RwJ>xZ&UH;T*pC==Lc4M+_gJW zUbnq`R%`6YI!`~kks{3ANg0FcEOPizczXAI}>>0<_;@Q?uFs| zPIs;PdC=PlZ_2pBcQS1np&~S5I8->kQ>+!@oXH_mFPAYx5C5y3*7oizSy@ul;pLxH za(bT2XHo92KE_8#bbsHW;9$WJv0kSRDlymS&-w^@^S3 z6+A9@uve^CxlUKDjSUXgh--yq3SGHgqSq1{sTwwDl};#CR|U(|I>=nX8snA`EDMQ= zk%vS@5aAIrkysoV5)vgAi6l{>lJE#|NOV+0bVNjqJURrt*VQ$+)EKiXyw`%SYK4-} z1euG%h2g@GpfI6G6dn~L42=vBg%MIhERu#x2x)kfNFEbGghoq6;+XJgnLI`=4U>e5 z32B5JDaEQXJwCxEd7BQgL2_^%BvnI@C^QPG^}1@kE~paD2q@X05?3jtum>5k6sk%> zX6j?4u7*8OF;T38?2k0YBpoCpB>J+TYNcL_s!h8AN>CY+s`bd!18?Gw5{5wh8mTZ8 zH$fDJn-CrrA`KUbrIHXr5)vLADvyYkg#sHIB@)ZS!Xv{X!w3-}lfG%f8*kMbqDDc~ zB9&=dLn$h}yWz7t9L!N+enL`8V17zgN*vhL}*`|2vOgX$bS_4!zhw!gqkVm?nJ2)lD$DCB^L*5 zoP<^=Q!3&0=x)#49jG2|8Ok3KzOqH}^JIgS?e?kX->#>(Cpcj;QBAa>R7q=;e>-mE+&*#{4?*o^WnYW%S~3_ALJ6-mO#I^gR`sKTMgEZ1Z3J7(*CY zYVip+b^Lz%6qFP3MI#r;{?{jWrR}@IbJj8KZe3Y|dWx8mckQMnB(!)3zMwYQhOl=&sj+Hq>H zbrGa7eF$WT3@|Qei5jwRh^;h|Qs!%7053|1f@4c751N*SQZhy(Xn>q6%J3 zO;ufwtd*crI(n2cs(z2QvQtxF`xEv0>%ol(v-1HQ3w4EQLkm88ysfY zpm|49nq%*z`CeGJ&}0fPPlQ;8d%sYut<+Lq@r}cAv;w9b#Ghj~bARr-A*u6zD1AKS zliz2U>_>`~dZJKYtu%+a0=E}>X?^$UU1==$^aIxCt_b}PNov&Kx+_$eqX8C{WcouE`VcPs&e4#biGX~#b1xH)4;8IGCi1fSST8RbYzj(4{L=_f+7!8O%_e@#dl$W_4A<26_sKpvVI9LT%^PIgv|Qo z-h7~m_1mxplWmFI=x|>~qZ@E>R(+?rm`BG~?sS9E>X5gQ9`VLq5z7NDcWev_WW9V} z91728I219R$>+3W{A@!$%Tw_)`N@o@D7HjA=Ao*$##LT8PG0>?qXoXTeLFt7e-CT0%R;W$Ix%&jrU>Fq;?JIF)^lU!(|k;IWC zHrz;JND}{nk@#bO!vetHKx(@~{BXHvKaLHmU_w9IGZE#K`QLZNu=i&9_idkV{n*eN zw$c_}rY&O9xcElX(-&&?qQfm{4HNNk^|;hK5=!|@7(ib-d{}@|JPO_%h@Qqo4DdlV z%86_utv?jQUFbj#CLT_Sq>K}YO&slwCb)>kC-v+(5n;e72LcCOVXB>jD|5zECKx{r zpu?SPp*37$k7mQID)hdqX~86^l}h8GKr1BA+X^Y~pKHm+R8&Yz(AnZ9FM3E@Y)f10 zLBav z06Ow9j)xQ99#2)h`+Mhs>7c-%EwRN@EC%;?Fq2I#RdQ$s*~5iAQ;&NjE|}v0_u{b; z9492iV*(ip>v#-Ms#uec1OBT8Zki;{}NQSVO@MX`(33J>* zPW~3~Eg*>FhBE2!HKhRmK<;4U=5TN}U?Dypb}`F<=imSbD@)utEL6f|+u5k4%y6CMhZg&z@2r9?5;8d)=C@Cc)YqAXD| cvo-|L>vG^b18K~&4q_kyb1Gb4GsmNU1EfEmBme*a From 1bde15bcd02262b19893d1d88cb6a996a3d8e3a6 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 17 Feb 2023 11:02:08 +0100 Subject: [PATCH 18/19] fix: add `#[non_exhaustive]` to SoureCode enum --- symbolic-debuginfo/src/base.rs | 1 + symbolic-debuginfo/tests/test_objects.rs | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/symbolic-debuginfo/src/base.rs b/symbolic-debuginfo/src/base.rs index 6f632d7f2..a21a04bff 100644 --- a/symbolic-debuginfo/src/base.rs +++ b/symbolic-debuginfo/src/base.rs @@ -671,6 +671,7 @@ impl fmt::Debug for Function<'_> { pub type DynIterator<'a, T> = Box + 'a>; /// Represents a source file referenced by a debug information object file. +#[non_exhaustive] #[derive(Debug, Clone)] pub enum SourceCode<'a> { /// Verbatim source code/file contents. diff --git a/symbolic-debuginfo/tests/test_objects.rs b/symbolic-debuginfo/tests/test_objects.rs index 27b3e7306..2346013af 100644 --- a/symbolic-debuginfo/tests/test_objects.rs +++ b/symbolic-debuginfo/tests/test_objects.rs @@ -778,7 +778,7 @@ fn test_ppdb_source_by_path() -> Result<(), Error> { .unwrap(); match source.unwrap() { SourceCode::Content(text) => assert_eq!(text.len(), 204), - SourceCode::Url(_) => panic!(), + _ => panic!(), } } @@ -816,6 +816,7 @@ fn test_ppdb_source_links() -> Result<(), Error> { .replace('\\', "/"); assert_eq!(url, expected); } + _ => panic!(), } } From b9473467e66d21527d368cdfdb4fcf611c9da82b Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Fri, 17 Feb 2023 11:44:37 +0100 Subject: [PATCH 19/19] test: fix source-link tests on mac/linux --- symbolic-ppdb/tests/test_ppdb.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/symbolic-ppdb/tests/test_ppdb.rs b/symbolic-ppdb/tests/test_ppdb.rs index 3c14ed7e4..032b34c19 100644 --- a/symbolic-ppdb/tests/test_ppdb.rs +++ b/symbolic-ppdb/tests/test_ppdb.rs @@ -170,7 +170,7 @@ fn test_source_links() { .get_embedded_sources() .unwrap() .map(|src| { - Path::new(src.unwrap().get_path()) + Path::new(&src.unwrap().get_path().replace('\\', "/")) .file_name() .unwrap() .to_string_lossy()