From cad86628dbffa7f773d2865e1fa9b0fb1782f908 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 24 Nov 2020 20:57:17 +0100 Subject: [PATCH 01/21] feat(wasm): Initial work for WASM support --- Cargo.lock | 74 ++++++++++++++++++++++++++++++++----- Cargo.toml | 2 +- src/actors/symbolication.rs | 65 +++++++++++++++++++++----------- src/sources.rs | 5 +++ src/types.rs | 14 +++++++ src/utils/paths.rs | 37 ++++++++++++++++--- 6 files changed, 159 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8d7f68a52..149a4fe95 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -575,10 +575,11 @@ checksum = "b3a71ab494c0b5b860bdc8407ae08978052417070c2ced38573a9157ad75b8ac" [[package]] name = "cpp_demangle" -version = "0.3.0" -source = "git+https://github.com/Swatinem/cpp_demangle?branch=sentry-patches#2d274c4865169e81cc84fc33c6227f61bceab1ae" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6656dbc7c586cf2965887c3d066d9b983b20400bc2d52ed4cc5e33d0ceb2c602" dependencies = [ - "cfg-if 0.1.10", + "cfg-if 1.0.0", "glob", ] @@ -1406,6 +1407,12 @@ dependencies = [ "tokio-tls 0.3.0", ] +[[package]] +name = "id-arena" +version = "2.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "25a2bc672d1148e28034f176e01fffebb08b35768468cc954630da77a1449005" + [[package]] name = "idna" version = "0.1.5" @@ -1595,6 +1602,12 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b294d6fa9ee409a054354afc4352b0b9ef7ca222c69b8812cbea9e7d2bf3783f" +[[package]] +name = "leb128" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3576a87f2ba00f6f106fdfcd16db1d698d648a26ad8e0573cad8537c3c362d2a" + [[package]] name = "libc" version = "0.2.80" @@ -1820,7 +1833,8 @@ dependencies = [ [[package]] name = "msvc-demangler" version = "0.8.0" -source = "git+https://github.com/Swatinem/msvc-demangler-rust?branch=sentry-patches#47c1c61d28f5a226d7f59aeff94fc6e41f550e40" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6584cf122f02fc0396420a116cd395a9a776ec4347dffe1c5119c3fcc917c060" dependencies = [ "bitflags", ] @@ -3121,7 +3135,7 @@ dependencies = [ [[package]] name = "symbolic" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#4619030e1ca0470c38190b89fb580d9aea5c5f07" +source = "git+https://github.com/getsentry/symbolic?branch=master#5d007d7c6605df7fa683749ef0f1b9380571fa05" dependencies = [ "symbolic-common", "symbolic-debuginfo", @@ -3133,7 +3147,7 @@ dependencies = [ [[package]] name = "symbolic-common" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#4619030e1ca0470c38190b89fb580d9aea5c5f07" +source = "git+https://github.com/getsentry/symbolic?branch=master#5d007d7c6605df7fa683749ef0f1b9380571fa05" dependencies = [ "debugid", "memmap", @@ -3145,7 +3159,7 @@ dependencies = [ [[package]] name = "symbolic-debuginfo" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#4619030e1ca0470c38190b89fb580d9aea5c5f07" +source = "git+https://github.com/getsentry/symbolic?branch=master#5d007d7c6605df7fa683749ef0f1b9380571fa05" dependencies = [ "dmsort", "fallible-iterator", @@ -3164,13 +3178,15 @@ dependencies = [ "smallvec 1.5.0", "symbolic-common", "thiserror", + "walrus", + "wasmparser 0.68.0", "zip", ] [[package]] name = "symbolic-demangle" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#4619030e1ca0470c38190b89fb580d9aea5c5f07" +source = "git+https://github.com/getsentry/symbolic?branch=master#5d007d7c6605df7fa683749ef0f1b9380571fa05" dependencies = [ "cc", "cpp_demangle", @@ -3182,7 +3198,7 @@ dependencies = [ [[package]] name = "symbolic-minidump" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#4619030e1ca0470c38190b89fb580d9aea5c5f07" +source = "git+https://github.com/getsentry/symbolic?branch=master#5d007d7c6605df7fa683749ef0f1b9380571fa05" dependencies = [ "cc", "lazy_static", @@ -3196,7 +3212,7 @@ dependencies = [ [[package]] name = "symbolic-symcache" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#4619030e1ca0470c38190b89fb580d9aea5c5f07" +source = "git+https://github.com/getsentry/symbolic?branch=master#5d007d7c6605df7fa683749ef0f1b9380571fa05" dependencies = [ "dmsort", "fnv", @@ -3963,6 +3979,32 @@ version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "078775d0255232fb988e6fccf26ddc9d1ac274299aaedcedce21c6f72cc533ce" +[[package]] +name = "walrus" +version = "0.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4d470d0583e65f4cab21a1ff3c1ba3dd23ae49e68f516f0afceaeb001b32af39" +dependencies = [ + "anyhow", + "id-arena", + "leb128", + "log", + "walrus-macro", + "wasmparser 0.59.0", +] + +[[package]] +name = "walrus-macro" +version = "0.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7c2bb690b44cb1b0fdcc54d4998d21f8bdaf706b93775425e440b174f39ad16" +dependencies = [ + "heck", + "proc-macro2 1.0.10", + "quote 1.0.3", + "syn 1.0.17", +] + [[package]] name = "want" version = "0.2.0" @@ -4064,6 +4106,18 @@ version = "0.2.60" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "daf76fe7d25ac79748a37538b7daeed1c7a6867c92d3245c12c6222e4a20d639" +[[package]] +name = "wasmparser" +version = "0.59.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a950e6a618f62147fd514ff445b2a0b53120d382751960797f85f058c7eda9b9" + +[[package]] +name = "wasmparser" +version = "0.68.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29a00e14eed9c2ecbbdbdd4fb284f49d21b6808965de24769a6379a13ec47d4c" + [[package]] name = "web-sys" version = "0.3.37" diff --git a/Cargo.toml b/Cargo.toml index c30cb0d6c..4b11af2b8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,7 +49,7 @@ serde = { version = "1.0.101", features = ["derive", "rc"] } serde_json = "1.0.45" serde_yaml = "0.8.11" structopt = "0.3.2" -symbolic = { git = "https://github.com/getsentry/symbolic", branch = "fix/demangle-fixes", features = ["common-serde", "demangle", "minidump-serde", "symcache"] } +symbolic = { git = "https://github.com/getsentry/symbolic", branch = "master", features = ["common-serde", "demangle", "minidump-serde", "symcache"] } tempfile = "3.1.0" thiserror = "1.0.20" tokio = "0.1.22" diff --git a/src/actors/symbolication.rs b/src/actors/symbolication.rs index 5b348de01..4af25de40 100644 --- a/src/actors/symbolication.rs +++ b/src/actors/symbolication.rs @@ -550,7 +550,9 @@ impl SymCacheLookup { for stacktrace in stacktraces { for frame in stacktrace.frames { - if let Some((i, ..)) = self.lookup_symcache(frame.instruction_addr.0) { + if let Some((i, ..)) = + self.lookup_symcache(frame.instruction_addr.0, frame.in_module) + { referenced_objects.insert(i); } } @@ -608,8 +610,16 @@ impl SymCacheLookup { fn lookup_symcache( &self, addr: u64, + in_module: Option, ) -> Option<(usize, &CompleteObjectInfo, Option<&SymCacheFile>)> { for (i, (info, cache)) in self.inner.iter().enumerate() { + // if `in_module` is defined we pick up the module directly + // by the index. + if Some(i) == in_module { + return Some((i, info, cache.as_ref().map(|x| &**x))); + } + + // lookup by address let start_addr = info.raw.image_addr.0; if start_addr > addr { @@ -640,21 +650,22 @@ fn symbolicate_frame( frame: &mut RawFrame, index: usize, ) -> Result, FrameStatus> { - let (object_info, symcache) = match caches.lookup_symcache(frame.instruction_addr.0) { - Some((_, info, Some(symcache))) => { - frame.package = info.raw.code_file.clone(); - (info, symcache) - } - Some((_, info, None)) => { - frame.package = info.raw.code_file.clone(); - if info.debug_status == ObjectFileStatus::Malformed { - return Err(FrameStatus::Malformed); - } else { - return Err(FrameStatus::Missing); + let (object_info, symcache) = + match caches.lookup_symcache(frame.instruction_addr.0, frame.in_module) { + Some((_, info, Some(symcache))) => { + frame.package = info.raw.code_file.clone(); + (info, symcache) } - } - None => return Err(FrameStatus::UnknownImage), - }; + Some((_, info, None)) => { + frame.package = info.raw.code_file.clone(); + if info.debug_status == ObjectFileStatus::Malformed { + return Err(FrameStatus::Malformed); + } else { + return Err(FrameStatus::Missing); + } + } + None => return Err(FrameStatus::UnknownImage), + }; log::trace!("Loading symcache"); let symcache = match symcache.parse() { @@ -681,12 +692,21 @@ fn symbolicate_frame( .ip_register_value(ip_register_value) .caller_address(); - let relative_addr = match caller_address.checked_sub(object_info.raw.image_addr.0) { - Some(x) => x, - None => { - log::warn!("Underflow when trying to subtract image start addr from caller address"); - metric!(counter("relative_addr.underflow") += 1); - return Err(FrameStatus::MissingSymbol); + // if `in_module` is provided the relative address is the caller address, + // otherwise we have to subtarct the image address to make it relative + // within the module. + let relative_addr = if frame.in_module.is_some() { + caller_address + } else { + match caller_address.checked_sub(object_info.raw.image_addr.0) { + Some(x) => x, + None => { + log::warn!( + "Underflow when trying to subtract image start addr from caller address" + ); + metric!(counter("relative_addr.underflow") += 1); + return Err(FrameStatus::MissingSymbol); + } } }; @@ -754,6 +774,7 @@ fn symbolicate_frame( instruction_addr: HexValue( object_info.raw.image_addr.0 + line_info.instruction_address(), ), + in_module: None, symbol: Some(line_info.symbol().to_string()), abs_path: if !abs_path.is_empty() { Some(abs_path) @@ -2043,7 +2064,7 @@ mod tests { let lookup = SymCacheLookup::from_iter(vec![info.clone()]); - let (a, b, c) = lookup.lookup_symcache(43).unwrap(); + let (a, b, c) = lookup.lookup_symcache(43, None).unwrap(); assert_eq!(a, 0); assert_eq!(b, &info); assert!(c.is_none()); diff --git a/src/sources.rs b/src/sources.rs index e2cdeacc2..a4b8b5cb4 100644 --- a/src/sources.rs +++ b/src/sources.rs @@ -481,6 +481,8 @@ pub enum FileType { ElfDebug, /// Linux/ELF code files ElfCode, + /// A WASM debug file + WasmDebug, /// Breakpad files (this is the reason we have a flat enum for what at first sight could've /// been two enums) Breakpad, @@ -501,6 +503,7 @@ impl FileType { Pe, MachCode, ElfCode, + WasmDebug, Breakpad, SourceBundle, ] @@ -519,6 +522,7 @@ impl FileType { ObjectType::Macho => &[FileType::MachDebug, FileType::MachCode, FileType::Breakpad], ObjectType::Pe => &[FileType::Pdb, FileType::Pe, FileType::Breakpad], ObjectType::Elf => &[FileType::ElfDebug, FileType::ElfCode, FileType::Breakpad], + ObjectType::Wasm => &[FileType::WasmDebug], _ => Self::all(), } } @@ -533,6 +537,7 @@ impl AsRef for FileType { FileType::MachCode => "mach_code", FileType::ElfDebug => "elf_debug", FileType::ElfCode => "elf_code", + FileType::WasmDebug => "wasm_debug", FileType::Breakpad => "breakpad", FileType::SourceBundle => "sourcebundle", } diff --git a/src/types.rs b/src/types.rs index 93bf4a23f..03f9efcc3 100644 --- a/src/types.rs +++ b/src/types.rs @@ -127,6 +127,11 @@ pub struct RawFrame { /// The absolute instruction address of this frame. pub instruction_addr: HexValue, + /// Switches the address to be interpreted to be within + /// a module. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub in_module: Option, + /// The path to the image this frame is located in. #[serde(default, skip_serializing_if = "Option::is_none")] pub package: Option, @@ -214,6 +219,12 @@ pub struct RawObjectInfo { pub debug_file: Option, /// Absolute address at which the image was mounted into virtual memory. + /// + /// We do allow the image addr to be skipped if it's zero. This is + /// because for instance systems like WASM do not actually require an + /// image to be mounted at a specific address. This makes sense mostly + /// when using `in_module` lookups. + #[serde(default)] pub image_addr: HexValue, /// Size of the image in virtual memory. @@ -228,6 +239,7 @@ pub enum ObjectType { Elf, Macho, Pe, + Wasm, Unknown, } @@ -239,6 +251,7 @@ impl FromStr for ObjectType { "elf" => ObjectType::Elf, "macho" => ObjectType::Macho, "pe" => ObjectType::Pe, + "wasm" => ObjectType::Wasm, _ => ObjectType::Unknown, }) } @@ -260,6 +273,7 @@ impl fmt::Display for ObjectType { ObjectType::Elf => write!(f, "elf"), ObjectType::Macho => write!(f, "macho"), ObjectType::Pe => write!(f, "pe"), + ObjectType::Wasm => write!(f, "wasm"), ObjectType::Unknown => write!(f, "unknown"), } } diff --git a/src/utils/paths.rs b/src/utils/paths.rs index f6e8088e0..c0e1c4bb8 100644 --- a/src/utils/paths.rs +++ b/src/utils/paths.rs @@ -99,6 +99,11 @@ fn get_pe_symstore_path(identifier: &ObjectId, ssqp_casing: bool) -> Option Option { + // wasm files never get a breakpad path + if identifier.object_type == ObjectType::Wasm { + return None; + } + let debug_file = identifier.debug_file_basename()?; let debug_id = identifier.debug_id.as_ref()?; @@ -123,8 +128,9 @@ fn get_native_paths(filetype: FileType, identifier: &ObjectId) -> Vec { match filetype { // ELF follows GDB "Build ID Method" conventions. // See: https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html + // We apply the same rule for WASM. FileType::ElfCode => get_gdb_path(identifier).into_iter().collect(), - FileType::ElfDebug => { + FileType::ElfDebug | FileType::WasmDebug => { if let Some(mut path) = get_gdb_path(identifier) { path.push_str(".debug"); vec![path] @@ -174,7 +180,7 @@ fn get_native_paths(filetype: FileType, identifier: &ObjectId) -> Vec { Some(path) => path, None => return vec![], }, - ObjectType::Elf => match get_gdb_path(identifier) { + ObjectType::Elf | ObjectType::Wasm => match get_gdb_path(identifier) { Some(path) => path, None => return vec![], }, @@ -248,6 +254,9 @@ fn get_symstore_path( // Microsoft SymbolServer does not specify Breakpad. FileType::Breakpad => None, + // Microsoft SymbolServer does not specify WASM. + FileType::WasmDebug => None, + // source bundles are available through an extension for PE/PDB only. FileType::SourceBundle => { let original_file_type = match identifier.object_type { @@ -293,6 +302,9 @@ fn get_debuginfod_path(filetype: FileType, identifier: &ObjectId) -> Option None, + // WASM is not supported + FileType::WasmDebug => None, + // Breakpad is not supported FileType::Breakpad => None, @@ -311,6 +323,7 @@ fn get_search_target_object_type(filetype: FileType, identifier: &ObjectId) -> O FileType::Pe | FileType::Pdb => ObjectType::Pe, FileType::MachCode | FileType::MachDebug => ObjectType::Macho, FileType::ElfCode | FileType::ElfDebug => ObjectType::Elf, + FileType::WasmDebug => ObjectType::Wasm, FileType::SourceBundle | FileType::Breakpad => identifier.object_type, } } @@ -319,7 +332,9 @@ fn get_unified_path(filetype: FileType, identifier: &ObjectId) -> Option // determine the suffix and object type let suffix = match filetype { FileType::ElfCode | FileType::MachCode | FileType::Pe => "executable", - FileType::ElfDebug | FileType::MachDebug | FileType::Pdb => "debuginfo", + FileType::ElfDebug | FileType::MachDebug | FileType::Pdb | FileType::WasmDebug => { + "debuginfo" + } FileType::Breakpad => "breakpad", FileType::SourceBundle => "sourcebundle", }; @@ -332,8 +347,9 @@ fn get_unified_path(filetype: FileType, identifier: &ObjectId) -> Option // we do not want to encode. ObjectType::Pe => Cow::Owned(identifier.debug_id?.breakpad().to_string().to_lowercase()), // On mach we can always determine the code ID from the debug ID if the - // code ID is unavailable. - ObjectType::Macho => { + // code ID is unavailable. We apply the same rule to WASM files as we + // suggest Uuids to be used as build ids. + ObjectType::Macho | ObjectType::Wasm => { if identifier.code_id.is_none() { Cow::Owned(identifier.debug_id?.uuid().to_simple_ref().to_string()) } else { @@ -533,6 +549,13 @@ mod tests { debug_file: Some("/lib/x86_64-linux-gnu/libm-2.23.so".into()), object_type: ObjectType::Elf, }; + static ref WASM_OBJECT_ID: ObjectId = ObjectId { + code_id: Some("67e9247c814e392ba027dbde6748fcbf".parse().unwrap()), + code_file: None, + debug_id: Some("67e9247c-814e-392b-a027-dbde6748fcbf".parse().unwrap()), + debug_file: Some("file://foo.invalid/demo.wasm".into()), + object_type: ObjectType::Wasm, + }; } fn pattern(x: &str) -> Glob { @@ -558,6 +581,8 @@ mod tests { crash/67E9247C814E392BA027DBDE6748FCBF0/crash.src.zip 67E9/247C/814E/392B/A027/DBDE6748FCBF.src.zip "###); + path_test!(FileType::WasmDebug, WASM_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf.debug"); + path_test!(FileType::SourceBundle, WASM_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf.src.zip"); path_test!(FileType::ElfCode, ELF_OBJECT_ID, @"df/b85de42daffd09640c8fe377d572de3e168920"); path_test!(FileType::ElfDebug, ELF_OBJECT_ID, @"df/b85de42daffd09640c8fe377d572de3e168920.debug"); path_test!(FileType::Breakpad, ELF_OBJECT_ID, @"libm-2.23.so/E45DB8DFAF2D09FD640C8FE377D572DE0/libm-2.23.so.sym"); @@ -581,12 +606,14 @@ mod tests { path_test!(FileType::SourceBundle, PE_OBJECT_ID, @"32/49d99d0c4049318610f4e4fb0b69361/sourcebundle"); path_test!(FileType::MachCode, MACHO_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf/executable"); path_test!(FileType::MachDebug, MACHO_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf/debuginfo"); + path_test!(FileType::WasmDebug, WASM_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf/debuginfo"); path_test!(FileType::Breakpad, MACHO_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf/breakpad"); path_test!(FileType::SourceBundle, MACHO_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf/sourcebundle"); path_test!(FileType::ElfCode, ELF_OBJECT_ID, @"df/b85de42daffd09640c8fe377d572de3e168920/executable"); path_test!(FileType::ElfDebug, ELF_OBJECT_ID, @"df/b85de42daffd09640c8fe377d572de3e168920/debuginfo"); path_test!(FileType::Breakpad, ELF_OBJECT_ID, @"df/b85de42daffd09640c8fe377d572de3e168920/breakpad"); path_test!(FileType::SourceBundle, ELF_OBJECT_ID, @"df/b85de42daffd09640c8fe377d572de3e168920/sourcebundle"); + path_test!(FileType::SourceBundle, WASM_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf/sourcebundle"); } #[test] From cb018fb8f35ef33087c3983212662c06a8ae9af2 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 24 Nov 2020 23:44:57 +0100 Subject: [PATCH 02/21] test: Added test for WASM --- .../bd/a18fd85d4a4eb893022d6bfad846b1.debug | Bin 0 -> 4293 bytes tests/integration/conftest.py | 11 +++ tests/integration/test_wasm.py | 90 ++++++++++++++++++ 3 files changed, 101 insertions(+) create mode 100755 tests/fixtures/symbols/bd/a18fd85d4a4eb893022d6bfad846b1.debug create mode 100644 tests/integration/test_wasm.py diff --git a/tests/fixtures/symbols/bd/a18fd85d4a4eb893022d6bfad846b1.debug b/tests/fixtures/symbols/bd/a18fd85d4a4eb893022d6bfad846b1.debug new file mode 100755 index 0000000000000000000000000000000000000000..55739be56d75e3874b274e8a2e4e67e1a4e4381c GIT binary patch literal 4293 zcmcIoU2Ggz6+U-%*538*jMv$EW2*s{Bv^Kd{}QL(Y${W?c3_e7BPR}OBWs!69j`~; zompmfow!1Zd8jA|395((DrkfhA)!d+M?3@pq7nsBpr`^=1qmu1K$TD`phLuUM=YO`~Yqm54|bi|eM* zD3%P*j3|8x5vSBP9_mlfpZXFL6|>Z;7AU%Y=C;d0AB1G z8cI?8Sym8z&oQr8c?F&FDmSlj^E#RYtKY#ZMDn-jD0!O)$v4p|$-khDke0<!^#opZh_pYTkE!VeNYgT)^aYSvCh;ooAUe56QE;(G|G}7&f(MZD z$ku9B2c)q?RR=!?$p-kCgQN7fkC`g9Wu& z^I(F#y8$vd@|kV0zKyX|z1dcMaknam@6kkdc@lX2NX|FRGVw_}XV_M`fZK=oatO>C z%*^N8GEQ)N30c;>B%?c-J+>|Wyl%J>#^-BR)#hXp@7S{GwQ5ZXnP(eK z%dsEpO8mWPU7aI7M`85DM>1G962*WRc#(n3N!$wk-f#7D^5S^XX(1dwZlU9g(-NmU zGnUT>JH%zbcaRd;*X+^znIFkM@ED9`$LE4G<;t|=^L~0B8y!2+dDs?R*Ky~FOF9hL zBR=9wPTAljN*Hb(J1W7td;fM+xXR_(ku^~YKRRlbZ7zeQ(m}NN=(($F++i-hB!uUd ziE379!u5De6ve!R6CCY}{h()2QJ@SSGb1SxjrSdlD(gOXL<_3UHSDVCvDqQKteW6K z_!kwa&;ftD@FPR=NuqBl;fzM9m<8zp7P9~aX2aGJm; z1uOZik3geW2$sY?NQLx4Z5B(5ABO9>L;_?VlZnI}$b<^L`Y;cl6aB!aQHTzJ(9#Gf zU?TY(6%=wX=|3h0;X0+kwPej;zJXzpfoxw|dX}!z9agOJ&@l2cI0-9WX{KKi$9jnN z56W1FqTq1WhGV6cRjU-O%8$PO#oMbNKXdDvGQRQd?Ni^O*JFLbwxFiVHEdyaY3btW z?5NxFn&s@K>3TSM+3AUy+{9FNbk;P=xg7GIJvw!CYJ7TneEQL`F~aeg3{KDFiU*s? zy4Cbr>(+)dxnORZHK)PYFnJlD=HuoSqu!{Qlb*#L1>F-Xg{6gJVS%BsSe##8o`0%% zzVP&7@xoK*7K=qHK7Hm|)ElY}pl(P)XfC0OZ#^ zBwIDdsE`3PZ#YteLbTs(8YQgK2l!4XmDpc5JNI zj%wkhf&*d#`f=#WPOW{{Jm7w5e<*Y-wE6Ta(eWK-8#v;(fI&Xg^LN-!F$!w&21wND zb#z>^-$aj^Ekl4+D(LPC{R#BAF8SA3P-Uwi zQS2XvZU`mxV+?$lI1Ool2;kv1Re25K1NiaZ^&f(`=f8l{@rLVET4kW05@R53p|FLz Z3>={(M5t%atc;hoL{wty`}V}t{{nR#palQ` literal 0 HcmV?d00001 diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 95330f365..6d6899217 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -201,6 +201,17 @@ def _handle_path(path, start_response): print(f"status code: {r.status_code}") start_response(f"{r.status_code} BOGUS", list(r.headers.items())) return [r.content] + elif path.startswith("/symbols/"): + print(f"got requested: {path}") + path = path[len("/symbols/") :] + try: + with open(os.path.join(os.path.dirname(__file__), "..", "fixtures", "symbols", path), "rb") as f: + d = f.read() + start_response("200 OK", [("Content-Length", str(len(d)))]) + return [d] + except IOError: + start_response("404 NOT FOUND", []) + return [b""] elif path.startswith("/respond_statuscode/"): statuscode = int(path.split("/")[2]) start_response(f"{statuscode} BOGUS", []) diff --git a/tests/integration/test_wasm.py b/tests/integration/test_wasm.py new file mode 100644 index 000000000..e8cebce21 --- /dev/null +++ b/tests/integration/test_wasm.py @@ -0,0 +1,90 @@ +import pytest + + +WASM_DATA = { + "stacktraces": [ + { + "registers": {}, + "frames": [ + { + "instruction_addr": "0x8c", + "in_module": 0, + } + ], + }, + ], + "modules": [ + { + "type": "wasm", + "debug_id": "bda18fd8-5d4a-4eb8-9302-2d6bfad846b1", + "code_id": "bda18fd85d4a4eb893022d6bfad846b1", + "debug_file": "file://foo.invalid/demo.wasm", + } + ], +} + +SUCCESS_WASM = { + "modules": [ + { + "arch": "wasm", + "code_id": "bda18fd85d4a4eb893022d6bfad846b1", + "debug_file": "file://foo.invalid/demo.wasm", + "debug_id": "bda18fd8-5d4a-4eb8-9302-2d6bfad846b1", + "debug_status": "found", + "features": { + "has_debug_info": True, + "has_sources": False, + "has_symbols": True, + "has_unwind_info": False + }, + "image_addr": "0x0", + "type": "wasm" + } + ], + "stacktraces": [ + { + "frames": [ + { + "abs_path": "/Users/mitsuhiko/Development/wasm-example/simple/src/lib.rs", + "filename": "src/lib.rs", + "function": "internal_func", + "instruction_addr": "0x8c", + "lang": "rust", + "lineno": 4, + "original_index": 0, + "status": "symbolicated", + "sym_addr": "0x8b", + "symbol": "internal_func" + } + ] + } + ], + "status": "completed" +} + + +def test_basic_wasm(symbolicator, hitcounter): + scope = "myscope" + + import json + input = dict( + **WASM_DATA, + sources=[ + { + "type": "http", + "id": "stuff", + "layout": {"type": "native"}, + "filters": {"filetypes": ["wasm_debug"]}, + "url": f"{hitcounter.url}/symbols/", + "is_public": False, + } + ] + ) + + service = symbolicator() + service.wait_healthcheck() + + response = service.post(f"/symbolicate?scope={scope}", json=input) + response.raise_for_status() + + assert response.json() == SUCCESS_WASM From 573e41a84e21897c0c862565f2c49673fc31b405 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Tue, 24 Nov 2020 23:47:15 +0100 Subject: [PATCH 03/21] ref: point symbolic back to fix/demangle-fixes --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 4b11af2b8..c30cb0d6c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,7 +49,7 @@ serde = { version = "1.0.101", features = ["derive", "rc"] } serde_json = "1.0.45" serde_yaml = "0.8.11" structopt = "0.3.2" -symbolic = { git = "https://github.com/getsentry/symbolic", branch = "master", features = ["common-serde", "demangle", "minidump-serde", "symcache"] } +symbolic = { git = "https://github.com/getsentry/symbolic", branch = "fix/demangle-fixes", features = ["common-serde", "demangle", "minidump-serde", "symcache"] } tempfile = "3.1.0" thiserror = "1.0.20" tokio = "0.1.22" From 904ad2f2fbe9850fd49f7fb9879e4cc62b609d84 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 25 Nov 2020 00:04:10 +0100 Subject: [PATCH 04/21] ref: reformat --- tests/integration/conftest.py | 7 ++++++- tests/integration/test_wasm.py | 11 ++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 6d6899217..5f1a60419 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -205,7 +205,12 @@ def _handle_path(path, start_response): print(f"got requested: {path}") path = path[len("/symbols/") :] try: - with open(os.path.join(os.path.dirname(__file__), "..", "fixtures", "symbols", path), "rb") as f: + with open( + os.path.join( + os.path.dirname(__file__), "..", "fixtures", "symbols", path + ), + "rb", + ) as f: d = f.read() start_response("200 OK", [("Content-Length", str(len(d)))]) return [d] diff --git a/tests/integration/test_wasm.py b/tests/integration/test_wasm.py index e8cebce21..b26e830eb 100644 --- a/tests/integration/test_wasm.py +++ b/tests/integration/test_wasm.py @@ -35,10 +35,10 @@ "has_debug_info": True, "has_sources": False, "has_symbols": True, - "has_unwind_info": False + "has_unwind_info": False, }, "image_addr": "0x0", - "type": "wasm" + "type": "wasm", } ], "stacktraces": [ @@ -54,12 +54,12 @@ "original_index": 0, "status": "symbolicated", "sym_addr": "0x8b", - "symbol": "internal_func" + "symbol": "internal_func", } ] } ], - "status": "completed" + "status": "completed", } @@ -67,6 +67,7 @@ def test_basic_wasm(symbolicator, hitcounter): scope = "myscope" import json + input = dict( **WASM_DATA, sources=[ @@ -78,7 +79,7 @@ def test_basic_wasm(symbolicator, hitcounter): "url": f"{hitcounter.url}/symbols/", "is_public": False, } - ] + ], ) service = symbolicator() From 1246e4a0a1f50bb0b448f574da47e170eb5b47f7 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 25 Nov 2020 00:09:44 +0100 Subject: [PATCH 05/21] fix: remove dead imports --- tests/integration/test_wasm.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/integration/test_wasm.py b/tests/integration/test_wasm.py index b26e830eb..53380a4f4 100644 --- a/tests/integration/test_wasm.py +++ b/tests/integration/test_wasm.py @@ -1,6 +1,3 @@ -import pytest - - WASM_DATA = { "stacktraces": [ { @@ -66,8 +63,6 @@ def test_basic_wasm(symbolicator, hitcounter): scope = "myscope" - import json - input = dict( **WASM_DATA, sources=[ From fe89021a63368d8aad0eb820d9fea15177d9d578 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 25 Nov 2020 00:26:18 +0100 Subject: [PATCH 06/21] ref: update lockfile --- Cargo.lock | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 149a4fe95..0903e6dd5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -576,8 +576,7 @@ checksum = "b3a71ab494c0b5b860bdc8407ae08978052417070c2ced38573a9157ad75b8ac" [[package]] name = "cpp_demangle" version = "0.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6656dbc7c586cf2965887c3d066d9b983b20400bc2d52ed4cc5e33d0ceb2c602" +source = "git+https://github.com/Swatinem/cpp_demangle?branch=sentry-patches#479281f0c8c2630c5a5a1aeec2ea74ce233d8cc6" dependencies = [ "cfg-if 1.0.0", "glob", @@ -1833,8 +1832,7 @@ dependencies = [ [[package]] name = "msvc-demangler" version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6584cf122f02fc0396420a116cd395a9a776ec4347dffe1c5119c3fcc917c060" +source = "git+https://github.com/Swatinem/msvc-demangler-rust?branch=sentry-patches#075432259fe0eaac2f3fc225550a4db479bf3c81" dependencies = [ "bitflags", ] @@ -3135,7 +3133,7 @@ dependencies = [ [[package]] name = "symbolic" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=master#5d007d7c6605df7fa683749ef0f1b9380571fa05" +source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#2ad654f6921509e3567575394e831931ee267158" dependencies = [ "symbolic-common", "symbolic-debuginfo", @@ -3147,7 +3145,7 @@ dependencies = [ [[package]] name = "symbolic-common" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=master#5d007d7c6605df7fa683749ef0f1b9380571fa05" +source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#2ad654f6921509e3567575394e831931ee267158" dependencies = [ "debugid", "memmap", @@ -3159,7 +3157,7 @@ dependencies = [ [[package]] name = "symbolic-debuginfo" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=master#5d007d7c6605df7fa683749ef0f1b9380571fa05" +source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#2ad654f6921509e3567575394e831931ee267158" dependencies = [ "dmsort", "fallible-iterator", @@ -3186,7 +3184,7 @@ dependencies = [ [[package]] name = "symbolic-demangle" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=master#5d007d7c6605df7fa683749ef0f1b9380571fa05" +source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#2ad654f6921509e3567575394e831931ee267158" dependencies = [ "cc", "cpp_demangle", @@ -3198,7 +3196,7 @@ dependencies = [ [[package]] name = "symbolic-minidump" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=master#5d007d7c6605df7fa683749ef0f1b9380571fa05" +source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#2ad654f6921509e3567575394e831931ee267158" dependencies = [ "cc", "lazy_static", @@ -3212,7 +3210,7 @@ dependencies = [ [[package]] name = "symbolic-symcache" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=master#5d007d7c6605df7fa683749ef0f1b9380571fa05" +source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#2ad654f6921509e3567575394e831931ee267158" dependencies = [ "dmsort", "fnv", From a07251336b450a55b56de45e7c87f042a1f2c4df Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 25 Nov 2020 00:51:05 +0100 Subject: [PATCH 07/21] doc: Document WASM lookups for symbol server --- docs/advanced/symbol-server-compatibility.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/advanced/symbol-server-compatibility.md b/docs/advanced/symbol-server-compatibility.md index 3415e4efb..0c0f4cbf1 100644 --- a/docs/advanced/symbol-server-compatibility.md +++ b/docs/advanced/symbol-server-compatibility.md @@ -75,6 +75,11 @@ Specifically, the code and debug identifiers are defined as follows: bidirectionally from the UUID. - **Debug ID:** The same UUID, amended by a `0` for age. +**WASM**: + +- **Code ID:** The bytes as specified in the `build_id` custom section. +- **Debug ID:** The same as code ID but truncated to 16 bytes. + **PE** / **PDB**: - **Code ID:** The hex value of the `time_date_stamp` in the COFF header @@ -266,6 +271,7 @@ The build-id hex representation is always provided in **lowercase**. - **ELF** (binary, potentially stripped) - **ELF** (debug info) +- **WASM** (debug info) Symbol bundles are supported by adding a `.src.zip` prefix to the ELF: @@ -297,7 +303,7 @@ The following layout types support this lookup: If you have no requirements to be compatible with another system you can also use the "unified" directory layout structure. This has the advantage that it's unified across all platforms and thus easier to manage. It can store breakpad -files, PDBs, PEs and everything else. The `symsorter` tool in the symbolicator +files, PDBs, PEs and everything else. The `symsorter` tool in the symbolicator repository can automatically sort debug symbols into this format and also automatically create source bundles. @@ -309,6 +315,7 @@ The debug id is in all cases lowercase in hex format and computed as follows: - **PDB**: `` (age in hex, not padded) - **ELF**: `` - **MachO**: `` +- **WASM**: `` The path format is then as follows: From a57aee8616851fbdd46a5740c3d8cb2c3e313c53 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 25 Nov 2020 10:26:30 +0100 Subject: [PATCH 08/21] meta: changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6efcff8cc..69cdbbe0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Add `processing_pool_size` configuration option that allows to set the size of the processing pool. ([#273](https://github.com/getsentry/symbolicator/pull/273)) - Use a dedicated `tmp` sub-directory in the cache directory to write temporary files into. ([#265](https://github.com/getsentry/symbolicator/pull/265)) - Use STATSD_SERVER environment variable to set metrics.statsd configuration option ([#182](https://github.com/getsentry/symbolicator/pull/182)) +- Added WASM support. ([#301](https://github.com/getsentry/symbolicator/pull/301)) ### Bug Fixes From 4236e4722f7e0a72a3cf2f8d4552321198ad1040 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 25 Nov 2020 14:10:07 +0100 Subject: [PATCH 09/21] Update src/actors/symbolication.rs Co-authored-by: Arpad Borsos --- src/actors/symbolication.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/actors/symbolication.rs b/src/actors/symbolication.rs index 4af25de40..47536b0d6 100644 --- a/src/actors/symbolication.rs +++ b/src/actors/symbolication.rs @@ -693,7 +693,7 @@ fn symbolicate_frame( .caller_address(); // if `in_module` is provided the relative address is the caller address, - // otherwise we have to subtarct the image address to make it relative + // otherwise we have to subtract the image address to make it relative // within the module. let relative_addr = if frame.in_module.is_some() { caller_address From 723df61140e9a9e466e4d81ffe9a3632dbb2b59b Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 25 Nov 2020 14:24:09 +0100 Subject: [PATCH 10/21] test: Fix bad test case and bump symbolic --- Cargo.lock | 12 ++++++------ tests/integration/test_wasm.py | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0903e6dd5..f38cd1c8b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3133,7 +3133,7 @@ dependencies = [ [[package]] name = "symbolic" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#2ad654f6921509e3567575394e831931ee267158" +source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#749a39e7f34b7c22339c0f9d44a0b57502a05655" dependencies = [ "symbolic-common", "symbolic-debuginfo", @@ -3145,7 +3145,7 @@ dependencies = [ [[package]] name = "symbolic-common" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#2ad654f6921509e3567575394e831931ee267158" +source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#749a39e7f34b7c22339c0f9d44a0b57502a05655" dependencies = [ "debugid", "memmap", @@ -3157,7 +3157,7 @@ dependencies = [ [[package]] name = "symbolic-debuginfo" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#2ad654f6921509e3567575394e831931ee267158" +source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#749a39e7f34b7c22339c0f9d44a0b57502a05655" dependencies = [ "dmsort", "fallible-iterator", @@ -3184,7 +3184,7 @@ dependencies = [ [[package]] name = "symbolic-demangle" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#2ad654f6921509e3567575394e831931ee267158" +source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#749a39e7f34b7c22339c0f9d44a0b57502a05655" dependencies = [ "cc", "cpp_demangle", @@ -3196,7 +3196,7 @@ dependencies = [ [[package]] name = "symbolic-minidump" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#2ad654f6921509e3567575394e831931ee267158" +source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#749a39e7f34b7c22339c0f9d44a0b57502a05655" dependencies = [ "cc", "lazy_static", @@ -3210,7 +3210,7 @@ dependencies = [ [[package]] name = "symbolic-symcache" version = "7.5.0" -source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#2ad654f6921509e3567575394e831931ee267158" +source = "git+https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#749a39e7f34b7c22339c0f9d44a0b57502a05655" dependencies = [ "dmsort", "fnv", diff --git a/tests/integration/test_wasm.py b/tests/integration/test_wasm.py index 53380a4f4..d6937e456 100644 --- a/tests/integration/test_wasm.py +++ b/tests/integration/test_wasm.py @@ -47,7 +47,7 @@ "function": "internal_func", "instruction_addr": "0x8c", "lang": "rust", - "lineno": 4, + "lineno": 19, "original_index": 0, "status": "symbolicated", "sym_addr": "0x8b", From 72b58896378ab63973496e8345df46a05890b3d0 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 25 Nov 2020 14:41:11 +0100 Subject: [PATCH 11/21] nits --- src/types.rs | 3 ++- tests/integration/conftest.py | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/types.rs b/src/types.rs index 03f9efcc3..8d4d75f6a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -128,7 +128,8 @@ pub struct RawFrame { pub instruction_addr: HexValue, /// Switches the address to be interpreted to be within - /// a module. + /// a module of the specified index. For instance sending `0` here + /// would use the first module. #[serde(default, skip_serializing_if = "Option::is_none")] pub in_module: Option, diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 5f1a60419..59a6c99bf 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -205,10 +205,11 @@ def _handle_path(path, start_response): print(f"got requested: {path}") path = path[len("/symbols/") :] try: + filename = os.path.join( + os.path.dirname(__file__), "..", "fixtures", "symbols", path + ) with open( - os.path.join( - os.path.dirname(__file__), "..", "fixtures", "symbols", path - ), + filename, "rb", ) as f: d = f.read() From 9d718bce39cff1075c66819f3d9bbbb470945922 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 25 Nov 2020 18:03:24 +0100 Subject: [PATCH 12/21] feat: add WasmCode to symbolicator --- src/sources.rs | 6 +++++- src/utils/paths.rs | 12 +++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/sources.rs b/src/sources.rs index a4b8b5cb4..f222ec1e1 100644 --- a/src/sources.rs +++ b/src/sources.rs @@ -483,6 +483,8 @@ pub enum FileType { ElfCode, /// A WASM debug file WasmDebug, + /// A WASM code file + WasmCode, /// Breakpad files (this is the reason we have a flat enum for what at first sight could've /// been two enums) Breakpad, @@ -503,6 +505,7 @@ impl FileType { Pe, MachCode, ElfCode, + WasmCode, WasmDebug, Breakpad, SourceBundle, @@ -522,7 +525,7 @@ impl FileType { ObjectType::Macho => &[FileType::MachDebug, FileType::MachCode, FileType::Breakpad], ObjectType::Pe => &[FileType::Pdb, FileType::Pe, FileType::Breakpad], ObjectType::Elf => &[FileType::ElfDebug, FileType::ElfCode, FileType::Breakpad], - ObjectType::Wasm => &[FileType::WasmDebug], + ObjectType::Wasm => &[FileType::WasmCode, FileType::WasmDebug], _ => Self::all(), } } @@ -538,6 +541,7 @@ impl AsRef for FileType { FileType::ElfDebug => "elf_debug", FileType::ElfCode => "elf_code", FileType::WasmDebug => "wasm_debug", + FileType::WasmCode => "wasm_code", FileType::Breakpad => "breakpad", FileType::SourceBundle => "sourcebundle", } diff --git a/src/utils/paths.rs b/src/utils/paths.rs index c0e1c4bb8..48e406d2e 100644 --- a/src/utils/paths.rs +++ b/src/utils/paths.rs @@ -129,7 +129,7 @@ fn get_native_paths(filetype: FileType, identifier: &ObjectId) -> Vec { // ELF follows GDB "Build ID Method" conventions. // See: https://sourceware.org/gdb/onlinedocs/gdb/Separate-Debug-Files.html // We apply the same rule for WASM. - FileType::ElfCode => get_gdb_path(identifier).into_iter().collect(), + FileType::ElfCode | FileType::WasmCode => get_gdb_path(identifier).into_iter().collect(), FileType::ElfDebug | FileType::WasmDebug => { if let Some(mut path) = get_gdb_path(identifier) { path.push_str(".debug"); @@ -255,7 +255,7 @@ fn get_symstore_path( FileType::Breakpad => None, // Microsoft SymbolServer does not specify WASM. - FileType::WasmDebug => None, + FileType::WasmDebug | FileType::WasmCode => None, // source bundles are available through an extension for PE/PDB only. FileType::SourceBundle => { @@ -303,7 +303,7 @@ fn get_debuginfod_path(filetype: FileType, identifier: &ObjectId) -> Option None, // WASM is not supported - FileType::WasmDebug => None, + FileType::WasmDebug | FileType::WasmCode => None, // Breakpad is not supported FileType::Breakpad => None, @@ -323,7 +323,7 @@ fn get_search_target_object_type(filetype: FileType, identifier: &ObjectId) -> O FileType::Pe | FileType::Pdb => ObjectType::Pe, FileType::MachCode | FileType::MachDebug => ObjectType::Macho, FileType::ElfCode | FileType::ElfDebug => ObjectType::Elf, - FileType::WasmDebug => ObjectType::Wasm, + FileType::WasmDebug | FileType::WasmCode => ObjectType::Wasm, FileType::SourceBundle | FileType::Breakpad => identifier.object_type, } } @@ -331,7 +331,7 @@ fn get_search_target_object_type(filetype: FileType, identifier: &ObjectId) -> O fn get_unified_path(filetype: FileType, identifier: &ObjectId) -> Option { // determine the suffix and object type let suffix = match filetype { - FileType::ElfCode | FileType::MachCode | FileType::Pe => "executable", + FileType::ElfCode | FileType::MachCode | FileType::Pe | FileType::WasmCode => "executable", FileType::ElfDebug | FileType::MachDebug | FileType::Pdb | FileType::WasmDebug => { "debuginfo" } @@ -582,6 +582,7 @@ mod tests { 67E9/247C/814E/392B/A027/DBDE6748FCBF.src.zip "###); path_test!(FileType::WasmDebug, WASM_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf.debug"); + path_test!(FileType::WasmCode, WASM_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf"); path_test!(FileType::SourceBundle, WASM_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf.src.zip"); path_test!(FileType::ElfCode, ELF_OBJECT_ID, @"df/b85de42daffd09640c8fe377d572de3e168920"); path_test!(FileType::ElfDebug, ELF_OBJECT_ID, @"df/b85de42daffd09640c8fe377d572de3e168920.debug"); @@ -607,6 +608,7 @@ mod tests { path_test!(FileType::MachCode, MACHO_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf/executable"); path_test!(FileType::MachDebug, MACHO_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf/debuginfo"); path_test!(FileType::WasmDebug, WASM_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf/debuginfo"); + path_test!(FileType::WasmCode, WASM_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf/executable"); path_test!(FileType::Breakpad, MACHO_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf/breakpad"); path_test!(FileType::SourceBundle, MACHO_OBJECT_ID, @"67/e9247c814e392ba027dbde6748fcbf/sourcebundle"); path_test!(FileType::ElfCode, ELF_OBJECT_ID, @"df/b85de42daffd09640c8fe377d572de3e168920/executable"); From 5723e2b3096614fd1e48a5d725892e4816870004 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Wed, 25 Nov 2020 23:30:37 +0100 Subject: [PATCH 13/21] ref: in_module -> addr_base_module --- src/actors/symbolication.rs | 26 ++++++++++++++++++-------- src/types.rs | 19 ++++++++++++++----- tests/integration/test_wasm.py | 3 ++- 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/src/actors/symbolication.rs b/src/actors/symbolication.rs index 47536b0d6..5f46176ed 100644 --- a/src/actors/symbolication.rs +++ b/src/actors/symbolication.rs @@ -551,7 +551,7 @@ impl SymCacheLookup { for stacktrace in stacktraces { for frame in stacktrace.frames { if let Some((i, ..)) = - self.lookup_symcache(frame.instruction_addr.0, frame.in_module) + self.lookup_symcache(frame.instruction_addr.0, frame.addr_base_module) { referenced_objects.insert(i); } @@ -610,12 +610,12 @@ impl SymCacheLookup { fn lookup_symcache( &self, addr: u64, - in_module: Option, + addr_base_module: Option, ) -> Option<(usize, &CompleteObjectInfo, Option<&SymCacheFile>)> { for (i, (info, cache)) in self.inner.iter().enumerate() { - // if `in_module` is defined we pick up the module directly + // if `addr_base_module` is defined we pick up the module directly // by the index. - if Some(i) == in_module { + if Some(i) == addr_base_module { return Some((i, info, cache.as_ref().map(|x| &**x))); } @@ -651,7 +651,7 @@ fn symbolicate_frame( index: usize, ) -> Result, FrameStatus> { let (object_info, symcache) = - match caches.lookup_symcache(frame.instruction_addr.0, frame.in_module) { + match caches.lookup_symcache(frame.instruction_addr.0, frame.addr_base_module) { Some((_, info, Some(symcache))) => { frame.package = info.raw.code_file.clone(); (info, symcache) @@ -692,10 +692,10 @@ fn symbolicate_frame( .ip_register_value(ip_register_value) .caller_address(); - // if `in_module` is provided the relative address is the caller address, + // if `addr_base_module` is provided the relative address is the caller address, // otherwise we have to subtract the image address to make it relative // within the module. - let relative_addr = if frame.in_module.is_some() { + let relative_addr = if frame.addr_base_module.is_some() { caller_address } else { match caller_address.checked_sub(object_info.raw.image_addr.0) { @@ -771,10 +771,19 @@ fn symbolicate_frame( original_index: Some(index), raw: RawFrame { package: object_info.raw.code_file.clone(), + // for the symbolicated frame we generally switch to absolute reporting of + // addresses. This is not done for images mounted at `0x0`. This is done + // because for instance WASM does not have a unified address space and so + // it's not possible for us to absolutize addresses. + addr_base_module: if object_info.raw.image_addr.0 > 0 { + None + } else { + frame.addr_base_module + }, + // make absolute. This is a noop for images mounted at 0x0 instruction_addr: HexValue( object_info.raw.image_addr.0 + line_info.instruction_address(), ), - in_module: None, symbol: Some(line_info.symbol().to_string()), abs_path: if !abs_path.is_empty() { Some(abs_path) @@ -794,6 +803,7 @@ fn symbolicate_frame( pre_context: vec![], context_line: None, post_context: vec![], + // make absolute. This is a noop for images mounted at 0x0 sym_addr: Some(HexValue( object_info.raw.image_addr.0 + line_info.function_address(), )), diff --git a/src/types.rs b/src/types.rs index 8d4d75f6a..6db971b1e 100644 --- a/src/types.rs +++ b/src/types.rs @@ -124,14 +124,21 @@ fn is_default_frame_trust(trust: &FrameTrust) -> bool { /// An unsymbolicated frame from a symbolication request. #[derive(Debug, Default, Clone, Deserialize, Serialize)] pub struct RawFrame { - /// The absolute instruction address of this frame. - pub instruction_addr: HexValue, - /// Switches the address to be interpreted to be within /// a module of the specified index. For instance sending `0` here /// would use the first module. + /// + /// This applies to `instruction_addr` and `sym_addr`. When the + /// `addr_base_module` is set then both `instruction_addr` and `sym_addr` + /// are relative within that module. If it's not set or `null` then + /// the adresses are absolute within a unified address space. #[serde(default, skip_serializing_if = "Option::is_none")] - pub in_module: Option, + pub addr_base_module: Option, + + /// The instruction address of this frame. + /// + /// See `addr_base_module` for the exact behavior of addresses. + pub instruction_addr: HexValue, /// The path to the image this frame is located in. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -146,6 +153,8 @@ pub struct RawFrame { pub symbol: Option, /// Start address of the function this frame is located in (lower or equal to instruction_addr). + /// + /// See `addr_base_module` for the exact behavior of addresses. #[serde(skip_serializing_if = "Option::is_none")] pub sym_addr: Option, @@ -224,7 +233,7 @@ pub struct RawObjectInfo { /// We do allow the image addr to be skipped if it's zero. This is /// because for instance systems like WASM do not actually require an /// image to be mounted at a specific address. This makes sense mostly - /// when using `in_module` lookups. + /// when using `addr_base_module`. #[serde(default)] pub image_addr: HexValue, diff --git a/tests/integration/test_wasm.py b/tests/integration/test_wasm.py index d6937e456..9fd3d47bf 100644 --- a/tests/integration/test_wasm.py +++ b/tests/integration/test_wasm.py @@ -5,7 +5,7 @@ "frames": [ { "instruction_addr": "0x8c", - "in_module": 0, + "addr_base_module": 0, } ], }, @@ -46,6 +46,7 @@ "filename": "src/lib.rs", "function": "internal_func", "instruction_addr": "0x8c", + "addr_base_module": 0, "lang": "rust", "lineno": 19, "original_index": 0, From edeb583e20f26db48533a01aaa9a72a17cd907e8 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 26 Nov 2020 00:50:03 +0100 Subject: [PATCH 14/21] doc: Update docs on wasm changes --- docs/api/response.md | 11 ++++++++++- docs/api/symbolication.md | 25 ++++++++++++++----------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/docs/api/response.md b/docs/api/response.md index 857de9db6..4744c073d 100644 --- a/docs/api/response.md +++ b/docs/api/response.md @@ -34,8 +34,8 @@ symbolication succeeds within a configured timeframe (around 20 seconds): // Frame information "instruction_addr": "0xfeedbeef", // actual address of the frame + "addr_base_module": 0, // when not null/set relative mode "sym_addr": "0xfeed0000", // start address of the function - "line_addr": "0xfeedbe00", // start address of the line "package": "/path/to/module.so", // path to the module's code file "symbol": "__1cGmemset6FpviI_0_", // original mangled function name "function": "memset", // demangled short version of symbol @@ -73,6 +73,15 @@ occurred during symbolication, such as missing symbol files or unresolvable addresses within symbols are reported as values for `status` in both modules and frames. +## Note on Addresses + +Addresses (`instruction_addr` and `sym_addr`) can come in two versions. They +can be absolute or relative. Symbolicator will always try to make addresses +absolute but in some cases this cannot be done. For instance WASM modules do +not have absolute addresses in which case the addresses stay relative. This is +identified by the presence of the `addr_base_module` property. If this exists +and is not `null` it's the index of the module containing the address. + ## Backoff Response If symbolication takes longer than the threshold `timeout`, the server instead diff --git a/docs/api/symbolication.md b/docs/api/symbolication.md index d0140fb6f..dbd9b39ff 100644 --- a/docs/api/symbolication.md +++ b/docs/api/symbolication.md @@ -22,7 +22,8 @@ Content-Type: application/json { "frames": [ { - "instruction_addr": "0xfeedbeef" + "instruction_addr": "0xfeedbeef", + "addr_base_module": 0 }, ... ], @@ -61,17 +62,19 @@ as well as external sources to pull symbols from: - `sources`: A list of descriptors for internal or external symbol sources. See [Sources](index.md). - `modules`: A list of code modules (aka debug images) that were loaded into the - process. All attributes other than `type` are required. The Symbolicator may - optimize lookups based on the `type` if present. Valid types are `macho`, - `pe`, `elf`. Invalid types are silently ignored. The Symbolicator still works - if the type is invalid, but less efficiently. However, a schematically valid - but _wrong_ type is fatal for finding symbols. + process. All attributes other than `type`, `image_addr` and `image_size` are + required. The Symbolicator may optimize lookups based on the `type` if present. + Valid types are `macho`, `pe`, `elf`. Invalid types are silently ignored. The + Symbolicator still works if the type is invalid, but less efficiently. However, + a schematically valid but _wrong_ type is fatal for finding symbols. - `threads`: A list of process threads to symbolicate. - - `registers`: Optional register values aiding symbolication heuristics. For - example, register values may be used to perform correction heuristics on the - instruction address of the top frame. - - `frames`: A list of frames with addresses. Arbitrary additional properties - may be passed with frames, but are discarded. + - `registers`: Optional register values aiding symbolication heuristics. For + example, register values may be used to perform correction heuristics on the + instruction address of the top frame. + - `frames`: A list of frames with addresses. Arbitrary additional properties + may be passed with frames, but are discarded. When `addr_base_module` is + defined the address in `instruction_addr` is relative within the module + (zero indexed), otherwise it's absolute. ## Response From d5b899864c691fb9ea83def46d031f1f29130db1 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 26 Nov 2020 13:52:18 +0100 Subject: [PATCH 15/21] feat: Introduce addr_mode --- docs/api/response.md | 6 +- docs/api/symbolication.md | 7 +- ...s__symbolication__tests__add_bucket-2.snap | 1 + ...ors__symbolication__tests__add_bucket.snap | 1 + ...bolication__tests__apple_crash_report.snap | 17 ++ ..._symbolication__tests__minidump_linux.snap | 18 ++ ..._symbolication__tests__minidump_macos.snap | 4 + ...ymbolication__tests__minidump_windows.snap | 14 ++ ...symbolication__tests__remove_bucket-2.snap | 1 + ...__symbolication__tests__remove_bucket.snap | 1 + src/actors/symbolication.rs | 231 +++++++++++------- src/types.rs | 57 +++-- src/utils/mod.rs | 1 + tests/integration/conftest.py | 2 +- tests/integration/test_applecrashreport.py | 17 ++ tests/integration/test_basic.py | 2 + tests/integration/test_gcs.py | 4 + tests/integration/test_minidump.py | 22 ++ tests/integration/test_wasm.py | 4 +- 19 files changed, 291 insertions(+), 119 deletions(-) diff --git a/docs/api/response.md b/docs/api/response.md index 4744c073d..3d45842ab 100644 --- a/docs/api/response.md +++ b/docs/api/response.md @@ -34,7 +34,7 @@ symbolication succeeds within a configured timeframe (around 20 seconds): // Frame information "instruction_addr": "0xfeedbeef", // actual address of the frame - "addr_base_module": 0, // when not null/set relative mode + "addr_mode": "abs", // address mode "sym_addr": "0xfeed0000", // start address of the function "package": "/path/to/module.so", // path to the module's code file "symbol": "__1cGmemset6FpviI_0_", // original mangled function name @@ -79,8 +79,8 @@ Addresses (`instruction_addr` and `sym_addr`) can come in two versions. They can be absolute or relative. Symbolicator will always try to make addresses absolute but in some cases this cannot be done. For instance WASM modules do not have absolute addresses in which case the addresses stay relative. This is -identified by the presence of the `addr_base_module` property. If this exists -and is not `null` it's the index of the module containing the address. +identified by the `addr_mode` property. When it's set to `"abs"` it means +the addresses are absolute, when `"rel:X"` it's relative to module index `X`. ## Backoff Response diff --git a/docs/api/symbolication.md b/docs/api/symbolication.md index dbd9b39ff..42c4b2524 100644 --- a/docs/api/symbolication.md +++ b/docs/api/symbolication.md @@ -23,7 +23,7 @@ Content-Type: application/json "frames": [ { "instruction_addr": "0xfeedbeef", - "addr_base_module": 0 + "addr_mode": "rel:0" }, ... ], @@ -72,9 +72,8 @@ as well as external sources to pull symbols from: example, register values may be used to perform correction heuristics on the instruction address of the top frame. - `frames`: A list of frames with addresses. Arbitrary additional properties - may be passed with frames, but are discarded. When `addr_base_module` is - defined the address in `instruction_addr` is relative within the module - (zero indexed), otherwise it's absolute. + may be passed with frames, but are discarded. The `addr_mode` property + defines the beahvior of `instruction_addr`. ## Response diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket-2.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket-2.snap index fa516fa49..e372d4230 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket-2.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket-2.snap @@ -7,6 +7,7 @@ stacktraces: - frames: - status: symbolicated original_index: 0 + addr_mode: abs instruction_addr: 0x100000fa0 lang: c symbol: main diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket.snap index 84ad3e76c..ffbf52a1a 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket.snap @@ -7,6 +7,7 @@ stacktraces: - frames: - status: missing original_index: 0 + addr_mode: abs instruction_addr: 0x100000fa0 modules: - debug_status: missing diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__apple_crash_report.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__apple_crash_report.snap index be5975064..1c48ca6b5 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__apple_crash_report.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__apple_crash_report.snap @@ -19,58 +19,72 @@ stacktraces: frames: - status: unknown_image original_index: 0 + addr_mode: abs instruction_addr: 0x7fff61bc6c2a package: libsystem_kernel.dylib - status: unknown_image original_index: 1 + addr_mode: abs instruction_addr: 0x7fff349f505e package: CoreFoundation - status: unknown_image original_index: 2 + addr_mode: abs instruction_addr: 0x7fff349f45ad package: CoreFoundation - status: unknown_image original_index: 3 + addr_mode: abs instruction_addr: 0x7fff349f3ce4 package: CoreFoundation - status: unknown_image original_index: 4 + addr_mode: abs instruction_addr: 0x7fff33c8d895 package: HIToolbox - status: unknown_image original_index: 5 + addr_mode: abs instruction_addr: 0x7fff33c8d5cb package: HIToolbox - status: unknown_image original_index: 6 + addr_mode: abs instruction_addr: 0x7fff33c8d348 package: HIToolbox - status: unknown_image original_index: 7 + addr_mode: abs instruction_addr: 0x7fff31f4a95b package: AppKit - status: unknown_image original_index: 8 + addr_mode: abs instruction_addr: 0x7fff31f496fa package: AppKit - status: unknown_image original_index: 9 + addr_mode: abs instruction_addr: 0x7fff31f4375d package: AppKit - status: missing original_index: 10 + addr_mode: abs instruction_addr: 0x108b7092b package: /Users/bruno/Documents/Unreal Projects/YetAnotherMac/MacNoEditor/YetAnotherMac.app/Contents/MacOS/YetAnotherMac - status: missing original_index: 11 + addr_mode: abs instruction_addr: 0x108b702a6 package: /Users/bruno/Documents/Unreal Projects/YetAnotherMac/MacNoEditor/YetAnotherMac.app/Contents/MacOS/YetAnotherMac - status: unknown_image original_index: 12 + addr_mode: abs instruction_addr: 0x7fff61a8e085 package: libdyld.dylib - status: unknown_image original_index: 13 + addr_mode: abs instruction_addr: 0xea004 package: YetanotherMac - thread_id: 1 @@ -100,14 +114,17 @@ stacktraces: frames: - status: unknown_image original_index: 0 + addr_mode: abs instruction_addr: 0x7fff61bc85be package: libsystem_kernel.dylib - status: unknown_image original_index: 1 + addr_mode: abs instruction_addr: 0x7fff61c7f415 package: libsystem_pthread.dylib - status: unknown_image original_index: 2 + addr_mode: abs instruction_addr: 0x54485244 modules: - debug_status: missing diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_linux.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_linux.snap index e16472c63..d81fafae6 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_linux.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_linux.snap @@ -37,91 +37,109 @@ stacktraces: frames: - status: missing original_index: 0 + addr_mode: abs instruction_addr: 0x401d72 package: /work/linux/build/crash trust: context - status: missing original_index: 1 + addr_mode: abs instruction_addr: 0x7f514025002e package: /lib/x86_64-linux-gnu/libc-2.23.so trust: scan - status: missing original_index: 2 + addr_mode: abs instruction_addr: 0x7f51401e4800 package: /lib/x86_64-linux-gnu/libc-2.23.so trust: scan - status: missing original_index: 3 + addr_mode: abs instruction_addr: 0x400000 package: /work/linux/build/crash trust: scan - status: missing original_index: 4 + addr_mode: abs instruction_addr: 0x7f5140cebac6 package: /lib/x86_64-linux-gnu/ld-2.23.so trust: scan - status: missing original_index: 5 + addr_mode: abs instruction_addr: 0x401ec0 package: /work/linux/build/crash trust: scan - status: missing original_index: 6 + addr_mode: abs instruction_addr: 0x414c30 package: /work/linux/build/crash trust: scan - status: missing original_index: 7 + addr_mode: abs instruction_addr: 0x7f514017d830 package: /lib/x86_64-linux-gnu/libc-2.23.so trust: scan - status: missing original_index: 8 + addr_mode: abs instruction_addr: 0x401c70 package: /work/linux/build/crash trust: scan - status: missing original_index: 9 + addr_mode: abs instruction_addr: 0x401dc0 package: /work/linux/build/crash trust: scan - status: missing original_index: 10 + addr_mode: abs instruction_addr: 0x401c70 package: /work/linux/build/crash trust: scan - status: missing original_index: 11 + addr_mode: abs instruction_addr: 0x414ca0 package: /work/linux/build/crash trust: scan - status: missing original_index: 12 + addr_mode: abs instruction_addr: 0x401dc0 package: /work/linux/build/crash trust: scan - status: missing original_index: 13 + addr_mode: abs instruction_addr: 0x401de9 package: /work/linux/build/crash trust: scan - status: missing original_index: 14 + addr_mode: abs instruction_addr: 0x7fff5aef1000 package: linux-gate.so trust: scan - status: missing original_index: 15 + addr_mode: abs instruction_addr: 0x400040 package: /work/linux/build/crash trust: scan - status: missing original_index: 16 + addr_mode: abs instruction_addr: 0x7f5140cdc000 package: /lib/x86_64-linux-gnu/ld-2.23.so trust: scan - status: missing original_index: 17 + addr_mode: abs instruction_addr: 0x401dc0 package: /work/linux/build/crash trust: scan diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_macos.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_macos.snap index a0e3608bd..9d8c355dc 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_macos.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_macos.snap @@ -37,21 +37,25 @@ stacktraces: frames: - status: missing original_index: 0 + addr_mode: abs instruction_addr: 0x109ba8c15 package: /Users/travis/build/getsentry/breakpad-tools/macos/build/./crash trust: context - status: missing original_index: 1 + addr_mode: abs instruction_addr: 0x109ba8c70 package: /Users/travis/build/getsentry/breakpad-tools/macos/build/./crash trust: scan - status: missing original_index: 2 + addr_mode: abs instruction_addr: 0x7fffe7eeb235 package: /usr/lib/system/libdyld.dylib trust: scan - status: missing original_index: 3 + addr_mode: abs instruction_addr: 0x7fffe7eeb235 package: /usr/lib/system/libdyld.dylib trust: scan diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_windows.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_windows.snap index 19b12c5cf..fec8d80c6 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_windows.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_windows.snap @@ -30,6 +30,7 @@ stacktraces: frames: - status: symbolicated original_index: 0 + addr_mode: abs instruction_addr: 0x2a2a3d package: "C:\\projects\\breakpad-tools\\windows\\Release\\crash.exe" symbol: main @@ -41,6 +42,7 @@ stacktraces: trust: context - status: symbolicated original_index: 1 + addr_mode: abs instruction_addr: 0x2a2d96 package: "C:\\projects\\breakpad-tools\\windows\\Release\\crash.exe" symbol: __scrt_common_main_seh @@ -52,16 +54,19 @@ stacktraces: trust: cfi - status: missing original_index: 2 + addr_mode: abs instruction_addr: 0x750662c4 package: "C:\\Windows\\System32\\kernel32.dll" trust: cfi - status: missing original_index: 3 + addr_mode: abs instruction_addr: 0x771d0f79 package: "C:\\Windows\\System32\\ntdll.dll" trust: fp - status: missing original_index: 4 + addr_mode: abs instruction_addr: 0x771d0f44 package: "C:\\Windows\\System32\\ntdll.dll" trust: fp @@ -81,21 +86,25 @@ stacktraces: frames: - status: missing original_index: 0 + addr_mode: abs instruction_addr: 0x771e016c package: "C:\\Windows\\System32\\ntdll.dll" trust: context - status: missing original_index: 1 + addr_mode: abs instruction_addr: 0x750662c4 package: "C:\\Windows\\System32\\kernel32.dll" trust: fp - status: missing original_index: 2 + addr_mode: abs instruction_addr: 0x771d0f79 package: "C:\\Windows\\System32\\ntdll.dll" trust: fp - status: missing original_index: 3 + addr_mode: abs instruction_addr: 0x771d0f44 package: "C:\\Windows\\System32\\ntdll.dll" trust: fp @@ -115,21 +124,25 @@ stacktraces: frames: - status: missing original_index: 0 + addr_mode: abs instruction_addr: 0x771e016c package: "C:\\Windows\\System32\\ntdll.dll" trust: context - status: missing original_index: 1 + addr_mode: abs instruction_addr: 0x750662c4 package: "C:\\Windows\\System32\\kernel32.dll" trust: fp - status: missing original_index: 2 + addr_mode: abs instruction_addr: 0x771d0f79 package: "C:\\Windows\\System32\\ntdll.dll" trust: fp - status: missing original_index: 3 + addr_mode: abs instruction_addr: 0x771d0f44 package: "C:\\Windows\\System32\\ntdll.dll" trust: fp @@ -149,6 +162,7 @@ stacktraces: frames: - status: missing original_index: 0 + addr_mode: abs instruction_addr: 0x771df3dc package: "C:\\Windows\\System32\\ntdll.dll" trust: context diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket-2.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket-2.snap index 84ad3e76c..ffbf52a1a 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket-2.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket-2.snap @@ -7,6 +7,7 @@ stacktraces: - frames: - status: missing original_index: 0 + addr_mode: abs instruction_addr: 0x100000fa0 modules: - debug_status: missing diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket.snap index fa516fa49..e372d4230 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket.snap @@ -7,6 +7,7 @@ stacktraces: - frames: - status: symbolicated original_index: 0 + addr_mode: abs instruction_addr: 0x100000fa0 lang: c symbol: main diff --git a/src/actors/symbolication.rs b/src/actors/symbolication.rs index 5f46176ed..13947acf8 100644 --- a/src/actors/symbolication.rs +++ b/src/actors/symbolication.rs @@ -42,6 +42,7 @@ use crate::types::{ ObjectFileStatus, ObjectId, ObjectType, RawFrame, RawObjectInfo, RawStacktrace, Registers, RequestId, Scope, Signal, SymbolicatedFrame, SymbolicationResponse, SystemInfo, }; +use crate::utils::addr::AddrMode; use crate::utils::futures::{CallOnDrop, ThreadPool}; use crate::utils::hex::HexValue; use crate::utils::sentry::SentryFutureExt as _; @@ -97,6 +98,39 @@ type ComputationChannel = Shared>>; +#[derive(Debug, Clone)] +pub struct SymCacheLookupResult<'a> { + module_index: usize, + object_info: &'a CompleteObjectInfo, + symcache: Option<&'a SymCacheFile>, + relative_addr: Option, +} + +impl<'a> SymCacheLookupResult<'a> { + /// The preferred addr mode for this lookup. + /// + /// for the symbolicated frame we generally switch to absolute reporting of + /// addresses. This is not done for images mounted at `0x0`. This is done + /// because for instance WASM does not have a unified address space and so + /// it's not possible for us to absolutize addresses. + pub fn preferred_addr_mode(&self) -> AddrMode { + if self.object_info.supports_absolute_addresses() { + AddrMode::Abs + } else { + AddrMode::ModRel(self.module_index) + } + } + + /// Exposes an address consistent with `preferred_addr_mode`. + pub fn expose_preferred_addr(&self, addr: u64) -> u64 { + if self.object_info.supports_absolute_addresses() { + self.object_info.rel_to_abs_addr(addr).unwrap_or(0) + } else { + addr + } + } +} + /// A builder for the modules list of the symbolication response. /// /// On several platforms, the list of code modules can include shared memory regions or memory @@ -550,10 +584,10 @@ impl SymCacheLookup { for stacktrace in stacktraces { for frame in stacktrace.frames { - if let Some((i, ..)) = - self.lookup_symcache(frame.instruction_addr.0, frame.addr_base_module) + if let Some(SymCacheLookupResult { module_index, .. }) = + self.lookup_symcache(frame.instruction_addr.0, frame.addr_mode) { - referenced_objects.insert(i); + referenced_objects.insert(module_index); } } } @@ -607,36 +641,44 @@ impl SymCacheLookup { }) } - fn lookup_symcache( - &self, - addr: u64, - addr_base_module: Option, - ) -> Option<(usize, &CompleteObjectInfo, Option<&SymCacheFile>)> { - for (i, (info, cache)) in self.inner.iter().enumerate() { - // if `addr_base_module` is defined we pick up the module directly - // by the index. - if Some(i) == addr_base_module { - return Some((i, info, cache.as_ref().map(|x| &**x))); - } + fn lookup_symcache(&self, addr: u64, addr_mode: AddrMode) -> Option> { + for (module_index, (object_info, symcache)) in self.inner.iter().enumerate() { + match addr_mode { + AddrMode::Abs => { + let start_addr = object_info.raw.image_addr.0; - // lookup by address - let start_addr = info.raw.image_addr.0; + if start_addr > addr { + // The debug image starts at a too high address + continue; + } - if start_addr > addr { - // The debug image starts at a too high address - continue; - } + let size = object_info.raw.image_size.unwrap_or(0); + if let Some(end_addr) = start_addr.checked_add(size) { + if end_addr < addr && size != 0 { + // The debug image ends at a too low address and we're also confident that + // end_addr is accurate (size != 0) + continue; + } + } - let size = info.raw.image_size.unwrap_or(0); - if let Some(end_addr) = start_addr.checked_add(size) { - if end_addr < addr && size != 0 { - // The debug image ends at a too low address and we're also confident that - // end_addr is accurate (size != 0) - continue; + return Some(SymCacheLookupResult { + module_index, + object_info, + symcache: symcache.as_ref().map(|x| &**x), + relative_addr: object_info.abs_to_rel_addr(addr), + }); + } + AddrMode::ModRel(this_module_index) => { + if this_module_index == module_index { + return Some(SymCacheLookupResult { + module_index, + object_info, + symcache: symcache.as_ref().map(|x| &**x), + relative_addr: Some(addr), + }); + } } } - - return Some((i, info, cache.as_ref().map(|x| &**x))); } None @@ -650,64 +692,73 @@ fn symbolicate_frame( frame: &mut RawFrame, index: usize, ) -> Result, FrameStatus> { - let (object_info, symcache) = - match caches.lookup_symcache(frame.instruction_addr.0, frame.addr_base_module) { - Some((_, info, Some(symcache))) => { - frame.package = info.raw.code_file.clone(); - (info, symcache) - } - Some((_, info, None)) => { - frame.package = info.raw.code_file.clone(); - if info.debug_status == ObjectFileStatus::Malformed { - return Err(FrameStatus::Malformed); - } else { - return Err(FrameStatus::Missing); - } + let lookup_result = if let Some(lookup_result) = + caches.lookup_symcache(frame.instruction_addr.0, frame.addr_mode) + { + frame.package = lookup_result.object_info.raw.code_file.clone(); + if lookup_result.symcache.is_none() { + if lookup_result.object_info.debug_status == ObjectFileStatus::Malformed { + return Err(FrameStatus::Malformed); + } else { + return Err(FrameStatus::Missing); } - None => return Err(FrameStatus::UnknownImage), - }; + } + lookup_result + } else { + return Err(FrameStatus::UnknownImage); + }; log::trace!("Loading symcache"); - let symcache = match symcache.parse() { + let symcache = match lookup_result + .symcache + .as_ref() + .expect("symcache should always be available at this point") + .parse() + { Ok(Some(x)) => x, Ok(None) => return Err(FrameStatus::Missing), Err(_) => return Err(FrameStatus::Malformed), }; - let is_crashing_frame = index == 0; - let ip_register_value = if is_crashing_frame { - symcache - .arch() - .cpu_family() - .ip_register_name() - .and_then(|ip_reg_name| registers.get(ip_reg_name)) - .map(|x| x.0) - } else { - None - }; - - let caller_address = InstructionInfo::new(symcache.arch(), frame.instruction_addr.0) - .is_crashing_frame(is_crashing_frame) - .signal(signal.map(|signal| signal.0)) - .ip_register_value(ip_register_value) - .caller_address(); - - // if `addr_base_module` is provided the relative address is the caller address, - // otherwise we have to subtract the image address to make it relative - // within the module. - let relative_addr = if frame.addr_base_module.is_some() { - caller_address - } else { - match caller_address.checked_sub(object_info.raw.image_addr.0) { - Some(x) => x, - None => { - log::warn!( - "Underflow when trying to subtract image start addr from caller address" - ); - metric!(counter("relative_addr.underflow") += 1); - return Err(FrameStatus::MissingSymbol); - } + // get the relative caller address + let relative_addr = if let Some(addr) = lookup_result.relative_addr { + // heuristics currently are only supported when we can work with absolute addresses. + // In cases where this is not possible we skip this part entirely and use the relative + // address calculated by the lookup result as lookup address in the module. + if let Some(absolute_addr) = lookup_result.object_info.rel_to_abs_addr(addr) { + let is_crashing_frame = index == 0; + let ip_register_value = if is_crashing_frame { + symcache + .arch() + .cpu_family() + .ip_register_name() + .and_then(|ip_reg_name| registers.get(ip_reg_name)) + .map(|x| x.0) + } else { + None + }; + let absolute_caller_addr = InstructionInfo::new(symcache.arch(), absolute_addr) + .is_crashing_frame(is_crashing_frame) + .signal(signal.map(|signal| signal.0)) + .ip_register_value(ip_register_value) + .caller_address(); + lookup_result + .object_info + .abs_to_rel_addr(absolute_caller_addr) + .ok_or_else(|| { + log::warn!( + "Underflow when trying to subtract image start addr from caller address after heuristics" + ); + metric!(counter("relative_addr.underflow") += 1); + FrameStatus::MissingSymbol + })? + } else { + addr } + } else { + log::warn!("Underflow when trying to subtract image start addr from caller address before heuristics"); + metric!(counter("relative_addr.underflow") += 1); + return Err(FrameStatus::MissingSymbol); }; log::trace!("Symbolicating {:#x}", relative_addr); @@ -770,19 +821,10 @@ fn symbolicate_frame( status: FrameStatus::Symbolicated, original_index: Some(index), raw: RawFrame { - package: object_info.raw.code_file.clone(), - // for the symbolicated frame we generally switch to absolute reporting of - // addresses. This is not done for images mounted at `0x0`. This is done - // because for instance WASM does not have a unified address space and so - // it's not possible for us to absolutize addresses. - addr_base_module: if object_info.raw.image_addr.0 > 0 { - None - } else { - frame.addr_base_module - }, - // make absolute. This is a noop for images mounted at 0x0 + package: lookup_result.object_info.raw.code_file.clone(), + addr_mode: lookup_result.preferred_addr_mode(), instruction_addr: HexValue( - object_info.raw.image_addr.0 + line_info.instruction_address(), + lookup_result.expose_preferred_addr(line_info.instruction_address()), ), symbol: Some(line_info.symbol().to_string()), abs_path: if !abs_path.is_empty() { @@ -803,9 +845,8 @@ fn symbolicate_frame( pre_context: vec![], context_line: None, post_context: vec![], - // make absolute. This is a noop for images mounted at 0x0 sym_addr: Some(HexValue( - object_info.raw.image_addr.0 + line_info.function_address(), + lookup_result.expose_preferred_addr(line_info.function_address()), )), lang: match line_info.language() { Language::Unknown => None, @@ -2074,10 +2115,10 @@ mod tests { let lookup = SymCacheLookup::from_iter(vec![info.clone()]); - let (a, b, c) = lookup.lookup_symcache(43, None).unwrap(); - assert_eq!(a, 0); - assert_eq!(b, &info); - assert!(c.is_none()); + let lookup_result = lookup.lookup_symcache(43, AddrMode::Abs).unwrap(); + assert_eq!(lookup_result.module_index, 0); + assert_eq!(lookup_result.object_info, &info); + assert!(lookup_result.symcache.is_none()); } fn create_object_info(has_id: bool, addr: u64, size: Option) -> CompleteObjectInfo { diff --git a/src/types.rs b/src/types.rs index 6db971b1e..b5a193f3c 100644 --- a/src/types.rs +++ b/src/types.rs @@ -20,6 +20,7 @@ use symbolic::debuginfo::Object; use symbolic::minidump::processor::FrameTrust; use uuid::Uuid; +use crate::utils::addr::AddrMode; use crate::utils::hex::HexValue; use crate::utils::sentry::WriteSentryScope; @@ -124,20 +125,16 @@ fn is_default_frame_trust(trust: &FrameTrust) -> bool { /// An unsymbolicated frame from a symbolication request. #[derive(Debug, Default, Clone, Deserialize, Serialize)] pub struct RawFrame { - /// Switches the address to be interpreted to be within - /// a module of the specified index. For instance sending `0` here - /// would use the first module. - /// - /// This applies to `instruction_addr` and `sym_addr`. When the - /// `addr_base_module` is set then both `instruction_addr` and `sym_addr` - /// are relative within that module. If it's not set or `null` then - /// the adresses are absolute within a unified address space. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub addr_base_module: Option, + /// Controls the addressing mode for `instruction_addr` and + /// `sym_addr`. If not defined it defaults to "abs". Can be + /// set to `"rel:INDEX"` to make the address relative to the + /// module at the given index. + #[serde(default)] + pub addr_mode: AddrMode, /// The instruction address of this frame. /// - /// See `addr_base_module` for the exact behavior of addresses. + /// See `addr_mode` for the exact behavior of addresses. pub instruction_addr: HexValue, /// The path to the image this frame is located in. @@ -154,7 +151,7 @@ pub struct RawFrame { /// Start address of the function this frame is located in (lower or equal to instruction_addr). /// - /// See `addr_base_module` for the exact behavior of addresses. + /// See `addr_mode` for the exact behavior of addresses. #[serde(skip_serializing_if = "Option::is_none")] pub sym_addr: Option, @@ -232,8 +229,8 @@ pub struct RawObjectInfo { /// /// We do allow the image addr to be skipped if it's zero. This is /// because for instance systems like WASM do not actually require an - /// image to be mounted at a specific address. This makes sense mostly - /// when using `addr_base_module`. + /// image to be mounted at a specific address. Per definition an image + /// mounted at 0 does not support absolute addressing. #[serde(default)] pub image_addr: HexValue, @@ -451,6 +448,38 @@ pub struct CompleteObjectInfo { pub raw: RawObjectInfo, } +impl CompleteObjectInfo { + /// Given an absolute address converts it into a relative one. + /// + /// If it does not fit into the object `None` is returned. + pub fn abs_to_rel_addr(&self, addr: u64) -> Option { + if self.supports_absolute_addresses() { + addr.checked_sub(self.raw.image_addr.0) + } else { + None + } + } + + /// Given a relative address returns the absolute address. + /// + /// Certain environments do not support absolute addresses in which + /// case this returns `None`. + pub fn rel_to_abs_addr(&self, addr: u64) -> Option { + if self.supports_absolute_addresses() { + self.raw.image_addr.0.checked_add(addr) + } else { + None + } + } + + /// Checks if this image supports absolute addressing. + /// + /// Per definition images at 0 do not support absolute addresses. + pub fn supports_absolute_addresses(&self) -> bool { + self.raw.image_addr.0 != 0 + } +} + impl From for CompleteObjectInfo { fn from(mut raw: RawObjectInfo) -> Self { raw.debug_id = raw diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 909b80e40..7c473ed7f 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -1,3 +1,4 @@ +pub mod addr; pub mod futures; pub mod hex; pub mod http; diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 59a6c99bf..b4811fb63 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -8,7 +8,6 @@ import traceback import uuid -import boto3 import pytest import requests from pytest_localserver.http import WSGIServer @@ -285,6 +284,7 @@ def s3(pytestconfig): """ if not AWS_ACCESS_KEY_ID or not AWS_SECRET_ACCESS_KEY: fail_missing_secrets("No AWS credentials") + import boto3 return boto3.resource( "s3", aws_access_key_id=AWS_ACCESS_KEY_ID, diff --git a/tests/integration/test_applecrashreport.py b/tests/integration/test_applecrashreport.py index 847df48ef..153814f4f 100644 --- a/tests/integration/test_applecrashreport.py +++ b/tests/integration/test_applecrashreport.py @@ -110,60 +110,70 @@ "original_index": 0, "package": "libsystem_kernel.dylib", "status": "unknown_image", + "addr_mode": "abs", }, { "instruction_addr": "0x7fff349f505e", "original_index": 1, "package": "CoreFoundation", "status": "unknown_image", + "addr_mode": "abs", }, { "instruction_addr": "0x7fff349f45ad", "original_index": 2, "package": "CoreFoundation", "status": "unknown_image", + "addr_mode": "abs", }, { "instruction_addr": "0x7fff349f3ce4", "original_index": 3, "package": "CoreFoundation", "status": "unknown_image", + "addr_mode": "abs", }, { "instruction_addr": "0x7fff33c8d895", "original_index": 4, "package": "HIToolbox", "status": "unknown_image", + "addr_mode": "abs", }, { "instruction_addr": "0x7fff33c8d5cb", "original_index": 5, "package": "HIToolbox", "status": "unknown_image", + "addr_mode": "abs", }, { "instruction_addr": "0x7fff33c8d348", "original_index": 6, "package": "HIToolbox", "status": "unknown_image", + "addr_mode": "abs", }, { "instruction_addr": "0x7fff31f4a95b", "original_index": 7, "package": "AppKit", "status": "unknown_image", + "addr_mode": "abs", }, { "instruction_addr": "0x7fff31f496fa", "original_index": 8, "package": "AppKit", "status": "unknown_image", + "addr_mode": "abs", }, { "instruction_addr": "0x7fff31f4375d", "original_index": 9, "package": "AppKit", "status": "unknown_image", + "addr_mode": "abs", }, { "instruction_addr": "0x108b7092b", @@ -171,6 +181,7 @@ "package": "/Users/bruno/Documents/Unreal " "Projects/YetAnotherMac/MacNoEditor/YetAnotherMac.app/Contents/MacOS/YetAnotherMac", "status": "missing", + "addr_mode": "abs", }, { "instruction_addr": "0x108b702a6", @@ -178,18 +189,21 @@ "package": "/Users/bruno/Documents/Unreal " "Projects/YetAnotherMac/MacNoEditor/YetAnotherMac.app/Contents/MacOS/YetAnotherMac", "status": "missing", + "addr_mode": "abs", }, { "instruction_addr": "0x7fff61a8e085", "original_index": 12, "package": "libdyld.dylib", "status": "unknown_image", + "addr_mode": "abs", }, { "instruction_addr": "0xea004", "original_index": 13, "package": "YetanotherMac", "status": "unknown_image", + "addr_mode": "abs", }, ], "is_requesting": False, @@ -202,17 +216,20 @@ "original_index": 0, "package": "libsystem_kernel.dylib", "status": "unknown_image", + "addr_mode": "abs", }, { "instruction_addr": "0x7fff61c7f415", "original_index": 1, "package": "libsystem_pthread.dylib", "status": "unknown_image", + "addr_mode": "abs", }, { "instruction_addr": "0x54485244", "original_index": 2, "status": "unknown_image", + "addr_mode": "abs", }, ], "is_requesting": True, diff --git a/tests/integration/test_basic.py b/tests/integration/test_basic.py index fda72d39b..aa98cec42 100644 --- a/tests/integration/test_basic.py +++ b/tests/integration/test_basic.py @@ -29,6 +29,7 @@ "frames": [ { "status": "symbolicated", + "addr_mode": "abs", "original_index": 0, "instruction_addr": "0x749e8630", "lineno": 0, @@ -69,6 +70,7 @@ def _make_unsuccessful_result(status): "registers": {"eip": "0x1509530"}, "frames": [ { + "addr_mode": "abs", "status": status, "original_index": 0, "package": "C:\\Windows\\System32\\kernel32.dll", diff --git a/tests/integration/test_gcs.py b/tests/integration/test_gcs.py index f15fd5939..90ad6f034 100644 --- a/tests/integration/test_gcs.py +++ b/tests/integration/test_gcs.py @@ -83,6 +83,7 @@ "status": "symbolicated", "sym_addr": "0x190609598", "symbol": "start", + "addr_mode": "abs", }, { "function": "__CFRunLoopRun", @@ -92,6 +93,7 @@ "status": "symbolicated", "sym_addr": "0x1916ca6c0", "symbol": "__CFRunLoopRun", + "addr_mode": "abs", }, { "function": "__CFRunLoopDoBlocks", @@ -101,6 +103,7 @@ "status": "symbolicated", "sym_addr": "0x1916cca08", "symbol": "__CFRunLoopDoBlocks", + "addr_mode": "abs", }, { "function": "__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__", @@ -110,6 +113,7 @@ "status": "symbolicated", "sym_addr": "0x1916cd414", "symbol": "__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__", + "addr_mode": "abs", }, ] } diff --git a/tests/integration/test_minidump.py b/tests/integration/test_minidump.py index ea79fb69e..9e1a16794 100644 --- a/tests/integration/test_minidump.py +++ b/tests/integration/test_minidump.py @@ -22,6 +22,7 @@ "package": "C:\\projects\\breakpad-tools\\windows\\Release\\crash.exe", "instruction_addr": "0x2a2a3d", "trust": "context", + "addr_mode": "abs", }, { "status": "missing", @@ -29,6 +30,7 @@ "package": "C:\\projects\\breakpad-tools\\windows\\Release\\crash.exe", "instruction_addr": "0x2a28d0", "trust": "fp", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -40,6 +42,7 @@ "function": "FreeWrapper(void *)", "lineno": 0, "trust": "scan", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -51,6 +54,7 @@ "function": "DetermineOutputProvider(class MiniDumpAllocationProvider *,void *,struct _MINIDUMP_CALLBACK_INFORMATION * const,class MiniDumpOutputProvider * *)", "lineno": 0, "trust": "scan", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -62,6 +66,7 @@ "function": "FreeWrapper(void *)", "lineno": 0, "trust": "scan", + "addr_mode": "abs", }, { "instruction_addr": "0x2a3435", @@ -69,6 +74,7 @@ "package": "C:\\projects\\breakpad-tools\\windows\\Release\\crash.exe", "status": "missing", "trust": "scan", + "addr_mode": "abs", }, { "instruction_addr": "0x2a2d97", @@ -76,6 +82,7 @@ "package": "C:\\projects\\breakpad-tools\\windows\\Release\\crash.exe", "status": "missing", "trust": "scan", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -87,6 +94,7 @@ "function": "@BaseThreadInitThunk@12", "lineno": 0, "trust": "fp", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -98,6 +106,7 @@ "function": "__RtlUserThreadStart@8", "lineno": 0, "trust": "cfi", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -109,6 +118,7 @@ "function": "_RtlUserThreadStart@8", "lineno": 0, "trust": "cfi", + "addr_mode": "abs", }, ], "is_requesting": True, @@ -138,6 +148,7 @@ "function": "ZwWaitForWorkViaWorkerFactory@20", "lineno": 0, "trust": "context", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -149,6 +160,7 @@ "function": "TppWorkerThread@4", "lineno": 0, "trust": "cfi", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -160,6 +172,7 @@ "function": "@BaseThreadInitThunk@12", "lineno": 0, "trust": "cfi", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -171,6 +184,7 @@ "function": "__RtlUserThreadStart@8", "lineno": 0, "trust": "cfi", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -182,6 +196,7 @@ "function": "_RtlUserThreadStart@8", "lineno": 0, "trust": "cfi", + "addr_mode": "abs", }, ], "is_requesting": False, @@ -211,6 +226,7 @@ "function": "ZwWaitForWorkViaWorkerFactory@20", "lineno": 0, "trust": "context", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -222,6 +238,7 @@ "function": "TppWorkerThread@4", "lineno": 0, "trust": "cfi", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -233,6 +250,7 @@ "function": "@BaseThreadInitThunk@12", "lineno": 0, "trust": "cfi", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -244,6 +262,7 @@ "function": "__RtlUserThreadStart@8", "lineno": 0, "trust": "cfi", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -255,6 +274,7 @@ "function": "_RtlUserThreadStart@8", "lineno": 0, "trust": "cfi", + "addr_mode": "abs", }, ], "is_requesting": False, @@ -284,6 +304,7 @@ "function": "ZwGetContextThread@8", "lineno": 0, "trust": "context", + "addr_mode": "abs", }, { "status": "symbolicated", @@ -295,6 +316,7 @@ "function": "NlsIsUserDefaultLocale@4", "lineno": 0, "trust": "cfi", + "addr_mode": "abs", }, ], "is_requesting": False, diff --git a/tests/integration/test_wasm.py b/tests/integration/test_wasm.py index 9fd3d47bf..d5cc84065 100644 --- a/tests/integration/test_wasm.py +++ b/tests/integration/test_wasm.py @@ -5,7 +5,7 @@ "frames": [ { "instruction_addr": "0x8c", - "addr_base_module": 0, + "addr_mode": "rel:0", } ], }, @@ -46,7 +46,7 @@ "filename": "src/lib.rs", "function": "internal_func", "instruction_addr": "0x8c", - "addr_base_module": 0, + "addr_mode": "rel:0", "lang": "rust", "lineno": 19, "original_index": 0, From fc564a84358d8a3c461e03083827879e6fab099d Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 26 Nov 2020 13:57:22 +0100 Subject: [PATCH 16/21] feat: Added missing address module --- src/utils/addr.rs | 71 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 src/utils/addr.rs diff --git a/src/utils/addr.rs b/src/utils/addr.rs new file mode 100644 index 000000000..a81b04ddb --- /dev/null +++ b/src/utils/addr.rs @@ -0,0 +1,71 @@ +use std::borrow::Cow; +use std::fmt; +use std::str::FromStr; + +use serde::de::{self, Deserialize, Deserializer}; +use serde::ser::{Serialize, Serializer}; +use thiserror::Error; + +#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] +pub enum AddrMode { + Abs, + ModRel(usize), +} + +impl Default for AddrMode { + fn default() -> AddrMode { + AddrMode::Abs + } +} + +impl fmt::Display for AddrMode { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match *self { + AddrMode::Abs => write!(f, "abs"), + AddrMode::ModRel(idx) => write!(f, "rel:{}", idx), + } + } +} + +impl Serialize for AddrMode { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&self.to_string()) + } +} + +#[derive(Debug, Error)] +#[error("invalid address mode")] +pub struct ParseAddrModeError; + +impl FromStr for AddrMode { + type Err = ParseAddrModeError; + + fn from_str(s: &str) -> Result { + if s == "abs" { + return Ok(AddrMode::Abs); + } + let mut iter = s.splitn(2, ':'); + let kind = iter.next().ok_or(ParseAddrModeError)?; + let index = iter + .next() + .and_then(|x| x.parse().ok()) + .ok_or(ParseAddrModeError)?; + match kind { + "rel" => Ok(AddrMode::ModRel(index)), + _ => Err(ParseAddrModeError), + } + } +} + +impl<'de> Deserialize<'de> for AddrMode { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let s = Cow::::deserialize(deserializer).map_err(de::Error::custom)?; + Ok(AddrMode::from_str(&s).map_err(de::Error::custom)?) + } +} From 2a1948142920c659116368223d1b1ed250ef4a2e Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 26 Nov 2020 14:23:43 +0100 Subject: [PATCH 17/21] fix: Retain original module order from request --- src/actors/symbolication.rs | 127 ++++++++++++++++++++++-------------- 1 file changed, 77 insertions(+), 50 deletions(-) diff --git a/src/actors/symbolication.rs b/src/actors/symbolication.rs index 13947acf8..5e3b889e1 100644 --- a/src/actors/symbolication.rs +++ b/src/actors/symbolication.rs @@ -377,7 +377,7 @@ fn object_info_from_minidump_module(ty: ObjectType, module: &CodeModule) -> RawO pub struct SourceObject(SelfCell, Object<'static>>); struct SourceLookup { - inner: Vec<(CompleteObjectInfo, Option>)>, + inner: Vec<(usize, CompleteObjectInfo, Option>)>, } impl SourceLookup { @@ -393,7 +393,9 @@ impl SourceLookup { for stacktrace in stacktraces { for frame in &stacktrace.frames { - if let Some(i) = self.get_object_index_by_addr(frame.raw.instruction_addr.0) { + if let Some(i) = + self.get_object_index_by_addr(frame.raw.instruction_addr.0, frame.raw.addr_mode) + { referenced_objects.insert(i); } } @@ -401,7 +403,7 @@ impl SourceLookup { let mut futures = Vec::new(); - for (i, (mut object_info, _)) in self.inner.into_iter().enumerate() { + for (i, mut object_info, _) in self.inner.into_iter() { let is_used = referenced_objects.contains(&i); let objects = objects.clone(); let scope = scope.clone(); @@ -410,7 +412,7 @@ impl SourceLookup { futures.push(async move { if !is_used { object_info.debug_status = ObjectFileStatus::Unused; - return (object_info, None); + return (i, object_info, None); } let opt_object_file_meta = objects @@ -443,7 +445,7 @@ impl SourceLookup { object_info.features.has_sources = true; } - (object_info, object_file_opt) + (i, object_info, object_file_opt) }); } @@ -455,7 +457,7 @@ impl SourceLookup { pub fn prepare_debug_sessions(&self) -> Vec>> { self.inner .iter() - .map(|&(_, ref o)| o.as_ref().and_then(|o| o.0.get().debug_session().ok())) + .map(|&(_, _, ref o)| o.as_ref().and_then(|o| o.0.get().debug_session().ok())) .collect() } @@ -463,11 +465,12 @@ impl SourceLookup { &self, debug_sessions: &[Option>], addr: u64, + addr_mode: AddrMode, abs_path: &str, lineno: u32, n: usize, ) -> Option<(Vec, String, Vec)> { - let index = self.get_object_index_by_addr(addr)?; + let index = self.get_object_index_by_addr(addr, addr_mode)?; let session = debug_sessions[index].as_ref()?; let source = session.source_by_path(abs_path).ok()??; @@ -486,44 +489,54 @@ impl SourceLookup { Some((pre_context, context, post_context)) } - fn get_object_index_by_addr(&self, addr: u64) -> Option { - for (i, (info, _)) in self.inner.iter().enumerate() { - let start_addr = info.raw.image_addr.0; + fn get_object_index_by_addr(&self, addr: u64, addr_mode: AddrMode) -> Option { + for &(module_index, ref info, _) in self.inner.iter() { + match addr_mode { + AddrMode::Abs => { + let start_addr = info.raw.image_addr.0; - if start_addr > addr { - // The debug image starts at a too high address - continue; - } + if start_addr > addr { + // The debug image starts at a too high address + continue; + } - let size = info.raw.image_size.unwrap_or(0); - if let Some(end_addr) = start_addr.checked_add(size) { - if end_addr < addr && size != 0 { - // The debug image ends at a too low address and we're also confident that - // end_addr is accurate (size != 0) - continue; + let size = info.raw.image_size.unwrap_or(0); + if let Some(end_addr) = start_addr.checked_add(size) { + if end_addr < addr && size != 0 { + // The debug image ends at a too low address and we're also confident that + // end_addr is accurate (size != 0) + continue; + } + } + + return Some(module_index); + } + AddrMode::ModRel(this_module_index) => { + if this_module_index == module_index { + return Some(module_index); + } } } - - return Some(i); } None } fn sort(&mut self) { - self.inner.sort_by_key(|(info, _)| info.raw.image_addr.0); + self.inner.sort_by_key(|(_, info, _)| info.raw.image_addr.0); // Ignore the name `dedup_by`, I just want to iterate over consecutive items and update // some. - self.inner.dedup_by(|(ref info2, _), (ref mut info1, _)| { - // If this underflows we didn't sort properly. - let size = info2.raw.image_addr.0 - info1.raw.image_addr.0; - if info1.raw.image_size.unwrap_or(0) == 0 { - info1.raw.image_size = Some(size); - } + self.inner + .dedup_by(|(_, ref info2, _), (_, ref mut info1, _)| { + // If this underflows we didn't sort properly. + let size = info2.raw.image_addr.0 - info1.raw.image_addr.0; + if info1.raw.image_size.unwrap_or(0) == 0 { + info1.raw.image_size = Some(size); + } - false - }); + false + }); } } @@ -533,7 +546,11 @@ impl FromIterator for SourceLookup { T: IntoIterator, { let mut rv = SourceLookup { - inner: iter.into_iter().map(|x| (x, None)).collect(), + inner: iter + .into_iter() + .enumerate() + .map(|(i, x)| (i, x, None)) + .collect(), }; rv.sort(); rv @@ -541,7 +558,7 @@ impl FromIterator for SourceLookup { } struct SymCacheLookup { - inner: Vec<(CompleteObjectInfo, Option>)>, + inner: Vec<(usize, CompleteObjectInfo, Option>)>, } impl FromIterator for SymCacheLookup { @@ -550,7 +567,11 @@ impl FromIterator for SymCacheLookup { T: IntoIterator, { let mut rv = SymCacheLookup { - inner: iter.into_iter().map(|x| (x, None)).collect(), + inner: iter + .into_iter() + .enumerate() + .map(|(idx, x)| (idx, x, None)) + .collect(), }; rv.sort(); rv @@ -559,19 +580,20 @@ impl FromIterator for SymCacheLookup { impl SymCacheLookup { fn sort(&mut self) { - self.inner.sort_by_key(|(info, _)| info.raw.image_addr.0); + self.inner.sort_by_key(|(_, info, _)| info.raw.image_addr.0); // Ignore the name `dedup_by`, I just want to iterate over consecutive items and update // some. - self.inner.dedup_by(|(ref info2, _), (ref mut info1, _)| { - // If this underflows we didn't sort properly. - let size = info2.raw.image_addr.0 - info1.raw.image_addr.0; - if info1.raw.image_size.unwrap_or(0) == 0 { - info1.raw.image_size = Some(size); - } + self.inner + .dedup_by(|(_, ref info2, _), (_, ref mut info1, _)| { + // If this underflows we didn't sort properly. + let size = info2.raw.image_addr.0 - info1.raw.image_addr.0; + if info1.raw.image_size.unwrap_or(0) == 0 { + info1.raw.image_size = Some(size); + } - false - }); + false + }); } async fn fetch_symcaches( @@ -594,7 +616,7 @@ impl SymCacheLookup { let mut futures = Vec::new(); - for (i, (mut object_info, _)) in self.inner.into_iter().enumerate() { + for (i, mut object_info, _) in self.inner.into_iter() { let is_used = referenced_objects.contains(&i); let sources = request.sources.clone(); let scope = request.scope.clone(); @@ -603,7 +625,7 @@ impl SymCacheLookup { futures.push(async move { if !is_used { object_info.debug_status = ObjectFileStatus::Unused; - return (object_info, None); + return (i, object_info, None); } let symcache_result = symcache_actor .fetch(FetchSymCache { @@ -632,7 +654,7 @@ impl SymCacheLookup { } object_info.debug_status = status; - (object_info, symcache) + (i, object_info, symcache) }); } @@ -642,7 +664,7 @@ impl SymCacheLookup { } fn lookup_symcache(&self, addr: u64, addr_mode: AddrMode) -> Option> { - for (module_index, (object_info, symcache)) in self.inner.iter().enumerate() { + for &(module_index, ref object_info, ref symcache) in self.inner.iter() { match addr_mode { AddrMode::Abs => { let start_addr = object_info.raw.image_addr.0; @@ -1014,19 +1036,23 @@ impl SymbolicationActor { .map(|trace| symbolicate_stacktrace(trace, &symcache_lookup, signal)) .collect(); - let modules: Vec<_> = symcache_lookup + let mut modules: Vec<_> = symcache_lookup .inner .into_iter() - .map(|(object_info, _)| { + .map(|(index, object_info, _)| { metric!( counter("symbolication.debug_status") += 1, "status" => object_info.debug_status.name() ); - object_info + (index, object_info) }) .collect(); + // bring modules back into the original order + modules.sort_by_key(|&(index, _)| index); + let modules: Vec<_> = modules.into_iter().map(|(_, module)| module).collect(); + metric!(time_raw("symbolication.num_modules") = modules.len() as u64); metric!(time_raw("symbolication.num_stacktraces") = stacktraces.len() as u64); metric!( @@ -1065,6 +1091,7 @@ impl SymbolicationActor { let result = source_lookup.get_context_lines( &debug_sessions, frame.raw.instruction_addr.0, + frame.raw.addr_mode, abs_path, lineno, 5, From 281f9061e7083dd8841701461bb7c80f05c3cae6 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 26 Nov 2020 14:30:23 +0100 Subject: [PATCH 18/21] fix: lint and tests --- tests/integration/conftest.py | 1 + tests/integration/test_s3.py | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index b4811fb63..72d310d5d 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -285,6 +285,7 @@ def s3(pytestconfig): if not AWS_ACCESS_KEY_ID or not AWS_SECRET_ACCESS_KEY: fail_missing_secrets("No AWS credentials") import boto3 + return boto3.resource( "s3", aws_access_key_id=AWS_ACCESS_KEY_ID, diff --git a/tests/integration/test_s3.py b/tests/integration/test_s3.py index ed709fb6b..e30afa4b6 100644 --- a/tests/integration/test_s3.py +++ b/tests/integration/test_s3.py @@ -59,6 +59,7 @@ "status": "symbolicated", "sym_addr": "0x100000fa0", "symbol": "main", + "addr_mode": "abs", } ] } From a334d7a4ff3e83cd021b89e8db40403474d6465d Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 26 Nov 2020 17:47:06 +0100 Subject: [PATCH 19/21] ref: make abs the implied addr mode --- ...s__symbolication__tests__add_bucket-2.snap | 1 - ...ors__symbolication__tests__add_bucket.snap | 1 - ...bolication__tests__apple_crash_report.snap | 17 -------------- ..._symbolication__tests__minidump_linux.snap | 18 --------------- ..._symbolication__tests__minidump_macos.snap | 4 ---- ...ymbolication__tests__minidump_windows.snap | 14 ------------ ...symbolication__tests__remove_bucket-2.snap | 1 - ...__symbolication__tests__remove_bucket.snap | 1 - src/types.rs | 9 ++++---- tests/integration/test_applecrashreport.py | 17 -------------- tests/integration/test_basic.py | 2 -- tests/integration/test_gcs.py | 4 ---- tests/integration/test_minidump.py | 22 ------------------- tests/integration/test_s3.py | 1 - 14 files changed, 4 insertions(+), 108 deletions(-) diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket-2.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket-2.snap index e372d4230..fa516fa49 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket-2.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket-2.snap @@ -7,7 +7,6 @@ stacktraces: - frames: - status: symbolicated original_index: 0 - addr_mode: abs instruction_addr: 0x100000fa0 lang: c symbol: main diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket.snap index ffbf52a1a..84ad3e76c 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__add_bucket.snap @@ -7,7 +7,6 @@ stacktraces: - frames: - status: missing original_index: 0 - addr_mode: abs instruction_addr: 0x100000fa0 modules: - debug_status: missing diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__apple_crash_report.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__apple_crash_report.snap index 1c48ca6b5..be5975064 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__apple_crash_report.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__apple_crash_report.snap @@ -19,72 +19,58 @@ stacktraces: frames: - status: unknown_image original_index: 0 - addr_mode: abs instruction_addr: 0x7fff61bc6c2a package: libsystem_kernel.dylib - status: unknown_image original_index: 1 - addr_mode: abs instruction_addr: 0x7fff349f505e package: CoreFoundation - status: unknown_image original_index: 2 - addr_mode: abs instruction_addr: 0x7fff349f45ad package: CoreFoundation - status: unknown_image original_index: 3 - addr_mode: abs instruction_addr: 0x7fff349f3ce4 package: CoreFoundation - status: unknown_image original_index: 4 - addr_mode: abs instruction_addr: 0x7fff33c8d895 package: HIToolbox - status: unknown_image original_index: 5 - addr_mode: abs instruction_addr: 0x7fff33c8d5cb package: HIToolbox - status: unknown_image original_index: 6 - addr_mode: abs instruction_addr: 0x7fff33c8d348 package: HIToolbox - status: unknown_image original_index: 7 - addr_mode: abs instruction_addr: 0x7fff31f4a95b package: AppKit - status: unknown_image original_index: 8 - addr_mode: abs instruction_addr: 0x7fff31f496fa package: AppKit - status: unknown_image original_index: 9 - addr_mode: abs instruction_addr: 0x7fff31f4375d package: AppKit - status: missing original_index: 10 - addr_mode: abs instruction_addr: 0x108b7092b package: /Users/bruno/Documents/Unreal Projects/YetAnotherMac/MacNoEditor/YetAnotherMac.app/Contents/MacOS/YetAnotherMac - status: missing original_index: 11 - addr_mode: abs instruction_addr: 0x108b702a6 package: /Users/bruno/Documents/Unreal Projects/YetAnotherMac/MacNoEditor/YetAnotherMac.app/Contents/MacOS/YetAnotherMac - status: unknown_image original_index: 12 - addr_mode: abs instruction_addr: 0x7fff61a8e085 package: libdyld.dylib - status: unknown_image original_index: 13 - addr_mode: abs instruction_addr: 0xea004 package: YetanotherMac - thread_id: 1 @@ -114,17 +100,14 @@ stacktraces: frames: - status: unknown_image original_index: 0 - addr_mode: abs instruction_addr: 0x7fff61bc85be package: libsystem_kernel.dylib - status: unknown_image original_index: 1 - addr_mode: abs instruction_addr: 0x7fff61c7f415 package: libsystem_pthread.dylib - status: unknown_image original_index: 2 - addr_mode: abs instruction_addr: 0x54485244 modules: - debug_status: missing diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_linux.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_linux.snap index d81fafae6..e16472c63 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_linux.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_linux.snap @@ -37,109 +37,91 @@ stacktraces: frames: - status: missing original_index: 0 - addr_mode: abs instruction_addr: 0x401d72 package: /work/linux/build/crash trust: context - status: missing original_index: 1 - addr_mode: abs instruction_addr: 0x7f514025002e package: /lib/x86_64-linux-gnu/libc-2.23.so trust: scan - status: missing original_index: 2 - addr_mode: abs instruction_addr: 0x7f51401e4800 package: /lib/x86_64-linux-gnu/libc-2.23.so trust: scan - status: missing original_index: 3 - addr_mode: abs instruction_addr: 0x400000 package: /work/linux/build/crash trust: scan - status: missing original_index: 4 - addr_mode: abs instruction_addr: 0x7f5140cebac6 package: /lib/x86_64-linux-gnu/ld-2.23.so trust: scan - status: missing original_index: 5 - addr_mode: abs instruction_addr: 0x401ec0 package: /work/linux/build/crash trust: scan - status: missing original_index: 6 - addr_mode: abs instruction_addr: 0x414c30 package: /work/linux/build/crash trust: scan - status: missing original_index: 7 - addr_mode: abs instruction_addr: 0x7f514017d830 package: /lib/x86_64-linux-gnu/libc-2.23.so trust: scan - status: missing original_index: 8 - addr_mode: abs instruction_addr: 0x401c70 package: /work/linux/build/crash trust: scan - status: missing original_index: 9 - addr_mode: abs instruction_addr: 0x401dc0 package: /work/linux/build/crash trust: scan - status: missing original_index: 10 - addr_mode: abs instruction_addr: 0x401c70 package: /work/linux/build/crash trust: scan - status: missing original_index: 11 - addr_mode: abs instruction_addr: 0x414ca0 package: /work/linux/build/crash trust: scan - status: missing original_index: 12 - addr_mode: abs instruction_addr: 0x401dc0 package: /work/linux/build/crash trust: scan - status: missing original_index: 13 - addr_mode: abs instruction_addr: 0x401de9 package: /work/linux/build/crash trust: scan - status: missing original_index: 14 - addr_mode: abs instruction_addr: 0x7fff5aef1000 package: linux-gate.so trust: scan - status: missing original_index: 15 - addr_mode: abs instruction_addr: 0x400040 package: /work/linux/build/crash trust: scan - status: missing original_index: 16 - addr_mode: abs instruction_addr: 0x7f5140cdc000 package: /lib/x86_64-linux-gnu/ld-2.23.so trust: scan - status: missing original_index: 17 - addr_mode: abs instruction_addr: 0x401dc0 package: /work/linux/build/crash trust: scan diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_macos.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_macos.snap index 9d8c355dc..a0e3608bd 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_macos.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_macos.snap @@ -37,25 +37,21 @@ stacktraces: frames: - status: missing original_index: 0 - addr_mode: abs instruction_addr: 0x109ba8c15 package: /Users/travis/build/getsentry/breakpad-tools/macos/build/./crash trust: context - status: missing original_index: 1 - addr_mode: abs instruction_addr: 0x109ba8c70 package: /Users/travis/build/getsentry/breakpad-tools/macos/build/./crash trust: scan - status: missing original_index: 2 - addr_mode: abs instruction_addr: 0x7fffe7eeb235 package: /usr/lib/system/libdyld.dylib trust: scan - status: missing original_index: 3 - addr_mode: abs instruction_addr: 0x7fffe7eeb235 package: /usr/lib/system/libdyld.dylib trust: scan diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_windows.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_windows.snap index fec8d80c6..19b12c5cf 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_windows.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__minidump_windows.snap @@ -30,7 +30,6 @@ stacktraces: frames: - status: symbolicated original_index: 0 - addr_mode: abs instruction_addr: 0x2a2a3d package: "C:\\projects\\breakpad-tools\\windows\\Release\\crash.exe" symbol: main @@ -42,7 +41,6 @@ stacktraces: trust: context - status: symbolicated original_index: 1 - addr_mode: abs instruction_addr: 0x2a2d96 package: "C:\\projects\\breakpad-tools\\windows\\Release\\crash.exe" symbol: __scrt_common_main_seh @@ -54,19 +52,16 @@ stacktraces: trust: cfi - status: missing original_index: 2 - addr_mode: abs instruction_addr: 0x750662c4 package: "C:\\Windows\\System32\\kernel32.dll" trust: cfi - status: missing original_index: 3 - addr_mode: abs instruction_addr: 0x771d0f79 package: "C:\\Windows\\System32\\ntdll.dll" trust: fp - status: missing original_index: 4 - addr_mode: abs instruction_addr: 0x771d0f44 package: "C:\\Windows\\System32\\ntdll.dll" trust: fp @@ -86,25 +81,21 @@ stacktraces: frames: - status: missing original_index: 0 - addr_mode: abs instruction_addr: 0x771e016c package: "C:\\Windows\\System32\\ntdll.dll" trust: context - status: missing original_index: 1 - addr_mode: abs instruction_addr: 0x750662c4 package: "C:\\Windows\\System32\\kernel32.dll" trust: fp - status: missing original_index: 2 - addr_mode: abs instruction_addr: 0x771d0f79 package: "C:\\Windows\\System32\\ntdll.dll" trust: fp - status: missing original_index: 3 - addr_mode: abs instruction_addr: 0x771d0f44 package: "C:\\Windows\\System32\\ntdll.dll" trust: fp @@ -124,25 +115,21 @@ stacktraces: frames: - status: missing original_index: 0 - addr_mode: abs instruction_addr: 0x771e016c package: "C:\\Windows\\System32\\ntdll.dll" trust: context - status: missing original_index: 1 - addr_mode: abs instruction_addr: 0x750662c4 package: "C:\\Windows\\System32\\kernel32.dll" trust: fp - status: missing original_index: 2 - addr_mode: abs instruction_addr: 0x771d0f79 package: "C:\\Windows\\System32\\ntdll.dll" trust: fp - status: missing original_index: 3 - addr_mode: abs instruction_addr: 0x771d0f44 package: "C:\\Windows\\System32\\ntdll.dll" trust: fp @@ -162,7 +149,6 @@ stacktraces: frames: - status: missing original_index: 0 - addr_mode: abs instruction_addr: 0x771df3dc package: "C:\\Windows\\System32\\ntdll.dll" trust: context diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket-2.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket-2.snap index ffbf52a1a..84ad3e76c 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket-2.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket-2.snap @@ -7,7 +7,6 @@ stacktraces: - frames: - status: missing original_index: 0 - addr_mode: abs instruction_addr: 0x100000fa0 modules: - debug_status: missing diff --git a/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket.snap b/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket.snap index e372d4230..fa516fa49 100644 --- a/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket.snap +++ b/src/actors/snapshots/symbolicator__actors__symbolication__tests__remove_bucket.snap @@ -7,7 +7,6 @@ stacktraces: - frames: - status: symbolicated original_index: 0 - addr_mode: abs instruction_addr: 0x100000fa0 lang: c symbol: main diff --git a/src/types.rs b/src/types.rs index b5a193f3c..b89f7f91a 100644 --- a/src/types.rs +++ b/src/types.rs @@ -117,9 +117,8 @@ impl fmt::Display for Scope { /// A map of register values. pub type Registers = BTreeMap; -#[allow(clippy::trivially_copy_pass_by_ref)] -fn is_default_frame_trust(trust: &FrameTrust) -> bool { - *trust == FrameTrust::None +fn is_default_value(value: &T) -> bool { + *value == T::default() } /// An unsymbolicated frame from a symbolication request. @@ -129,7 +128,7 @@ pub struct RawFrame { /// `sym_addr`. If not defined it defaults to "abs". Can be /// set to `"rel:INDEX"` to make the address relative to the /// module at the given index. - #[serde(default)] + #[serde(default, skip_serializing_if = "is_default_value")] pub addr_mode: AddrMode, /// The instruction address of this frame. @@ -184,7 +183,7 @@ pub struct RawFrame { pub post_context: Vec, /// Information about how the raw frame was created. - #[serde(default, skip_serializing_if = "is_default_frame_trust")] + #[serde(default, skip_serializing_if = "is_default_value")] pub trust: FrameTrust, } diff --git a/tests/integration/test_applecrashreport.py b/tests/integration/test_applecrashreport.py index 153814f4f..847df48ef 100644 --- a/tests/integration/test_applecrashreport.py +++ b/tests/integration/test_applecrashreport.py @@ -110,70 +110,60 @@ "original_index": 0, "package": "libsystem_kernel.dylib", "status": "unknown_image", - "addr_mode": "abs", }, { "instruction_addr": "0x7fff349f505e", "original_index": 1, "package": "CoreFoundation", "status": "unknown_image", - "addr_mode": "abs", }, { "instruction_addr": "0x7fff349f45ad", "original_index": 2, "package": "CoreFoundation", "status": "unknown_image", - "addr_mode": "abs", }, { "instruction_addr": "0x7fff349f3ce4", "original_index": 3, "package": "CoreFoundation", "status": "unknown_image", - "addr_mode": "abs", }, { "instruction_addr": "0x7fff33c8d895", "original_index": 4, "package": "HIToolbox", "status": "unknown_image", - "addr_mode": "abs", }, { "instruction_addr": "0x7fff33c8d5cb", "original_index": 5, "package": "HIToolbox", "status": "unknown_image", - "addr_mode": "abs", }, { "instruction_addr": "0x7fff33c8d348", "original_index": 6, "package": "HIToolbox", "status": "unknown_image", - "addr_mode": "abs", }, { "instruction_addr": "0x7fff31f4a95b", "original_index": 7, "package": "AppKit", "status": "unknown_image", - "addr_mode": "abs", }, { "instruction_addr": "0x7fff31f496fa", "original_index": 8, "package": "AppKit", "status": "unknown_image", - "addr_mode": "abs", }, { "instruction_addr": "0x7fff31f4375d", "original_index": 9, "package": "AppKit", "status": "unknown_image", - "addr_mode": "abs", }, { "instruction_addr": "0x108b7092b", @@ -181,7 +171,6 @@ "package": "/Users/bruno/Documents/Unreal " "Projects/YetAnotherMac/MacNoEditor/YetAnotherMac.app/Contents/MacOS/YetAnotherMac", "status": "missing", - "addr_mode": "abs", }, { "instruction_addr": "0x108b702a6", @@ -189,21 +178,18 @@ "package": "/Users/bruno/Documents/Unreal " "Projects/YetAnotherMac/MacNoEditor/YetAnotherMac.app/Contents/MacOS/YetAnotherMac", "status": "missing", - "addr_mode": "abs", }, { "instruction_addr": "0x7fff61a8e085", "original_index": 12, "package": "libdyld.dylib", "status": "unknown_image", - "addr_mode": "abs", }, { "instruction_addr": "0xea004", "original_index": 13, "package": "YetanotherMac", "status": "unknown_image", - "addr_mode": "abs", }, ], "is_requesting": False, @@ -216,20 +202,17 @@ "original_index": 0, "package": "libsystem_kernel.dylib", "status": "unknown_image", - "addr_mode": "abs", }, { "instruction_addr": "0x7fff61c7f415", "original_index": 1, "package": "libsystem_pthread.dylib", "status": "unknown_image", - "addr_mode": "abs", }, { "instruction_addr": "0x54485244", "original_index": 2, "status": "unknown_image", - "addr_mode": "abs", }, ], "is_requesting": True, diff --git a/tests/integration/test_basic.py b/tests/integration/test_basic.py index aa98cec42..fda72d39b 100644 --- a/tests/integration/test_basic.py +++ b/tests/integration/test_basic.py @@ -29,7 +29,6 @@ "frames": [ { "status": "symbolicated", - "addr_mode": "abs", "original_index": 0, "instruction_addr": "0x749e8630", "lineno": 0, @@ -70,7 +69,6 @@ def _make_unsuccessful_result(status): "registers": {"eip": "0x1509530"}, "frames": [ { - "addr_mode": "abs", "status": status, "original_index": 0, "package": "C:\\Windows\\System32\\kernel32.dll", diff --git a/tests/integration/test_gcs.py b/tests/integration/test_gcs.py index 90ad6f034..f15fd5939 100644 --- a/tests/integration/test_gcs.py +++ b/tests/integration/test_gcs.py @@ -83,7 +83,6 @@ "status": "symbolicated", "sym_addr": "0x190609598", "symbol": "start", - "addr_mode": "abs", }, { "function": "__CFRunLoopRun", @@ -93,7 +92,6 @@ "status": "symbolicated", "sym_addr": "0x1916ca6c0", "symbol": "__CFRunLoopRun", - "addr_mode": "abs", }, { "function": "__CFRunLoopDoBlocks", @@ -103,7 +101,6 @@ "status": "symbolicated", "sym_addr": "0x1916cca08", "symbol": "__CFRunLoopDoBlocks", - "addr_mode": "abs", }, { "function": "__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__", @@ -113,7 +110,6 @@ "status": "symbolicated", "sym_addr": "0x1916cd414", "symbol": "__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__", - "addr_mode": "abs", }, ] } diff --git a/tests/integration/test_minidump.py b/tests/integration/test_minidump.py index 9e1a16794..ea79fb69e 100644 --- a/tests/integration/test_minidump.py +++ b/tests/integration/test_minidump.py @@ -22,7 +22,6 @@ "package": "C:\\projects\\breakpad-tools\\windows\\Release\\crash.exe", "instruction_addr": "0x2a2a3d", "trust": "context", - "addr_mode": "abs", }, { "status": "missing", @@ -30,7 +29,6 @@ "package": "C:\\projects\\breakpad-tools\\windows\\Release\\crash.exe", "instruction_addr": "0x2a28d0", "trust": "fp", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -42,7 +40,6 @@ "function": "FreeWrapper(void *)", "lineno": 0, "trust": "scan", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -54,7 +51,6 @@ "function": "DetermineOutputProvider(class MiniDumpAllocationProvider *,void *,struct _MINIDUMP_CALLBACK_INFORMATION * const,class MiniDumpOutputProvider * *)", "lineno": 0, "trust": "scan", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -66,7 +62,6 @@ "function": "FreeWrapper(void *)", "lineno": 0, "trust": "scan", - "addr_mode": "abs", }, { "instruction_addr": "0x2a3435", @@ -74,7 +69,6 @@ "package": "C:\\projects\\breakpad-tools\\windows\\Release\\crash.exe", "status": "missing", "trust": "scan", - "addr_mode": "abs", }, { "instruction_addr": "0x2a2d97", @@ -82,7 +76,6 @@ "package": "C:\\projects\\breakpad-tools\\windows\\Release\\crash.exe", "status": "missing", "trust": "scan", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -94,7 +87,6 @@ "function": "@BaseThreadInitThunk@12", "lineno": 0, "trust": "fp", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -106,7 +98,6 @@ "function": "__RtlUserThreadStart@8", "lineno": 0, "trust": "cfi", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -118,7 +109,6 @@ "function": "_RtlUserThreadStart@8", "lineno": 0, "trust": "cfi", - "addr_mode": "abs", }, ], "is_requesting": True, @@ -148,7 +138,6 @@ "function": "ZwWaitForWorkViaWorkerFactory@20", "lineno": 0, "trust": "context", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -160,7 +149,6 @@ "function": "TppWorkerThread@4", "lineno": 0, "trust": "cfi", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -172,7 +160,6 @@ "function": "@BaseThreadInitThunk@12", "lineno": 0, "trust": "cfi", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -184,7 +171,6 @@ "function": "__RtlUserThreadStart@8", "lineno": 0, "trust": "cfi", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -196,7 +182,6 @@ "function": "_RtlUserThreadStart@8", "lineno": 0, "trust": "cfi", - "addr_mode": "abs", }, ], "is_requesting": False, @@ -226,7 +211,6 @@ "function": "ZwWaitForWorkViaWorkerFactory@20", "lineno": 0, "trust": "context", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -238,7 +222,6 @@ "function": "TppWorkerThread@4", "lineno": 0, "trust": "cfi", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -250,7 +233,6 @@ "function": "@BaseThreadInitThunk@12", "lineno": 0, "trust": "cfi", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -262,7 +244,6 @@ "function": "__RtlUserThreadStart@8", "lineno": 0, "trust": "cfi", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -274,7 +255,6 @@ "function": "_RtlUserThreadStart@8", "lineno": 0, "trust": "cfi", - "addr_mode": "abs", }, ], "is_requesting": False, @@ -304,7 +284,6 @@ "function": "ZwGetContextThread@8", "lineno": 0, "trust": "context", - "addr_mode": "abs", }, { "status": "symbolicated", @@ -316,7 +295,6 @@ "function": "NlsIsUserDefaultLocale@4", "lineno": 0, "trust": "cfi", - "addr_mode": "abs", }, ], "is_requesting": False, diff --git a/tests/integration/test_s3.py b/tests/integration/test_s3.py index e30afa4b6..ed709fb6b 100644 --- a/tests/integration/test_s3.py +++ b/tests/integration/test_s3.py @@ -59,7 +59,6 @@ "status": "symbolicated", "sym_addr": "0x100000fa0", "symbol": "main", - "addr_mode": "abs", } ] } From 81de0bc1839f6fff241cf18724b435237fc4bd7c Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 26 Nov 2020 18:49:09 +0100 Subject: [PATCH 20/21] ref: structs are greater than tuples --- docs/advanced/symbol-server-compatibility.md | 2 +- src/actors/symbolication.rs | 218 ++++++++++--------- src/utils/addr.rs | 9 +- 3 files changed, 127 insertions(+), 102 deletions(-) diff --git a/docs/advanced/symbol-server-compatibility.md b/docs/advanced/symbol-server-compatibility.md index 0c0f4cbf1..b19a7d641 100644 --- a/docs/advanced/symbol-server-compatibility.md +++ b/docs/advanced/symbol-server-compatibility.md @@ -78,7 +78,7 @@ Specifically, the code and debug identifiers are defined as follows: **WASM**: - **Code ID:** The bytes as specified in the `build_id` custom section. -- **Debug ID:** The same as code ID but truncated to 16 bytes. +- **Debug ID:** The same as code ID but truncated to 16 bytes + `0` for age. **PE** / **PDB**: diff --git a/src/actors/symbolication.rs b/src/actors/symbolication.rs index 5e3b889e1..b59a9cb68 100644 --- a/src/actors/symbolication.rs +++ b/src/actors/symbolication.rs @@ -117,7 +117,7 @@ impl<'a> SymCacheLookupResult<'a> { if self.object_info.supports_absolute_addresses() { AddrMode::Abs } else { - AddrMode::ModRel(self.module_index) + AddrMode::Rel(self.module_index) } } @@ -376,8 +376,14 @@ fn object_info_from_minidump_module(ty: ObjectType, module: &CodeModule) -> RawO pub struct SourceObject(SelfCell, Object<'static>>); +struct SourceObjectEntry { + module_index: usize, + object_info: CompleteObjectInfo, + source_object: Option>, +} + struct SourceLookup { - inner: Vec<(usize, CompleteObjectInfo, Option>)>, + inner: Vec, } impl SourceLookup { @@ -403,16 +409,17 @@ impl SourceLookup { let mut futures = Vec::new(); - for (i, mut object_info, _) in self.inner.into_iter() { - let is_used = referenced_objects.contains(&i); + for mut entry in self.inner.into_iter() { + let is_used = referenced_objects.contains(&entry.module_index); let objects = objects.clone(); let scope = scope.clone(); let sources = sources.clone(); futures.push(async move { if !is_used { - object_info.debug_status = ObjectFileStatus::Unused; - return (i, object_info, None); + entry.object_info.debug_status = ObjectFileStatus::Unused; + entry.source_object = None; + return entry; } let opt_object_file_meta = objects @@ -420,14 +427,14 @@ impl SourceLookup { filetypes: FileType::sources(), purpose: ObjectPurpose::Source, scope: scope.clone(), - identifier: object_id_from_object_info(&object_info.raw), + identifier: object_id_from_object_info(&entry.object_info.raw), sources, }) .compat() .await .unwrap_or(None); - let object_file_opt = match opt_object_file_meta { + entry.source_object = match opt_object_file_meta { None => None, Some(object_file_meta) => objects .fetch(object_file_meta) @@ -441,11 +448,11 @@ impl SourceLookup { }), }; - if object_file_opt.is_some() { - object_info.features.has_sources = true; + if entry.source_object.is_some() { + entry.object_info.features.has_sources = true; } - (i, object_info, object_file_opt) + entry }); } @@ -457,7 +464,12 @@ impl SourceLookup { pub fn prepare_debug_sessions(&self) -> Vec>> { self.inner .iter() - .map(|&(_, _, ref o)| o.as_ref().and_then(|o| o.0.get().debug_session().ok())) + .map(|entry| { + entry + .source_object + .as_ref() + .and_then(|o| o.0.get().debug_session().ok()) + }) .collect() } @@ -490,17 +502,17 @@ impl SourceLookup { } fn get_object_index_by_addr(&self, addr: u64, addr_mode: AddrMode) -> Option { - for &(module_index, ref info, _) in self.inner.iter() { - match addr_mode { - AddrMode::Abs => { - let start_addr = info.raw.image_addr.0; + match addr_mode { + AddrMode::Abs => { + for entry in self.inner.iter() { + let start_addr = entry.object_info.raw.image_addr.0; if start_addr > addr { // The debug image starts at a too high address continue; } - let size = info.raw.image_size.unwrap_or(0); + let size = entry.object_info.raw.image_size.unwrap_or(0); if let Some(end_addr) = start_addr.checked_add(size) { if end_addr < addr && size != 0 { // The debug image ends at a too low address and we're also confident that @@ -509,34 +521,33 @@ impl SourceLookup { } } - return Some(module_index); - } - AddrMode::ModRel(this_module_index) => { - if this_module_index == module_index { - return Some(module_index); - } + return Some(entry.module_index); } + None } + AddrMode::Rel(this_module_index) => self + .inner + .iter() + .find(|x| x.module_index == this_module_index) + .map(|x| x.module_index), } - - None } fn sort(&mut self) { - self.inner.sort_by_key(|(_, info, _)| info.raw.image_addr.0); + self.inner + .sort_by_key(|entry| entry.object_info.raw.image_addr.0); // Ignore the name `dedup_by`, I just want to iterate over consecutive items and update // some. - self.inner - .dedup_by(|(_, ref info2, _), (_, ref mut info1, _)| { - // If this underflows we didn't sort properly. - let size = info2.raw.image_addr.0 - info1.raw.image_addr.0; - if info1.raw.image_size.unwrap_or(0) == 0 { - info1.raw.image_size = Some(size); - } + self.inner.dedup_by(|entry2, entry1| { + // If this underflows we didn't sort properly. + let size = entry2.object_info.raw.image_addr.0 - entry1.object_info.raw.image_addr.0; + if entry1.object_info.raw.image_size.unwrap_or(0) == 0 { + entry1.object_info.raw.image_size = Some(size); + } - false - }); + false + }); } } @@ -549,7 +560,11 @@ impl FromIterator for SourceLookup { inner: iter .into_iter() .enumerate() - .map(|(i, x)| (i, x, None)) + .map(|(module_index, object_info)| SourceObjectEntry { + module_index, + object_info, + source_object: None, + }) .collect(), }; rv.sort(); @@ -557,8 +572,14 @@ impl FromIterator for SourceLookup { } } +struct SymCacheEntry { + module_index: usize, + object_info: CompleteObjectInfo, + symcache: Option>, +} + struct SymCacheLookup { - inner: Vec<(usize, CompleteObjectInfo, Option>)>, + inner: Vec, } impl FromIterator for SymCacheLookup { @@ -570,7 +591,11 @@ impl FromIterator for SymCacheLookup { inner: iter .into_iter() .enumerate() - .map(|(idx, x)| (idx, x, None)) + .map(|(module_index, object_info)| SymCacheEntry { + module_index, + object_info, + symcache: None, + }) .collect(), }; rv.sort(); @@ -580,20 +605,20 @@ impl FromIterator for SymCacheLookup { impl SymCacheLookup { fn sort(&mut self) { - self.inner.sort_by_key(|(_, info, _)| info.raw.image_addr.0); + self.inner + .sort_by_key(|entry| entry.object_info.raw.image_addr.0); // Ignore the name `dedup_by`, I just want to iterate over consecutive items and update // some. - self.inner - .dedup_by(|(_, ref info2, _), (_, ref mut info1, _)| { - // If this underflows we didn't sort properly. - let size = info2.raw.image_addr.0 - info1.raw.image_addr.0; - if info1.raw.image_size.unwrap_or(0) == 0 { - info1.raw.image_size = Some(size); - } + self.inner.dedup_by(|entry2, entry1| { + // If this underflows we didn't sort properly. + let size = entry2.object_info.raw.image_addr.0 - entry1.object_info.raw.image_addr.0; + if entry1.object_info.raw.image_size.unwrap_or(0) == 0 { + entry1.object_info.raw.image_size = Some(size); + } - false - }); + false + }); } async fn fetch_symcaches( @@ -616,21 +641,21 @@ impl SymCacheLookup { let mut futures = Vec::new(); - for (i, mut object_info, _) in self.inner.into_iter() { - let is_used = referenced_objects.contains(&i); + for mut entry in self.inner.into_iter() { + let is_used = referenced_objects.contains(&entry.module_index); let sources = request.sources.clone(); let scope = request.scope.clone(); let symcache_actor = symcache_actor.clone(); futures.push(async move { if !is_used { - object_info.debug_status = ObjectFileStatus::Unused; - return (i, object_info, None); + entry.object_info.debug_status = ObjectFileStatus::Unused; + return entry; } let symcache_result = symcache_actor .fetch(FetchSymCache { - object_type: object_info.raw.ty, - identifier: object_id_from_object_info(&object_info.raw), + object_type: entry.object_info.raw.ty, + identifier: object_id_from_object_info(&entry.object_info.raw), sources, scope, }) @@ -646,15 +671,16 @@ impl SymCacheLookup { Err(e) => (None, (&*e).into()), }; - object_info.arch = Default::default(); + entry.object_info.arch = Default::default(); if let Some(ref symcache) = symcache { - object_info.arch = symcache.arch(); - object_info.features.merge(symcache.features()); + entry.object_info.arch = symcache.arch(); + entry.object_info.features.merge(symcache.features()); } - object_info.debug_status = status; - (i, object_info, symcache) + entry.symcache = symcache; + entry.object_info.debug_status = status; + entry }); } @@ -664,17 +690,17 @@ impl SymCacheLookup { } fn lookup_symcache(&self, addr: u64, addr_mode: AddrMode) -> Option> { - for &(module_index, ref object_info, ref symcache) in self.inner.iter() { - match addr_mode { - AddrMode::Abs => { - let start_addr = object_info.raw.image_addr.0; + match addr_mode { + AddrMode::Abs => { + for entry in self.inner.iter() { + let start_addr = entry.object_info.raw.image_addr.0; if start_addr > addr { // The debug image starts at a too high address continue; } - let size = object_info.raw.image_size.unwrap_or(0); + let size = entry.object_info.raw.image_size.unwrap_or(0); if let Some(end_addr) = start_addr.checked_add(size) { if end_addr < addr && size != 0 { // The debug image ends at a too low address and we're also confident that @@ -684,26 +710,25 @@ impl SymCacheLookup { } return Some(SymCacheLookupResult { - module_index, - object_info, - symcache: symcache.as_ref().map(|x| &**x), - relative_addr: object_info.abs_to_rel_addr(addr), + module_index: entry.module_index, + object_info: &entry.object_info, + symcache: entry.symcache.as_ref().map(|x| &**x), + relative_addr: entry.object_info.abs_to_rel_addr(addr), }); } - AddrMode::ModRel(this_module_index) => { - if this_module_index == module_index { - return Some(SymCacheLookupResult { - module_index, - object_info, - symcache: symcache.as_ref().map(|x| &**x), - relative_addr: Some(addr), - }); - } - } + None } + AddrMode::Rel(this_module_index) => self + .inner + .iter() + .find(|x| x.module_index == this_module_index) + .map(|entry| SymCacheLookupResult { + module_index: entry.module_index, + object_info: &entry.object_info, + symcache: entry.symcache.as_ref().map(|x| &**x), + relative_addr: Some(addr), + }), } - - None } } @@ -714,21 +739,18 @@ fn symbolicate_frame( frame: &mut RawFrame, index: usize, ) -> Result, FrameStatus> { - let lookup_result = if let Some(lookup_result) = - caches.lookup_symcache(frame.instruction_addr.0, frame.addr_mode) - { - frame.package = lookup_result.object_info.raw.code_file.clone(); - if lookup_result.symcache.is_none() { - if lookup_result.object_info.debug_status == ObjectFileStatus::Malformed { - return Err(FrameStatus::Malformed); - } else { - return Err(FrameStatus::Missing); - } + let lookup_result = caches + .lookup_symcache(frame.instruction_addr.0, frame.addr_mode) + .ok_or(FrameStatus::UnknownImage)?; + + frame.package = lookup_result.object_info.raw.code_file.clone(); + if lookup_result.symcache.is_none() { + if lookup_result.object_info.debug_status == ObjectFileStatus::Malformed { + return Err(FrameStatus::Malformed); + } else { + return Err(FrameStatus::Missing); } - lookup_result - } else { - return Err(FrameStatus::UnknownImage); - }; + } log::trace!("Loading symcache"); let symcache = match lookup_result @@ -1039,13 +1061,13 @@ impl SymbolicationActor { let mut modules: Vec<_> = symcache_lookup .inner .into_iter() - .map(|(index, object_info, _)| { + .map(|entry| { metric!( counter("symbolication.debug_status") += 1, - "status" => object_info.debug_status.name() + "status" => entry.object_info.debug_status.name() ); - (index, object_info) + (entry.module_index, entry.object_info) }) .collect(); @@ -1526,7 +1548,7 @@ impl SymbolicationActor { // Start building the module list to be returned in the // symbolication response. For all modules in the minidump we - // create the CompletedObjectInfo and start populating it. + // create the CompleteObjectInfo and start populating it. let mut module_builder = process_state .modules() .into_iter() diff --git a/src/utils/addr.rs b/src/utils/addr.rs index a81b04ddb..2d5bce376 100644 --- a/src/utils/addr.rs +++ b/src/utils/addr.rs @@ -6,10 +6,13 @@ use serde::de::{self, Deserialize, Deserializer}; use serde::ser::{Serialize, Serializer}; use thiserror::Error; +/// Defines the addressing mode. #[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] pub enum AddrMode { + /// Declares addresses to be absolute with a shared memory space. Abs, - ModRel(usize), + /// Declares an address to be relative to an indexed module. + Rel(usize), } impl Default for AddrMode { @@ -22,7 +25,7 @@ impl fmt::Display for AddrMode { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self { AddrMode::Abs => write!(f, "abs"), - AddrMode::ModRel(idx) => write!(f, "rel:{}", idx), + AddrMode::Rel(idx) => write!(f, "rel:{}", idx), } } } @@ -54,7 +57,7 @@ impl FromStr for AddrMode { .and_then(|x| x.parse().ok()) .ok_or(ParseAddrModeError)?; match kind { - "rel" => Ok(AddrMode::ModRel(index)), + "rel" => Ok(AddrMode::Rel(index)), _ => Err(ParseAddrModeError), } } From ecf82f61852522caa3da8c175356e527fb286ed6 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 26 Nov 2020 19:00:37 +0100 Subject: [PATCH 21/21] happy clippy --- src/actors/symbolication.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/actors/symbolication.rs b/src/actors/symbolication.rs index b59a9cb68..936112af0 100644 --- a/src/actors/symbolication.rs +++ b/src/actors/symbolication.rs @@ -712,7 +712,7 @@ impl SymCacheLookup { return Some(SymCacheLookupResult { module_index: entry.module_index, object_info: &entry.object_info, - symcache: entry.symcache.as_ref().map(|x| &**x), + symcache: entry.symcache.as_deref(), relative_addr: entry.object_info.abs_to_rel_addr(addr), }); } @@ -725,7 +725,7 @@ impl SymCacheLookup { .map(|entry| SymCacheLookupResult { module_index: entry.module_index, object_info: &entry.object_info, - symcache: entry.symcache.as_ref().map(|x| &**x), + symcache: entry.symcache.as_deref(), relative_addr: Some(addr), }), }