Skip to content
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
ebdbaee
feat(debuginfo): Extract srcsrv data from PDB for file mapping
mujacica Nov 10, 2025
e832235
Add srcsrv to pdb
mujacica Nov 10, 2025
2a0e0d8
Cleanup implementation
mujacica Nov 10, 2025
b234ddb
Fix format/changelog
mujacica Nov 10, 2025
8fff473
Fix path handling
mujacica Nov 11, 2025
b7cb2d1
Don't prepend
mujacica Nov 11, 2025
12a41bb
Rework to use PdbDebugSession
mujacica Nov 11, 2025
2c8521f
Revert the rest of the files
mujacica Nov 11, 2025
f9f84e1
Support function iterator
mujacica Nov 11, 2025
63ef28c
Review comments
mujacica Nov 13, 2025
8262b0e
Update symbolic-debuginfo/src/pdb.rs
mujacica Nov 13, 2025
1ca76a3
Update symbolic-debuginfo/src/pdb.rs
mujacica Nov 13, 2025
a5afa5e
Extract the revision and support both formats
mujacica Dec 9, 2025
6b9f3c1
Rework based on feedback from Sebastian
mujacica Dec 15, 2025
6290cd3
Remove unused import
mujacica Dec 15, 2025
d879081
Merge branch 'master' into feat/perforce-srcsrv-support
loewenheim Apr 14, 2026
524623f
Remove orphaned modules
loewenheim Apr 15, 2026
72df305
Refactor
loewenheim Apr 15, 2026
6fbf4ec
Put srcsrv information besides original instead of overwriting
loewenheim Apr 15, 2026
2c2c298
Remove function
loewenheim Apr 15, 2026
bcc01e1
Expand Perforce variant documentation
loewenheim Apr 15, 2026
1f3e73c
Document when parsing SourceServerMappings fails
loewenheim Apr 15, 2026
9603f9e
Typo
loewenheim Apr 15, 2026
e3e841a
Fix doc link
loewenheim Apr 15, 2026
4ef2e49
More minor corrections
loewenheim Apr 15, 2026
9b2ff63
Move "no vcs" error
loewenheim Apr 17, 2026
c6ff18f
Don't error when failing to open srcsrv data
loewenheim Apr 17, 2026
c9e477e
correct some comments
loewenheim Apr 17, 2026
6f86859
Don't needlessly use pub(crate)
loewenheim Apr 17, 2026
7639205
Prefix revision with srcsrv
loewenheim Apr 17, 2026
5cbc898
Simplify tests
loewenheim Apr 17, 2026
a93142c
Add unit test
loewenheim Apr 17, 2026
ac444aa
Merge branch 'master' into feat/perforce-srcsrv-support
loewenheim Apr 17, 2026
ed90fde
Fixup test_cache.rs
loewenheim Apr 17, 2026
d0a2953
Expand comment
loewenheim Apr 17, 2026
c144b3b
Some more doc comments
loewenheim Apr 17, 2026
0fbed54
Merge branch 'master' into feat/perforce-srcsrv-support
loewenheim May 4, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

**Features**

- Extract srcsrv data from PDB for file mapping ([#943](https://github.com/getsentry/symbolic/pull/943))

## 12.18.0

**Improvements**
Expand Down
88 changes: 86 additions & 2 deletions symbolic-debuginfo/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,13 +450,34 @@ pub struct FileInfo<'data> {
name: Cow<'data, [u8]>,
/// Path to the file.
dir: Cow<'data, [u8]>,
/// The base name on the source server.
///
/// This only exists if we have a debug file containing
/// source server information.
srcsrv_name: Option<Cow<'data, [u8]>>,
/// The path to the file on the source server.
///
/// This only exists if we have a debug file containing
/// source server information.
srcsrv_dir: Option<Cow<'data, [u8]>>,
/// The optional VCS revision (e.g., Perforce changelist, git commit hash).
///
/// This only exists if we have a debug file containing
/// source server information.
revision: Option<Cow<'data, str>>,
Comment thread
loewenheim marked this conversation as resolved.
Outdated
}

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 {
FileInfo { name, dir }
FileInfo {
name,
dir,
srcsrv_name: None,
srcsrv_dir: None,
revision: None,
}
}

/// Creates a `FileInfo` from a joined path by trying to split it.
Expand All @@ -470,12 +491,14 @@ impl<'data> FileInfo<'data> {
Some(dir) => Cow::Borrowed(dir),
None => Cow::default(),
},
srcsrv_name: None,
srcsrv_dir: None,
revision: None,
}
}

/// Creates a `FileInfo` from a joined path by trying to split it.
/// Unlike from_path(), copies the given data instead of referencing it.
#[cfg(feature = "ppdb")]
pub(crate) fn from_path_owned(path: &[u8]) -> Self {
let (dir, name) = symbolic_common::split_path_bytes(path);

Expand All @@ -485,6 +508,9 @@ impl<'data> FileInfo<'data> {
Some(dir) => Cow::Owned(dir.to_vec()),
None => Cow::default(),
},
srcsrv_name: None,
srcsrv_dir: None,
revision: None,
}
}

Expand All @@ -493,6 +519,9 @@ impl<'data> FileInfo<'data> {
FileInfo {
name: Cow::Borrowed(name),
dir: Cow::default(),
srcsrv_name: None,
srcsrv_dir: None,
revision: None,
}
}

Expand All @@ -511,6 +540,53 @@ impl<'data> FileInfo<'data> {
let joined = join_path(&self.dir_str(), &self.name_str());
clean_path(&joined).into_owned()
}

/// The file name on the source server as UTF-8 string.
///
/// This only exists if we have a debug file containing
/// source server information.
pub fn srcsrv_name_str(&self) -> Option<Cow<'data, str>> {
self.srcsrv_name.as_ref().map(from_utf8_cow_lossy)
}

/// Path to the file on the source server.
///
/// This only exists if we have a debug file containing
/// source server information.
pub fn srcsrv_dir_str(&self) -> Option<Cow<'data, str>> {
self.srcsrv_dir.as_ref().map(from_utf8_cow_lossy)
}

/// The full path to the file on the source server.
///
/// This only exists if we have a debug file containing
/// source server information.
pub fn srcsrv_path_str(&self) -> Option<String> {
let joined = join_path(
&self.srcsrv_dir_str().unwrap_or_default(),
&self.srcsrv_name_str()?,
);
Some(clean_path(&joined).into_owned())
}

/// The optional VCS revision (e.g., Perforce changelist, git commit hash).
///
/// This only exists if we have a debug file containing
/// source server information.
pub fn revision(&self) -> Option<&str> {
self.revision.as_deref()
}

pub(crate) fn set_srcsrv_path(&mut self, path: &[u8]) {
let (dir, name) = symbolic_common::split_path_bytes(path);

self.srcsrv_name = Some(Cow::Owned(name.to_owned()));
self.srcsrv_dir = dir.map(|d| Cow::Owned(d.to_owned()));
}

pub(crate) fn set_revision(&mut self, revision: Option<String>) {
self.revision = revision.map(Cow::Owned);
}
}

#[allow(clippy::ptr_arg)] // false positive https://github.com/rust-lang/rust-clippy/issues/9218
Expand Down Expand Up @@ -562,6 +638,14 @@ impl<'data> FileEntry<'data> {
let joined = join_path(&self.compilation_dir_str(), &joined_path);
clean_path(&joined).into_owned()
}

/// The full path to the file on the source server.
///
/// This only exists if we have a debug file containing
/// source server information.
pub fn srcsrv_path_str(&self) -> Option<String> {
self.info.srcsrv_path_str()
}
}

impl fmt::Debug for FileEntry<'_> {
Expand Down
107 changes: 79 additions & 28 deletions symbolic-debuginfo/src/pdb.rs → symbolic-debuginfo/src/pdb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use pdb_addr2line::pdb::{
};
use pdb_addr2line::ModuleProvider;
use smallvec::SmallVec;
use srcsrv;
use thiserror::Error;

use symbolic_common::{
Expand All @@ -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";
Expand All @@ -47,6 +49,9 @@ pub enum PdbErrorKind {

/// Formatting of a type name failed.
FormattingFailed,

/// The srcsrv stream doesn't contain a VCS name.
MissingSourceServerVcs,
Comment thread
loewenheim marked this conversation as resolved.
Outdated
}

impl fmt::Display for PdbErrorKind {
Expand All @@ -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"),
}
}
}
Expand Down Expand Up @@ -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()),
}
}
Comment thread
cursor[bot] marked this conversation as resolved.

/// The kind of this object, which is always `Debug`.
pub fn kind(&self) -> ObjectKind {
ObjectKind::Debug
Expand Down Expand Up @@ -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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 SrcSrvStream, which constructs various indices and maps. That's really more appropriate for a function on the debug session.

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
Expand Down Expand Up @@ -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>>>,

Expand All @@ -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(),
})
Expand Down Expand Up @@ -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> {
Expand All @@ -574,6 +581,12 @@ impl<'d> PdbDebugInfo<'d> {

drop(p);

let srcsrv = streams
.srcsrv
.as_deref()
.map(SourceServerMappings::parse)
.transpose()?;
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 debug_session if parsing the srcsrv stream fails since it's not essential. Any opinions on this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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,
Expand All @@ -587,6 +600,7 @@ impl<'d> PdbDebugInfo<'d> {
streams.string_table.as_ref(),
Default::default(),
)?,
srcsrv,
})
}

Expand Down Expand Up @@ -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 })
})?;
Expand Down Expand Up @@ -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<'_> {
Expand Down Expand Up @@ -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
Comment thread
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(),
});
}
Expand Down Expand Up @@ -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
Comment thread
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);
}
Expand Down
Loading
Loading