diff --git a/Cargo.lock b/Cargo.lock index bffe758c11..358ee0477b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2517,9 +2517,9 @@ checksum = "6bdef32e8150c2a081110b42772ffe7d7c9032b606bc226c8260fd97e0976601" [[package]] name = "symbolic" -version = "11.0.0" +version = "11.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6f5219610b220c2114858461fa469f8ead9dd9920556c4fbb41878d669c92278" +checksum = "220047ad8c7b304542d390969a5b4be62b9a8c95a55bee8f905b0bd8ea5664db" dependencies = [ "symbolic-common", "symbolic-debuginfo", @@ -2529,9 +2529,9 @@ dependencies = [ [[package]] name = "symbolic-common" -version = "11.0.0" +version = "11.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a446214493923a25931088b6b0d94a6e8ccc9896f0e992d10ff0000395322cf1" +checksum = "46939659856f0595dbbdbe0c8271d11c6788c780c859bf9afd834a723dd78fa3" dependencies = [ "debugid", "memmap2", @@ -2542,9 +2542,9 @@ dependencies = [ [[package]] name = "symbolic-debuginfo" -version = "11.0.0" +version = "11.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6fb3d8648a0e2de0c194b7221d34e1009af574b692bd496dac592fe466c0d42" +checksum = "a442c100d3884c3834be145adc3583de3c755f89d89887dc1f1bbc9bb67c464c" dependencies = [ "bitvec", "dmsort", @@ -2574,9 +2574,9 @@ dependencies = [ [[package]] name = "symbolic-il2cpp" -version = "11.0.0" +version = "11.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "06db0c594e1dfcc1e7993bf63635ffc596a65881c81375de264a78e7270ac696" +checksum = "fce6b51dc2691cb7e281ec371cd6307622545261989c060491c1604775335031" dependencies = [ "indexmap", "serde_json", @@ -2586,9 +2586,9 @@ dependencies = [ [[package]] name = "symbolic-ppdb" -version = "11.0.0" +version = "11.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1ae4243c9bab0da49a75c2d3aeee737d82e65d6d94b68fec9a7a6e6e977ace00" +checksum = "8845b582c7ec58a155f44020cf4e7de16fe5db69cf2368bca174ff4331504b25" dependencies = [ "flate2", "indexmap", @@ -2600,9 +2600,9 @@ dependencies = [ [[package]] name = "symbolic-symcache" -version = "11.0.0" +version = "11.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fb5cb538fa9dea7b76619252ac19617af25e3e5608230cfbe105cea3453e945" +checksum = "6cb55e8429481c312f635f8842182b3ec17bd173a407549257c311a333536b2a" dependencies = [ "indexmap", "symbolic-common", diff --git a/Cargo.toml b/Cargo.toml index 45b00ebf8f..6e284d5a78 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -61,7 +61,7 @@ serde = { version = "1.0.152", features = ["derive"] } serde_json = "1.0.91" sha1_smol = { version = "1.0.0", features = ["serde"] } sourcemap = { version = "6.2.1", features = ["ram_bundle"] } -symbolic = { version = "11.0.0", features = ["debuginfo-serde", "il2cpp"] } +symbolic = { version = "11.1.0", features = ["debuginfo-serde", "il2cpp"] } thiserror = "1.0.38" url = "2.3.1" username = "0.2.0" diff --git a/src/utils/dif.rs b/src/utils/dif.rs index f24dee1fd9..c120b034b3 100644 --- a/src/utils/dif.rs +++ b/src/utils/dif.rs @@ -335,11 +335,30 @@ impl<'a> DifFile<'a> { match self { DifFile::Archive(archive) => { let mut features = ObjectDifFeatures::none(); - for object in archive.get().objects().filter_map(Result::ok) { + + let mut add_object_features = |object: &Object| { features.symtab = features.symtab || object.has_symbols(); features.debug = features.debug || object.has_debug_info(); features.unwind = features.unwind || object.has_unwind_info(); features.sources = features.sources || object.has_sources(); + }; + + for object in archive.get().objects().filter_map(Result::ok) { + add_object_features(&object); + + // Combine features with an embedded Portable PDB, if any to show up correctly in `dif check` cmd. + // Note: this is intentionally different than `DifUpload.valid_features()` because we don't want to + // upload the PE file separately, unless it has features we need. The PPDB is extracted instead. + if let Ok(Some(Object::Pe(pe))) = archive.get().object_by_index(0) { + if let Ok(Some(ppdb_data)) = pe.embedded_ppdb() { + let mut buf = Vec::new(); + if ppdb_data.decompress_to(&mut buf).is_ok() { + if let Ok(ppdb) = Object::parse(&buf) { + add_object_features(&ppdb); + } + } + } + } } features } diff --git a/src/utils/dif_upload.rs b/src/utils/dif_upload.rs index 56d652e64a..c63520e50d 100644 --- a/src/utils/dif_upload.rs +++ b/src/utils/dif_upload.rs @@ -24,6 +24,7 @@ use log::{debug, info, warn}; use sha1_smol::Digest; use symbolic::common::{AsSelf, ByteView, DebugId, SelfCell, Uuid}; use symbolic::debuginfo::macho::{BcSymbolMap, UuidMapping}; +use symbolic::debuginfo::pe::PeObject; use symbolic::debuginfo::sourcebundle::SourceBundleWriter; use symbolic::debuginfo::{Archive, FileEntry, FileFormat, Object}; use symbolic::il2cpp::ObjectLineMapping; @@ -114,7 +115,7 @@ struct DifMatch<'data> { } impl<'data> DifMatch<'data> { - fn from_temp_object(temp_file: TempFile, name: S) -> Result + fn from_temp_object(temp_file: TempFile, name: S, debug_id: Option) -> Result where S: Into, { @@ -123,14 +124,11 @@ impl<'data> DifMatch<'data> { Object::parse(unsafe { &*b }).map(|object| ParsedDif::Object(Box::new(object))) })?; - // Even though we could supply the debug_id here from the object we do not, the - // server will do the same anyway and we actually have control over the version of - // the code running there so can fix bugs more reliably. Ok(DifMatch { _backing: Some(DifBacking::Temp(temp_file)), dif, name: name.into(), - debug_id: None, + debug_id, attachments: None, }) } @@ -196,8 +194,11 @@ impl<'data> DifMatch<'data> { P: AsRef, S: Into, { + // Even though we could supply the debug_id here from the object we do not, the + // server will do the same anyway and we actually have control over the version of + // the code running there so can fix bugs more reliably. let temp_file = TempFile::take(path)?; - Self::from_temp_object(temp_file, name) + Self::from_temp_object(temp_file, name, None) } /// Returns the parsed [`Object`] of this DIF. @@ -729,16 +730,13 @@ fn search_difs(options: &DifUpload) -> Result>> { } ); - let count_with_sources = match options.include_sources { - true => collected - .iter() - .filter(|dif| match dif.object() { - Some(object) => object.has_sources(), - None => false, - }) - .count(), - false => 0, - }; + let count_with_sources = collected + .iter() + .filter(|dif| match dif.object() { + Some(object) => object.has_sources(), + None => false, + }) + .count(); match count_with_sources { 0 => println!(), @@ -853,7 +851,7 @@ fn collect_object_dif<'a>( // Each `FatObject` might contain multiple matching objects, each of // which needs to retain a reference to the original fat file. We - // create a shared instance here and clone it into `DifMatche`s + // create a shared instance here and clone it into `DifMatch`es // below. for object in archive.objects() { // Silently skip all objects that we cannot process. This can @@ -872,6 +870,15 @@ fn collect_object_dif<'a>( continue; } + // If this is a PE file with an embedded Portable PDB, we extract and process the PPDB separately. + if let Object::Pe(pe) = &object { + if let Ok(Some(ppdb_dif)) = extract_embedded_ppdb(pe, name.as_str()) { + if options.validate_dif(&ppdb_dif) { + collected.push(ppdb_dif); + } + } + }; + // Store a mapping of "age" values for all encountered PE files, // regardless of whether they will be uploaded. This is used later // to fix up PDB files. @@ -1077,6 +1084,25 @@ fn process_symbol_maps<'a>( Ok(without_hidden) } +/// Checks whether the `PeObject` contains an embedded Portable PDB and extracts it as a separate `DifMatch`. +fn extract_embedded_ppdb<'a>(pe: &PeObject, pe_name: &str) -> Result>> { + if let Some(embedded_ppdb) = pe.embedded_ppdb()? { + let temp_file = TempFile::create()?; + temp_file + .open() + .map(|f| embedded_ppdb.decompress_to(BufWriter::new(f)))??; + + let dif = DifMatch::from_temp_object( + temp_file, + Path::new(pe_name).with_extension("pdb").to_string_lossy(), + Some(pe.debug_id()), + )?; + Ok(Some(dif)) + } else { + Ok(None) + } +} + /// Default filter function to skip over bad sources we do not want to include. pub fn filter_bad_sources(entry: &FileEntry) -> bool { let max_size = Config::current().get_max_dif_item_size(); @@ -1151,9 +1177,7 @@ fn create_source_bundles<'a>( continue; } - let mut source_bundle = DifMatch::from_temp_object(temp_file, name)?; - source_bundle.debug_id = dif.debug_id; - source_bundles.push(source_bundle); + source_bundles.push(DifMatch::from_temp_object(temp_file, name, dif.debug_id)?); } let len = source_bundles.len(); diff --git a/tests/integration/_cases/debug_files/debug_files-check-dll-embedded-ppdb-with-sources.trycmd b/tests/integration/_cases/debug_files/debug_files-check-dll-embedded-ppdb-with-sources.trycmd new file mode 100644 index 0000000000..98afe62d1f --- /dev/null +++ b/tests/integration/_cases/debug_files/debug_files-check-dll-embedded-ppdb-with-sources.trycmd @@ -0,0 +1,14 @@ +``` +$ SENTRY_ALLOW_FAILURE=1 sentry-cli debug-files check tests/integration/_fixtures/Sentry.Samples.Console.Basic-embedded-ppdb-with-sources.dll +? success +Debug Info File Check + Type: pe executable + Contained debug identifiers: + > Debug ID: 623535c7-c0ea-4dee-b99b-4669e99a7ecc-a878d1fa + Code ID: d1dde1d5a000 + Arch: x86 + Contained debug information: + > debug, sources + Usable: yes + +``` diff --git a/tests/integration/_cases/debug_files/debug_files-upload-dll-embedded-ppdb-with-sources.trycmd b/tests/integration/_cases/debug_files/debug_files-upload-dll-embedded-ppdb-with-sources.trycmd new file mode 100644 index 0000000000..74d9c45e8e --- /dev/null +++ b/tests/integration/_cases/debug_files/debug_files-upload-dll-embedded-ppdb-with-sources.trycmd @@ -0,0 +1,8 @@ +``` +$ sentry-cli debug-files upload tests/integration/_fixtures/Sentry.Samples.Console.Basic-embedded-ppdb-with-sources.dll +? success +> Found 1 debug information file (1 with embedded sources) +> Prepared debug information file for upload +> Nothing to upload, all files are on the server + +``` diff --git a/tests/integration/_fixtures/Sentry.Samples.Console.Basic-embedded-ppdb-with-sources.dll b/tests/integration/_fixtures/Sentry.Samples.Console.Basic-embedded-ppdb-with-sources.dll new file mode 100644 index 0000000000..1117428524 Binary files /dev/null and b/tests/integration/_fixtures/Sentry.Samples.Console.Basic-embedded-ppdb-with-sources.dll differ diff --git a/tests/integration/debug_files/check.rs b/tests/integration/debug_files/check.rs index e1f4c306fc..66326f6f6c 100644 --- a/tests/integration/debug_files/check.rs +++ b/tests/integration/debug_files/check.rs @@ -28,3 +28,8 @@ fn command_debug_files_check_no_file_allow_failure_env() { #[cfg(windows)] register_test("debug_files/debug_files-check-no-file-allow-failure-env-windows.trycmd"); } + +#[test] +fn command_debug_files_check_dll_embedded_ppdb_with_sources() { + register_test("debug_files/debug_files-check-dll-embedded-ppdb-with-sources.trycmd"); +} diff --git a/tests/integration/debug_files/upload.rs b/tests/integration/debug_files/upload.rs index 70a6ba4f39..65d05d8769 100644 --- a/tests/integration/debug_files/upload.rs +++ b/tests/integration/debug_files/upload.rs @@ -99,6 +99,38 @@ fn command_debug_files_upload_pdb_embedded_sources() { register_test("debug_files/debug_files-upload-pdb-embedded-sources.trycmd"); } +#[test] +fn command_debug_files_upload_dll_embedded_ppdb_with_sources() { + let _chunk_upload = mock_endpoint( + EndpointOptions::new("GET", "/api/0/organizations/wat-org/chunk-upload/", 200) + .with_response_file("debug_files/get-chunk-upload.json"), + ); + let _assemble = mock_endpoint( + EndpointOptions::new( + "POST", + "/api/0/projects/wat-org/wat-project/files/difs/assemble/", + 200, + ) + .with_response_body( + r#"{ + "fc1c9e58a65bd4eaf973bbb7e7a7cc01bfdaf15e": { + "state": "ok", + "missingChunks": [] + } + }"#, + ), + ); + let _reprocessing = mock_endpoint( + EndpointOptions::new( + "POST", + "/api/0/projects/wat-org/wat-project/reprocessing/", + 200, + ) + .with_response_body("[]"), + ); + register_test("debug_files/debug_files-upload-dll-embedded-ppdb-with-sources.trycmd"); +} + #[test] fn command_debug_files_upload_no_upload() { let _chunk_upload = mock_endpoint(