-
-
Notifications
You must be signed in to change notification settings - Fork 85
feat(debuginfo): Extract srcsrv data from PDB for file mapping #943
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 25 commits
ebdbaee
e832235
2a0e0d8
b234ddb
8fff473
b7cb2d1
12a41bb
2c8521f
f9f84e1
63ef28c
8262b0e
1ca76a3
a5afa5e
6b9f3c1
6290cd3
d879081
524623f
72df305
6fbf4ec
2c2c298
bcc01e1
1f3e73c
9603f9e
e3e841a
4ef2e49
9b2ff63
c6ff18f
c9e477e
6f86859
7639205
5cbc898
a93142c
ac444aa
ed90fde
d0a2953
c144b3b
0fbed54
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,6 @@ use pdb_addr2line::pdb::{ | |
| }; | ||
| use pdb_addr2line::ModuleProvider; | ||
| use smallvec::SmallVec; | ||
| use srcsrv; | ||
| use thiserror::Error; | ||
|
|
||
| use symbolic_common::{ | ||
|
|
@@ -25,8 +24,11 @@ use symbolic_common::{ | |
|
|
||
| use crate::base::*; | ||
| use crate::function_stack::FunctionStack; | ||
| use crate::pdb::srcsrv::{SourceServerInfo, SourceServerMappings}; | ||
| use crate::sourcebundle::SourceFileDescriptor; | ||
|
|
||
| mod srcsrv; | ||
|
|
||
| type Pdb<'data> = pdb::PDB<'data, Cursor<&'data [u8]>>; | ||
|
|
||
| const MAGIC_BIG: &[u8] = b"Microsoft C/C++ MSF 7.00\r\n\x1a\x44\x53\x00\x00\x00"; | ||
|
|
@@ -47,6 +49,9 @@ pub enum PdbErrorKind { | |
|
|
||
| /// Formatting of a type name failed. | ||
| FormattingFailed, | ||
|
|
||
| /// The srcsrv stream doesn't contain a VCS name. | ||
| MissingSourceServerVcs, | ||
|
loewenheim marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| impl fmt::Display for PdbErrorKind { | ||
|
|
@@ -55,6 +60,7 @@ impl fmt::Display for PdbErrorKind { | |
| Self::BadObject => write!(f, "invalid pdb file"), | ||
| Self::UnexpectedInline => write!(f, "unexpected inline function without parent"), | ||
| Self::FormattingFailed => write!(f, "failed to format type name"), | ||
| Self::MissingSourceServerVcs => write!(f, "missing VCS name in srcsrv stream"), | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -201,6 +207,19 @@ impl<'data> PdbObject<'data> { | |
| .unwrap_or_default() | ||
| } | ||
|
|
||
| /// Returns true if this object contains source server information. | ||
| pub fn has_source_server_data(&self) -> Result<bool, PdbError> { | ||
| let mut pdb = self.pdb.write(); | ||
| match pdb.named_stream(b"srcsrv") { | ||
| Ok(_) => Ok(true), | ||
| Err(pdb::Error::StreamNameNotFound) => { | ||
| // No source server info is normal for many PDBs | ||
| Ok(false) | ||
| } | ||
| Err(e) => Err(e.into()), | ||
| } | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
| /// The kind of this object, which is always `Debug`. | ||
| pub fn kind(&self) -> ObjectKind { | ||
| ObjectKind::Debug | ||
|
|
@@ -252,31 +271,6 @@ impl<'data> PdbObject<'data> { | |
| false | ||
| } | ||
|
|
||
| /// Returns the SRCSRV VCS integration name if available. | ||
| /// | ||
| /// This extracts the version control system identifier from the SRCSRV stream, | ||
| /// if present. Common values include "perforce", "tfs", "git", etc. | ||
| /// Returns `None` if no SRCSRV stream exists or if it cannot be parsed. | ||
| pub fn srcsrv_vcs_name(&self) -> Option<String> { | ||
| let mut pdb = self.pdb.write(); | ||
|
|
||
| // Try to open the "srcsrv" named stream | ||
| let stream = match pdb.named_stream(b"srcsrv") { | ||
| Ok(stream) => stream, | ||
| Err(_) => return None, | ||
| }; | ||
|
|
||
| // Parse the stream to extract VCS name | ||
| let stream_data = stream.as_slice(); | ||
| if let Ok(parsed_stream) = srcsrv::SrcSrvStream::parse(stream_data) { | ||
| parsed_stream | ||
| .version_control_description() | ||
| .map(|s| s.to_string()) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
|
|
||
|
Comment on lines
-255
to
-279
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding this function in the first place was probably a mistake. It parses a Since the symcache version bump causes a major release anyway, we can remove it. |
||
| /// Determines whether this object is malformed and was only partially parsed | ||
| pub fn is_malformed(&self) -> bool { | ||
| false | ||
|
|
@@ -500,6 +494,7 @@ struct PdbStreams<'d> { | |
| type_info: pdb::TypeInformation<'d>, | ||
| id_info: pdb::IdInformation<'d>, | ||
| string_table: Option<pdb::StringTable<'d>>, | ||
| srcsrv: Option<Vec<u8>>, | ||
|
|
||
| pdb: Arc<RwLock<Pdb<'d>>>, | ||
|
|
||
|
|
@@ -523,11 +518,22 @@ impl<'d> PdbStreams<'d> { | |
| Err(e) => return Err(e.into()), | ||
| }; | ||
|
|
||
| // Try to open the "srcsrv" named stream | ||
| let srcsrv = match p.named_stream(b"srcsrv") { | ||
| Ok(stream) => Some(stream.as_slice().to_vec()), | ||
| Err(pdb::Error::StreamNameNotFound) => { | ||
| // No source server info is normal for many PDBs | ||
| None | ||
| } | ||
| Err(e) => return Err(e.into()), | ||
| }; | ||
|
|
||
| Ok(Self { | ||
| string_table, | ||
| debug_info: pdb.debug_info.clone(), | ||
| type_info: p.type_information()?, | ||
| id_info: p.id_information()?, | ||
| srcsrv, | ||
| pdb: pdb.pdb.clone(), | ||
| module_infos: FrozenMap::new(), | ||
| }) | ||
|
|
@@ -561,6 +567,7 @@ struct PdbDebugInfo<'d> { | |
| string_table: Option<&'d pdb::StringTable<'d>>, | ||
| /// Type formatter for function name strings. | ||
| type_formatter: pdb_addr2line::TypeFormatter<'d, 'd>, | ||
| srcsrv: Option<SourceServerMappings<'d>>, | ||
| } | ||
|
|
||
| impl<'d> PdbDebugInfo<'d> { | ||
|
|
@@ -574,6 +581,12 @@ impl<'d> PdbDebugInfo<'d> { | |
|
|
||
| drop(p); | ||
|
|
||
| let srcsrv = streams | ||
| .srcsrv | ||
| .as_deref() | ||
| .map(SourceServerMappings::parse) | ||
| .transpose()?; | ||
|
cursor[bot] marked this conversation as resolved.
Outdated
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The clankers are correct in that we could be more lenient here and not abort the entire
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels very much optional, but it would also be nice if errors would be visible to the caller. Is it an option to parse it lazily instead of the constructor and propagate an error on use that is not fatal? Overall I'd more lean towards swallowing the error, we wouldn't want PDBs with srcsrv information we just don't understand to fail, I think. |
||
|
|
||
| Ok(PdbDebugInfo { | ||
| address_map, | ||
| streams, | ||
|
|
@@ -587,6 +600,7 @@ impl<'d> PdbDebugInfo<'d> { | |
| streams.string_table.as_ref(), | ||
| Default::default(), | ||
| )?, | ||
| srcsrv, | ||
| }) | ||
| } | ||
|
|
||
|
|
@@ -638,6 +652,7 @@ pub struct PdbDebugSession<'d> { | |
| impl<'d> PdbDebugSession<'d> { | ||
| fn build(pdb: &PdbObject<'d>) -> Result<Self, PdbError> { | ||
| let streams = PdbStreams::from_pdb(pdb)?; | ||
|
|
||
| let cell = SelfCell::try_new(Box::new(streams), |streams| { | ||
| PdbDebugInfo::build(pdb, unsafe { &*streams }) | ||
| })?; | ||
|
|
@@ -671,6 +686,19 @@ impl<'d> PdbDebugSession<'d> { | |
| ) -> Result<Option<SourceFileDescriptor<'_>>, PdbError> { | ||
| Ok(None) | ||
| } | ||
|
|
||
| /// Returns the SRCSRV VCS integration name if available. | ||
| /// | ||
| /// This extracts the version control system identifier from the SRCSRV stream, | ||
| /// if present. Common values include "perforce", "tfs", "git", etc. | ||
| /// Returns `None` if no SRCSRV stream exists or if it cannot be parsed. | ||
| pub fn srcsrv_vcs_name(&self) -> Option<String> { | ||
| self.cell | ||
| .get() | ||
| .srcsrv | ||
| .as_ref() | ||
| .map(|srcsrv| srcsrv.vcs_name().to_owned()) | ||
| } | ||
| } | ||
|
|
||
| impl<'session> DebugSession<'session> for PdbDebugSession<'_> { | ||
|
|
@@ -735,11 +763,22 @@ impl<'s> Unit<'s> { | |
| } | ||
|
|
||
| let file_info = program.get_file_info(line_info.file_index)?; | ||
| let mut file = self.debug_info.file_info(file_info)?; | ||
|
|
||
| // Apply SRCSRV remapping if available | ||
|
loewenheim marked this conversation as resolved.
Outdated
|
||
| if let Some(mappings) = self.debug_info.srcsrv.as_ref() { | ||
| let original_path = file.path_str(); | ||
| let info = mappings.get_info(&original_path); | ||
| if let Some(SourceServerInfo { path, revision }) = info { | ||
| file.set_srcsrv_path(path.as_bytes()); | ||
| file.set_revision(revision); | ||
| } | ||
| } | ||
|
|
||
| lines.push(LineInfo { | ||
| address: rva, | ||
| size, | ||
| file: self.debug_info.file_info(file_info)?, | ||
| file, | ||
| line: line_info.line_start.into(), | ||
| }); | ||
| } | ||
|
|
@@ -1062,7 +1101,19 @@ impl<'s> Iterator for PdbFileIterator<'s> { | |
| let result = file_result | ||
| .map_err(|err| err.into()) | ||
| .and_then(|i| self.debug_info.file_info(i)) | ||
| .map(|info| FileEntry::new(Cow::default(), info)); | ||
| .map(|mut file| { | ||
| // Apply source server remapping if available | ||
|
loewenheim marked this conversation as resolved.
Outdated
|
||
| if let Some(mappings) = &self.debug_info.srcsrv { | ||
| let original_path = file.path_str(); | ||
| let info = mappings.get_info(&original_path); | ||
| if let Some(SourceServerInfo { path, revision }) = info { | ||
| file.set_srcsrv_path(path.as_bytes()); | ||
| file.set_revision(revision); | ||
| } | ||
| } | ||
|
|
||
| FileEntry::new(Cow::default(), file) | ||
| }); | ||
|
|
||
| return Some(result); | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.