fix: Correctly read the debug_id of Deterministic PE files

Deterministic PE files have a slightly different way of storing their `debug_id`, which
should match the corresponding ID inside a portable pdb `#Pdb` stream.

See https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md#codeview-debug-directory-entry-type-2

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+**Features**:
+
+- Correctly read the `debug_id` of Deterministic PE files ([#658](https://github.com/getsentry/symbolic/pull/658))
+
 ## 11.0.0
 
 **Fixes**:

symbolic-debuginfo/src/pe.rs

@@ -8,7 +8,7 @@ use gimli::RunTimeEndian;
 use goblin::pe;
 use thiserror::Error;
 
-use symbolic_common::{Arch, AsSelf, CodeId, DebugId, Uuid};
+use symbolic_common::{Arch, AsSelf, CodeId, DebugId};
 
 use crate::base::*;
 use crate::dwarf::*;
@@ -118,18 +118,31 @@ impl<'data> PeObject<'data> {
         self.pe
             .debug_data
             .as_ref()
-            .and_then(|debug_data| debug_data.codeview_pdb70_debug_info.as_ref())
-            .and_then(|debug_info| {
-                // PE always stores the signature with little endian UUID fields.
-                // Convert to network byte order (big endian) to match the
-                // Breakpad processor's expectations.
-                let mut data = debug_info.signature;
-                data[0..4].reverse(); // uuid field 1
-                data[4..6].reverse(); // uuid field 2
-                data[6..8].reverse(); // uuid field 3
-
-                let uuid = Uuid::from_slice(&data).ok()?;
-                Some(DebugId::from_parts(uuid, debug_info.age))
+            .and_then(|debug_data| {
+                debug_data
+                    .codeview_pdb70_debug_info
+                    .as_ref()
+                    .map(|cv_record| (debug_data.image_debug_directory, cv_record))
+            })
+            .and_then(|(debug_directory, cv_record)| {
+                let guid = &cv_record.signature;
+
+                // Deterministic PE files have a different debug_id format:
+                //
+                // > Version Major=any, Minor=0x504d of the data format has the same structure as above.
+                // > The Age shall be 1. The format of the .pdb file that this PE/COFF file was built with is Portable PDB.
+                // > The Major version specified in the entry indicates the version of the Portable PDB format.
+                // > Together 16B of the Guid concatenated with 4B of the TimeDateStamp field of the entry form a PDB ID that should be used to match the PE/COFF image with the associated PDB (instead of Guid and Age).
+                // > Matching PDB ID is stored in the #Pdb stream of the .pdb file.
+                //
+                // See https://github.com/dotnet/runtime/blob/main/docs/design/specs/PE-COFF.md#codeview-debug-directory-entry-type-2
+                let age = if debug_directory.minor_version == 0x504d {
+                    debug_directory.time_date_stamp
+                } else {
+                    cv_record.age
+                };
+
+                DebugId::from_guid_age(guid, age).ok()
             })
             .unwrap_or_default()
     }
@@ -392,7 +405,7 @@ impl<'data> Dwarf<'data> for PeObject<'data> {
 
     fn raw_section(&self, name: &str) -> Option<DwarfSection<'data>> {
         // Name is given without leading "."
-        let sect = self.section(&format!(".{}", name))?;
+        let sect = self.section(&format!(".{name}"))?;
         let start = sect.pointer_to_raw_data as usize;
         let end = start + (sect.virtual_size as usize);
         let dwarf_data: &'data [u8] = self.data.get(start..end)?;

symbolic-ppdb/Cargo.toml

@@ -28,4 +28,5 @@ uuid = "1.0.0"
 flate2 = { version ="1.0.13", default-features = false, features = [ "rust_backend" ] }
 
 [dev-dependencies]
+symbolic-debuginfo = { path = "../symbolic-debuginfo" }
 symbolic-testutils = { path = "../symbolic-testutils" }

symbolic-ppdb/src/lib.rs

@@ -29,8 +29,8 @@
 //! converter.serialize(&mut buf).unwrap();
 //!
 //! let cache = PortablePdbCache::parse(&buf).unwrap();
-//! let line_info = cache.lookup(6, 10).unwrap();
-//! assert_eq!(line_info.line, 55);
+//! let line_info = cache.lookup(7, 10).unwrap();
+//! assert_eq!(line_info.line, 81);
 //! ```
 //!
 //! # Structure of a Portable PDB file

symbolic-ppdb/tests/test_caches.rs

@@ -47,9 +47,9 @@ fn test_integration() {
     let cache = PortablePdbCache::parse(&buf).unwrap();
 
     assert_eq!(
-        cache.lookup(6, 10),
+        cache.lookup(7, 10),
         Some(LineInfo {
-            line: 55,
+            line: 81,
             file_name: "/Users/swatinem/Coding/sentry-dotnet/samples/foo/Program.cs",
             file_lang: Language::CSharp
         })
@@ -58,7 +58,7 @@ fn test_integration() {
     assert_eq!(
         cache.lookup(5, 6),
         Some(LineInfo {
-            line: 48,
+            line: 37,
             file_name: "/Users/swatinem/Coding/sentry-dotnet/samples/foo/Program.cs",
             file_lang: Language::CSharp
         })
@@ -67,7 +67,7 @@ fn test_integration() {
     assert_eq!(
         cache.lookup(3, 0),
         Some(LineInfo {
-            line: 41,
+            line: 30,
             file_name: "/Users/swatinem/Coding/sentry-dotnet/samples/foo/Program.cs",
             file_lang: Language::CSharp
         })
@@ -76,7 +76,7 @@ fn test_integration() {
     assert_eq!(
         cache.lookup(2, 0),
         Some(LineInfo {
-            line: 36,
+            line: 25,
             file_name: "/Users/swatinem/Coding/sentry-dotnet/samples/foo/Program.cs",
             file_lang: Language::CSharp
         })
@@ -85,9 +85,22 @@ fn test_integration() {
     assert_eq!(
         cache.lookup(1, 45),
         Some(LineInfo {
-            line: 18,
+            line: 20,
             file_name: "/Users/swatinem/Coding/sentry-dotnet/samples/foo/Program.cs",
             file_lang: Language::CSharp
         })
     );
 }
+
+#[test]
+fn test_matching_ids() {
+    let pdb_buf = std::fs::read(fixture("windows/portable.pdb")).unwrap();
+    let pdb = PortablePdb::parse(&pdb_buf).unwrap();
+    let pdb_debug_id = pdb.pdb_id().unwrap();
+
+    let pe_buf = std::fs::read("tests/fixtures/integration.dll").unwrap();
+    let pe = symbolic_debuginfo::pe::PeObject::parse(&pe_buf).unwrap();
+    let pe_debug_id = pe.debug_id();
+
+    assert_eq!(pe_debug_id, pdb_debug_id);
+}