diff --git a/CHANGELOG.md b/CHANGELOG.md index c26472c019..6cdf3dbbb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- (debug-files) Add the `dxil` debug file type for uploading compiled shaders used to symbolicate GPU crashes ([#3489](https://github.com/getsentry/sentry-cli/pull/3489)) + ### Internal Changes - (docker) Bump the Docker image base from Alpine 3.22.0 to 3.22.7, whose `ca-certificates` already drops the distrusted eMudhra/emSign roots, and remove the `ca-certificates` version pin. diff --git a/src/api/data_types/chunking/dif.rs b/src/api/data_types/chunking/dif.rs index f8ddc20b24..3c80aa08fa 100644 --- a/src/api/data_types/chunking/dif.rs +++ b/src/api/data_types/chunking/dif.rs @@ -14,6 +14,10 @@ pub struct ChunkedDifRequest<'a> { pub name: Cow<'a, str>, #[serde(skip_serializing_if = "Option::is_none")] pub debug_id: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub code_id: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub shader_group: Option>, pub chunks: &'a [Digest], #[serde(skip)] hash: Digest, @@ -28,6 +32,8 @@ impl<'a> ChunkedDifRequest<'a> { chunks, hash, debug_id: None, + code_id: None, + shader_group: None, } } @@ -37,6 +43,20 @@ impl<'a> ChunkedDifRequest<'a> { self.debug_id = debug_id; self } + + /// Set the provided code_id on the request, or clear it if + /// `None` is passed. + pub fn with_code_id(mut self, code_id: Option>) -> Self { + self.code_id = code_id; + self + } + + /// Set the provided shader_group on the request, or clear it if + /// `None` is passed. + pub fn with_shader_group(mut self, shader_group: Option>) -> Self { + self.shader_group = shader_group; + self + } } #[derive(Debug, Deserialize)] diff --git a/src/commands/debug_files/find.rs b/src/commands/debug_files/find.rs index 4cd78c313b..56cc295e58 100644 --- a/src/commands/debug_files/find.rs +++ b/src/commands/debug_files/find.rs @@ -150,7 +150,7 @@ fn find_ids( DifType::Breakpad => find_ids_for_breakpad(&dirent, &remaining), DifType::Proguard => find_ids_for_proguard(&dirent, &proguard_uuids), DifType::Jvm => find_ids_for_sourcebundle(&dirent, &remaining), - DifType::Wasm => None, + DifType::Wasm | DifType::Dxil => None, }) .flatten() .collect(); diff --git a/src/commands/debug_files/upload.rs b/src/commands/debug_files/upload.rs index d8e30d00fd..514f59ec6d 100644 --- a/src/commands/debug_files/upload.rs +++ b/src/commands/debug_files/upload.rs @@ -247,6 +247,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { "portablepdb" => upload.filter_format(DifFormat::Object(FileFormat::PortablePdb)), "jvm" => upload.filter_format(DifFormat::Object(FileFormat::SourceBundle)), "wasm" => upload.filter_format(DifFormat::Object(FileFormat::Wasm)), + "dxil" => upload.filter_format(DifFormat::Dxil), "bcsymbolmap" => { upload.filter_format(DifFormat::BcSymbolMap); upload.filter_format(DifFormat::PList) diff --git a/src/utils/chunks/mod.rs b/src/utils/chunks/mod.rs index 1a984e49bc..2b86e5a5fa 100644 --- a/src/utils/chunks/mod.rs +++ b/src/utils/chunks/mod.rs @@ -29,6 +29,9 @@ use crate::utils::progress::{ProgressBar, ProgressBarMode, ProgressStyle}; /// Timeout for polling all assemble endpoints. pub const ASSEMBLE_POLL_INTERVAL: Duration = Duration::from_millis(1000); +/// Maximum number of files per assemble request. +pub const ASSEMBLE_BATCH_SIZE: usize = 1000; + /// A trait required by `BatchedSliceExt` to determine the logical size of a /// batch. Semantics and unit of the size depend on the batching use case, /// likely a number of bytes. See `BatchedSliceExt` for more information. diff --git a/src/utils/chunks/types.rs b/src/utils/chunks/types.rs index a8a1ceca2d..6151ed5f9d 100644 --- a/src/utils/chunks/types.rs +++ b/src/utils/chunks/types.rs @@ -23,6 +23,18 @@ pub trait Assemblable { /// Returns the debug ID of the object. /// Types which do not have a debug ID should return `None`. fn debug_id(&self) -> Option; + + /// Returns the code ID of the object, if it has one that is not + /// derived by the server from the file contents. + fn code_id(&self) -> Option> { + None + } + + /// Returns the shader group of the object, if it is a shader debug file + /// that belongs to one. + fn shader_group(&self) -> Option> { + None + } } /// Chunked arbitrary data with computed SHA1 checksums. @@ -105,6 +117,14 @@ where fn debug_id(&self) -> Option { (*self).debug_id() } + + fn code_id(&self) -> Option> { + (*self).code_id() + } + + fn shader_group(&self) -> Option> { + (*self).shader_group() + } } impl Assemblable for Chunked @@ -118,6 +138,14 @@ where fn debug_id(&self) -> Option { self.object().debug_id() } + + fn code_id(&self) -> Option> { + self.object().code_id() + } + + fn shader_group(&self) -> Option> { + self.object().shader_group() + } } impl<'c, T> From<&'c Chunked> for ChunkedDifRequest<'c> @@ -127,5 +155,7 @@ where fn from(value: &'c Chunked) -> Self { Self::new(value.name(), value.chunk_hashes(), value.checksum()) .with_debug_id(value.debug_id()) + .with_code_id(value.code_id()) + .with_shader_group(value.shader_group()) } } diff --git a/src/utils/chunks/upload.rs b/src/utils/chunks/upload.rs index a472737dee..000821f6b7 100644 --- a/src/utils/chunks/upload.rs +++ b/src/utils/chunks/upload.rs @@ -7,12 +7,16 @@ use anyhow::Result; use indicatif::ProgressStyle; use crate::{ - api::{Api, AssembleDifsRequest, ChunkServerOptions, ChunkedFileState, DebugInfoFile}, + api::{ + Api, AssembleDifsRequest, AssembleDifsResponse, ChunkServerOptions, ChunkedFileState, + DebugInfoFile, + }, utils::progress::ProgressBar, }; use super::{ - Assemblable, Chunk, ChunkOptions, Chunked, MissingObjectsInfo, ASSEMBLE_POLL_INTERVAL, + Assemblable, Chunk, ChunkOptions, Chunked, MissingObjectsInfo, ASSEMBLE_BATCH_SIZE, + ASSEMBLE_POLL_INTERVAL, }; pub fn upload_chunked_objects( @@ -40,6 +44,26 @@ where } } +/// Sends the assemble requests one by one and merges their responses. +/// +/// The server processes every file in a request before answering, so files are +/// split into batches of `ASSEMBLE_BATCH_SIZE` to keep each request short. +fn assemble_difs( + requests: &[AssembleDifsRequest<'_>], + options: &ChunkOptions<'_>, +) -> Result { + let api = Api::current(); + let mut response = AssembleDifsResponse::new(); + for request in requests { + response.extend(api.authenticated()?.assemble_difs( + options.org(), + options.project(), + request, + )?); + } + Ok(response) +} + /// Calls the assemble endpoint and returns the state for every object along /// with info on missing chunks. /// @@ -52,12 +76,11 @@ fn try_assemble<'m, T>( where T: AsRef<[u8]> + Assemblable, { - let api = Api::current(); - let request: AssembleDifsRequest<'_> = objects.iter().collect(); - - let response = - api.authenticated()? - .assemble_difs(options.org(), options.project(), &request)?; + let requests: Vec> = objects + .chunks(ASSEMBLE_BATCH_SIZE) + .map(|batch| batch.iter().collect()) + .collect(); + let response = assemble_difs(&requests, options)?; // We map all objects by their checksum, so we can access them faster when // iterating through the server response below. Since the caller will invoke @@ -177,19 +200,19 @@ where \n{wide_bar} {pos}/{len}", ); - let api = Api::current(); let pb = ProgressBar::new(chunked_objects.len()); pb.set_style(progress_style); pb.set_prefix(">"); let assemble_start = Instant::now(); - let request: AssembleDifsRequest<'_> = chunked_objects.iter().copied().collect(); + let requests: Vec> = chunked_objects + .chunks(ASSEMBLE_BATCH_SIZE) + .map(|batch| batch.iter().copied().collect()) + .collect(); let response = loop { - let response = - api.authenticated()? - .assemble_difs(options.org(), options.project(), &request)?; + let response = assemble_difs(&requests, options)?; let chunks_missing = response .values() diff --git a/src/utils/dif.rs b/src/utils/dif.rs index 321244f378..be14305c1e 100644 --- a/src/utils/dif.rs +++ b/src/utils/dif.rs @@ -2,7 +2,7 @@ use std::path::Path; use std::str; use std::{fmt, iter}; -use anyhow::{bail, Context as _, Error, Result}; +use anyhow::{bail, format_err, Context as _, Error, Result}; use proguard::ProguardMapping; use serde::ser::{SerializeStruct as _, Serializer}; use serde::Serialize; @@ -22,6 +22,7 @@ pub enum DifType { PortablePdb, Wasm, Jvm, + Dxil, } impl DifType { @@ -37,6 +38,7 @@ impl DifType { DifType::Proguard => "proguard", DifType::Wasm => "wasm", DifType::Jvm => "jvm", + DifType::Dxil => "dxil", } } @@ -52,6 +54,7 @@ impl DifType { DifType::Proguard, DifType::Wasm, DifType::Jvm, + DifType::Dxil, ] } @@ -67,6 +70,7 @@ impl DifType { "proguard", "wasm", "jvm", + "dxil", ] } } @@ -92,6 +96,7 @@ impl str::FromStr for DifType { "proguard" => Ok(DifType::Proguard), "wasm" => Ok(DifType::Wasm), "jvm" => Ok(DifType::Jvm), + "dxil" => Ok(DifType::Dxil), _ => bail!("Invalid debug info file type"), } } @@ -257,6 +262,7 @@ impl DifFile<'static> { Some(DifType::Breakpad) => DifFile::open_object(&path, FileFormat::Breakpad), Some(DifType::Proguard) => DifFile::open_proguard(&path), Some(DifType::Jvm) => DifFile::open_object(&path, FileFormat::SourceBundle), + Some(DifType::Dxil) => Err(format_err!("Shader debug files can only be uploaded")), None => DifFile::try_open(&path), }; diff --git a/src/utils/dif_upload/mod.rs b/src/utils/dif_upload/mod.rs index b3fef2312e..4ff676a548 100644 --- a/src/utils/dif_upload/mod.rs +++ b/src/utils/dif_upload/mod.rs @@ -4,6 +4,7 @@ //! `DifUpload` for more information. mod error; +mod shader; use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; @@ -34,6 +35,7 @@ use zip::result::ZipError; use zip::ZipArchive; use self::error::ValidationError; +use self::shader::{ShaderInfo, ShaderManifest}; use crate::api::{Api, ChunkServerOptions}; use crate::config::Config; use crate::constants::{DEFAULT_MAX_DIF_SIZE, DEFAULT_MAX_WAIT}; @@ -56,6 +58,7 @@ enum ParsedDif<'a> { BcSymbolMap, UuidMap, Il2Cpp, + Dxil(Option), } impl<'slf, 'data: 'slf> AsSelf<'slf> for ParsedDif<'data> { @@ -145,6 +148,23 @@ impl<'data> DifMatch<'data> { }) } + fn from_dxil( + debug_id: DebugId, + name: String, + data: ByteView<'static>, + info: Option, + ) -> Self { + let dif = SelfCell::new(data, |_| ParsedDif::Dxil(info)); + + Self { + _backing: None, + dif, + name, + debug_id: Some(debug_id), + attachments: None, + } + } + fn from_plist(uuid: DebugId, name: String, data: ByteView<'static>) -> Result { let dif = SelfCell::try_new(data, |buf| { UuidMapping::parse_plist(uuid, unsafe { &*buf }).map(|_| ParsedDif::UuidMap) @@ -183,6 +203,7 @@ impl<'data> DifMatch<'data> { ParsedDif::BcSymbolMap => None, ParsedDif::UuidMap => None, ParsedDif::Il2Cpp => None, + ParsedDif::Dxil(_) => None, } } @@ -192,6 +213,7 @@ impl<'data> DifMatch<'data> { ParsedDif::BcSymbolMap => DifFormat::BcSymbolMap, ParsedDif::UuidMap => DifFormat::PList, ParsedDif::Il2Cpp => DifFormat::Il2Cpp, + ParsedDif::Dxil(_) => DifFormat::Dxil, } } @@ -202,6 +224,7 @@ impl<'data> DifMatch<'data> { ParsedDif::BcSymbolMap => self.dif.owner(), ParsedDif::UuidMap => self.dif.owner(), ParsedDif::Il2Cpp => self.dif.owner(), + ParsedDif::Dxil(_) => self.dif.owner(), } } @@ -272,6 +295,7 @@ impl Display for DifMatch<'_> { ParsedDif::BcSymbolMap => String::from("bcsymbolmap"), ParsedDif::UuidMap => String::from("uuidmap"), ParsedDif::Il2Cpp => String::from("il2cpp"), + ParsedDif::Dxil(_) => String::from("dxil"), }; write!( @@ -301,6 +325,20 @@ impl Assemblable for DifMatch<'_> { fn debug_id(&self) -> Option { self.debug_id } + + fn code_id(&self) -> Option> { + match self.dif.get() { + ParsedDif::Dxil(Some(info)) => Some(Cow::Borrowed(&info.code_id)), + _ => None, + } + } + + fn shader_group(&self) -> Option> { + match self.dif.get() { + ParsedDif::Dxil(Some(info)) => info.shader_group.as_deref().map(Cow::Borrowed), + _ => None, + } + } } type ZipFileArchive = ZipArchive>; @@ -595,6 +633,7 @@ fn search_difs(options: &DifUpload) -> Result>> { pb.set_style(progress_style); let mut age_overrides = BTreeMap::new(); + let mut shader_manifests = BTreeMap::new(); let mut collected = Vec::new(); for base_path in &options.paths { if base_path == Path::new("") { @@ -608,7 +647,13 @@ fn search_difs(options: &DifUpload) -> Result>> { debug!("trying to process {name}"); pb.set_message(&name); - if Archive::peek(&buffer) != FileFormat::Unknown { + if shader::is_dxil(&buffer) { + if let Some(dif) = + collect_dxil_dif(&source, name, buffer, options, &mut shader_manifests)? + { + collected.push(dif); + } + } else if Archive::peek(&buffer) != FileFormat::Unknown { let mut difs = collect_object_dif(source, name, buffer, options, &mut age_overrides)?; collected.append(difs.as_mut()); @@ -674,6 +719,50 @@ impl Display for AuxDifKind { } } +/// Collects a DXIL shader container into a [`DifMatch`]. +/// +/// The debug ID is the 32-hex file stem. The code ID and shader group come from +/// the manifest next to the file, loaded once per directory into `manifests`. +fn collect_dxil_dif<'a>( + source: &DifSource<'_>, + name: String, + buffer: ByteView<'static>, + options: &DifUpload, + manifests: &mut BTreeMap, +) -> Result>> { + let file_stem = Path::new(&name) + .file_stem() + .map(|stem| stem.to_string_lossy()) + .unwrap_or_default(); + let debug_id = match file_stem.parse() { + Ok(debug_id) => debug_id, + Err(_) => { + warn!("Skipping DXIL with invalid filename: {name}"); + return Ok(None); + } + }; + + let info = match source { + DifSource::FileSystem(path) => path.parent().and_then(|dir| { + manifests + .entry(dir.to_path_buf()) + .or_insert_with(|| shader::load_manifest(dir)) + .get(path.file_name()?.to_str()?) + .cloned() + }), + _ => None, + }; + + let dif = DifMatch::from_dxil(debug_id, name.clone(), buffer, info); + + if let Err(e) = options.validate_dif(&dif) { + error::handle(&name, &e)?; + return Ok(None); + } + + Ok(Some(dif)) +} + /// Collects a possible BCSymbolmap or PList into a [`DifMatch`]. /// /// The `name` is the relative path of the file processed, while `buffer` contains the @@ -1224,6 +1313,8 @@ pub enum DifFormat { PList, /// A Unity il2cpp line mapping file. Il2Cpp, + /// A DXIL shader container produced by DXC. + Dxil, } /// Searches, processes and uploads debug information files (DIFs). diff --git a/src/utils/dif_upload/shader.rs b/src/utils/dif_upload/shader.rs new file mode 100644 index 0000000000..a9e2b7cd76 --- /dev/null +++ b/src/utils/dif_upload/shader.rs @@ -0,0 +1,107 @@ +//! Detection and manifest lookup for DXIL shader containers. + +use std::collections::HashMap; +use std::fs; +use std::path::Path; + +use log::warn; + +const MANIFEST_FILE_NAME: &str = "sentry-shader-manifest.tsv"; + +/// Returns `true` if the buffer is a DXIL container. +pub fn is_dxil(data: &[u8]) -> bool { + data.starts_with(b"DXBC") +} + +/// Manifest metadata of a single `.dxil` file. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ShaderInfo { + /// NVIDIA Aftermath shader hash as 16 hex digits. + pub code_id: String, + pub shader_group: Option, +} + +/// `.dxil` file names mapped to their manifest metadata. +pub type ShaderManifest = HashMap; + +/// Loads the manifest written next to the shader files in `dir`. +pub fn load_manifest(dir: &Path) -> ShaderManifest { + let path = dir.join(MANIFEST_FILE_NAME); + match fs::read_to_string(&path) { + Ok(contents) => parse_manifest(&contents), + Err(err) => { + warn!( + "Could not read {}: {err}; DXIL files are uploaded without code ID", + path.display() + ); + ShaderManifest::new() + } + } +} + +/// Parses lines of the form `\t\t[\t]`. +fn parse_manifest(contents: &str) -> ShaderManifest { + contents + .lines() + .filter_map(|line| { + let mut columns = line.split('\t'); + let hash: u64 = columns.next()?.parse().ok()?; + let _pdb_name = columns.next()?; + let dxil_name = columns.next()?.rsplit(['\\', '/']).next()?; + let shader_group = columns + .next() + .map(str::trim) + .filter(|group| !group.is_empty()) + .map(str::to_owned); + let info = ShaderInfo { + code_id: format!("{hash:016x}"), + shader_group, + }; + Some((dxil_name.to_owned(), info)) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_is_dxil() { + assert!(is_dxil(b"DXBC\x01\x02\x03")); + assert!(!is_dxil(b"\x7fELF")); + } + + #[test] + fn test_parse_manifest() { + let manifest = parse_manifest( + "709220027517385083\t000057e2ed5c597bf3ca2651c6e13a1d.pdb\tD:\\x\\000057e2ed5c597bf3ca2651c6e13a1d.dxil\r\n\ + 2\tb.pdb\tD:/y/b.dxil\tGlobalShaders\n\ + 3\tc.pdb\tc.dxil\t Padded Group \r\n\ + 4\td.pdb\td.dxil\t \n\ + 123\t\n\ + notanumber\tfoo.pdb\tfoo.dxil\n", + ); + + assert_eq!( + manifest["000057e2ed5c597bf3ca2651c6e13a1d.dxil"], + ShaderInfo { + code_id: "09d7a7db7fc4397b".into(), + shader_group: None, + } + ); + assert_eq!( + manifest["b.dxil"], + ShaderInfo { + code_id: "0000000000000002".into(), + shader_group: Some("GlobalShaders".into()), + } + ); + assert_eq!( + manifest["c.dxil"].shader_group.as_deref(), + Some("Padded Group") + ); + assert_eq!(manifest["d.dxil"].shader_group, None); + assert_eq!(manifest.len(), 4); + } +} diff --git a/tests/integration/_cases/debug_files/not_windows/debug_files-upload-help.trycmd b/tests/integration/_cases/debug_files/not_windows/debug_files-upload-help.trycmd index d663e891d8..fcbfd7a85a 100644 --- a/tests/integration/_cases/debug_files/not_windows/debug_files-upload-help.trycmd +++ b/tests/integration/_cases/debug_files/not_windows/debug_files-upload-help.trycmd @@ -16,7 +16,8 @@ Options: --auth-token Use the given Sentry auth token. -t, --type Only consider debug information files of the given type. By default, all types are considered. [possible values: bcsymbolmap, - breakpad, dsym, elf, jvm, pdb, pe, portablepdb, sourcebundle, wasm] + breakpad, dsym, dxil, elf, jvm, pdb, pe, portablepdb, sourcebundle, + wasm] --log-level Set the log output verbosity. [possible values: trace, debug, info, warn, error] --no-unwind Do not scan for stack unwinding information. Specify this flag for diff --git a/tests/integration/_cases/upload_dif/upload_dif-help.trycmd b/tests/integration/_cases/upload_dif/upload_dif-help.trycmd index 3c212473cf..4737b1b409 100644 --- a/tests/integration/_cases/upload_dif/upload_dif-help.trycmd +++ b/tests/integration/_cases/upload_dif/upload_dif-help.trycmd @@ -16,7 +16,8 @@ Options: --auth-token Use the given Sentry auth token. -t, --type Only consider debug information files of the given type. By default, all types are considered. [possible values: bcsymbolmap, - breakpad, dsym, elf, jvm, pdb, pe, portablepdb, sourcebundle, wasm] + breakpad, dsym, dxil, elf, jvm, pdb, pe, portablepdb, sourcebundle, + wasm] --log-level Set the log output verbosity. [possible values: trace, debug, info, warn, error] --no-unwind Do not scan for stack unwinding information. Specify this flag for diff --git a/tests/integration/_cases/upload_dsym/upload_dsym-help.trycmd b/tests/integration/_cases/upload_dsym/upload_dsym-help.trycmd index b5d4a76c30..e5e68eeb77 100644 --- a/tests/integration/_cases/upload_dsym/upload_dsym-help.trycmd +++ b/tests/integration/_cases/upload_dsym/upload_dsym-help.trycmd @@ -16,7 +16,8 @@ Options: --auth-token Use the given Sentry auth token. -t, --type Only consider debug information files of the given type. By default, all types are considered. [possible values: bcsymbolmap, - breakpad, dsym, elf, jvm, pdb, pe, portablepdb, sourcebundle, wasm] + breakpad, dsym, dxil, elf, jvm, pdb, pe, portablepdb, sourcebundle, + wasm] --log-level Set the log output verbosity. [possible values: trace, debug, info, warn, error] --no-unwind Do not scan for stack unwinding information. Specify this flag for