From b9edb1b31f98103a93e5b53fb3318a2681ac731a Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sun, 2 Jul 2023 15:41:27 +0200 Subject: [PATCH 01/42] Upgrade to 2021 ed and inline panics Panics and asserts do not work the same way with inlined format args, so this updates all crates to 2021 edition. --- Cargo.toml | 6 +++--- crates/as-if-std/Cargo.toml | 2 +- crates/debuglink/Cargo.toml | 2 +- crates/dylib-dep/Cargo.toml | 2 +- crates/line-tables-only/Cargo.toml | 2 +- crates/line-tables-only/src/lib.rs | 15 ++++++++------- crates/macos_frames_test/Cargo.toml | 2 +- crates/without_debuginfo/Cargo.toml | 2 +- tests/accuracy/main.rs | 2 +- tests/smoke.rs | 18 ++++-------------- 10 files changed, 22 insertions(+), 31 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6714b3b7d..03deefdb8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,7 @@ A library to acquire a stack trace (backtrace) at runtime in a Rust program. """ autoexamples = true autotests = true -edition = "2018" +edition = "2021" exclude = ["/ci/"] [workspace] @@ -118,12 +118,12 @@ required-features = ["std"] [[test]] name = "smoke" required-features = ["std"] -edition = '2018' +edition = '2021' [[test]] name = "accuracy" required-features = ["std"] -edition = '2018' +edition = '2021' [[test]] name = "concurrent-panics" diff --git a/crates/as-if-std/Cargo.toml b/crates/as-if-std/Cargo.toml index bcbcfe159..1d8d066f1 100644 --- a/crates/as-if-std/Cargo.toml +++ b/crates/as-if-std/Cargo.toml @@ -2,7 +2,7 @@ name = "as-if-std" version = "0.1.0" authors = ["Alex Crichton "] -edition = "2018" +edition = "2021" publish = false [lib] diff --git a/crates/debuglink/Cargo.toml b/crates/debuglink/Cargo.toml index 6b55b1394..5e62abd37 100644 --- a/crates/debuglink/Cargo.toml +++ b/crates/debuglink/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "debuglink" version = "0.1.0" -edition = "2018" +edition = "2021" [dependencies] backtrace = { path = "../.." } diff --git a/crates/dylib-dep/Cargo.toml b/crates/dylib-dep/Cargo.toml index c3d4a8c2f..e6cc9c23b 100644 --- a/crates/dylib-dep/Cargo.toml +++ b/crates/dylib-dep/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "dylib-dep" version = "0.1.0" -edition = "2018" +edition = "2021" authors = [] publish = false diff --git a/crates/line-tables-only/Cargo.toml b/crates/line-tables-only/Cargo.toml index e2967d3d3..8d17db58c 100644 --- a/crates/line-tables-only/Cargo.toml +++ b/crates/line-tables-only/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "line-tables-only" version = "0.1.0" -edition = "2018" +edition = "2021" [build-dependencies] cc = "1.0" diff --git a/crates/line-tables-only/src/lib.rs b/crates/line-tables-only/src/lib.rs index bd5afcb3a..b292b8441 100644 --- a/crates/line-tables-only/src/lib.rs +++ b/crates/line-tables-only/src/lib.rs @@ -1,8 +1,8 @@ #[cfg(test)] mod tests { - use std::path::Path; use backtrace::Backtrace; use libc::c_void; + use std::path::Path; pub type Callback = extern "C" fn(data: *mut c_void); @@ -15,11 +15,12 @@ mod tests { unsafe { *(data as *mut Option) = Some(bt) }; } - fn assert_contains(backtrace: &Backtrace, - expected_name: &str, - expected_file: &str, - expected_line: u32) { - + fn assert_contains( + backtrace: &Backtrace, + expected_name: &str, + expected_file: &str, + expected_line: u32, + ) { let expected_file = Path::new(expected_file); for frame in backtrace.frames() { @@ -34,7 +35,7 @@ mod tests { } } - panic!("symbol {:?} not found in backtrace: {:?}", expected_name, backtrace); + panic!("symbol {expected_name:?} not found in backtrace: {backtrace:?}"); } /// Verifies that when debug info includes only lines tables the generated diff --git a/crates/macos_frames_test/Cargo.toml b/crates/macos_frames_test/Cargo.toml index 278d51e79..849e76414 100644 --- a/crates/macos_frames_test/Cargo.toml +++ b/crates/macos_frames_test/Cargo.toml @@ -2,7 +2,7 @@ name = "macos_frames_test" version = "0.1.0" authors = ["Aaron Hill "] -edition = "2018" +edition = "2021" [dependencies.backtrace] path = "../.." diff --git a/crates/without_debuginfo/Cargo.toml b/crates/without_debuginfo/Cargo.toml index 19d76cbec..38e559971 100644 --- a/crates/without_debuginfo/Cargo.toml +++ b/crates/without_debuginfo/Cargo.toml @@ -2,7 +2,7 @@ name = "without_debuginfo" version = "0.1.0" authors = ["Alex Crichton "] -edition = "2018" +edition = "2021" [dependencies.backtrace] path = "../.." diff --git a/tests/accuracy/main.rs b/tests/accuracy/main.rs index 149203a1b..7570b2784 100644 --- a/tests/accuracy/main.rs +++ b/tests/accuracy/main.rs @@ -103,7 +103,7 @@ fn verify(filelines: &[Pos]) { loop { let sym = match symbols.next() { Some(sym) => sym, - None => panic!("failed to find {}:{}", file, line), + None => panic!("failed to find {file}:{line}"), }; if let Some(filename) = sym.filename() { if let Some(lineno) = sym.lineno() { diff --git a/tests/smoke.rs b/tests/smoke.rs index 683a6f0db..299cb820c 100644 --- a/tests/smoke.rs +++ b/tests/smoke.rs @@ -150,9 +150,7 @@ fn smoke_test_frames() { if cfg!(debug_assertions) { assert!( name.contains(expected_name), - "didn't find `{}` in `{}`", - expected_name, - name + "didn't find `{expected_name}` in `{name}`" ); } @@ -164,18 +162,13 @@ fn smoke_test_frames() { if !expected_file.is_empty() { assert!( file.ends_with(expected_file), - "{:?} didn't end with {:?}", - file, - expected_file + "{file:?} didn't end with {expected_file:?}" ); } if expected_line != 0 { assert!( line == expected_line, - "bad line number on frame for `{}`: {} != {}", - expected_name, - line, - expected_line + "bad line number on frame for `{expected_name}`: {line} != {expected_line}" ); } @@ -185,10 +178,7 @@ fn smoke_test_frames() { if expected_col != 0 { assert!( col == expected_col, - "bad column number on frame for `{}`: {} != {}", - expected_name, - col, - expected_col + "bad column number on frame for `{expected_name}`: {col} != {expected_col}", ); } } From 4b291636ada00a1641f557e640e1c102a433daa9 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sun, 2 Jul 2023 19:58:06 +0200 Subject: [PATCH 02/42] Specify MSRV in cargo.toml --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 03deefdb8..5a715e15d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,7 @@ autoexamples = true autotests = true edition = "2021" exclude = ["/ci/"] +rust-version = "1.65.0" [workspace] members = ['crates/cpp_smoke_test', 'crates/as-if-std'] From 038b456730eb19e7fa1bfa4fe9dd392659ec2a62 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 19 Aug 2023 22:22:02 -0700 Subject: [PATCH 03/42] Write down MSRV policy This was effectively Alex Crichton's policy, so this reflects only the decision to formalize a policy. --- README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/README.md b/README.md index cd80a697c..f4c7ad257 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,20 @@ fn main() { } ``` +# Supported Rust Versions + +The `backtrace` crate is a core component of the standard library, and must +at times keep up with the evolution of various platforms in order to serve +the standard library's needs. This often means using recent libraries +that provide unwinding and symbolication for various platforms. +Thus `backtrace` is likely to use recent Rust features or depend on a library +which itself uses them. Its minimum supported Rust version, by policy, is +within a few versions of current stable, approximately "stable - 2". + +This policy takes precedence over any Rust version stated anywhere else: +the Cargo.toml `rust-version` may reflect what actually builds in CI instead. +It is not a guarantee the version remains as-is from release to release. + # License This project is licensed under either of From ecb214b955e4cff5b342cf3d325dc4234b5a2cea Mon Sep 17 00:00:00 2001 From: Kai Luo Date: Fri, 6 Jan 2023 04:47:34 -0500 Subject: [PATCH 04/42] Add AIX support - Use `loadquery` to get loaded libraries - Use object crate to parse XCOFF and get image information --- Cargo.toml | 2 +- crates/as-if-std/Cargo.toml | 2 +- src/symbolize/gimli.rs | 43 +++++++- src/symbolize/gimli/libs_aix.rs | 74 +++++++++++++ src/symbolize/gimli/xcoff.rs | 186 ++++++++++++++++++++++++++++++++ tests/accuracy/main.rs | 2 + tests/smoke.rs | 33 ++++-- 7 files changed, 330 insertions(+), 12 deletions(-) create mode 100644 src/symbolize/gimli/libs_aix.rs create mode 100644 src/symbolize/gimli/xcoff.rs diff --git a/Cargo.toml b/Cargo.toml index cff2c9e66..80de4171d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,7 +43,7 @@ miniz_oxide = { version = "0.7.0", default-features = false } [dependencies.object] version = "0.31.1" default-features = false -features = ['read_core', 'elf', 'macho', 'pe', 'unaligned', 'archive'] +features = ['read_core', 'elf', 'macho', 'pe', 'xcoff', 'unaligned', 'archive'] [target.'cfg(windows)'.dependencies] winapi = { version = "0.3.9", optional = true } diff --git a/crates/as-if-std/Cargo.toml b/crates/as-if-std/Cargo.toml index 012e60f8f..8fbacae4c 100644 --- a/crates/as-if-std/Cargo.toml +++ b/crates/as-if-std/Cargo.toml @@ -22,7 +22,7 @@ miniz_oxide = { version = "0.7", default-features = false } version = "0.31.1" default-features = false optional = true -features = ['read_core', 'elf', 'macho', 'pe', 'unaligned', 'archive'] +features = ['read_core', 'elf', 'macho', 'pe', 'xcoff', 'unaligned', 'archive'] [features] default = ['backtrace'] diff --git a/src/symbolize/gimli.rs b/src/symbolize/gimli.rs index 7f1c6a528..5d2f62746 100644 --- a/src/symbolize/gimli.rs +++ b/src/symbolize/gimli.rs @@ -41,6 +41,7 @@ cfg_if::cfg_if! { target_os = "openbsd", target_os = "solaris", target_os = "illumos", + target_os = "aix", ))] { #[path = "gimli/mmap_unix.rs"] mod mmap; @@ -116,8 +117,17 @@ impl<'data> Context<'data> { dwp: Option>, ) -> Option> { let mut sections = gimli::Dwarf::load(|id| -> Result<_, ()> { - let data = object.section(stash, id.name()).unwrap_or(&[]); - Ok(EndianSlice::new(data, Endian)) + if cfg!(not(target_os = "aix")) { + let data = object.section(stash, id.name()).unwrap_or(&[]); + Ok(EndianSlice::new(data, Endian)) + } else { + if let Some(name) = id.xcoff_name() { + let data = object.section(stash, name).unwrap_or(&[]); + Ok(EndianSlice::new(data, Endian)) + } else { + Ok(EndianSlice::new(&[], Endian)) + } + } }) .ok()?; @@ -192,6 +202,9 @@ cfg_if::cfg_if! { ))] { mod macho; use self::macho::{handle_split_dwarf, Object}; + } else if #[cfg(target_os = "aix")] { + mod xcoff; + use self::xcoff::{handle_split_dwarf, Object}; } else { mod elf; use self::elf::{handle_split_dwarf, Object}; @@ -234,6 +247,9 @@ cfg_if::cfg_if! { } else if #[cfg(target_os = "haiku")] { mod libs_haiku; use libs_haiku::native_libraries; + } else if #[cfg(target_os = "aix")] { + mod libs_aix; + use libs_aix::native_libraries; } else { // Everything else should doesn't know how to load native libraries. fn native_libraries() -> Vec { @@ -261,6 +277,13 @@ struct Cache { struct Library { name: OsString, + #[cfg(target_os = "aix")] + /// On AIX, the library mmapped can be a member of a big-archive file. + /// For example, with a big-archive named libfoo.a containing libbar.so, + /// one can use `dlopen("libfoo.a(libbar.so)", RTLD_MEMBER | RTLD_LAZY)` + /// to use the `libbar.so` library. In this case, only `libbar.so` is + /// mmapped, not the whole `libfoo.a`. + member_name: OsString, /// Segments of this library loaded into memory, and where they're loaded. segments: Vec, /// The "bias" of this library, typically where it's loaded into memory. @@ -280,6 +303,19 @@ struct LibrarySegment { len: usize, } +#[cfg(target_os = "aix")] +fn create_mapping(lib: &Library) -> Option { + let name = &lib.name; + let member_name = &lib.member_name; + Mapping::new(name.as_ref(), member_name) +} + +#[cfg(not(target_os = "aix"))] +fn create_mapping(lib: &Library) -> Option { + let name = &lib.name; + Mapping::new(name.as_ref()) +} + // unsafe because this is required to be externally synchronized pub unsafe fn clear_symbol_cache() { Cache::with_global(|cache| cache.mappings.clear()); @@ -360,8 +396,7 @@ impl Cache { // When the mapping is not in the cache, create a new mapping, // insert it into the front of the cache, and evict the oldest cache // entry if necessary. - let name = &self.libraries[lib].name; - let mapping = Mapping::new(name.as_ref())?; + let mapping = create_mapping(&self.libraries[lib])?; if self.mappings.len() == MAPPINGS_CACHE_SIZE { self.mappings.pop(); diff --git a/src/symbolize/gimli/libs_aix.rs b/src/symbolize/gimli/libs_aix.rs new file mode 100644 index 000000000..8cac11d4d --- /dev/null +++ b/src/symbolize/gimli/libs_aix.rs @@ -0,0 +1,74 @@ +use super::mystd::borrow::ToOwned; +use super::mystd::env; +use super::mystd::ffi::{CStr, OsStr}; +use super::mystd::io::Error; +use super::mystd::os::unix::prelude::*; +use super::xcoff; +use super::{Library, LibrarySegment, Vec}; +use alloc::vec; +use core::mem; + +const EXE_IMAGE_BASE: u64 = 0x100000000; + +/// On AIX, we use `loadquery` with `L_GETINFO` flag to query libraries mmapped. +/// See https://www.ibm.com/docs/en/aix/7.2?topic=l-loadquery-subroutine for +/// detailed information of `loadquery`. +pub(super) fn native_libraries() -> Vec { + let mut ret = Vec::new(); + unsafe { + let mut buffer = vec![mem::zeroed::(); 64]; + loop { + if libc::loadquery( + libc::L_GETINFO, + buffer.as_mut_ptr() as *mut libc::c_char, + (mem::size_of::() * buffer.len()) as u32, + ) != -1 + { + break; + } else { + match Error::last_os_error().raw_os_error() { + Some(libc::ENOMEM) => { + buffer.resize(buffer.len() * 2, mem::zeroed::()); + } + Some(_) => { + // If other error occurs, return empty libraries. + return Vec::new(); + } + _ => unreachable!(), + } + } + } + let mut current = buffer.as_mut_ptr(); + loop { + let text_base = (*current).ldinfo_textorg as usize; + let filename_ptr: *const libc::c_char = &(*current).ldinfo_filename[0]; + let bytes = CStr::from_ptr(filename_ptr).to_bytes(); + let member_name_ptr = filename_ptr.offset((bytes.len() + 1) as isize); + let mut filename = OsStr::from_bytes(bytes).to_owned(); + if text_base == EXE_IMAGE_BASE as usize { + if let Ok(exe) = env::current_exe() { + filename = exe.into_os_string(); + } + } + let bytes = CStr::from_ptr(member_name_ptr).to_bytes(); + let member_name = OsStr::from_bytes(bytes).to_owned(); + if let Some(image) = xcoff::parse_image(filename.as_ref(), &member_name) { + ret.push(Library { + name: filename, + member_name, + segments: vec![LibrarySegment { + stated_virtual_memory_address: image.base as usize, + len: image.size, + }], + bias: (text_base + image.offset).wrapping_sub(image.base as usize), + }); + } + if (*current).ldinfo_next == 0 { + break; + } + current = (current as *mut libc::c_char).offset((*current).ldinfo_next as isize) + as *mut libc::ld_info; + } + } + return ret; +} diff --git a/src/symbolize/gimli/xcoff.rs b/src/symbolize/gimli/xcoff.rs new file mode 100644 index 000000000..dd308840f --- /dev/null +++ b/src/symbolize/gimli/xcoff.rs @@ -0,0 +1,186 @@ +use super::mystd::ffi::{OsStr, OsString}; +use super::mystd::os::unix::ffi::OsStrExt; +use super::mystd::str; +use super::{gimli, Context, Endian, EndianSlice, Mapping, Path, Stash, Vec}; +use alloc::sync::Arc; +use core::ops::Deref; +use object::read::archive::ArchiveFile; +use object::read::xcoff::{FileHeader, SectionHeader, XcoffFile, XcoffSymbol}; +use object::Object as _; +use object::ObjectSection as _; +use object::ObjectSymbol as _; +use object::SymbolFlags; + +#[cfg(target_pointer_width = "32")] +type Xcoff = object::xcoff::FileHeader32; +#[cfg(target_pointer_width = "64")] +type Xcoff = object::xcoff::FileHeader64; + +impl Mapping { + pub fn new(path: &Path, member_name: &OsString) -> Option { + let map = super::mmap(path)?; + Mapping::mk(map, |data, stash| { + if member_name.is_empty() { + Context::new(stash, Object::parse(data)?, None, None) + } else { + let archive = ArchiveFile::parse(data).ok()?; + for member in archive + .members() + .filter_map(|m| m.ok()) + .filter(|m| OsStr::from_bytes(m.name()) == member_name) + { + let member_data = member.data(data).ok()?; + if let Some(obj) = Object::parse(member_data) { + return Context::new(stash, obj, None, None); + } + } + None + } + }) + } +} + +struct ParsedSym<'a> { + address: u64, + size: u64, + name: &'a str, +} + +pub struct Object<'a> { + syms: Vec>, + file: XcoffFile<'a, Xcoff>, +} + +pub struct Image { + pub offset: usize, + pub base: u64, + pub size: usize, +} + +pub fn parse_xcoff(data: &[u8]) -> Option { + let mut offset = 0; + let header = Xcoff::parse(data, &mut offset).ok()?; + let _ = header.aux_header(data, &mut offset).ok()?; + let sections = header.sections(data, &mut offset).ok()?; + if let Some(section) = sections.iter().find(|s| { + if let Ok(name) = str::from_utf8(&s.s_name()[0..5]) { + name == ".text" + } else { + false + } + }) { + Some(Image { + offset: section.s_scnptr() as usize, + base: section.s_paddr() as u64, + size: section.s_size() as usize, + }) + } else { + None + } +} + +pub fn parse_image(path: &Path, member_name: &OsString) -> Option { + let map = super::mmap(path)?; + let data = map.deref(); + if member_name.is_empty() { + return parse_xcoff(data); + } else { + let archive = ArchiveFile::parse(data).ok()?; + for member in archive + .members() + .filter_map(|m| m.ok()) + .filter(|m| OsStr::from_bytes(m.name()) == member_name) + { + let member_data = member.data(data).ok()?; + if let Some(image) = parse_xcoff(member_data) { + return Some(image); + } + } + None + } +} + +impl<'a> Object<'a> { + fn get_concrete_size(file: &XcoffFile<'a, Xcoff>, sym: &XcoffSymbol<'a, '_, Xcoff>) -> u64 { + match sym.flags() { + SymbolFlags::Xcoff { + n_sclass: _, + x_smtyp: _, + x_smclas: _, + containing_csect: Some(index), + } => { + if let Ok(tgt_sym) = file.symbol_by_index(index) { + Self::get_concrete_size(file, &tgt_sym) + } else { + 0 + } + } + _ => sym.size(), + } + } + + fn parse(data: &'a [u8]) -> Option> { + let file = XcoffFile::parse(data).ok()?; + let mut syms = file + .symbols() + .filter_map(|sym| { + let name = sym.name().map_or("", |v| v); + let address = sym.address(); + let size = Self::get_concrete_size(&file, &sym); + if name == ".text" || name == ".data" { + // We don't want to include ".text" and ".data" symbols. + // If they are included, since their ranges cover other + // symbols, when searching a symbol for a given address, + // ".text" or ".data" is returned. That's not what we expect. + None + } else { + Some(ParsedSym { + address, + size, + name, + }) + } + }) + .collect::>(); + syms.sort_by_key(|s| s.address); + Some(Object { syms, file }) + } + + pub fn section(&self, _: &Stash, name: &str) -> Option<&'a [u8]> { + Some(self.file.section_by_name(name)?.data().ok()?) + } + + pub fn search_symtab<'b>(&'b self, addr: u64) -> Option<&'b [u8]> { + // Symbols, except ".text" and ".data", are sorted and are not overlapped each other, + // so we can just perform a binary search here. + let i = match self.syms.binary_search_by_key(&addr, |sym| sym.address) { + Ok(i) => i, + Err(i) => i.checked_sub(1)?, + }; + let sym = self.syms.get(i)?; + if (sym.address..sym.address + sym.size).contains(&addr) { + // On AIX, for a function call, for example, `foo()`, we have + // two symbols `foo` and `.foo`. `foo` references the function + // descriptor and `.foo` references the function entry. + // See https://www.ibm.com/docs/en/xl-fortran-aix/16.1.0?topic=calls-linkage-convention-function + // for more information. + // We trim the prefix `.` here, so that the rust demangler can work + // properly. + Some(sym.name.trim_start_matches(".").as_bytes()) + } else { + None + } + } + + pub(super) fn search_object_map(&self, _addr: u64) -> Option<(&Context<'_>, u64)> { + None + } +} + +pub(super) fn handle_split_dwarf<'data>( + _package: Option<&gimli::DwarfPackage>>, + _stash: &'data Stash, + _load: addr2line::SplitDwarfLoad>, +) -> Option>>> { + None +} diff --git a/tests/accuracy/main.rs b/tests/accuracy/main.rs index 149203a1b..fbf23c5a7 100644 --- a/tests/accuracy/main.rs +++ b/tests/accuracy/main.rs @@ -31,6 +31,8 @@ fn doit() { dir.push("dylib_dep.dll"); } else if cfg!(target_os = "macos") { dir.push("libdylib_dep.dylib"); + } else if cfg!(target_os = "aix") { + dir.push("libdylib_dep.a"); } else { dir.push("libdylib_dep.so"); } diff --git a/tests/smoke.rs b/tests/smoke.rs index 683a6f0db..4b1839e7b 100644 --- a/tests/smoke.rs +++ b/tests/smoke.rs @@ -1,6 +1,27 @@ use backtrace::Frame; use std::thread; +fn get_actual_fn_pointer(fp: usize) -> usize { + // On AIX, the function name references a function descriptor. + // A function descriptor consists of (See https://reviews.llvm.org/D62532) + // * The address of the entry point of the function. + // * The TOC base address for the function. + // * The environment pointer. + // Deref `fp` directly so that we can get the address of `fp`'s + // entry point in text section. + // + // For TOC, one can find more information in + // https://www.ibm.com/docs/en/aix/7.2?topic=program-understanding-programming-toc + if cfg!(target_os = "aix") { + unsafe { + let actual_fn_entry = *(fp as *const usize); + actual_fn_entry + } + } else { + fp + } +} + #[test] // FIXME: shouldn't ignore this test on i686-msvc, unsure why it's failing #[cfg_attr(all(target_arch = "x86", target_env = "msvc"), ignore)] @@ -20,7 +41,7 @@ fn smoke_test_frames() { // Various platforms have various bits of weirdness about their // backtraces. To find a good starting spot let's search through the // frames - let target = frame_4 as usize; + let target = get_actual_fn_pointer(frame_4 as usize); let offset = v .iter() .map(|frame| frame.symbol_address() as usize) @@ -39,7 +60,7 @@ fn smoke_test_frames() { assert_frame( frames.next().unwrap(), - frame_4 as usize, + get_actual_fn_pointer(frame_4 as usize), "frame_4", "tests/smoke.rs", start_line + 6, @@ -47,7 +68,7 @@ fn smoke_test_frames() { ); assert_frame( frames.next().unwrap(), - frame_3 as usize, + get_actual_fn_pointer(frame_3 as usize), "frame_3", "tests/smoke.rs", start_line + 3, @@ -55,7 +76,7 @@ fn smoke_test_frames() { ); assert_frame( frames.next().unwrap(), - frame_2 as usize, + get_actual_fn_pointer(frame_2 as usize), "frame_2", "tests/smoke.rs", start_line + 2, @@ -63,7 +84,7 @@ fn smoke_test_frames() { ); assert_frame( frames.next().unwrap(), - frame_1 as usize, + get_actual_fn_pointer(frame_1 as usize), "frame_1", "tests/smoke.rs", start_line + 1, @@ -71,7 +92,7 @@ fn smoke_test_frames() { ); assert_frame( frames.next().unwrap(), - smoke_test_frames as usize, + get_actual_fn_pointer(smoke_test_frames as usize), "smoke_test_frames", "", 0, From ebfc57e94693c7842dc8ea8dd61196734c679061 Mon Sep 17 00:00:00 2001 From: Jason Heeris Date: Sat, 8 Jul 2023 09:51:57 +0800 Subject: [PATCH 05/42] Modularise CI workflow and validate outputs for binary size checks. --- .../actions/build-with-patched-std/action.yml | 48 +++++++++ .github/workflows/check-binary-size.yml | 97 +++++++++++++------ 2 files changed, 117 insertions(+), 28 deletions(-) create mode 100644 .github/actions/build-with-patched-std/action.yml diff --git a/.github/actions/build-with-patched-std/action.yml b/.github/actions/build-with-patched-std/action.yml new file mode 100644 index 000000000..5466a2289 --- /dev/null +++ b/.github/actions/build-with-patched-std/action.yml @@ -0,0 +1,48 @@ +# Github composite action to build a single-source-file test binary with an +# already-checked-out version of Rust's stdlib, that will be patched with a +# given revision of the backtrace crate. + +name: Build with patched std +description: > + Build a binary with a version of std that's had a specific revision of + backtrace patched in. +inputs: + backtrace-commit: + description: The git commit of backtrace to patch in to std + required: true + main-rs: + description: The (single) source code file to compile + required: true + rustc-dir: + description: The root directory of the rustc repo + required: true +outputs: + test-binary-size: + description: The size in bytes of the built test binary + value: ${{ steps.measure.outputs.test-binary-size }} +runs: + using: composite + steps: + - shell: bash + id: measure + env: + RUSTC_FLAGS: -Copt-level=3 -Cstrip=symbols + # This symlink is made by Build::new() in the bootstrap crate, using a + # symlink on Linux and a junction on Windows, so it will exist on both + # platforms. + RUSTC_BUILD_DIR: build/host + working-directory: ${{ inputs.rustc-dir }} + run: | + rm -rf "$RUSTC_BUILD_DIR/stage0-std" + + (cd library/backtrace && git checkout ${{ inputs.backtrace-commit }}) + git add library/backtrace + + python3 x.py build library --stage 0 + + TEMP_BUILD_OUTPUT=$(mktemp test-binary-XXXXXXXX) + "$RUSTC_BUILD_DIR/stage0-sysroot/bin/rustc" $RUSTC_FLAGS "${{ inputs.main-rs }}" -o "$TEMP_BUILD_OUTPUT" + BINARY_SIZE=$(stat -c '%s' "$TEMP_BUILD_OUTPUT") + rm "$TEMP_BUILD_OUTPUT" + + echo "test-binary-size=$BINARY_SIZE" >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/check-binary-size.yml b/.github/workflows/check-binary-size.yml index 0beae1da9..f01583afb 100644 --- a/.github/workflows/check-binary-size.yml +++ b/.github/workflows/check-binary-size.yml @@ -15,51 +15,92 @@ jobs: runs-on: ubuntu-latest permissions: pull-requests: write + env: + # This cannot be used as a context variable in the 'uses' key later. If it + # changes, update those steps too. + BACKTRACE_DIR: backtrace + RUSTC_DIR: rustc + TEST_MAIN_RS: foo.rs + BASE_COMMIT: ${{ github.event.pull_request.base.sha }} + HEAD_COMMIT: ${{ github.event.pull_request.head.sha }} steps: - name: Print info + shell: bash run: | - echo "Current SHA: ${{ github.event.pull_request.head.sha }}" - echo "Base SHA: ${{ github.event.pull_request.base.sha }}" + echo "Current SHA: $HEAD_COMMIT" + echo "Base SHA: $BASE_COMMIT" + # Note: the backtrace source that's cloned here is NOT the version to be + # patched in to std. It's cloned here to access the Github action for + # building the test binary and measuring its size. + - name: Clone backtrace to access Github action + uses: actions/checkout@v3 + with: + path: ${{ env.BACKTRACE_DIR }} - name: Clone Rustc uses: actions/checkout@v3 with: repository: rust-lang/rust - fetch-depth: 1 - - name: Fetch backtrace - run: git submodule update --init library/backtrace - - name: Create hello world program that uses backtrace - run: printf "fn main() { panic!(); }" > foo.rs - - name: Build binary with base version of backtrace + path: ${{ env.RUSTC_DIR }} + - name: Set up std repository and backtrace submodule for size test + shell: bash + working-directory: ${{ env.RUSTC_DIR }} + env: + PR_SOURCE_REPO: ${{ github.event.pull_request.head.repo.full_name }} run: | - printf "[llvm]\ndownload-ci-llvm = true\n\n[rust]\nincremental = false\n" > config.toml + # Bootstrap config + cat < config.toml + [llvm] + download-ci-llvm = true + [rust] + incremental = false + EOF + + # Test program source + cat < $TEST_MAIN_RS + fn main() { + panic!(); + } + EOF + + git submodule update --init library/backtrace + cd library/backtrace - git remote add head-pr https://github.com/${{ github.event.pull_request.head.repo.full_name }} + git remote add head-pr "https://github.com/$PR_SOURCE_REPO" git fetch --all - git checkout ${{ github.event.pull_request.base.sha }} - cd ../.. - git add library/backtrace - python3 x.py build library --stage 0 - ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O foo.rs -o binary-reference + - name: Build binary with base version of backtrace + uses: ./backtrace/.github/actions/build-with-patched-std + with: + backtrace-commit: ${{ env.BASE_COMMIT }} + main-rs: ${{ env.TEST_MAIN_RS }} + rustc-dir: ${{ env.RUSTC_DIR }} + id: size-reference - name: Build binary with PR version of backtrace - run: | - cd library/backtrace - git checkout ${{ github.event.pull_request.head.sha }} - cd ../.. - git add library/backtrace - rm -rf build/x86_64-unknown-linux-gnu/stage0-std - python3 x.py build library --stage 0 - ./build/x86_64-unknown-linux-gnu/stage0-sysroot/bin/rustc -O foo.rs -o binary-updated - - name: Display binary size - run: | - ls -la binary-* - echo "SIZE_REFERENCE=$(stat -c '%s' binary-reference)" >> "$GITHUB_ENV" - echo "SIZE_UPDATED=$(stat -c '%s' binary-updated)" >> "$GITHUB_ENV" + uses: ./backtrace/.github/actions/build-with-patched-std + with: + backtrace-commit: ${{ env.HEAD_COMMIT }} + main-rs: ${{ env.TEST_MAIN_RS }} + rustc-dir: ${{ env.RUSTC_DIR }} + id: size-updated - name: Post a PR comment if the size has changed uses: actions/github-script@v6 + env: + SIZE_REFERENCE: ${{ steps.size-reference.outputs.test-binary-size }} + SIZE_UPDATED: ${{ steps.size-updated.outputs.test-binary-size }} with: script: | const reference = process.env.SIZE_REFERENCE; const updated = process.env.SIZE_UPDATED; + + if (!(reference > 0)) { + core.setFailed(`Reference size invalid: ${reference}`); + return; + } + + if (!(updated > 0)) { + core.setFailed(`Updated size invalid: ${updated}`); + return; + } + const diff = updated - reference; const plus = diff > 0 ? "+" : ""; const diff_str = `${plus}${diff}B`; From 6a68ad8e37ff64bc8b57a3592efbc29238ee2294 Mon Sep 17 00:00:00 2001 From: Jason Heeris Date: Tue, 18 Jul 2023 09:36:43 +0800 Subject: [PATCH 06/42] Split the CI code size measurement job into two jobs: one with read-only permission to build (and potentially run) untrusted code, and another with PR-write permission that only reports the sizes from the first job. --- .github/workflows/check-binary-size.yml | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/.github/workflows/check-binary-size.yml b/.github/workflows/check-binary-size.yml index f01583afb..485204d23 100644 --- a/.github/workflows/check-binary-size.yml +++ b/.github/workflows/check-binary-size.yml @@ -9,12 +9,15 @@ on: branches: - master +# Responsibility is divided between two jobs "measure" and "report", so that the +# job that builds (and potentnially runs) untrusted code does not have PR write +# permission, and vice-versa. jobs: - test: + measure: name: Check binary size runs-on: ubuntu-latest permissions: - pull-requests: write + contents: read env: # This cannot be used as a context variable in the 'uses' key later. If it # changes, update those steps too. @@ -23,6 +26,9 @@ jobs: TEST_MAIN_RS: foo.rs BASE_COMMIT: ${{ github.event.pull_request.base.sha }} HEAD_COMMIT: ${{ github.event.pull_request.head.sha }} + outputs: + binary-size-reference: ${{ steps.size-reference.outputs.test-binary-size }} + binary-size-updated: ${{ steps.size-updated.outputs.test-binary-size }} steps: - name: Print info shell: bash @@ -81,11 +87,18 @@ jobs: main-rs: ${{ env.TEST_MAIN_RS }} rustc-dir: ${{ env.RUSTC_DIR }} id: size-updated + report: + name: Report binary size changes + runs-on: ubuntu-latest + needs: measure + permissions: + pull-requests: write + steps: - name: Post a PR comment if the size has changed uses: actions/github-script@v6 env: - SIZE_REFERENCE: ${{ steps.size-reference.outputs.test-binary-size }} - SIZE_UPDATED: ${{ steps.size-updated.outputs.test-binary-size }} + SIZE_REFERENCE: ${{ needs.measure.outputs.binary-size-reference }} + SIZE_UPDATED: ${{ needs.measure.outputs.binary-size-updated }} with: script: | const reference = process.env.SIZE_REFERENCE; From 54e4ec8b0d7e7d231e9a95eff355188bb3537411 Mon Sep 17 00:00:00 2001 From: Jason Heeris Date: Sun, 27 Aug 2023 10:36:35 +0800 Subject: [PATCH 07/42] Move size reporting job into separate composite workflow. --- .../report-code-size-changes/action.yml | 56 +++++++++++++++++++ .github/workflows/check-binary-size.yml | 46 ++------------- 2 files changed, 62 insertions(+), 40 deletions(-) create mode 100644 .github/actions/report-code-size-changes/action.yml diff --git a/.github/actions/report-code-size-changes/action.yml b/.github/actions/report-code-size-changes/action.yml new file mode 100644 index 000000000..6d71feab2 --- /dev/null +++ b/.github/actions/report-code-size-changes/action.yml @@ -0,0 +1,56 @@ +# Github composite action to report on code size changes +name: Report binary size changes on PR +description: | + Report on code size changes resulting from a PR as a comment on the PR + (accessed via context). +inputs: + reference: + description: The size in bytes of the reference binary (base of PR). + required: true + updated: + description: The size in bytes of the updated binary (head of PR). + required: true +runs: + using: composite + steps: + - name: Post a PR comment if the size has changed + uses: actions/github-script@v6 + env: + SIZE_REFERENCE: ${{ inputs.reference }} + SIZE_UPDATED: ${{ inputs.updated }} + with: + script: | + const reference = process.env.SIZE_REFERENCE; + const updated = process.env.SIZE_UPDATED; + + if (!(reference > 0)) { + core.setFailed(`Reference size invalid: ${reference}`); + return; + } + + if (!(updated > 0)) { + core.setFailed(`Updated size invalid: ${updated}`); + return; + } + + const diff = updated - reference; + const plus = diff > 0 ? "+" : ""; + const diff_str = `${plus}${diff}B`; + + if (diff !== 0) { + const percent = (((updated / reference) - 1) * 100).toFixed(2); + // The body is created here and wrapped so "weirdly" to avoid whitespace at the start of the lines, + // which is interpreted as a code block by Markdown. + const body = `Below is the size of a hello-world Rust program linked with libstd with backtrace. + + Original binary size: **${reference}B** + Updated binary size: **${updated}B** + Difference: **${diff_str}** (${percent}%)`; + + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body + }) + } diff --git a/.github/workflows/check-binary-size.yml b/.github/workflows/check-binary-size.yml index 485204d23..35dc99a63 100644 --- a/.github/workflows/check-binary-size.yml +++ b/.github/workflows/check-binary-size.yml @@ -94,44 +94,10 @@ jobs: permissions: pull-requests: write steps: - - name: Post a PR comment if the size has changed - uses: actions/github-script@v6 - env: - SIZE_REFERENCE: ${{ needs.measure.outputs.binary-size-reference }} - SIZE_UPDATED: ${{ needs.measure.outputs.binary-size-updated }} + # Clone backtrace to access Github composite actions to report size. + - uses: actions/checkout@v3 + # Run the size reporting action. + - uses: ./.github/actions/report-code-size-changes with: - script: | - const reference = process.env.SIZE_REFERENCE; - const updated = process.env.SIZE_UPDATED; - - if (!(reference > 0)) { - core.setFailed(`Reference size invalid: ${reference}`); - return; - } - - if (!(updated > 0)) { - core.setFailed(`Updated size invalid: ${updated}`); - return; - } - - const diff = updated - reference; - const plus = diff > 0 ? "+" : ""; - const diff_str = `${plus}${diff}B`; - - if (diff !== 0) { - const percent = (((updated / reference) - 1) * 100).toFixed(2); - // The body is created here and wrapped so "weirdly" to avoid whitespace at the start of the lines, - // which is interpreted as a code block by Markdown. - const body = `Below is the size of a hello-world Rust program linked with libstd with backtrace. - - Original binary size: **${reference}B** - Updated binary size: **${updated}B** - Difference: **${diff_str}** (${percent}%)`; - - github.rest.issues.createComment({ - issue_number: context.issue.number, - owner: context.repo.owner, - repo: context.repo.repo, - body - }) - } + reference: ${{ needs.measure.outputs.binary-size-reference }} + updated: ${{ needs.measure.outputs.binary-size-updated }} From 17fbabb571f9f8f4308b90be238fa995b3681884 Mon Sep 17 00:00:00 2001 From: Jason Heeris Date: Wed, 19 Jul 2023 13:46:47 +0800 Subject: [PATCH 08/42] Format code size differences with grouping and unit display. --- .../report-code-size-changes/action.yml | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/.github/actions/report-code-size-changes/action.yml b/.github/actions/report-code-size-changes/action.yml index 6d71feab2..b029aa025 100644 --- a/.github/actions/report-code-size-changes/action.yml +++ b/.github/actions/report-code-size-changes/action.yml @@ -33,19 +33,34 @@ runs: return; } + const formatter = Intl.NumberFormat("en", {useGrouping: "always"}); + + const updated_str = formatter.format(updated); + const reference_str = formatter.format(reference); + const diff = updated - reference; - const plus = diff > 0 ? "+" : ""; - const diff_str = `${plus}${diff}B`; + const diff_pct = (updated / reference) - 1; + + const diff_str = Intl.NumberFormat("en", { + useGrouping: "always", + sign: "exceptZero" + }).format(diff); + + const diff_pct_str = Intl.NumberFormat("en", { + style: "percent", + useGrouping: "always", + sign: "exceptZero", + maximumFractionDigits: 2 + }).format(diff_pct); if (diff !== 0) { - const percent = (((updated / reference) - 1) * 100).toFixed(2); // The body is created here and wrapped so "weirdly" to avoid whitespace at the start of the lines, // which is interpreted as a code block by Markdown. const body = `Below is the size of a hello-world Rust program linked with libstd with backtrace. - Original binary size: **${reference}B** - Updated binary size: **${updated}B** - Difference: **${diff_str}** (${percent}%)`; + Original binary size: **${reference_str} B** + Updated binary size: **${updated_str} B** + Difference: **${diff_str} B** (${diff_pct_str})`; github.rest.issues.createComment({ issue_number: context.issue.number, From 43c3ecb414d4de731a9f68c3793970c68938143c Mon Sep 17 00:00:00 2001 From: Jason Heeris Date: Thu, 20 Jul 2023 13:24:29 +0800 Subject: [PATCH 09/42] Use matrix strategy to measure size data on multiple platforms. This involves writing the size measurements to separate JSON files (one per matrix job) and retaining them as artifacts, as there's no built-in way to collect all matrix job outputs into a subsequent job's inputs. --- .../report-code-size-changes/action.yml | 126 ++++++++++++------ .github/workflows/check-binary-size.yml | 66 +++++++-- 2 files changed, 140 insertions(+), 52 deletions(-) diff --git a/.github/actions/report-code-size-changes/action.yml b/.github/actions/report-code-size-changes/action.yml index b029aa025..ede26975a 100644 --- a/.github/actions/report-code-size-changes/action.yml +++ b/.github/actions/report-code-size-changes/action.yml @@ -1,14 +1,21 @@ -# Github composite action to report on code size changes +# Github composite action to report on code size changes across different +# platforms. + name: Report binary size changes on PR description: | - Report on code size changes resulting from a PR as a comment on the PR - (accessed via context). + Report on code size changes across different platforms resulting from a PR. + The only input argument is the path to a directory containing a set of + "*.json" files (extension required), each file containing the keys: + + - platform: the platform that the code size change was measured on + - reference: the size in bytes of the reference binary (base of PR) + - updated: the size in bytes of the updated binary (head of PR) + + The size is reported as a comment on the PR (accessed via context). inputs: - reference: - description: The size in bytes of the reference binary (base of PR). - required: true - updated: - description: The size in bytes of the updated binary (head of PR). + data-directory: + description: > + Path to directory containing size data as a set of "*.json" files. required: true runs: using: composite @@ -16,56 +23,89 @@ runs: - name: Post a PR comment if the size has changed uses: actions/github-script@v6 env: - SIZE_REFERENCE: ${{ inputs.reference }} - SIZE_UPDATED: ${{ inputs.updated }} + DATA_DIRECTORY: ${{ inputs.data-directory }} with: script: | - const reference = process.env.SIZE_REFERENCE; - const updated = process.env.SIZE_UPDATED; + const fs = require("fs"); - if (!(reference > 0)) { - core.setFailed(`Reference size invalid: ${reference}`); - return; - } + const size_dir = process.env.DATA_DIRECTORY; - if (!(updated > 0)) { - core.setFailed(`Updated size invalid: ${updated}`); - return; - } + // Map the set of all the *.json files into an array of objects. + const globber = await glob.create(`${size_dir}/*.json`); + const files = await globber.glob(); + const sizes = files.map(path => { + const contents = fs.readFileSync(path); + return JSON.parse(contents); + }); + + // Map each object into some text, but only if it shows any difference + // to report. + const size_reports = sizes.flatMap(size_data => { + const platform = size_data["platform"]; + const reference = size_data["reference"]; + const updated = size_data["updated"]; + + if (!(reference > 0)) { + core.setFailed(`Reference size invalid: ${reference}`); + return; + } + + if (!(updated > 0)) { + core.setFailed(`Updated size invalid: ${updated}`); + return; + } + + const formatter = Intl.NumberFormat("en", { + useGrouping: "always" + }); + + const updated_str = formatter.format(updated); + const reference_str = formatter.format(reference); + + const diff = updated - reference; + const diff_pct = (updated / reference) - 1; + + const diff_str = Intl.NumberFormat("en", { + useGrouping: "always", + sign: "exceptZero" + }).format(diff); - const formatter = Intl.NumberFormat("en", {useGrouping: "always"}); + const diff_pct_str = Intl.NumberFormat("en", { + style: "percent", + useGrouping: "always", + sign: "exceptZero", + maximumFractionDigits: 2 + }).format(diff_pct); - const updated_str = formatter.format(updated); - const reference_str = formatter.format(reference); + if (diff !== 0) { + // The body is created here and wrapped so "weirdly" to avoid whitespace at the start of the lines, + // which is interpreted as a code block by Markdown. + const report = `On platform \`${platform}\`: - const diff = updated - reference; - const diff_pct = (updated / reference) - 1; + - Original binary size: **${reference_str} B** + - Updated binary size: **${updated_str} B** + - Difference: **${diff_str} B** (${diff_pct_str}) - const diff_str = Intl.NumberFormat("en", { - useGrouping: "always", - sign: "exceptZero" - }).format(diff); + `; - const diff_pct_str = Intl.NumberFormat("en", { - style: "percent", - useGrouping: "always", - sign: "exceptZero", - maximumFractionDigits: 2 - }).format(diff_pct); + return [report]; + } else { + return []; + } + }); - if (diff !== 0) { - // The body is created here and wrapped so "weirdly" to avoid whitespace at the start of the lines, - // which is interpreted as a code block by Markdown. - const body = `Below is the size of a hello-world Rust program linked with libstd with backtrace. + // If there are any size changes to report, format a comment and post + // it. + if (size_reports.length > 0) { + const comment_sizes = size_reports.join(""); + const body = `Code size changes for a hello-world Rust program linked with libstd with backtrace: - Original binary size: **${reference_str} B** - Updated binary size: **${updated_str} B** - Difference: **${diff_str} B** (${diff_pct_str})`; + ${comment_sizes}`; github.rest.issues.createComment({ issue_number: context.issue.number, owner: context.repo.owner, repo: context.repo.repo, body - }) + }); } diff --git a/.github/workflows/check-binary-size.yml b/.github/workflows/check-binary-size.yml index 35dc99a63..d045fb7b3 100644 --- a/.github/workflows/check-binary-size.yml +++ b/.github/workflows/check-binary-size.yml @@ -9,13 +9,20 @@ on: branches: - master +# Both the "measure" and "report" jobs need to know this. +env: + SIZE_DATA_DIR: sizes + # Responsibility is divided between two jobs "measure" and "report", so that the # job that builds (and potentnially runs) untrusted code does not have PR write # permission, and vice-versa. jobs: measure: name: Check binary size - runs-on: ubuntu-latest + strategy: + matrix: + platform: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.platform }} permissions: contents: read env: @@ -26,9 +33,7 @@ jobs: TEST_MAIN_RS: foo.rs BASE_COMMIT: ${{ github.event.pull_request.base.sha }} HEAD_COMMIT: ${{ github.event.pull_request.head.sha }} - outputs: - binary-size-reference: ${{ steps.size-reference.outputs.test-binary-size }} - binary-size-updated: ${{ steps.size-updated.outputs.test-binary-size }} + SIZE_DATA_FILE: size-${{ strategy.job-index }}.json steps: - name: Print info shell: bash @@ -37,7 +42,7 @@ jobs: echo "Base SHA: $BASE_COMMIT" # Note: the backtrace source that's cloned here is NOT the version to be # patched in to std. It's cloned here to access the Github action for - # building the test binary and measuring its size. + # building and measuring the test binary. - name: Clone backtrace to access Github action uses: actions/checkout@v3 with: @@ -87,6 +92,45 @@ jobs: main-rs: ${{ env.TEST_MAIN_RS }} rustc-dir: ${{ env.RUSTC_DIR }} id: size-updated + # There is no built-in way to "collect" all the outputs of a set of jobs + # run with a matrix strategy. Subsequent jobs that have a "needs" + # dependency on this one will be run once, when the last matrix job is + # run. Appending data to a single file within a matrix is subject to race + # conditions. So we write the size data to files with distinct names + # generated from the job index. + - name: Write sizes to file + uses: actions/github-script@v6 + env: + SIZE_REFERENCE: ${{ steps.size-reference.outputs.test-binary-size }} + SIZE_UPDATED: ${{ steps.size-updated.outputs.test-binary-size }} + PLATFORM: ${{ matrix.platform }} + with: + script: | + const fs = require("fs"); + const path = require("path"); + + fs.mkdirSync(process.env.SIZE_DATA_DIR, {recursive: true}); + + const output_data = JSON.stringify({ + platform: process.env.PLATFORM, + reference: process.env.SIZE_REFERENCE, + updated: process.env.SIZE_UPDATED, + }); + + // The "wx" flag makes this fail if the file exists, which we want, + // because there should be no collisions. + fs.writeFileSync( + path.join(process.env.SIZE_DATA_DIR, process.env.SIZE_DATA_FILE), + output_data, + { flag: "wx" }, + ); + - name: Upload size data + uses: actions/upload-artifact@v3 + with: + name: size-files + path: ${{ env.SIZE_DATA_DIR }}/${{ env.SIZE_DATA_FILE }} + retention-days: 1 + if-no-files-found: error report: name: Report binary size changes runs-on: ubuntu-latest @@ -96,8 +140,12 @@ jobs: steps: # Clone backtrace to access Github composite actions to report size. - uses: actions/checkout@v3 - # Run the size reporting action. - - uses: ./.github/actions/report-code-size-changes + - name: Download size data + uses: actions/download-artifact@v3 + with: + name: size-files + path: ${{ env.SIZE_DATA_DIR }} + - name: Analyze and report size changes + uses: ./.github/actions/report-code-size-changes with: - reference: ${{ needs.measure.outputs.binary-size-reference }} - updated: ${{ needs.measure.outputs.binary-size-updated }} + data-directory: ${{ env.SIZE_DATA_DIR }} From eb44677cf4e40c89de12d88647c4a911be06dce5 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sun, 3 Sep 2023 17:32:16 -0700 Subject: [PATCH 10/42] Simplify version supremacy clause --- README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index f4c7ad257..f090d0ca0 100644 --- a/README.md +++ b/README.md @@ -65,9 +65,7 @@ Thus `backtrace` is likely to use recent Rust features or depend on a library which itself uses them. Its minimum supported Rust version, by policy, is within a few versions of current stable, approximately "stable - 2". -This policy takes precedence over any Rust version stated anywhere else: -the Cargo.toml `rust-version` may reflect what actually builds in CI instead. -It is not a guarantee the version remains as-is from release to release. +This policy takes precedence over versions written anywhere else in this repo. # License From df3464661bfd651f1046bd732041b821aea16f79 Mon Sep 17 00:00:00 2001 From: Mohsen Zohrevandi Date: Thu, 21 Sep 2023 13:19:02 -0700 Subject: [PATCH 11/42] Adjust frame IP in SGX relative to image base --- src/backtrace/libunwind.rs | 13 ++++++++- src/backtrace/mod.rs | 31 +++++++++++++++++++++ src/lib.rs | 6 ++++ src/print.rs | 11 +------- tests/sgx-image-base.rs | 56 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 106 insertions(+), 11 deletions(-) create mode 100644 tests/sgx-image-base.rs diff --git a/src/backtrace/libunwind.rs b/src/backtrace/libunwind.rs index aefa8b094..0cf6365f7 100644 --- a/src/backtrace/libunwind.rs +++ b/src/backtrace/libunwind.rs @@ -40,7 +40,18 @@ impl Frame { Frame::Raw(ctx) => ctx, Frame::Cloned { ip, .. } => return ip, }; - unsafe { uw::_Unwind_GetIP(ctx) as *mut c_void } + #[allow(unused_mut)] + let mut ip = unsafe { uw::_Unwind_GetIP(ctx) as *mut c_void }; + + // To reduce TCB size in SGX enclaves, we do not want to implement + // symbol resolution functionality. Rather, we can print the offset of + // the address here, which could be later mapped to correct function. + #[cfg(all(target_env = "sgx", target_vendor = "fortanix"))] + { + let image_base = super::get_image_base(); + ip = usize::wrapping_sub(ip as usize, image_base as _) as _; + } + ip } pub fn sp(&self) -> *mut c_void { diff --git a/src/backtrace/mod.rs b/src/backtrace/mod.rs index 6ca1080c4..c414d0514 100644 --- a/src/backtrace/mod.rs +++ b/src/backtrace/mod.rs @@ -125,6 +125,37 @@ impl fmt::Debug for Frame { } } +#[cfg(all(target_env = "sgx", target_vendor = "fortanix", not(feature = "std")))] +mod sgx_no_std_image_base { + use core::sync::atomic::{AtomicU64, Ordering::SeqCst}; + + static IMAGE_BASE: AtomicU64 = AtomicU64::new(0); + + /// Set the image base address. This is only available for Fortanix SGX + /// target when the `std` feature is not enabled. This can be used in the + /// standard library to set the correct base address. + pub fn set_image_base(base_addr: u64) { + IMAGE_BASE.store(base_addr, SeqCst); + } + + pub(crate) fn get_image_base() -> u64 { + IMAGE_BASE.load(SeqCst) + } +} + +#[cfg(all(target_env = "sgx", target_vendor = "fortanix", not(feature = "std")))] +pub use self::sgx_no_std_image_base::set_image_base; + +#[cfg(all(target_env = "sgx", target_vendor = "fortanix", not(feature = "std")))] +#[deny(unused)] +pub(crate) use self::sgx_no_std_image_base::get_image_base; + +#[cfg(all(target_env = "sgx", target_vendor = "fortanix", feature = "std"))] +#[deny(unused)] +pub(crate) fn get_image_base() -> u64 { + std::os::fortanix_sgx::mem::image_base() +} + cfg_if::cfg_if! { // This needs to come first, to ensure that // Miri takes priority over the host platform diff --git a/src/lib.rs b/src/lib.rs index 4615e1f96..090687fce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -134,6 +134,12 @@ cfg_if::cfg_if! { } } +cfg_if::cfg_if! { + if #[cfg(all(target_env = "sgx", target_vendor = "fortanix", not(feature = "std")))] { + pub use self::backtrace::set_image_base; + } +} + #[allow(dead_code)] struct Bomb { enabled: bool, diff --git a/src/print.rs b/src/print.rs index 395328a0a..de8569182 100644 --- a/src/print.rs +++ b/src/print.rs @@ -219,7 +219,7 @@ impl BacktraceFrameFmt<'_, '_, '_> { #[allow(unused_mut)] fn print_raw_generic( &mut self, - mut frame_ip: *mut c_void, + frame_ip: *mut c_void, symbol_name: Option>, filename: Option>, lineno: Option, @@ -233,15 +233,6 @@ impl BacktraceFrameFmt<'_, '_, '_> { } } - // To reduce TCB size in Sgx enclave, we do not want to implement symbol - // resolution functionality. Rather, we can print the offset of the - // address here, which could be later mapped to correct function. - #[cfg(all(feature = "std", target_env = "sgx", target_vendor = "fortanix"))] - { - let image_base = std::os::fortanix_sgx::mem::image_base(); - frame_ip = usize::wrapping_sub(frame_ip as usize, image_base as _) as _; - } - // Print the index of the frame as well as the optional instruction // pointer of the frame. If we're beyond the first symbol of this frame // though we just print appropriate whitespace. diff --git a/tests/sgx-image-base.rs b/tests/sgx-image-base.rs new file mode 100644 index 000000000..d8bacf3da --- /dev/null +++ b/tests/sgx-image-base.rs @@ -0,0 +1,56 @@ +#![cfg(all(target_env = "sgx", target_vendor = "fortanix"))] +#![feature(sgx_platform)] + +#[cfg(feature = "std")] +#[test] +fn sgx_image_base_with_std() { + use backtrace::trace; + + let image_base = std::os::fortanix_sgx::mem::image_base(); + + let mut frame_ips = Vec::new(); + trace(|frame| { + frame_ips.push(frame.ip()); + true + }); + + assert!(frame_ips.len() > 0); + for ip in frame_ips { + let ip: u64 = ip as _; + assert!(ip < image_base); + } +} + +#[cfg(not(feature = "std"))] +#[test] +fn sgx_image_base_no_std() { + use backtrace::trace_unsynchronized; + + fn guess_image_base() -> u64 { + let mut top_frame_ip = None; + unsafe { + trace_unsynchronized(|frame| { + top_frame_ip = Some(frame.ip()); + false + }); + } + top_frame_ip.unwrap() as u64 & 0xFFFFFF000000 + } + + let image_base = guess_image_base(); + backtrace::set_image_base(image_base); + + let mut frame_ips = Vec::new(); + unsafe { + trace_unsynchronized(|frame| { + frame_ips.push(frame.ip()); + true + }); + } + + assert!(frame_ips.len() > 0); + for ip in frame_ips { + let ip: u64 = ip as _; + assert!(ip < image_base); + } +} From f09114ad5aaa74fa254854201e1ff1b944fc73d9 Mon Sep 17 00:00:00 2001 From: Mohsen Zohrevandi Date: Mon, 2 Oct 2023 16:04:46 -0700 Subject: [PATCH 12/42] Hide set_image_base API --- src/backtrace/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/backtrace/mod.rs b/src/backtrace/mod.rs index c414d0514..c098755a3 100644 --- a/src/backtrace/mod.rs +++ b/src/backtrace/mod.rs @@ -134,6 +134,7 @@ mod sgx_no_std_image_base { /// Set the image base address. This is only available for Fortanix SGX /// target when the `std` feature is not enabled. This can be used in the /// standard library to set the correct base address. + #[doc(hidden)] pub fn set_image_base(base_addr: u64) { IMAGE_BASE.store(base_addr, SeqCst); } From d80896c072a97948b6d9dcb4148f4907cbeecc4e Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Thu, 5 Oct 2023 02:19:13 -0600 Subject: [PATCH 13/42] Enable calling build.rs externally v2 (rust-lang/backtrace-rs#568) this time we write the file into OUT_DIR, then there's no CWD issues --- build.rs | 31 ++++++++++++++++++++----------- src/android-api.c | 4 ---- 2 files changed, 20 insertions(+), 15 deletions(-) delete mode 100644 src/android-api.c diff --git a/build.rs b/build.rs index 9bd3abd16..ed4e07a85 100644 --- a/build.rs +++ b/build.rs @@ -11,17 +11,27 @@ pub fn main() { } } +// Used to detect the value of the `__ANDROID_API__` +// builtin #define +const MARKER: &str = "BACKTRACE_RS_ANDROID_APIVERSION"; +const ANDROID_API_C: &str = " +BACKTRACE_RS_ANDROID_APIVERSION __ANDROID_API__ +"; + fn build_android() { - // Resolve `src/android-api.c` relative to this file. + // Create `android-api.c` on demand. // Required to support calling this from the `std` build script. - let android_api_c = Path::new(file!()) - .parent() - .unwrap() - .join("src/android-api.c"); - let expansion = match cc::Build::new().file(android_api_c).try_expand() { + let out_dir = env::var_os("OUT_DIR").unwrap(); + let android_api_c = Path::new(&out_dir).join("android-api.c"); + std::fs::write(&android_api_c, ANDROID_API_C).unwrap(); + + let expansion = match cc::Build::new().file(&android_api_c).try_expand() { Ok(result) => result, Err(e) => { - println!("failed to run C compiler: {}", e); + eprintln!( + "warning: android version detection failed while running C compiler: {}", + e + ); return; } }; @@ -29,13 +39,12 @@ fn build_android() { Ok(s) => s, Err(_) => return, }; - println!("expanded android version detection:\n{}", expansion); - let marker = "APIVERSION"; - let i = match expansion.find(marker) { + eprintln!("expanded android version detection:\n{}", expansion); + let i = match expansion.find(MARKER) { Some(i) => i, None => return, }; - let version = match expansion[i + marker.len() + 1..].split_whitespace().next() { + let version = match expansion[i + MARKER.len() + 1..].split_whitespace().next() { Some(s) => s, None => return, }; diff --git a/src/android-api.c b/src/android-api.c deleted file mode 100644 index 1bfeadf5b..000000000 --- a/src/android-api.c +++ /dev/null @@ -1,4 +0,0 @@ -// Used from the build script to detect the value of the `__ANDROID_API__` -// builtin #define - -APIVERSION __ANDROID_API__ From 43d785942c40e8fd4f8a190e1cc75208ead0857c Mon Sep 17 00:00:00 2001 From: Mohsen Zohrevandi Date: Fri, 6 Oct 2023 11:42:35 -0700 Subject: [PATCH 14/42] Use pointer types for SGX image base address --- src/backtrace/mod.rs | 17 +++++++++-------- tests/sgx-image-base.rs | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/backtrace/mod.rs b/src/backtrace/mod.rs index c098755a3..b6cfb1669 100644 --- a/src/backtrace/mod.rs +++ b/src/backtrace/mod.rs @@ -127,20 +127,21 @@ impl fmt::Debug for Frame { #[cfg(all(target_env = "sgx", target_vendor = "fortanix", not(feature = "std")))] mod sgx_no_std_image_base { - use core::sync::atomic::{AtomicU64, Ordering::SeqCst}; + use core::ffi::c_void; + use core::sync::atomic::{AtomicUsize, Ordering::SeqCst}; - static IMAGE_BASE: AtomicU64 = AtomicU64::new(0); + static IMAGE_BASE: AtomicUsize = AtomicUsize::new(0); /// Set the image base address. This is only available for Fortanix SGX /// target when the `std` feature is not enabled. This can be used in the /// standard library to set the correct base address. #[doc(hidden)] - pub fn set_image_base(base_addr: u64) { - IMAGE_BASE.store(base_addr, SeqCst); + pub fn set_image_base(base_addr: *mut c_void) { + IMAGE_BASE.store(base_addr as _, SeqCst); } - pub(crate) fn get_image_base() -> u64 { - IMAGE_BASE.load(SeqCst) + pub(crate) fn get_image_base() -> *mut c_void { + IMAGE_BASE.load(SeqCst) as _ } } @@ -153,8 +154,8 @@ pub(crate) use self::sgx_no_std_image_base::get_image_base; #[cfg(all(target_env = "sgx", target_vendor = "fortanix", feature = "std"))] #[deny(unused)] -pub(crate) fn get_image_base() -> u64 { - std::os::fortanix_sgx::mem::image_base() +pub(crate) fn get_image_base() -> *mut c_void { + std::os::fortanix_sgx::mem::image_base() as _ } cfg_if::cfg_if! { diff --git a/tests/sgx-image-base.rs b/tests/sgx-image-base.rs index d8bacf3da..c29a8b67a 100644 --- a/tests/sgx-image-base.rs +++ b/tests/sgx-image-base.rs @@ -38,7 +38,7 @@ fn sgx_image_base_no_std() { } let image_base = guess_image_base(); - backtrace::set_image_base(image_base); + backtrace::set_image_base(image_base as _); let mut frame_ips = Vec::new(); unsafe { From 953297198c329c67ec2dd5a7a036c3c48dedf969 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 7 Oct 2023 01:44:15 +0200 Subject: [PATCH 15/42] Commit Cargo.lock (rust-lang/backtrace-rs#562) As of rust-lang/cargo#8728 it is now recommended to always check in Cargo.lock. This will help with reproducability. It will also allow tidy to run on the backtrace workspace. --- .gitignore | 1 - Cargo.lock | 221 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 221 insertions(+), 1 deletion(-) create mode 100644 Cargo.lock diff --git a/.gitignore b/.gitignore index a9d37c560..eb5a316cb 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1 @@ target -Cargo.lock diff --git a/Cargo.lock b/Cargo.lock new file mode 100644 index 000000000..619a1392f --- /dev/null +++ b/Cargo.lock @@ -0,0 +1,221 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "addr2line" +version = "0.21.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a30b2e23b9e17a9f90641c7ab1549cd9b44f296d3ccbf309d2863cfe398a0cb" +dependencies = [ + "gimli", +] + +[[package]] +name = "adler" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe" + +[[package]] +name = "as-if-std" +version = "0.1.0" +dependencies = [ + "addr2line", + "cc", + "cfg-if", + "libc", + "miniz_oxide", + "object", + "rustc-demangle", +] + +[[package]] +name = "backtrace" +version = "0.3.69" +dependencies = [ + "addr2line", + "cc", + "cfg-if", + "cpp_demangle", + "dylib-dep", + "libc", + "libloading", + "miniz_oxide", + "object", + "rustc-demangle", + "rustc-serialize", + "serde", + "winapi", +] + +[[package]] +name = "cc" +version = "1.0.83" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1174fb0b6ec23863f8b971027804a42614e347eafb0a95bf0b12cdae21fc4d0" +dependencies = [ + "libc", +] + +[[package]] +name = "cfg-if" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" + +[[package]] +name = "cpp_demangle" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e8227005286ec39567949b33df9896bcadfa6051bccca2488129f108ca23119" +dependencies = [ + "cfg-if", +] + +[[package]] +name = "cpp_smoke_test" +version = "0.1.0" +dependencies = [ + "backtrace", + "cc", +] + +[[package]] +name = "dylib-dep" +version = "0.1.0" + +[[package]] +name = "gimli" +version = "0.28.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6fb8d784f27acf97159b40fc4db5ecd8aa23b9ad5ef69cdd136d3bc80665f0c0" + +[[package]] +name = "libc" +version = "0.2.147" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b4668fb0ea861c1df094127ac5f1da3409a82116a4ba74fca2e58ef927159bb3" + +[[package]] +name = "libloading" +version = "0.7.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b67380fd3b2fbe7527a606e18729d21c6f3951633d0500574c4dc22d2d638b9f" +dependencies = [ + "cfg-if", + "winapi", +] + +[[package]] +name = "memchr" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2dffe52ecf27772e601905b7522cb4ef790d2cc203488bbd0e2fe85fcb74566d" + +[[package]] +name = "miniz_oxide" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7810e0be55b428ada41041c41f32c9f1a42817901b4ccf45fa3d4b6561e74c7" +dependencies = [ + "adler", +] + +[[package]] +name = "object" +version = "0.32.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77ac5bbd07aea88c60a577a1ce218075ffd59208b2d7ca97adf9bfc5aeb21ebe" +dependencies = [ + "memchr", +] + +[[package]] +name = "proc-macro2" +version = "1.0.66" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "18fb31db3f9bddb2ea821cde30a9f70117e3f119938b5ee630b7403aa6e2ead9" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5267fca4496028628a95160fc423a33e8b2e6af8a5302579e322e4b520293cae" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "rustc-demangle" +version = "0.1.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d626bb9dae77e28219937af045c257c28bfd3f69333c512553507f5f9798cb76" + +[[package]] +name = "rustc-serialize" +version = "0.3.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dcf128d1287d2ea9d80910b5f1120d0b8eede3fbf1abe91c40d39ea7d51e6fda" + +[[package]] +name = "serde" +version = "1.0.188" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf9e0fcba69a370eed61bcf2b728575f726b50b55cba78064753d708ddc7549e" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.188" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4eca7ac642d82aa35b60049a6eccb4be6be75e599bd2e9adb5f875a737654af2" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "syn" +version = "2.0.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c324c494eba9d92503e6f1ef2e6df781e78f6a7705a0202d9801b198807d518a" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "unicode-ident" +version = "1.0.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "301abaae475aa91687eb82514b328ab47a211a533026cb25fc3e519b86adfc3c" + +[[package]] +name = "winapi" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" +dependencies = [ + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +] + +[[package]] +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" + +[[package]] +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" From 11e0a4bdee2714a3711b3df1607dbe979119fd34 Mon Sep 17 00:00:00 2001 From: Samuel Thibault Date: Sat, 7 Oct 2023 01:45:09 +0200 Subject: [PATCH 16/42] Add GNU/Hurd support (rust-lang/backtrace-rs#567) --- src/symbolize/gimli.rs | 2 ++ src/symbolize/gimli/elf.rs | 2 +- src/symbolize/gimli/libs_dl_iterate_phdr.rs | 20 ++++++++++++-------- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/symbolize/gimli.rs b/src/symbolize/gimli.rs index 5d2f62746..3b28bf741 100644 --- a/src/symbolize/gimli.rs +++ b/src/symbolize/gimli.rs @@ -35,6 +35,7 @@ cfg_if::cfg_if! { target_os = "freebsd", target_os = "fuchsia", target_os = "haiku", + target_os = "hurd", target_os = "ios", target_os = "linux", target_os = "macos", @@ -231,6 +232,7 @@ cfg_if::cfg_if! { target_os = "linux", target_os = "fuchsia", target_os = "freebsd", + target_os = "hurd", target_os = "openbsd", target_os = "netbsd", all(target_os = "android", feature = "dl_iterate_phdr"), diff --git a/src/symbolize/gimli/elf.rs b/src/symbolize/gimli/elf.rs index b0eec0762..906a30054 100644 --- a/src/symbolize/gimli/elf.rs +++ b/src/symbolize/gimli/elf.rs @@ -308,7 +308,7 @@ const DEBUG_PATH: &[u8] = b"/usr/lib/debug"; fn debug_path_exists() -> bool { cfg_if::cfg_if! { - if #[cfg(any(target_os = "freebsd", target_os = "linux"))] { + if #[cfg(any(target_os = "freebsd", target_os = "hurd", target_os = "linux"))] { use core::sync::atomic::{AtomicU8, Ordering}; static DEBUG_PATH_EXISTS: AtomicU8 = AtomicU8::new(0); diff --git a/src/symbolize/gimli/libs_dl_iterate_phdr.rs b/src/symbolize/gimli/libs_dl_iterate_phdr.rs index 9f0304ce8..518512fff 100644 --- a/src/symbolize/gimli/libs_dl_iterate_phdr.rs +++ b/src/symbolize/gimli/libs_dl_iterate_phdr.rs @@ -18,14 +18,18 @@ pub(super) fn native_libraries() -> Vec { } fn infer_current_exe(base_addr: usize) -> OsString { - if let Ok(entries) = super::parse_running_mmaps::parse_maps() { - let opt_path = entries - .iter() - .find(|e| e.ip_matches(base_addr) && e.pathname().len() > 0) - .map(|e| e.pathname()) - .cloned(); - if let Some(path) = opt_path { - return path; + cfg_if::cfg_if! { + if #[cfg(not(target_os = "hurd"))] { + if let Ok(entries) = super::parse_running_mmaps::parse_maps() { + let opt_path = entries + .iter() + .find(|e| e.ip_matches(base_addr) && e.pathname().len() > 0) + .map(|e| e.pathname()) + .cloned(); + if let Some(path) = opt_path { + return path; + } + } } } env::current_exe().map(|e| e.into()).unwrap_or_default() From c1464fadd7c870c87818b86b6161ee188be6f3a4 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Tue, 10 Oct 2023 14:54:10 -0400 Subject: [PATCH 17/42] Rewrite msvc backtrace support to be much faster on 64-bit platforms Currently, capturing the stack backtrace is done on Windows by calling into `dbghelp!StackWalkEx` (or `dbghelp!StackWalk64` if the version of `dbghelp` we loaded is too old to contain that function). This is very convenient since `StackWalkEx` handles everything for us but there are two issues with doing so: 1. `dbghelp` is not safe to use from multiple threads at the same time so all calls into it must be serialized. 2. `StackWalkEx` returns inlined frames as if they were regular stack frames which requires loading debug info just to walk the stack. As a result, simply capturing a backtrace without resolving it is much more expensive on Windows than *nix. This change rewrites our Windows support to call `RtlVirtualUnwind` instead on platforms which support this API (`x86_64` and `aarch64`). This API walks the actual (ie, not inlined) stack frames so it does not require loading any debug info and is significantly faster. For platforms that do not support `RtlVirtualUnwind` (ie, `i686`), we fall back to the current implementation which calls into `dbghelp`. To recover the inlined frame information when we are asked to resolve symbols, we use `SymAddrIncludeInlineTrace` to load debug info and detect inlined frames and then `SymQueryInlineTrace` to get the appropriate inline context to resolve them. The result is significant performance improvements to backtrace capture and symbolizing on Windows! Before: ``` > cargo +nightly bench Running benches\benchmarks.rs running 6 tests test new ... bench: 658,652 ns/iter (+/- 30,741) test new_unresolved ... bench: 343,240 ns/iter (+/- 13,108) test new_unresolved_and_resolve_separate ... bench: 648,890 ns/iter (+/- 31,651) test trace ... bench: 304,815 ns/iter (+/- 19,633) test trace_and_resolve_callback ... bench: 463,645 ns/iter (+/- 12,893) test trace_and_resolve_separate ... bench: 474,290 ns/iter (+/- 73,858) test result: ok. 0 passed; 0 failed; 0 ignored; 6 measured; 0 filtered out; finished in 8.26s ``` After: ``` > cargo +nightly bench Running benches\benchmarks.rs running 6 tests test new ... bench: 495,468 ns/iter (+/- 31,215) test new_unresolved ... bench: 1,241 ns/iter (+/- 251) test new_unresolved_and_resolve_separate ... bench: 436,730 ns/iter (+/- 32,482) test trace ... bench: 850 ns/iter (+/- 162) test trace_and_resolve_callback ... bench: 410,790 ns/iter (+/- 19,424) test trace_and_resolve_separate ... bench: 408,090 ns/iter (+/- 29,324) test result: ok. 0 passed; 0 failed; 0 ignored; 6 measured; 0 filtered out; finished in 7.02s ``` The changes to the symbolize step also allow us to report inlined frames when resolving from just an instruction address which was not previously possible. --- src/backtrace/dbghelp.rs | 290 ++++++++++++++++++--------------------- src/backtrace/mod.rs | 2 - src/dbghelp.rs | 40 ++++-- src/lib.rs | 2 +- src/symbolize/dbghelp.rs | 94 ++++++++----- src/windows.rs | 52 +++++++ 6 files changed, 270 insertions(+), 210 deletions(-) diff --git a/src/backtrace/dbghelp.rs b/src/backtrace/dbghelp.rs index ba0f05f3b..05458b99f 100644 --- a/src/backtrace/dbghelp.rs +++ b/src/backtrace/dbghelp.rs @@ -1,30 +1,32 @@ //! Backtrace strategy for MSVC platforms. //! -//! This module contains the ability to generate a backtrace on MSVC using one -//! of two possible methods. The `StackWalkEx` function is primarily used if -//! possible, but not all systems have that. Failing that the `StackWalk64` -//! function is used instead. Note that `StackWalkEx` is favored because it -//! handles debuginfo internally and returns inline frame information. +//! This module contains the ability to capture a backtrace on MSVC using one +//! of three possible methods. For `x86_64` and `aarch64`, we use `RtlVirtualUnwind` +//! to walk the stack one frame at a time. This function is much faster than using +//! `dbghelp!StackWalk*` because it does not load debug info to report inlined frames. +//! We still report inlined frames during symbolization by consulting the appropriate +//! `dbghelp` functions. +//! +//! For all other platforms, primarily `i686`, the `StackWalkEx` function is used if +//! possible, but not all systems have that. Failing that the `StackWalk64` function +//! is used instead. Note that `StackWalkEx` is favored because it handles debuginfo +//! internally and returns inline frame information. //! //! Note that all dbghelp support is loaded dynamically, see `src/dbghelp.rs` //! for more information about that. #![allow(bad_style)] -use super::super::{dbghelp, windows::*}; +use super::super::windows::*; use core::ffi::c_void; -use core::mem; - -#[derive(Clone, Copy)] -pub enum StackFrame { - New(STACKFRAME_EX), - Old(STACKFRAME64), -} #[derive(Clone, Copy)] pub struct Frame { - pub(crate) stack_frame: StackFrame, base_address: *mut c_void, + ip: *mut c_void, + sp: *mut c_void, + #[cfg(not(target_env = "gnu"))] + inline_context: Option, } // we're just sending around raw pointers and reading them, never interpreting @@ -34,62 +36,108 @@ unsafe impl Sync for Frame {} impl Frame { pub fn ip(&self) -> *mut c_void { - self.addr_pc().Offset as *mut _ + self.ip } pub fn sp(&self) -> *mut c_void { - self.addr_stack().Offset as *mut _ + self.sp } pub fn symbol_address(&self) -> *mut c_void { - self.ip() + self.ip } pub fn module_base_address(&self) -> Option<*mut c_void> { Some(self.base_address) } - fn addr_pc(&self) -> &ADDRESS64 { - match self.stack_frame { - StackFrame::New(ref new) => &new.AddrPC, - StackFrame::Old(ref old) => &old.AddrPC, - } + #[cfg(not(target_env = "gnu"))] + pub fn inline_context(&self) -> Option { + self.inline_context } +} - fn addr_pc_mut(&mut self) -> &mut ADDRESS64 { - match self.stack_frame { - StackFrame::New(ref mut new) => &mut new.AddrPC, - StackFrame::Old(ref mut old) => &mut old.AddrPC, - } +#[repr(C, align(16))] // required by `CONTEXT`, is a FIXME in winapi right now +struct MyContext(CONTEXT); + +#[cfg(target_arch = "x86_64")] +impl MyContext { + #[inline(always)] + fn ip(&self) -> DWORD64 { + self.0.Rip } - fn addr_frame_mut(&mut self) -> &mut ADDRESS64 { - match self.stack_frame { - StackFrame::New(ref mut new) => &mut new.AddrFrame, - StackFrame::Old(ref mut old) => &mut old.AddrFrame, - } + #[inline(always)] + fn sp(&self) -> DWORD64 { + self.0.Rsp } +} - fn addr_stack(&self) -> &ADDRESS64 { - match self.stack_frame { - StackFrame::New(ref new) => &new.AddrStack, - StackFrame::Old(ref old) => &old.AddrStack, - } +#[cfg(target_arch = "aarch64")] +impl MyContext { + #[inline(always)] + fn ip(&self) -> DWORD64 { + self.0.Pc } - fn addr_stack_mut(&mut self) -> &mut ADDRESS64 { - match self.stack_frame { - StackFrame::New(ref mut new) => &mut new.AddrStack, - StackFrame::Old(ref mut old) => &mut old.AddrStack, - } + #[inline(always)] + fn sp(&self) -> DWORD64 { + self.0.Sp } } -#[repr(C, align(16))] // required by `CONTEXT`, is a FIXME in winapi right now -struct MyContext(CONTEXT); +#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] +#[inline(always)] +pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { + use core::ptr; + + let mut context = core::mem::zeroed::(); + RtlCaptureContext(&mut context.0); + + // Call `RtlVirtualUnwind` to find the previous stack frame, walking until we hit ip = 0. + while context.ip() != 0 { + let mut base = 0; + + let fn_entry = RtlLookupFunctionEntry(context.ip(), &mut base, ptr::null_mut()); + if fn_entry.is_null() { + break; + } + let frame = super::Frame { + inner: Frame { + base_address: fn_entry as *mut c_void, + ip: context.ip() as *mut c_void, + sp: context.sp() as *mut c_void, + #[cfg(not(target_env = "gnu"))] + inline_context: None, + }, + }; + + if !cb(&frame) { + break; + } + + let mut handler_data = 0usize; + let mut establisher_frame = 0; + + RtlVirtualUnwind( + 0, + base, + context.ip(), + fn_entry, + &mut context.0, + &mut handler_data as *mut usize as *mut PVOID, + &mut establisher_frame, + ptr::null_mut(), + ); + } +} + +#[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))] #[inline(always)] pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { + use core::mem; + // Allocate necessary structures for doing the stack walk let process = GetCurrentProcess(); let thread = GetCurrentThread(); @@ -98,38 +146,13 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { RtlCaptureContext(&mut context.0); // Ensure this process's symbols are initialized - let dbghelp = match dbghelp::init() { + let dbghelp = match super::super::dbghelp::init() { Ok(dbghelp) => dbghelp, Err(()) => return, // oh well... }; - // On x86_64 and ARM64 we opt to not use the default `Sym*` functions from - // dbghelp for getting the function table and module base. Instead we use - // the `RtlLookupFunctionEntry` function in kernel32 which will account for - // JIT compiler frames as well. These should be equivalent, but using - // `Rtl*` allows us to backtrace through JIT frames. - // - // Note that `RtlLookupFunctionEntry` only works for in-process backtraces, - // but that's all we support anyway, so it all lines up well. - cfg_if::cfg_if! { - if #[cfg(target_pointer_width = "64")] { - use core::ptr; - - unsafe extern "system" fn function_table_access(_process: HANDLE, addr: DWORD64) -> PVOID { - let mut base = 0; - RtlLookupFunctionEntry(addr, &mut base, ptr::null_mut()).cast() - } - - unsafe extern "system" fn get_module_base(_process: HANDLE, addr: DWORD64) -> DWORD64 { - let mut base = 0; - RtlLookupFunctionEntry(addr, &mut base, ptr::null_mut()); - base - } - } else { - let function_table_access = dbghelp.SymFunctionTableAccess64(); - let get_module_base = dbghelp.SymGetModuleBase64(); - } - } + let function_table_access = dbghelp.SymFunctionTableAccess64(); + let get_module_base = dbghelp.SymGetModuleBase64(); let process_handle = GetCurrentProcess(); @@ -137,26 +160,21 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { // since it's in theory supported on more systems. match (*dbghelp.dbghelp()).StackWalkEx() { Some(StackWalkEx) => { - let mut inner: STACKFRAME_EX = mem::zeroed(); - inner.StackFrameSize = mem::size_of::() as DWORD; - let mut frame = super::Frame { - inner: Frame { - stack_frame: StackFrame::New(inner), - base_address: 0 as _, - }, - }; - let image = init_frame(&mut frame.inner, &context.0); - let frame_ptr = match &mut frame.inner.stack_frame { - StackFrame::New(ptr) => ptr as *mut STACKFRAME_EX, - _ => unreachable!(), - }; + let mut stack_frame_ex: STACKFRAME_EX = mem::zeroed(); + stack_frame_ex.StackFrameSize = mem::size_of::() as DWORD; + stack_frame_ex.AddrPC.Offset = context.0.Eip as u64; + stack_frame_ex.AddrPC.Mode = AddrModeFlat; + stack_frame_ex.AddrStack.Offset = context.0.Esp as u64; + stack_frame_ex.AddrStack.Mode = AddrModeFlat; + stack_frame_ex.AddrFrame.Offset = context.0.Ebp as u64; + stack_frame_ex.AddrFrame.Mode = AddrModeFlat; while StackWalkEx( - image as DWORD, + IMAGE_FILE_MACHINE_I386 as DWORD, process, thread, - frame_ptr, - &mut context.0 as *mut CONTEXT as *mut _, + &mut stack_frame_ex, + &mut context.0 as *mut CONTEXT as PVOID, None, Some(function_table_access), Some(get_module_base), @@ -164,7 +182,16 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { 0, ) == TRUE { - frame.inner.base_address = get_module_base(process_handle, frame.ip() as _) as _; + let frame = super::Frame { + inner: Frame { + base_address: get_module_base(process_handle, stack_frame_ex.AddrPC.Offset) + as *mut c_void, + ip: stack_frame_ex.AddrPC.Offset as *mut c_void, + sp: stack_frame_ex.AddrStack.Offset as *mut c_void, + #[cfg(not(target_env = "gnu"))] + inline_context: Some(stack_frame_ex.InlineFrameContext), + }, + }; if !cb(&frame) { break; @@ -172,31 +199,36 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { } } None => { - let mut frame = super::Frame { - inner: Frame { - stack_frame: StackFrame::Old(mem::zeroed()), - base_address: 0 as _, - }, - }; - let image = init_frame(&mut frame.inner, &context.0); - let frame_ptr = match &mut frame.inner.stack_frame { - StackFrame::Old(ptr) => ptr as *mut STACKFRAME64, - _ => unreachable!(), - }; + let mut stack_frame64: STACKFRAME64 = mem::zeroed(); + stack_frame64.AddrPC.Offset = context.0.Eip as u64; + stack_frame64.AddrPC.Mode = AddrModeFlat; + stack_frame64.AddrStack.Offset = context.0.Esp as u64; + stack_frame64.AddrStack.Mode = AddrModeFlat; + stack_frame64.AddrFrame.Offset = context.0.Ebp as u64; + stack_frame64.AddrFrame.Mode = AddrModeFlat; while dbghelp.StackWalk64()( - image as DWORD, + IMAGE_FILE_MACHINE_I386 as DWORD, process, thread, - frame_ptr, - &mut context.0 as *mut CONTEXT as *mut _, + &mut stack_frame64, + &mut context.0 as *mut CONTEXT as PVOID, None, Some(function_table_access), Some(get_module_base), None, ) == TRUE { - frame.inner.base_address = get_module_base(process_handle, frame.ip() as _) as _; + let frame = super::Frame { + inner: Frame { + base_address: get_module_base(process_handle, stack_frame64.AddrPC.Offset) + as *mut c_void, + ip: stack_frame64.AddrPC.Offset as *mut c_void, + sp: stack_frame64.AddrStack.Offset as *mut c_void, + #[cfg(not(target_env = "gnu"))] + inline_context: None, + }, + }; if !cb(&frame) { break; @@ -205,53 +237,3 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { } } } - -#[cfg(target_arch = "x86_64")] -fn init_frame(frame: &mut Frame, ctx: &CONTEXT) -> WORD { - frame.addr_pc_mut().Offset = ctx.Rip as u64; - frame.addr_pc_mut().Mode = AddrModeFlat; - frame.addr_stack_mut().Offset = ctx.Rsp as u64; - frame.addr_stack_mut().Mode = AddrModeFlat; - frame.addr_frame_mut().Offset = ctx.Rbp as u64; - frame.addr_frame_mut().Mode = AddrModeFlat; - - IMAGE_FILE_MACHINE_AMD64 -} - -#[cfg(target_arch = "x86")] -fn init_frame(frame: &mut Frame, ctx: &CONTEXT) -> WORD { - frame.addr_pc_mut().Offset = ctx.Eip as u64; - frame.addr_pc_mut().Mode = AddrModeFlat; - frame.addr_stack_mut().Offset = ctx.Esp as u64; - frame.addr_stack_mut().Mode = AddrModeFlat; - frame.addr_frame_mut().Offset = ctx.Ebp as u64; - frame.addr_frame_mut().Mode = AddrModeFlat; - - IMAGE_FILE_MACHINE_I386 -} - -#[cfg(target_arch = "aarch64")] -fn init_frame(frame: &mut Frame, ctx: &CONTEXT) -> WORD { - frame.addr_pc_mut().Offset = ctx.Pc as u64; - frame.addr_pc_mut().Mode = AddrModeFlat; - frame.addr_stack_mut().Offset = ctx.Sp as u64; - frame.addr_stack_mut().Mode = AddrModeFlat; - unsafe { - frame.addr_frame_mut().Offset = ctx.u.s().Fp as u64; - } - frame.addr_frame_mut().Mode = AddrModeFlat; - IMAGE_FILE_MACHINE_ARM64 -} - -#[cfg(target_arch = "arm")] -fn init_frame(frame: &mut Frame, ctx: &CONTEXT) -> WORD { - frame.addr_pc_mut().Offset = ctx.Pc as u64; - frame.addr_pc_mut().Mode = AddrModeFlat; - frame.addr_stack_mut().Offset = ctx.Sp as u64; - frame.addr_stack_mut().Mode = AddrModeFlat; - unsafe { - frame.addr_frame_mut().Offset = ctx.R11 as u64; - } - frame.addr_frame_mut().Mode = AddrModeFlat; - IMAGE_FILE_MACHINE_ARMNT -} diff --git a/src/backtrace/mod.rs b/src/backtrace/mod.rs index 6ca1080c4..c0b4cb0d7 100644 --- a/src/backtrace/mod.rs +++ b/src/backtrace/mod.rs @@ -153,8 +153,6 @@ cfg_if::cfg_if! { mod dbghelp; use self::dbghelp::trace as trace_imp; pub(crate) use self::dbghelp::Frame as FrameImp; - #[cfg(target_env = "msvc")] // only used in dbghelp symbolize - pub(crate) use self::dbghelp::StackFrame; } else { mod noop; use self::noop::trace as trace_imp; diff --git a/src/dbghelp.rs b/src/dbghelp.rs index c81766bae..30033fc88 100644 --- a/src/dbghelp.rs +++ b/src/dbghelp.rs @@ -34,8 +34,8 @@ use core::ptr; mod dbghelp { use crate::windows::*; pub use winapi::um::dbghelp::{ - StackWalk64, StackWalkEx, SymCleanup, SymFromAddrW, SymFunctionTableAccess64, - SymGetLineFromAddrW64, SymGetModuleBase64, SymGetOptions, SymInitializeW, SymSetOptions, + StackWalk64, StackWalkEx, SymFunctionTableAccess64, SymGetModuleBase64, SymGetOptions, + SymInitializeW, SymSetOptions, }; extern "system" { @@ -55,6 +55,16 @@ mod dbghelp { pdwDisplacement: PDWORD, Line: PIMAGEHLP_LINEW64, ) -> BOOL; + pub fn SymAddrIncludeInlineTrace(hProcess: HANDLE, Address: DWORD64) -> DWORD; + pub fn SymQueryInlineTrace( + hProcess: HANDLE, + StartAddress: DWORD64, + StartContext: DWORD, + StartRetAddress: DWORD64, + CurAddress: DWORD64, + CurContext: LPDWORD, + CurFrameIndex: LPDWORD, + ) -> BOOL; } pub fn assert_equal_types(a: T, _b: T) -> T { @@ -164,7 +174,6 @@ dbghelp! { path: PCWSTR, invade: BOOL ) -> BOOL; - fn SymCleanup(handle: HANDLE) -> BOOL; fn StackWalk64( MachineType: DWORD, hProcess: HANDLE, @@ -184,18 +193,6 @@ dbghelp! { hProcess: HANDLE, AddrBase: DWORD64 ) -> DWORD64; - fn SymFromAddrW( - hProcess: HANDLE, - Address: DWORD64, - Displacement: PDWORD64, - Symbol: PSYMBOL_INFOW - ) -> BOOL; - fn SymGetLineFromAddrW64( - hProcess: HANDLE, - dwAddr: DWORD64, - pdwDisplacement: PDWORD, - Line: PIMAGEHLP_LINEW64 - ) -> BOOL; fn StackWalkEx( MachineType: DWORD, hProcess: HANDLE, @@ -223,6 +220,19 @@ dbghelp! { pdwDisplacement: PDWORD, Line: PIMAGEHLP_LINEW64 ) -> BOOL; + fn SymAddrIncludeInlineTrace( + hProcess: HANDLE, + Address: DWORD64 + ) -> DWORD; + fn SymQueryInlineTrace( + hProcess: HANDLE, + StartAddress: DWORD64, + StartContext: DWORD, + StartRetAddress: DWORD64, + CurAddress: DWORD64, + CurContext: LPDWORD, + CurFrameIndex: LPDWORD + ) -> BOOL; } } diff --git a/src/lib.rs b/src/lib.rs index 4615e1f96..c9fa42918 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -186,7 +186,7 @@ mod lock { } } -#[cfg(all(windows, not(target_vendor = "uwp")))] +#[cfg(all(windows, target_env = "msvc", not(target_vendor = "uwp")))] mod dbghelp; #[cfg(windows)] mod windows; diff --git a/src/symbolize/dbghelp.rs b/src/symbolize/dbghelp.rs index 181dba731..0ca58c839 100644 --- a/src/symbolize/dbghelp.rs +++ b/src/symbolize/dbghelp.rs @@ -17,7 +17,7 @@ #![allow(bad_style)] -use super::super::{backtrace::StackFrame, dbghelp, windows::*}; +use super::super::{dbghelp, windows::*}; use super::{BytesOrWideString, ResolveWhat, SymbolName}; use core::char; use core::ffi::c_void; @@ -79,53 +79,71 @@ pub unsafe fn resolve(what: ResolveWhat<'_>, cb: &mut dyn FnMut(&super::Symbol)) }; match what { - ResolveWhat::Address(_) => resolve_without_inline(&dbghelp, what.address_or_ip(), cb), - ResolveWhat::Frame(frame) => match &frame.inner.stack_frame { - StackFrame::New(frame) => resolve_with_inline(&dbghelp, frame, cb), - StackFrame::Old(_) => resolve_without_inline(&dbghelp, frame.ip(), cb), - }, + ResolveWhat::Address(_) => resolve_with_inline(&dbghelp, what.address_or_ip(), None, cb), + ResolveWhat::Frame(frame) => { + resolve_with_inline(&dbghelp, frame.ip(), frame.inner.inline_context(), cb) + } } } unsafe fn resolve_with_inline( dbghelp: &dbghelp::Init, - frame: &STACKFRAME_EX, + addr: *mut c_void, + inline_context: Option, cb: &mut dyn FnMut(&super::Symbol), ) { - do_resolve( - |info| { - dbghelp.SymFromInlineContextW()( - GetCurrentProcess(), - super::adjust_ip(frame.AddrPC.Offset as *mut _) as u64, - frame.InlineFrameContext, - &mut 0, - info, - ) - }, - |line| { - dbghelp.SymGetLineFromInlineContextW()( - GetCurrentProcess(), - super::adjust_ip(frame.AddrPC.Offset as *mut _) as u64, - frame.InlineFrameContext, + let current_process = GetCurrentProcess(); + + let addr = super::adjust_ip(addr) as DWORD64; + + let (inlined_frame_count, inline_context) = if let Some(ic) = inline_context { + (0, ic) + } else { + let mut inlined_frame_count = dbghelp.SymAddrIncludeInlineTrace()(current_process, addr); + + let mut inline_context = 0; + + // If there is are inlined frames but we can't load them for some reason OR if there are no + // inlined frames, then we disregard inlined_frame_count and inline_context. + if (inlined_frame_count > 0 + && dbghelp.SymQueryInlineTrace()( + current_process, + addr, 0, + addr, + addr, + &mut inline_context, &mut 0, - line, - ) - }, - cb, - ) -} + ) != TRUE) + || inlined_frame_count == 0 + { + inlined_frame_count = 0; + inline_context = 0; + } -unsafe fn resolve_without_inline( - dbghelp: &dbghelp::Init, - addr: *mut c_void, - cb: &mut dyn FnMut(&super::Symbol), -) { - do_resolve( - |info| dbghelp.SymFromAddrW()(GetCurrentProcess(), addr as DWORD64, &mut 0, info), - |line| dbghelp.SymGetLineFromAddrW64()(GetCurrentProcess(), addr as DWORD64, &mut 0, line), - cb, - ) + (inlined_frame_count, inline_context) + }; + + let last_inline_context = inline_context + 1 + inlined_frame_count; + + for inline_context in inline_context..last_inline_context { + do_resolve( + |info| { + dbghelp.SymFromInlineContextW()(current_process, addr, inline_context, &mut 0, info) + }, + |line| { + dbghelp.SymGetLineFromInlineContextW()( + current_process, + addr, + inline_context, + 0, + &mut 0, + line, + ) + }, + cb, + ); + } } unsafe fn do_resolve( diff --git a/src/windows.rs b/src/windows.rs index 92c2b2e66..13287f7c3 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -19,6 +19,9 @@ cfg_if::cfg_if! { pub use self::winapi::PUNWIND_HISTORY_TABLE; #[cfg(target_pointer_width = "64")] pub use self::winapi::PRUNTIME_FUNCTION; + pub use self::winapi::PEXCEPTION_ROUTINE; + #[cfg(target_pointer_width = "64")] + pub use self::winapi::PKNONVOLATILE_CONTEXT_POINTERS; mod winapi { pub use winapi::ctypes::*; @@ -35,6 +38,22 @@ cfg_if::cfg_if! { pub use winapi::um::tlhelp32::*; pub use winapi::um::winbase::*; pub use winapi::um::winnt::*; + + // Work around winapi not having this function on aarch64. + #[cfg(target_arch = "aarch64")] + #[link(name = "kernel32")] + extern "system" { + pub fn RtlVirtualUnwind( + HandlerType: ULONG, + ImageBase: ULONG64, + ControlPc: ULONG64, + FunctionEntry: PRUNTIME_FUNCTION, + ContextRecord: PCONTEXT, + HandlerData: *mut PVOID, + EstablisherFrame: PULONG64, + ContextPointers: PKNONVOLATILE_CONTEXT_POINTERS + ) -> PEXCEPTION_ROUTINE; + } } } else { pub use core::ffi::c_void; @@ -45,6 +64,9 @@ cfg_if::cfg_if! { pub type PRUNTIME_FUNCTION = *mut c_void; #[cfg(target_pointer_width = "64")] pub type PUNWIND_HISTORY_TABLE = *mut c_void; + pub type PEXCEPTION_ROUTINE = *mut c_void; + #[cfg(target_pointer_width = "64")] + pub type PKNONVOLATILE_CONTEXT_POINTERS = *mut c_void; } } @@ -359,6 +381,7 @@ ffi! { pub type LPCSTR = *const i8; pub type PWSTR = *mut u16; pub type WORD = u16; + pub type USHORT = u16; pub type ULONG = u32; pub type ULONG64 = u64; pub type WCHAR = u16; @@ -370,6 +393,8 @@ ffi! { pub type LPVOID = *mut c_void; pub type LPCVOID = *const c_void; pub type LPMODULEENTRY32W = *mut MODULEENTRY32W; + pub type PULONG = *mut ULONG; + pub type PULONG64 = *mut ULONG64; #[link(name = "kernel32")] extern "system" { @@ -435,6 +460,33 @@ ffi! { lpme: LPMODULEENTRY32W, ) -> BOOL; } + + #[link(name = "ntdll")] + extern "system" { + pub fn RtlCaptureStackBackTrace( + FramesToSkip: ULONG, + FramesToCapture: ULONG, + BackTrace: *mut PVOID, + BackTraceHash: PULONG, + ) -> USHORT; + } +} + +#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] +ffi! { + #[link(name = "kernel32")] + extern "system" { + pub fn RtlVirtualUnwind( + HandlerType: ULONG, + ImageBase: ULONG64, + ControlPc: ULONG64, + FunctionEntry: PRUNTIME_FUNCTION, + ContextRecord: PCONTEXT, + HandlerData: *mut PVOID, + EstablisherFrame: PULONG64, + ContextPointers: PKNONVOLATILE_CONTEXT_POINTERS + ) -> PEXCEPTION_ROUTINE; + } } #[cfg(target_pointer_width = "64")] From 61a0380b9eb33fd9168c74fe81b253fa51bb925b Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Wed, 11 Oct 2023 10:42:09 -0400 Subject: [PATCH 18/42] Update cfg for x86/arm and fix image param --- src/backtrace/dbghelp.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/backtrace/dbghelp.rs b/src/backtrace/dbghelp.rs index 05458b99f..9d7427ce6 100644 --- a/src/backtrace/dbghelp.rs +++ b/src/backtrace/dbghelp.rs @@ -133,7 +133,7 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { } } -#[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))] +#[cfg(any(target_arch = "x86", target_arch = "arm"))] #[inline(always)] pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { use core::mem; @@ -156,6 +156,11 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { let process_handle = GetCurrentProcess(); + #[cfg(target_arch = "x86")] + let image = IMAGE_FILE_MACHINE_I386; + #[cfg(target_arch = "arm")] + let image = IMAGE_FILE_MACHINE_ARMNT; + // Attempt to use `StackWalkEx` if we can, but fall back to `StackWalk64` // since it's in theory supported on more systems. match (*dbghelp.dbghelp()).StackWalkEx() { @@ -170,7 +175,7 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { stack_frame_ex.AddrFrame.Mode = AddrModeFlat; while StackWalkEx( - IMAGE_FILE_MACHINE_I386 as DWORD, + image as DWORD, process, thread, &mut stack_frame_ex, @@ -208,7 +213,7 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { stack_frame64.AddrFrame.Mode = AddrModeFlat; while dbghelp.StackWalk64()( - IMAGE_FILE_MACHINE_I386 as DWORD, + image as DWORD, process, thread, &mut stack_frame64, From 44a71e5920f1bef696d6e1735e2f2a11dc862e00 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Mon, 23 Oct 2023 11:36:58 -0400 Subject: [PATCH 19/42] Fix i686-pc-windows-gnu missing dbghelp module --- src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 08da8686e..7965d94ba 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -192,7 +192,11 @@ mod lock { } } -#[cfg(all(windows, target_env = "msvc", not(target_vendor = "uwp")))] +#[cfg(all( + windows, + any(target_env = "msvc", all(target_env = "gnu", target_arch = "x86")), + not(target_vendor = "uwp") +))] mod dbghelp; #[cfg(windows)] mod windows; From 3f9d175d402b7ecdd52524bf01e774a6059b5c3e Mon Sep 17 00:00:00 2001 From: Kleis Auke Wolthuizen Date: Sat, 4 Nov 2023 12:01:36 +0100 Subject: [PATCH 20/42] Fix build errors on thumbv7a-*-windows-msvc targets Resolves: #572 --- src/backtrace/dbghelp.rs | 48 +++++++++++++++++++++++++++++++++++----- src/lib.rs | 5 ++++- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/backtrace/dbghelp.rs b/src/backtrace/dbghelp.rs index 9d7427ce6..d1b76e281 100644 --- a/src/backtrace/dbghelp.rs +++ b/src/backtrace/dbghelp.rs @@ -86,6 +86,42 @@ impl MyContext { } } +#[cfg(target_arch = "x86")] +impl MyContext { + #[inline(always)] + fn ip(&self) -> DWORD { + self.0.Eip + } + + #[inline(always)] + fn sp(&self) -> DWORD { + self.0.Esp + } + + #[inline(always)] + fn fp(&self) -> DWORD { + self.0.Ebp + } +} + +#[cfg(target_arch = "arm")] +impl MyContext { + #[inline(always)] + fn ip(&self) -> DWORD { + self.0.Pc + } + + #[inline(always)] + fn sp(&self) -> DWORD { + self.0.Sp + } + + #[inline(always)] + fn fp(&self) -> DWORD { + self.0.R11 + } +} + #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] #[inline(always)] pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { @@ -167,11 +203,11 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { Some(StackWalkEx) => { let mut stack_frame_ex: STACKFRAME_EX = mem::zeroed(); stack_frame_ex.StackFrameSize = mem::size_of::() as DWORD; - stack_frame_ex.AddrPC.Offset = context.0.Eip as u64; + stack_frame_ex.AddrPC.Offset = context.ip() as u64; stack_frame_ex.AddrPC.Mode = AddrModeFlat; - stack_frame_ex.AddrStack.Offset = context.0.Esp as u64; + stack_frame_ex.AddrStack.Offset = context.sp() as u64; stack_frame_ex.AddrStack.Mode = AddrModeFlat; - stack_frame_ex.AddrFrame.Offset = context.0.Ebp as u64; + stack_frame_ex.AddrFrame.Offset = context.fp() as u64; stack_frame_ex.AddrFrame.Mode = AddrModeFlat; while StackWalkEx( @@ -205,11 +241,11 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { } None => { let mut stack_frame64: STACKFRAME64 = mem::zeroed(); - stack_frame64.AddrPC.Offset = context.0.Eip as u64; + stack_frame64.AddrPC.Offset = context.ip() as u64; stack_frame64.AddrPC.Mode = AddrModeFlat; - stack_frame64.AddrStack.Offset = context.0.Esp as u64; + stack_frame64.AddrStack.Offset = context.sp() as u64; stack_frame64.AddrStack.Mode = AddrModeFlat; - stack_frame64.AddrFrame.Offset = context.0.Ebp as u64; + stack_frame64.AddrFrame.Offset = context.fp() as u64; stack_frame64.AddrFrame.Mode = AddrModeFlat; while dbghelp.StackWalk64()( diff --git a/src/lib.rs b/src/lib.rs index 7965d94ba..44a0bc64e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -194,7 +194,10 @@ mod lock { #[cfg(all( windows, - any(target_env = "msvc", all(target_env = "gnu", target_arch = "x86")), + any( + target_env = "msvc", + all(target_env = "gnu", any(target_arch = "x86", target_arch = "arm")) + ), not(target_vendor = "uwp") ))] mod dbghelp; From b891125913228468b3fb33f07c3a1b4b3807644e Mon Sep 17 00:00:00 2001 From: roblabla Date: Tue, 21 Nov 2023 12:24:24 +0100 Subject: [PATCH 21/42] Fix panic in backtrace symbolication on win7 Since https://github.com/rust-lang/backtrace-rs/pull/569 was merged, symbolication of backtraces would panic on Windows 7, due to using symbols that do not exist in the version of dbghelp.dll that ships there (Windows 7 ships with dbghelp.dll version 6.1, but the symbols were only added in version 6.2). This commit checks for the presence of the newer symbols, and if they are not found, falls back to the old resolution method. --- src/dbghelp.rs | 16 ++++++++++++++-- src/symbolize/dbghelp.rs | 35 +++++++++++++++++++++++++++++++++-- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/dbghelp.rs b/src/dbghelp.rs index 30033fc88..e456dd45d 100644 --- a/src/dbghelp.rs +++ b/src/dbghelp.rs @@ -34,8 +34,8 @@ use core::ptr; mod dbghelp { use crate::windows::*; pub use winapi::um::dbghelp::{ - StackWalk64, StackWalkEx, SymFunctionTableAccess64, SymGetModuleBase64, SymGetOptions, - SymInitializeW, SymSetOptions, + StackWalk64, StackWalkEx, SymFromAddrW, SymFunctionTableAccess64, SymGetLineFromAddrW64, + SymGetModuleBase64, SymGetOptions, SymInitializeW, SymSetOptions, }; extern "system" { @@ -233,6 +233,18 @@ dbghelp! { CurContext: LPDWORD, CurFrameIndex: LPDWORD ) -> BOOL; + fn SymFromAddrW( + hProcess: HANDLE, + Address: DWORD64, + Displacement: PDWORD64, + Symbol: PSYMBOL_INFOW + ) -> BOOL; + fn SymGetLineFromAddrW64( + hProcess: HANDLE, + dwAddr: DWORD64, + pdwDisplacement: PDWORD, + Line: PIMAGEHLP_LINEW64 + ) -> BOOL; } } diff --git a/src/symbolize/dbghelp.rs b/src/symbolize/dbghelp.rs index 0ca58c839..8c47d58e8 100644 --- a/src/symbolize/dbghelp.rs +++ b/src/symbolize/dbghelp.rs @@ -78,14 +78,45 @@ pub unsafe fn resolve(what: ResolveWhat<'_>, cb: &mut dyn FnMut(&super::Symbol)) Err(()) => return, // oh well... }; + let resolve_inner = if (*dbghelp.dbghelp()).SymAddrIncludeInlineTrace().is_some() { + // We are on a version of dbghelp 6.2+, which contains the more modern + // Inline APIs. + resolve_with_inline + } else { + // We are on an older version of dbghelp which doesn't contain the Inline + // APIs. + resolve_legacy + }; match what { - ResolveWhat::Address(_) => resolve_with_inline(&dbghelp, what.address_or_ip(), None, cb), + ResolveWhat::Address(_) => resolve_inner(&dbghelp, what.address_or_ip(), None, cb), ResolveWhat::Frame(frame) => { - resolve_with_inline(&dbghelp, frame.ip(), frame.inner.inline_context(), cb) + resolve_inner(&dbghelp, frame.ip(), frame.inner.inline_context(), cb) } } } +/// Resolve the address using the legacy dbghelp API. +/// +/// This should work all the way down to Windows XP. The inline context is +/// ignored, since this concept was only introduced in dbghelp 6.2+. +unsafe fn resolve_legacy( + dbghelp: &dbghelp::Init, + addr: *mut c_void, + _inline_context: Option, + cb: &mut dyn FnMut(&super::Symbol), +) { + let addr = super::adjust_ip(addr) as DWORD64; + do_resolve( + |info| dbghelp.SymFromAddrW()(GetCurrentProcess(), addr, &mut 0, info), + |line| dbghelp.SymGetLineFromAddrW64()(GetCurrentProcess(), addr, &mut 0, line), + cb, + ) +} + +/// Resolve the address using the modern dbghelp APIs. +/// +/// Note that calling this function requires having dbghelp 6.2+ loaded - and +/// will panic otherwise. unsafe fn resolve_with_inline( dbghelp: &dbghelp::Init, addr: *mut c_void, From b760f44e3b35cbae06a5f22cabde0073cc5d981f Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sat, 2 Dec 2023 21:26:17 -0700 Subject: [PATCH 22/42] Fix unused_imports warning on latest nightly (rust-lang/backtrace-rs#575) This was causing CI to fail. --- crates/as-if-std/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/as-if-std/src/lib.rs b/crates/as-if-std/src/lib.rs index e1d8faaeb..89ed961f5 100644 --- a/crates/as-if-std/src/lib.rs +++ b/crates/as-if-std/src/lib.rs @@ -9,6 +9,7 @@ extern crate alloc; // We want to `pub use std::*` in the root but we don't want `std` available in // the root namespace, so do this in a funky inner module. +#[allow(unused_imports)] mod __internal { extern crate std; pub use std::*; From 7b7c10325e189e6ceace30c58baf6ee12490b746 Mon Sep 17 00:00:00 2001 From: klensy Date: Sun, 3 Dec 2023 07:42:15 +0300 Subject: [PATCH 23/42] remove few unused pub fn windows ffi (rust-lang/backtrace-rs#576) --- src/windows.rs | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/src/windows.rs b/src/windows.rs index 13287f7c3..e85fbf167 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -403,23 +403,8 @@ ffi! { pub fn RtlCaptureContext(ContextRecord: PCONTEXT) -> (); pub fn LoadLibraryA(a: *const i8) -> HMODULE; pub fn GetProcAddress(h: HMODULE, name: *const i8) -> FARPROC; - pub fn GetModuleHandleA(name: *const i8) -> HMODULE; - pub fn OpenProcess( - dwDesiredAccess: DWORD, - bInheitHandle: BOOL, - dwProcessId: DWORD, - ) -> HANDLE; pub fn GetCurrentProcessId() -> DWORD; pub fn CloseHandle(h: HANDLE) -> BOOL; - pub fn CreateFileA( - lpFileName: LPCSTR, - dwDesiredAccess: DWORD, - dwShareMode: DWORD, - lpSecurityAttributes: LPSECURITY_ATTRIBUTES, - dwCreationDisposition: DWORD, - dwFlagsAndAttributes: DWORD, - hTemplateFile: HANDLE, - ) -> HANDLE; pub fn CreateMutexA( attrs: LPSECURITY_ATTRIBUTES, initial: BOOL, @@ -460,16 +445,6 @@ ffi! { lpme: LPMODULEENTRY32W, ) -> BOOL; } - - #[link(name = "ntdll")] - extern "system" { - pub fn RtlCaptureStackBackTrace( - FramesToSkip: ULONG, - FramesToCapture: ULONG, - BackTrace: *mut PVOID, - BackTraceHash: PULONG, - ) -> USHORT; - } } #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] From 27f8f799cbb204db24ac6b1013b6b7e9f4382db8 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 5 Jan 2024 18:10:21 -0500 Subject: [PATCH 24/42] Bump rustc-serialize --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 619a1392f..dc930c70b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -157,9 +157,9 @@ checksum = "d626bb9dae77e28219937af045c257c28bfd3f69333c512553507f5f9798cb76" [[package]] name = "rustc-serialize" -version = "0.3.24" +version = "0.3.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dcf128d1287d2ea9d80910b5f1120d0b8eede3fbf1abe91c40d39ea7d51e6fda" +checksum = "fe834bc780604f4674073badbad26d7219cadfb4a2275802db12cbae17498401" [[package]] name = "serde" From d0371d425be54b1444604fb9f88994503a71b564 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 5 Jan 2024 18:16:25 -0500 Subject: [PATCH 25/42] Allow unused imports in windows module --- src/windows.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/windows.rs b/src/windows.rs index e85fbf167..4e72de8a8 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -7,7 +7,7 @@ //! This module largely exists to integrate into libstd itself where winapi is //! not currently available. -#![allow(bad_style, dead_code)] +#![allow(bad_style, dead_code, unused)] cfg_if::cfg_if! { if #[cfg(feature = "verify-winapi")] { From 1d0161b508fd7b46e92249d9d659673a6648fca6 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 5 Jan 2024 20:22:33 -0500 Subject: [PATCH 26/42] Fix linkage in the musl test --- ci/run-docker.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/run-docker.sh b/ci/run-docker.sh index 8aa6d84a4..ea77c785d 100755 --- a/ci/run-docker.sh +++ b/ci/run-docker.sh @@ -18,7 +18,7 @@ run() { --volume `pwd`/target:/checkout/target \ --workdir /checkout \ --privileged \ - --env RUSTFLAGS \ + --env RUSTFLAGS="-Lnative=/usr/lib/x86_64-linux-musl/" \ backtrace \ bash \ -c 'PATH=$PATH:/rust/bin exec ci/run.sh' From bcd6212176b90e920e9ca87d9b4077acdc8cc3dc Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 5 Jan 2024 20:26:42 -0500 Subject: [PATCH 27/42] Delete unused tuple fields --- src/print/fuchsia.rs | 6 +++--- tests/current-exe-mismatch.rs | 15 +++++---------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/print/fuchsia.rs b/src/print/fuchsia.rs index cb872697d..6e0e7ddd0 100644 --- a/src/print/fuchsia.rs +++ b/src/print/fuchsia.rs @@ -336,7 +336,7 @@ fn get_build_id<'a>(info: &'a dl_phdr_info) -> Option<&'a [u8]> { enum Error { /// NameError means that an error occurred while converting a C style string /// into a rust string. - NameError(core::str::Utf8Error), + NameError, /// BuildIDError means that we didn't find a build ID. This could either be /// because the DSO had no build ID or because the segment containing the /// build ID was malformed. @@ -362,8 +362,8 @@ fn for_each_dso(mut visitor: &mut DsoPrinter<'_, '_>) { unsafe { core::slice::from_raw_parts(info.name as *const u8, name_len) }; let name = match core::str::from_utf8(name_slice) { Ok(name) => name, - Err(err) => { - return visitor.error(Error::NameError(err)) as i32; + Err(_) => { + return visitor.error(Error::NameError) as i32; } }; let build_id = match get_build_id(info) { diff --git a/tests/current-exe-mismatch.rs b/tests/current-exe-mismatch.rs index b655827fb..e305b6677 100644 --- a/tests/current-exe-mismatch.rs +++ b/tests/current-exe-mismatch.rs @@ -16,7 +16,7 @@ fn main() { Ok(()) => { println!("test result: ok"); } - Err(EarlyExit::IgnoreTest(_)) => { + Err(EarlyExit::IgnoreTest) => { println!("test result: ignored"); } Err(EarlyExit::IoError(e)) => { @@ -34,7 +34,7 @@ const VAR: &str = "__THE_TEST_YOU_ARE_LUKE"; #[derive(Debug)] enum EarlyExit { - IgnoreTest(String), + IgnoreTest, IoError(std::io::Error), } @@ -47,7 +47,7 @@ impl From for EarlyExit { fn parent() -> Result<(), EarlyExit> { // If we cannot re-exec this test, there's no point in trying to do it. if common::cannot_reexec_the_test() { - return Err(EarlyExit::IgnoreTest("(cannot reexec)".into())); + return Err(EarlyExit::IgnoreTest); } let me = std::env::current_exe().unwrap(); @@ -111,7 +111,7 @@ fn find_interpreter(me: &Path) -> Result { .arg("-l") .arg(me) .output() - .map_err(|_err| EarlyExit::IgnoreTest("readelf invocation failed".into()))?; + .map_err(|_| EarlyExit::IgnoreTest)?; if result.status.success() { let r = BufReader::new(&result.stdout[..]); for line in r.lines() { @@ -124,11 +124,6 @@ fn find_interpreter(me: &Path) -> Result { } } } - - Err(EarlyExit::IgnoreTest( - "could not find interpreter from readelf output".into(), - )) - } else { - Err(EarlyExit::IgnoreTest("readelf returned non-success".into())) } + Err(EarlyExit::IgnoreTest) } From 891e91bf28d1f487524cd7aff43f74d1eda7df06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20W=C3=B6rister?= Date: Fri, 16 Feb 2024 13:25:59 +0100 Subject: [PATCH 28/42] Make dbghelp look for PDBs next to their exe/dll. --- .github/workflows/main.yml | 10 +++ src/dbghelp.rs | 150 +++++++++++++++++++++++++++++++++++++ src/windows.rs | 12 +++ 3 files changed, 172 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 21d7cb492..927582b79 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -110,6 +110,16 @@ jobs: - run: ./ci/debuglink-docker.sh if: contains(matrix.os, 'ubuntu') + # Test that backtraces are still symbolicated if we don't embed an absolute + # path to the PDB file in the binary. + - run: | + cargo clean + cargo test + if: contains(matrix.rust, 'msvc') + name: "Test that backtraces are symbolicated without absolute PDB path" + env: + RUSTFLAGS: "-C link-arg=/PDBALTPATH:%_PDB%" + # Test that including as a submodule will still work, both with and without # the `backtrace` feature enabled. - run: cargo build --manifest-path crates/as-if-std/Cargo.toml diff --git a/src/dbghelp.rs b/src/dbghelp.rs index e456dd45d..690d39881 100644 --- a/src/dbghelp.rs +++ b/src/dbghelp.rs @@ -23,9 +23,12 @@ #![allow(non_snake_case)] +use alloc::vec::Vec; + use super::windows::*; use core::mem; use core::ptr; +use core::slice; // Work around `SymGetOptions` and `SymSetOptions` not being present in winapi // itself. Otherwise this is only used when we're double-checking types against @@ -65,6 +68,17 @@ mod dbghelp { CurContext: LPDWORD, CurFrameIndex: LPDWORD, ) -> BOOL; + pub fn SymGetSearchPathW( + hprocess: HANDLE, + searchpatha: PWSTR, + searchpathlength: u32, + ) -> BOOL; + pub fn SymSetSearchPathW(hprocess: HANDLE, searchpatha: PCWSTR) -> BOOL; + pub fn EnumerateLoadedModulesW64( + hprocess: HANDLE, + enumloadedmodulescallback: PENUMLOADED_MODULES_CALLBACKW64, + usercontext: *const c_void, + ) -> BOOL; } pub fn assert_equal_types(a: T, _b: T) -> T { @@ -174,6 +188,20 @@ dbghelp! { path: PCWSTR, invade: BOOL ) -> BOOL; + fn SymGetSearchPathW( + hprocess: HANDLE, + searchpatha: PWSTR, + searchpathlength: u32 + ) -> BOOL; + fn SymSetSearchPathW( + hprocess: HANDLE, + searchpatha: PCWSTR + ) -> BOOL; + fn EnumerateLoadedModulesW64( + hprocess: HANDLE, + enumloadedmodulescallback: PENUMLOADED_MODULES_CALLBACKW64, + usercontext: *const c_void + ) -> BOOL; fn StackWalk64( MachineType: DWORD, hProcess: HANDLE, @@ -372,11 +400,133 @@ pub fn init() -> Result { // get to initialization first and the other will pick up that // initialization. DBGHELP.SymInitializeW().unwrap()(GetCurrentProcess(), ptr::null_mut(), TRUE); + + // The default search path for dbghelp will only look in the current working + // directory and (possibly) `_NT_SYMBOL_PATH` and `_NT_ALT_SYMBOL_PATH`. + // However, we also want to look in the directory of the executable + // and each DLL that is loaded. To do this, we need to update the search path + // to include these directories. + // + // See https://learn.microsoft.com/cpp/build/reference/pdbpath for an + // example of where symbols are usually searched for. + let mut search_path_buf = Vec::new(); + search_path_buf.resize(1024, 0); + + // Prefill the buffer with the current search path. + if DBGHELP.SymGetSearchPathW().unwrap()( + GetCurrentProcess(), + search_path_buf.as_mut_ptr(), + search_path_buf.len() as _, + ) == TRUE + { + // Trim the buffer to the actual length of the string. + let len = lstrlenW(search_path_buf.as_mut_ptr()); + assert!(len >= 0); + search_path_buf.truncate(len as usize); + } else { + // If getting the search path fails, at least include the current directory. + search_path_buf.clear(); + search_path_buf.push(utf16_char('.')); + search_path_buf.push(utf16_char(';')); + } + + let mut search_path = SearchPath::new(search_path_buf); + + // Update the search path to include the directory of the executable and each DLL. + DBGHELP.EnumerateLoadedModulesW64().unwrap()( + GetCurrentProcess(), + Some(enum_loaded_modules_callback), + &mut search_path as *mut _ as *mut _, + ); + + let new_search_path = search_path.finalize(); + + // Set the new search path. + DBGHELP.SymSetSearchPathW().unwrap()(GetCurrentProcess(), new_search_path.as_ptr()); + INITIALIZED = true; Ok(ret) } } +struct SearchPath { + search_path_utf16: Vec, +} + +fn utf16_char(c: char) -> u16 { + let buf = &mut [0u16; 2]; + let buf = c.encode_utf16(buf); + assert!(buf.len() == 1); + buf[0] +} + +impl SearchPath { + fn new(initial_search_path: Vec) -> Self { + Self { + search_path_utf16: initial_search_path, + } + } + + /// Add a path to the search path if it is not already present. + fn add(&mut self, path: &[u16]) { + let sep = utf16_char(';'); + + // We could deduplicate in a case-insensitive way, but case-sensitivity + // can be configured by directory on Windows, so let's not do that. + // https://learn.microsoft.com/windows/wsl/case-sensitivity + if !self + .search_path_utf16 + .split(|&c| c == sep) + .any(|p| p == path) + { + if self.search_path_utf16.last() != Some(&sep) { + self.search_path_utf16.push(sep); + } + self.search_path_utf16.extend_from_slice(path); + } + } + + fn finalize(mut self) -> Vec { + // Add a null terminator. + self.search_path_utf16.push(0); + self.search_path_utf16 + } +} + +extern "system" fn enum_loaded_modules_callback( + module_name: PCWSTR, + _: u64, + _: u32, + user_context: *const c_void, +) -> BOOL { + // `module_name` is an absolute path like `C:\path\to\module.dll` + // or `C:\path\to\module.exe` + let len: usize = unsafe { lstrlenW(module_name).try_into().unwrap() }; + + if len == 0 { + // This should not happen, but if it does, we can just ignore it. + return TRUE; + } + + let module_name = unsafe { slice::from_raw_parts(module_name, len) }; + let path_sep = utf16_char('\\'); + let alt_path_sep = utf16_char('/'); + + let Some(end_of_directory) = module_name + .iter() + .rposition(|&c| c == path_sep || c == alt_path_sep) + else { + // `module_name` being an absolute path, it should always contain at least one + // path separator. If not, there is nothing we can do. + return TRUE; + }; + + let search_path = unsafe { &mut *(user_context as *mut SearchPath) }; + search_path.add(&module_name[..end_of_directory]); + + TRUE +} + impl Drop for Init { fn drop(&mut self) { unsafe { diff --git a/src/windows.rs b/src/windows.rs index e85fbf167..8472ee66a 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -54,6 +54,16 @@ cfg_if::cfg_if! { ContextPointers: PKNONVOLATILE_CONTEXT_POINTERS ) -> PEXCEPTION_ROUTINE; } + + // winapi doesn't have this type + pub type PENUMLOADED_MODULES_CALLBACKW64 = Option< + unsafe extern "system" fn( + modulename: PCWSTR, + modulebase: u64, + modulesize: u32, + usercontext: *const c_void, + ) -> BOOL, + >; } } else { pub use core::ffi::c_void; @@ -291,6 +301,7 @@ ffi! { pub type PTRANSLATE_ADDRESS_ROUTINE64 = Option< unsafe extern "system" fn(hProcess: HANDLE, hThread: HANDLE, lpaddr: LPADDRESS64) -> DWORD64, >; + pub type PENUMLOADED_MODULES_CALLBACKW64 = Option BOOL>; pub type PGET_MODULE_BASE_ROUTINE64 = Option DWORD64>; pub type PFUNCTION_TABLE_ACCESS_ROUTINE64 = @@ -444,6 +455,7 @@ ffi! { hSnapshot: HANDLE, lpme: LPMODULEENTRY32W, ) -> BOOL; + pub fn lstrlenW(lpstring: PCWSTR) -> i32; } } From 17148e55f8a0a3e3488652fcc4a9035fd1792fec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20W=C3=B6rister?= Date: Thu, 22 Feb 2024 11:14:55 +0100 Subject: [PATCH 29/42] Use Win32 typedefs for new Win32 API decls. --- src/dbghelp.rs | 16 ++++++++-------- src/windows.rs | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/dbghelp.rs b/src/dbghelp.rs index 690d39881..e739c8668 100644 --- a/src/dbghelp.rs +++ b/src/dbghelp.rs @@ -71,13 +71,13 @@ mod dbghelp { pub fn SymGetSearchPathW( hprocess: HANDLE, searchpatha: PWSTR, - searchpathlength: u32, + searchpathlength: DWORD, ) -> BOOL; pub fn SymSetSearchPathW(hprocess: HANDLE, searchpatha: PCWSTR) -> BOOL; pub fn EnumerateLoadedModulesW64( hprocess: HANDLE, enumloadedmodulescallback: PENUMLOADED_MODULES_CALLBACKW64, - usercontext: *const c_void, + usercontext: PVOID, ) -> BOOL; } @@ -191,7 +191,7 @@ dbghelp! { fn SymGetSearchPathW( hprocess: HANDLE, searchpatha: PWSTR, - searchpathlength: u32 + searchpathlength: DWORD ) -> BOOL; fn SymSetSearchPathW( hprocess: HANDLE, @@ -200,7 +200,7 @@ dbghelp! { fn EnumerateLoadedModulesW64( hprocess: HANDLE, enumloadedmodulescallback: PENUMLOADED_MODULES_CALLBACKW64, - usercontext: *const c_void + usercontext: PVOID ) -> BOOL; fn StackWalk64( MachineType: DWORD, @@ -436,7 +436,7 @@ pub fn init() -> Result { DBGHELP.EnumerateLoadedModulesW64().unwrap()( GetCurrentProcess(), Some(enum_loaded_modules_callback), - &mut search_path as *mut _ as *mut _, + ((&mut search_path) as *mut SearchPath) as *mut c_void, ); let new_search_path = search_path.finalize(); @@ -495,9 +495,9 @@ impl SearchPath { extern "system" fn enum_loaded_modules_callback( module_name: PCWSTR, - _: u64, - _: u32, - user_context: *const c_void, + _: DWORD64, + _: ULONG, + user_context: PVOID, ) -> BOOL { // `module_name` is an absolute path like `C:\path\to\module.dll` // or `C:\path\to\module.exe` diff --git a/src/windows.rs b/src/windows.rs index 8472ee66a..17bfd34b0 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -59,9 +59,9 @@ cfg_if::cfg_if! { pub type PENUMLOADED_MODULES_CALLBACKW64 = Option< unsafe extern "system" fn( modulename: PCWSTR, - modulebase: u64, - modulesize: u32, - usercontext: *const c_void, + modulebase: DWORD64, + modulesize: ULONG, + usercontext: PVOID, ) -> BOOL, >; } @@ -301,7 +301,7 @@ ffi! { pub type PTRANSLATE_ADDRESS_ROUTINE64 = Option< unsafe extern "system" fn(hProcess: HANDLE, hThread: HANDLE, lpaddr: LPADDRESS64) -> DWORD64, >; - pub type PENUMLOADED_MODULES_CALLBACKW64 = Option BOOL>; + pub type PENUMLOADED_MODULES_CALLBACKW64 = Option BOOL>; pub type PGET_MODULE_BASE_ROUTINE64 = Option DWORD64>; pub type PFUNCTION_TABLE_ACCESS_ROUTINE64 = From de87ddf5bc6abea7c9db1dd45beca10701fe4d91 Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Sat, 24 Feb 2024 21:54:21 +0300 Subject: [PATCH 30/42] use `addr_of!` --- crates/line-tables-only/src/lib.rs | 3 ++- src/backtrace/dbghelp.rs | 8 ++++---- src/backtrace/libunwind.rs | 9 ++++++--- src/dbghelp.rs | 2 +- src/symbolize/dbghelp.rs | 2 +- src/symbolize/gimli/libs_dl_iterate_phdr.rs | 2 +- src/symbolize/gimli/libs_illumos.rs | 2 +- src/symbolize/gimli/libs_libnx.rs | 2 +- tests/smoke.rs | 3 ++- 9 files changed, 19 insertions(+), 14 deletions(-) diff --git a/crates/line-tables-only/src/lib.rs b/crates/line-tables-only/src/lib.rs index b292b8441..3c71a73c3 100644 --- a/crates/line-tables-only/src/lib.rs +++ b/crates/line-tables-only/src/lib.rs @@ -3,6 +3,7 @@ mod tests { use backtrace::Backtrace; use libc::c_void; use std::path::Path; + use std::ptr::addr_of_mut; pub type Callback = extern "C" fn(data: *mut c_void); @@ -49,7 +50,7 @@ mod tests { #[cfg_attr(windows, ignore)] fn backtrace_works_with_line_tables_only() { let mut backtrace: Option = None; - unsafe { foo(store_backtrace, &mut backtrace as *mut _ as *mut c_void) }; + unsafe { foo(store_backtrace, addr_of_mut!(backtrace) as *mut c_void) }; let backtrace = backtrace.expect("backtrace"); assert_contains(&backtrace, "foo", "src/callback.c", 13); assert_contains(&backtrace, "bar", "src/callback.c", 9); diff --git a/src/backtrace/dbghelp.rs b/src/backtrace/dbghelp.rs index d1b76e281..ecc126c70 100644 --- a/src/backtrace/dbghelp.rs +++ b/src/backtrace/dbghelp.rs @@ -162,7 +162,7 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { context.ip(), fn_entry, &mut context.0, - &mut handler_data as *mut usize as *mut PVOID, + ptr::addr_of_mut!(handler_data) as *mut PVOID, &mut establisher_frame, ptr::null_mut(), ); @@ -172,7 +172,7 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { #[cfg(any(target_arch = "x86", target_arch = "arm"))] #[inline(always)] pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { - use core::mem; + use core::{mem, ptr}; // Allocate necessary structures for doing the stack walk let process = GetCurrentProcess(); @@ -215,7 +215,7 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { process, thread, &mut stack_frame_ex, - &mut context.0 as *mut CONTEXT as PVOID, + ptr::addr_of_mut!(context.0) as PVOID, None, Some(function_table_access), Some(get_module_base), @@ -253,7 +253,7 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { process, thread, &mut stack_frame64, - &mut context.0 as *mut CONTEXT as PVOID, + ptr::addr_of_mut!(context.0) as PVOID, None, Some(function_table_access), Some(get_module_base), diff --git a/src/backtrace/libunwind.rs b/src/backtrace/libunwind.rs index 0cf6365f7..9861d7d32 100644 --- a/src/backtrace/libunwind.rs +++ b/src/backtrace/libunwind.rs @@ -17,6 +17,7 @@ use super::super::Bomb; use core::ffi::c_void; +use core::ptr::addr_of_mut; pub enum Frame { Raw(*mut uw::_Unwind_Context), @@ -101,7 +102,7 @@ impl Clone for Frame { #[inline(always)] pub unsafe fn trace(mut cb: &mut dyn FnMut(&super::Frame) -> bool) { - uw::_Unwind_Backtrace(trace_fn, &mut cb as *mut _ as *mut _); + uw::_Unwind_Backtrace(trace_fn, addr_of_mut!(cb) as *mut _); extern "C" fn trace_fn( ctx: *mut uw::_Unwind_Context, @@ -198,6 +199,8 @@ mod uw { _Unwind_GetGR(ctx, 15) } } else { + use core::ptr::addr_of_mut; + // On android and arm, the function `_Unwind_GetIP` and a bunch of // others are macros, so we define functions containing the // expansion of the macros. @@ -242,7 +245,7 @@ mod uw { pub unsafe fn _Unwind_GetIP(ctx: *mut _Unwind_Context) -> libc::uintptr_t { let mut val: _Unwind_Word = 0; - let ptr = &mut val as *mut _Unwind_Word; + let ptr = addr_of_mut!(val); let _ = _Unwind_VRS_Get( ctx, _Unwind_VRS_RegClass::_UVRSC_CORE, @@ -258,7 +261,7 @@ mod uw { pub unsafe fn get_sp(ctx: *mut _Unwind_Context) -> libc::uintptr_t { let mut val: _Unwind_Word = 0; - let ptr = &mut val as *mut _Unwind_Word; + let ptr = addr_of_mut!(val); let _ = _Unwind_VRS_Get( ctx, _Unwind_VRS_RegClass::_UVRSC_CORE, diff --git a/src/dbghelp.rs b/src/dbghelp.rs index e456dd45d..c524d85a9 100644 --- a/src/dbghelp.rs +++ b/src/dbghelp.rs @@ -155,7 +155,7 @@ macro_rules! dbghelp { pub fn dbghelp(&self) -> *mut Dbghelp { unsafe { - &mut DBGHELP + ptr::addr_of_mut!(DBGHELP) } } } diff --git a/src/symbolize/dbghelp.rs b/src/symbolize/dbghelp.rs index 8c47d58e8..1e6295f3c 100644 --- a/src/symbolize/dbghelp.rs +++ b/src/symbolize/dbghelp.rs @@ -222,7 +222,7 @@ unsafe fn do_resolve( } } } - let name = &name_buffer[..name_len] as *const [u8]; + let name = core::ptr::addr_of!(name_buffer[..name_len]); let mut line = mem::zeroed::(); line.SizeOfStruct = mem::size_of::() as DWORD; diff --git a/src/symbolize/gimli/libs_dl_iterate_phdr.rs b/src/symbolize/gimli/libs_dl_iterate_phdr.rs index 518512fff..9fcc8161a 100644 --- a/src/symbolize/gimli/libs_dl_iterate_phdr.rs +++ b/src/symbolize/gimli/libs_dl_iterate_phdr.rs @@ -12,7 +12,7 @@ use core::slice; pub(super) fn native_libraries() -> Vec { let mut ret = Vec::new(); unsafe { - libc::dl_iterate_phdr(Some(callback), &mut ret as *mut Vec<_> as *mut _); + libc::dl_iterate_phdr(Some(callback), core::ptr::addr_of_mut!(ret) as *mut _); } return ret; } diff --git a/src/symbolize/gimli/libs_illumos.rs b/src/symbolize/gimli/libs_illumos.rs index e64975e0c..7da16c6d0 100644 --- a/src/symbolize/gimli/libs_illumos.rs +++ b/src/symbolize/gimli/libs_illumos.rs @@ -41,7 +41,7 @@ pub(super) fn native_libraries() -> Vec { if dlinfo( RTLD_SELF, RTLD_DI_LINKMAP, - (&mut map) as *mut *const LinkMap as *mut libc::c_void, + core::ptr::addr_of_mut!(map) as *mut libc::c_void, ) != 0 { return libs; diff --git a/src/symbolize/gimli/libs_libnx.rs b/src/symbolize/gimli/libs_libnx.rs index 93b5ba17e..803008ef3 100644 --- a/src/symbolize/gimli/libs_libnx.rs +++ b/src/symbolize/gimli/libs_libnx.rs @@ -7,7 +7,7 @@ pub(super) fn native_libraries() -> Vec { static __start__: u8; } - let bias = unsafe { &__start__ } as *const u8 as usize; + let bias = core::ptr::addr_of!(__start__) as usize; let mut ret = Vec::new(); let mut segments = Vec::new(); diff --git a/tests/smoke.rs b/tests/smoke.rs index 715f567f3..a20c2d981 100644 --- a/tests/smoke.rs +++ b/tests/smoke.rs @@ -1,4 +1,5 @@ use backtrace::Frame; +use std::ptr; use std::thread; fn get_actual_fn_pointer(fp: usize) -> usize { @@ -264,7 +265,7 @@ fn sp_smoke_test() { assert!(refs.len() < 5); let x = refs.len(); - refs.push(&x as *const _ as usize); + refs.push(ptr::addr_of!(x) as usize); if refs.len() < 5 { recursive_stack_references(refs); From b31831e2d466fdb5c5600b9221611e229e757d1c Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Sat, 24 Feb 2024 23:10:13 +0300 Subject: [PATCH 31/42] Remove many `as`-casts of pointers --- crates/line-tables-only/src/lib.rs | 4 ++-- src/backtrace/dbghelp.rs | 4 ++-- src/backtrace/libunwind.rs | 8 ++++---- src/backtrace/miri.rs | 11 ++++++++--- src/backtrace/noop.rs | 7 ++++--- src/capture.rs | 8 +++----- src/dbghelp.rs | 6 +++--- src/lib.rs | 3 ++- src/print/fuchsia.rs | 2 +- src/symbolize/dbghelp.rs | 12 ++++++------ src/symbolize/gimli.rs | 2 +- src/symbolize/gimli/libs_aix.rs | 8 +++++--- src/symbolize/gimli/libs_dl_iterate_phdr.rs | 4 ++-- src/symbolize/gimli/libs_illumos.rs | 2 +- src/symbolize/gimli/libs_macos.rs | 15 +++++++++++---- src/symbolize/gimli/mmap_windows.rs | 4 ++-- src/symbolize/mod.rs | 2 +- 17 files changed, 58 insertions(+), 44 deletions(-) diff --git a/crates/line-tables-only/src/lib.rs b/crates/line-tables-only/src/lib.rs index 3c71a73c3..860d3db59 100644 --- a/crates/line-tables-only/src/lib.rs +++ b/crates/line-tables-only/src/lib.rs @@ -13,7 +13,7 @@ mod tests { extern "C" fn store_backtrace(data: *mut c_void) { let bt = backtrace::Backtrace::new(); - unsafe { *(data as *mut Option) = Some(bt) }; + unsafe { *data.cast::>() = Some(bt) }; } fn assert_contains( @@ -50,7 +50,7 @@ mod tests { #[cfg_attr(windows, ignore)] fn backtrace_works_with_line_tables_only() { let mut backtrace: Option = None; - unsafe { foo(store_backtrace, addr_of_mut!(backtrace) as *mut c_void) }; + unsafe { foo(store_backtrace, addr_of_mut!(backtrace).cast::()) }; let backtrace = backtrace.expect("backtrace"); assert_contains(&backtrace, "foo", "src/callback.c", 13); assert_contains(&backtrace, "bar", "src/callback.c", 9); diff --git a/src/backtrace/dbghelp.rs b/src/backtrace/dbghelp.rs index ecc126c70..40c5ec458 100644 --- a/src/backtrace/dbghelp.rs +++ b/src/backtrace/dbghelp.rs @@ -141,7 +141,7 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { let frame = super::Frame { inner: Frame { - base_address: fn_entry as *mut c_void, + base_address: fn_entry.cast::(), ip: context.ip() as *mut c_void, sp: context.sp() as *mut c_void, #[cfg(not(target_env = "gnu"))] @@ -162,7 +162,7 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { context.ip(), fn_entry, &mut context.0, - ptr::addr_of_mut!(handler_data) as *mut PVOID, + ptr::addr_of_mut!(handler_data).cast::(), &mut establisher_frame, ptr::null_mut(), ); diff --git a/src/backtrace/libunwind.rs b/src/backtrace/libunwind.rs index 9861d7d32..fb02c4cd1 100644 --- a/src/backtrace/libunwind.rs +++ b/src/backtrace/libunwind.rs @@ -102,13 +102,13 @@ impl Clone for Frame { #[inline(always)] pub unsafe fn trace(mut cb: &mut dyn FnMut(&super::Frame) -> bool) { - uw::_Unwind_Backtrace(trace_fn, addr_of_mut!(cb) as *mut _); + uw::_Unwind_Backtrace(trace_fn, addr_of_mut!(cb).cast()); extern "C" fn trace_fn( ctx: *mut uw::_Unwind_Context, arg: *mut c_void, ) -> uw::_Unwind_Reason_Code { - let cb = unsafe { &mut *(arg as *mut &mut dyn FnMut(&super::Frame) -> bool) }; + let cb = unsafe { &mut *arg.cast::<&mut dyn FnMut(&super::Frame) -> bool>() }; let cx = super::Frame { inner: Frame::Raw(ctx), }; @@ -251,7 +251,7 @@ mod uw { _Unwind_VRS_RegClass::_UVRSC_CORE, 15, _Unwind_VRS_DataRepresentation::_UVRSD_UINT32, - ptr as *mut c_void, + ptr.cast::(), ); (val & !1) as libc::uintptr_t } @@ -267,7 +267,7 @@ mod uw { _Unwind_VRS_RegClass::_UVRSC_CORE, SP, _Unwind_VRS_DataRepresentation::_UVRSD_UINT32, - ptr as *mut c_void, + ptr.cast::(), ); val as libc::uintptr_t } diff --git a/src/backtrace/miri.rs b/src/backtrace/miri.rs index f8c496428..ce06ed6bd 100644 --- a/src/backtrace/miri.rs +++ b/src/backtrace/miri.rs @@ -65,7 +65,7 @@ pub fn trace bool>(cb: F) { pub fn resolve_addr(ptr: *mut c_void) -> Frame { // SAFETY: Miri will stop execution with an error if this pointer // is invalid. - let frame = unsafe { miri_resolve_frame(ptr as *mut (), 1) }; + let frame = unsafe { miri_resolve_frame(ptr.cast::<()>(), 1) }; let mut name = Vec::with_capacity(frame.name_len); let mut filename = Vec::with_capacity(frame.filename_len); @@ -73,7 +73,12 @@ pub fn resolve_addr(ptr: *mut c_void) -> Frame { // SAFETY: name and filename have been allocated with the amount // of memory miri has asked for, and miri guarantees it will initialize it unsafe { - miri_resolve_frame_names(ptr as *mut (), 0, name.as_mut_ptr(), filename.as_mut_ptr()); + miri_resolve_frame_names( + ptr.cast::<()>(), + 0, + name.as_mut_ptr(), + filename.as_mut_ptr(), + ); name.set_len(frame.name_len); filename.set_len(frame.filename_len); @@ -101,7 +106,7 @@ unsafe fn trace_unsynchronized bool>(mut cb: F) { frames.set_len(len); for ptr in frames.iter() { - let frame = resolve_addr(*ptr as *mut c_void); + let frame = resolve_addr((*ptr).cast::()); if !cb(&super::Frame { inner: frame }) { return; } diff --git a/src/backtrace/noop.rs b/src/backtrace/noop.rs index 7bcea67aa..cae53df14 100644 --- a/src/backtrace/noop.rs +++ b/src/backtrace/noop.rs @@ -2,6 +2,7 @@ //! appropriate. use core::ffi::c_void; +use core::ptr::null_mut; #[inline(always)] pub fn trace(_cb: &mut dyn FnMut(&super::Frame) -> bool) {} @@ -11,15 +12,15 @@ pub struct Frame; impl Frame { pub fn ip(&self) -> *mut c_void { - 0 as *mut _ + null_mut() } pub fn sp(&self) -> *mut c_void { - 0 as *mut _ + null_mut() } pub fn symbol_address(&self) -> *mut c_void { - 0 as *mut _ + null_mut() } pub fn module_base_address(&self) -> Option<*mut c_void> { diff --git a/src/capture.rs b/src/capture.rs index e0dd9c474..a7ee93834 100644 --- a/src/capture.rs +++ b/src/capture.rs @@ -272,7 +272,7 @@ impl BacktraceFrame { /// This function requires the `std` feature of the `backtrace` crate to be /// enabled, and the `std` feature is enabled by default. pub fn ip(&self) -> *mut c_void { - self.frame.ip() as *mut c_void + self.frame.ip() } /// Same as `Frame::symbol_address` @@ -282,7 +282,7 @@ impl BacktraceFrame { /// This function requires the `std` feature of the `backtrace` crate to be /// enabled, and the `std` feature is enabled by default. pub fn symbol_address(&self) -> *mut c_void { - self.frame.symbol_address() as *mut c_void + self.frame.symbol_address() } /// Same as `Frame::module_base_address` @@ -292,9 +292,7 @@ impl BacktraceFrame { /// This function requires the `std` feature of the `backtrace` crate to be /// enabled, and the `std` feature is enabled by default. pub fn module_base_address(&self) -> Option<*mut c_void> { - self.frame - .module_base_address() - .map(|addr| addr as *mut c_void) + self.frame.module_base_address() } /// Returns the list of symbols that this frame corresponds to. diff --git a/src/dbghelp.rs b/src/dbghelp.rs index c524d85a9..54d320bce 100644 --- a/src/dbghelp.rs +++ b/src/dbghelp.rs @@ -88,7 +88,7 @@ macro_rules! dbghelp { static mut DBGHELP: Dbghelp = Dbghelp { // Initially we haven't loaded the DLL - dll: 0 as *mut _, + dll: ptr::null_mut(), // Initially all functions are set to zero to say they need to be // dynamically loaded. $($name: 0,)* @@ -108,7 +108,7 @@ macro_rules! dbghelp { } let lib = b"dbghelp.dll\0"; unsafe { - self.dll = LoadLibraryA(lib.as_ptr() as *const i8); + self.dll = LoadLibraryA(lib.as_ptr().cast::()); if self.dll.is_null() { Err(()) } else { @@ -135,7 +135,7 @@ macro_rules! dbghelp { fn symbol(&self, symbol: &[u8]) -> Option { unsafe { - match GetProcAddress(self.dll, symbol.as_ptr() as *const _) as usize { + match GetProcAddress(self.dll, symbol.as_ptr().cast()) as usize { 0 => None, n => Some(n), } diff --git a/src/lib.rs b/src/lib.rs index 44a0bc64e..5b97281a7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -159,11 +159,12 @@ impl Drop for Bomb { mod lock { use std::boxed::Box; use std::cell::Cell; + use std::ptr; use std::sync::{Mutex, MutexGuard, Once}; pub struct LockGuard(Option>); - static mut LOCK: *mut Mutex<()> = 0 as *mut _; + static mut LOCK: *mut Mutex<()> = ptr::null_mut(); static INIT: Once = Once::new(); thread_local!(static LOCK_HELD: Cell = Cell::new(false)); diff --git a/src/print/fuchsia.rs b/src/print/fuchsia.rs index 6e0e7ddd0..9bae98dd0 100644 --- a/src/print/fuchsia.rs +++ b/src/print/fuchsia.rs @@ -359,7 +359,7 @@ fn for_each_dso(mut visitor: &mut DsoPrinter<'_, '_>) { // location. let name_len = unsafe { libc::strlen(info.name) }; let name_slice: &[u8] = - unsafe { core::slice::from_raw_parts(info.name as *const u8, name_len) }; + unsafe { core::slice::from_raw_parts(info.name.cast::(), name_len) }; let name = match core::str::from_utf8(name_slice) { Ok(name) => name, Err(_) => { diff --git a/src/symbolize/dbghelp.rs b/src/symbolize/dbghelp.rs index 1e6295f3c..8a7bdf623 100644 --- a/src/symbolize/dbghelp.rs +++ b/src/symbolize/dbghelp.rs @@ -23,6 +23,7 @@ use core::char; use core::ffi::c_void; use core::marker; use core::mem; +use core::ptr; use core::slice; // Store an OsString on std so we can provide the symbol name and filename. @@ -44,7 +45,7 @@ impl Symbol<'_> { } pub fn addr(&self) -> Option<*mut c_void> { - Some(self.addr as *mut _) + Some(self.addr) } pub fn filename_raw(&self) -> Option> { @@ -184,8 +185,7 @@ unsafe fn do_resolve( ) { const SIZE: usize = 2 * MAX_SYM_NAME + mem::size_of::(); let mut data = Aligned8([0u8; SIZE]); - let data = &mut data.0; - let info = &mut *(data.as_mut_ptr() as *mut SYMBOL_INFOW); + let info = &mut *data.0.as_mut_ptr().cast::(); info.MaxNameLen = MAX_SYM_NAME as ULONG; // the struct size in C. the value is different to // `size_of::() - MAX_SYM_NAME + 1` (== 81) @@ -200,7 +200,7 @@ unsafe fn do_resolve( // give a buffer of (MaxNameLen - 1) characters and set NameLen to // the real value. let name_len = ::core::cmp::min(info.NameLen as usize, info.MaxNameLen as usize - 1); - let name_ptr = info.Name.as_ptr() as *const u16; + let name_ptr = info.Name.as_ptr().cast::(); let name = slice::from_raw_parts(name_ptr, name_len); // Reencode the utf-16 symbol to utf-8 so we can use `SymbolName::new` like @@ -222,7 +222,7 @@ unsafe fn do_resolve( } } } - let name = core::ptr::addr_of!(name_buffer[..name_len]); + let name = ptr::addr_of!(name_buffer[..name_len]); let mut line = mem::zeroed::(); line.SizeOfStruct = mem::size_of::() as DWORD; @@ -240,7 +240,7 @@ unsafe fn do_resolve( let len = len as usize; - filename = Some(slice::from_raw_parts(base, len) as *const [u16]); + filename = Some(ptr::from_ref(slice::from_raw_parts(base, len))); } cb(&super::Symbol { diff --git a/src/symbolize/gimli.rs b/src/symbolize/gimli.rs index 3b28bf741..fa8e59723 100644 --- a/src/symbolize/gimli.rs +++ b/src/symbolize/gimli.rs @@ -430,7 +430,7 @@ pub unsafe fn resolve(what: ResolveWhat<'_>, cb: &mut dyn FnMut(&super::Symbol)) }; Cache::with_global(|cache| { - let (lib, addr) = match cache.avma_to_svma(addr as *const u8) { + let (lib, addr) = match cache.avma_to_svma(addr.cast_const().cast::()) { Some(pair) => pair, None => return, }; diff --git a/src/symbolize/gimli/libs_aix.rs b/src/symbolize/gimli/libs_aix.rs index 8cac11d4d..aa4eaff89 100644 --- a/src/symbolize/gimli/libs_aix.rs +++ b/src/symbolize/gimli/libs_aix.rs @@ -20,7 +20,7 @@ pub(super) fn native_libraries() -> Vec { loop { if libc::loadquery( libc::L_GETINFO, - buffer.as_mut_ptr() as *mut libc::c_char, + buffer.as_mut_ptr().cast::(), (mem::size_of::() * buffer.len()) as u32, ) != -1 { @@ -66,8 +66,10 @@ pub(super) fn native_libraries() -> Vec { if (*current).ldinfo_next == 0 { break; } - current = (current as *mut libc::c_char).offset((*current).ldinfo_next as isize) - as *mut libc::ld_info; + current = current + .cast::() + .offset((*current).ldinfo_next as isize) + .cast::(); } } return ret; diff --git a/src/symbolize/gimli/libs_dl_iterate_phdr.rs b/src/symbolize/gimli/libs_dl_iterate_phdr.rs index 9fcc8161a..c8558e626 100644 --- a/src/symbolize/gimli/libs_dl_iterate_phdr.rs +++ b/src/symbolize/gimli/libs_dl_iterate_phdr.rs @@ -12,7 +12,7 @@ use core::slice; pub(super) fn native_libraries() -> Vec { let mut ret = Vec::new(); unsafe { - libc::dl_iterate_phdr(Some(callback), core::ptr::addr_of_mut!(ret) as *mut _); + libc::dl_iterate_phdr(Some(callback), core::ptr::addr_of_mut!(ret).cast()); } return ret; } @@ -43,7 +43,7 @@ unsafe extern "C" fn callback( vec: *mut libc::c_void, ) -> libc::c_int { let info = &*info; - let libs = &mut *(vec as *mut Vec); + let libs = &mut *vec.cast::>(); let is_main_prog = info.dlpi_name.is_null() || *info.dlpi_name == 0; let name = if is_main_prog { // The man page for dl_iterate_phdr says that the first object visited by diff --git a/src/symbolize/gimli/libs_illumos.rs b/src/symbolize/gimli/libs_illumos.rs index 7da16c6d0..4fc510cf5 100644 --- a/src/symbolize/gimli/libs_illumos.rs +++ b/src/symbolize/gimli/libs_illumos.rs @@ -41,7 +41,7 @@ pub(super) fn native_libraries() -> Vec { if dlinfo( RTLD_SELF, RTLD_DI_LINKMAP, - core::ptr::addr_of_mut!(map) as *mut libc::c_void, + core::ptr::addr_of_mut!(map).cast::(), ) != 0 { return libs; diff --git a/src/symbolize/gimli/libs_macos.rs b/src/symbolize/gimli/libs_macos.rs index 438bbff6f..671505a13 100644 --- a/src/symbolize/gimli/libs_macos.rs +++ b/src/symbolize/gimli/libs_macos.rs @@ -7,6 +7,13 @@ use super::{Library, LibrarySegment}; use core::convert::TryInto; use core::mem; +// FIXME: replace with ptr::from_ref once MSRV is high enough +#[inline(always)] +#[must_use] +const fn ptr_from_ref(r: &T) -> *const T { + r +} + pub(super) fn native_libraries() -> Vec { let mut ret = Vec::new(); let images = unsafe { libc::_dyld_image_count() }; @@ -42,18 +49,18 @@ fn native_library(i: u32) -> Option { match (*header).magic { macho::MH_MAGIC => { let endian = NativeEndian; - let header = &*(header as *const macho::MachHeader32); + let header = &*header.cast::>(); let data = core::slice::from_raw_parts( - header as *const _ as *const u8, + ptr_from_ref(header).cast::(), mem::size_of_val(header) + header.sizeofcmds.get(endian) as usize, ); (header.load_commands(endian, data, 0).ok()?, endian) } macho::MH_MAGIC_64 => { let endian = NativeEndian; - let header = &*(header as *const macho::MachHeader64); + let header = &*header.cast::>(); let data = core::slice::from_raw_parts( - header as *const _ as *const u8, + ptr_from_ref(header).cast::(), mem::size_of_val(header) + header.sizeofcmds.get(endian) as usize, ); (header.load_commands(endian, data, 0).ok()?, endian) diff --git a/src/symbolize/gimli/mmap_windows.rs b/src/symbolize/gimli/mmap_windows.rs index b39509d8c..c49ab083c 100644 --- a/src/symbolize/gimli/mmap_windows.rs +++ b/src/symbolize/gimli/mmap_windows.rs @@ -17,7 +17,7 @@ impl Mmap { pub unsafe fn map(file: &File, len: usize) -> Option { let file = file.try_clone().ok()?; let mapping = CreateFileMappingA( - file.as_raw_handle() as *mut _, + file.as_raw_handle().cast(), ptr::null_mut(), PAGE_READONLY, 0, @@ -43,7 +43,7 @@ impl Deref for Mmap { type Target = [u8]; fn deref(&self) -> &[u8] { - unsafe { slice::from_raw_parts(self.ptr as *const u8, self.len) } + unsafe { slice::from_raw_parts(self.ptr.cast_const().cast::(), self.len) } } } diff --git a/src/symbolize/mod.rs b/src/symbolize/mod.rs index a7c199506..f8571d7be 100644 --- a/src/symbolize/mod.rs +++ b/src/symbolize/mod.rs @@ -210,7 +210,7 @@ impl Symbol { /// Returns the starting address of this function. pub fn addr(&self) -> Option<*mut c_void> { - self.inner.addr().map(|p| p as *mut _) + self.inner.addr() } /// Returns the raw filename as a slice. This is mainly useful for `no_std` From b3a2a03cec2600e72d76b0d3f5b3b710bada4eb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20W=C3=B6rister?= Date: Tue, 27 Feb 2024 10:46:04 +0100 Subject: [PATCH 32/42] Add -Cforce-frame-pointers to make the PDBALTPATH test case non-flaky on i686-pc-windows-msvc --- .github/workflows/main.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 927582b79..e28c55a97 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -112,13 +112,14 @@ jobs: # Test that backtraces are still symbolicated if we don't embed an absolute # path to the PDB file in the binary. - - run: | - cargo clean - cargo test + # Add -Cforce-frame-pointers for stability. The test otherwise fails + # non-deterministically on i686-pc-windows-msvc because the stack cannot be + # unwound reliably. This failure is not related to the feature being tested. + - run: cargo clean && cargo test if: contains(matrix.rust, 'msvc') name: "Test that backtraces are symbolicated without absolute PDB path" env: - RUSTFLAGS: "-C link-arg=/PDBALTPATH:%_PDB%" + RUSTFLAGS: "-Clink-arg=/PDBALTPATH:%_PDB% -Cforce-frame-pointers" # Test that including as a submodule will still work, both with and without # the `backtrace` feature enabled. From ef39a7d7da58b4cae8c8f3fc67a8300fd8d2d0d9 Mon Sep 17 00:00:00 2001 From: Daniel Paoliello Date: Fri, 8 Mar 2024 10:12:24 -0800 Subject: [PATCH 33/42] Add support for arm64ec (rust-lang/backtrace-rs#587) The Rust Compiler has recently added support for ARM64EC (rust-lang/rust#119199), so this change adds support for ARM64EC to backtrace by using the same OS structures and functions as x64 NOT AArch64: this is because an ARM64EC binary runs within an x64 process (on an AArch64 machine), thus all the OS binaries it is interfacing with expose the x64 API. --- src/backtrace/dbghelp.rs | 8 ++++++-- src/windows.rs | 10 +++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/backtrace/dbghelp.rs b/src/backtrace/dbghelp.rs index d1b76e281..193fc82e7 100644 --- a/src/backtrace/dbghelp.rs +++ b/src/backtrace/dbghelp.rs @@ -60,7 +60,7 @@ impl Frame { #[repr(C, align(16))] // required by `CONTEXT`, is a FIXME in winapi right now struct MyContext(CONTEXT); -#[cfg(target_arch = "x86_64")] +#[cfg(any(target_arch = "x86_64", target_arch = "arm64ec"))] impl MyContext { #[inline(always)] fn ip(&self) -> DWORD64 { @@ -122,7 +122,11 @@ impl MyContext { } } -#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] +#[cfg(any( + target_arch = "x86_64", + target_arch = "aarch64", + target_arch = "arm64ec" +))] #[inline(always)] pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { use core::ptr; diff --git a/src/windows.rs b/src/windows.rs index 268362f64..a95762d35 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -459,7 +459,11 @@ ffi! { } } -#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))] +#[cfg(any( + target_arch = "x86_64", + target_arch = "aarch64", + target_arch = "arm64ec" +))] ffi! { #[link(name = "kernel32")] extern "system" { @@ -606,7 +610,7 @@ ffi! { } } -#[cfg(target_arch = "x86_64")] +#[cfg(any(target_arch = "x86_64", target_arch = "arm64ec"))] ffi! { #[repr(C, align(8))] pub struct CONTEXT { @@ -674,7 +678,7 @@ ffi! { } #[repr(C)] -#[cfg(target_arch = "x86_64")] +#[cfg(any(target_arch = "x86_64", target_arch = "arm64ec"))] #[derive(Copy, Clone)] pub struct FLOATING_SAVE_AREA { _Dummy: [u8; 512], From da34864acf354bcfe4258d2f23beff593f9fee8b Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Fri, 8 Mar 2024 13:22:30 -0500 Subject: [PATCH 34/42] Fix deny(unused) of an unused import with SGX + Miri (rust-lang/backtrace-rs#581) The `deny(unused)` use that used to be here is incompatible with building this crate with Miri. The import in question is only used in a module that is only declared when we are not `cfg(miri)`. I tried to fix this by grouping items into modules so that I could apply `cfg(not(miri))` to all the SGX code. One could also make the build work by deleting the `#[deny(unused)]`, but that felt hacky. --- src/backtrace/libunwind.rs | 2 +- src/backtrace/mod.rs | 64 ++++++++++++++++++++++---------------- 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/src/backtrace/libunwind.rs b/src/backtrace/libunwind.rs index fb02c4cd1..c3d88605c 100644 --- a/src/backtrace/libunwind.rs +++ b/src/backtrace/libunwind.rs @@ -49,7 +49,7 @@ impl Frame { // the address here, which could be later mapped to correct function. #[cfg(all(target_env = "sgx", target_vendor = "fortanix"))] { - let image_base = super::get_image_base(); + let image_base = super::sgx_image_base::get_image_base(); ip = usize::wrapping_sub(ip as usize, image_base as _) as _; } ip diff --git a/src/backtrace/mod.rs b/src/backtrace/mod.rs index 1b812d84e..c872b1c37 100644 --- a/src/backtrace/mod.rs +++ b/src/backtrace/mod.rs @@ -125,39 +125,49 @@ impl fmt::Debug for Frame { } } -#[cfg(all(target_env = "sgx", target_vendor = "fortanix", not(feature = "std")))] -mod sgx_no_std_image_base { - use core::ffi::c_void; - use core::sync::atomic::{AtomicUsize, Ordering::SeqCst}; - - static IMAGE_BASE: AtomicUsize = AtomicUsize::new(0); - - /// Set the image base address. This is only available for Fortanix SGX - /// target when the `std` feature is not enabled. This can be used in the - /// standard library to set the correct base address. - #[doc(hidden)] - pub fn set_image_base(base_addr: *mut c_void) { - IMAGE_BASE.store(base_addr as _, SeqCst); +#[cfg(all(target_env = "sgx", target_vendor = "fortanix", not(miri)))] +mod sgx_image_base { + + #[cfg(not(feature = "std"))] + pub(crate) mod imp { + use core::ffi::c_void; + use core::sync::atomic::{AtomicUsize, Ordering::SeqCst}; + + static IMAGE_BASE: AtomicUsize = AtomicUsize::new(0); + + /// Set the image base address. This is only available for Fortanix SGX + /// target when the `std` feature is not enabled. This can be used in the + /// standard library to set the correct base address. + #[doc(hidden)] + pub fn set_image_base(base_addr: *mut c_void) { + IMAGE_BASE.store(base_addr as _, SeqCst); + } + + pub(crate) fn get_image_base() -> *mut c_void { + IMAGE_BASE.load(SeqCst) as _ + } } - pub(crate) fn get_image_base() -> *mut c_void { - IMAGE_BASE.load(SeqCst) as _ - } -} - -#[cfg(all(target_env = "sgx", target_vendor = "fortanix", not(feature = "std")))] -pub use self::sgx_no_std_image_base::set_image_base; + #[cfg(feature = "std")] + mod imp { + use core::ffi::c_void; -#[cfg(all(target_env = "sgx", target_vendor = "fortanix", not(feature = "std")))] -#[deny(unused)] -pub(crate) use self::sgx_no_std_image_base::get_image_base; + pub(crate) fn get_image_base() -> *mut c_void { + std::os::fortanix_sgx::mem::image_base() as _ + } + } -#[cfg(all(target_env = "sgx", target_vendor = "fortanix", feature = "std"))] -#[deny(unused)] -pub(crate) fn get_image_base() -> *mut c_void { - std::os::fortanix_sgx::mem::image_base() as _ + pub(crate) use imp::get_image_base; } +#[cfg(all( + target_env = "sgx", + target_vendor = "fortanix", + not(feature = "std"), + not(miri) +))] +pub use sgx_image_base::imp::set_image_base; + cfg_if::cfg_if! { // This needs to come first, to ensure that // Miri takes priority over the host platform From 4dea00c98ff120834c3071919320b9ed51129c39 Mon Sep 17 00:00:00 2001 From: Mindaugas Vinkelis Date: Sat, 9 Mar 2024 08:01:56 +0200 Subject: [PATCH 35/42] make it easy to resolve symbols by frame (#526) Hello, Resolving backtrace (at least for the first time) is very slow, but this is not something that can be solved (I guess). However, not all frames are equally useful, and at least in the code base that I'm working on, we simply ignore bunch of them. We have code something like this: ```rust let mut bt = Backtrace::new_unresolved(); // ... bt.resolve(); let bt: Vec = bt.frames() .iter() .flat_map(BacktraceFrame::symbols) .take_while(is_not_async_stuff) .map(format_symbols) .collect(); ``` For us, it takes around 700~1500ms to resolve whole backtrace. With this PR we're able to write like this: ```rust let bt: Vec = bt.frames_mut() .iter_mut() .flat_map(BacktraceFrame::symbols_resolved) .take_while(is_not_async_stuff) .map(format_symbols) .collect(); ``` And it takes around 25ms to resolve the frames that we actually need. I'm aware of low level utils `backtrace::trace` and `backtrace::resolve_frame`, but this would basically mean that I would basically need to reimplement Backtrace, including Serialize and Deserialize capability, which is too much. --- src/capture.rs | 96 ++++++++++++++++++++++++++++---------------------- 1 file changed, 53 insertions(+), 43 deletions(-) diff --git a/src/capture.rs b/src/capture.rs index a7ee93834..4cb398068 100644 --- a/src/capture.rs +++ b/src/capture.rs @@ -26,9 +26,6 @@ use serde::{Deserialize, Serialize}; pub struct Backtrace { // Frames here are listed from top-to-bottom of the stack frames: Vec, - // The index we believe is the actual start of the backtrace, omitting - // frames like `Backtrace::new` and `backtrace::trace`. - actual_start_index: usize, } fn _assert_send_sync() { @@ -86,6 +83,27 @@ impl Frame { } => module_base_address.map(|addr| addr as *mut c_void), } } + + /// Resolve all addresses in the frame to their symbolic names. + fn resolve_symbols(&self) -> Vec { + let mut symbols = Vec::new(); + let sym = |symbol: &Symbol| { + symbols.push(BacktraceSymbol { + name: symbol.name().map(|m| m.as_bytes().to_vec()), + addr: symbol.addr().map(|a| a as usize), + filename: symbol.filename().map(|m| m.to_owned()), + lineno: symbol.lineno(), + colno: symbol.colno(), + }); + }; + match *self { + Frame::Raw(ref f) => resolve_frame(f, sym), + Frame::Deserialized { ip, .. } => { + resolve(ip as *mut c_void, sym); + } + } + symbols + } } /// Captured version of a symbol in a backtrace. @@ -172,23 +190,22 @@ impl Backtrace { fn create(ip: usize) -> Backtrace { let mut frames = Vec::new(); - let mut actual_start_index = None; trace(|frame| { frames.push(BacktraceFrame { frame: Frame::Raw(frame.clone()), symbols: None, }); - if frame.symbol_address() as usize == ip && actual_start_index.is_none() { - actual_start_index = Some(frames.len()); + // clear inner frames, and start with call site. + if frame.symbol_address() as usize == ip { + frames.clear(); } + true }); + frames.shrink_to_fit(); - Backtrace { - frames, - actual_start_index: actual_start_index.unwrap_or(0), - } + Backtrace { frames } } /// Returns the frames from when this backtrace was captured. @@ -202,7 +219,7 @@ impl Backtrace { /// This function requires the `std` feature of the `backtrace` crate to be /// enabled, and the `std` feature is enabled by default. pub fn frames(&self) -> &[BacktraceFrame] { - &self.frames[self.actual_start_index..] + self.frames.as_slice() } /// If this backtrace was created from `new_unresolved` then this function @@ -216,41 +233,18 @@ impl Backtrace { /// This function requires the `std` feature of the `backtrace` crate to be /// enabled, and the `std` feature is enabled by default. pub fn resolve(&mut self) { - for frame in self.frames.iter_mut().filter(|f| f.symbols.is_none()) { - let mut symbols = Vec::new(); - { - let sym = |symbol: &Symbol| { - symbols.push(BacktraceSymbol { - name: symbol.name().map(|m| m.as_bytes().to_vec()), - addr: symbol.addr().map(|a| a as usize), - filename: symbol.filename().map(|m| m.to_owned()), - lineno: symbol.lineno(), - colno: symbol.colno(), - }); - }; - match frame.frame { - Frame::Raw(ref f) => resolve_frame(f, sym), - Frame::Deserialized { ip, .. } => { - resolve(ip as *mut c_void, sym); - } - } - } - frame.symbols = Some(symbols); - } + self.frames.iter_mut().for_each(BacktraceFrame::resolve); } } impl From> for Backtrace { fn from(frames: Vec) -> Self { - Backtrace { - frames, - actual_start_index: 0, - } + Backtrace { frames } } } impl From for BacktraceFrame { - fn from(frame: crate::Frame) -> BacktraceFrame { + fn from(frame: crate::Frame) -> Self { BacktraceFrame { frame: Frame::Raw(frame), symbols: None, @@ -258,6 +252,9 @@ impl From for BacktraceFrame { } } +// we don't want implementing `impl From for Vec` on purpose, +// because "... additional directions for Vec can weaken type inference ..." +// more information on https://github.com/rust-lang/backtrace-rs/pull/526 impl Into> for Backtrace { fn into(self) -> Vec { self.frames @@ -312,6 +309,20 @@ impl BacktraceFrame { pub fn symbols(&self) -> &[BacktraceSymbol] { self.symbols.as_ref().map(|s| &s[..]).unwrap_or(&[]) } + + /// Resolve all addresses in this frame to their symbolic names. + /// + /// If this frame has been previously resolved, this function does nothing. + /// + /// # Required features + /// + /// This function requires the `std` feature of the `backtrace` crate to be + /// enabled, and the `std` feature is enabled by default. + pub fn resolve(&mut self) { + if self.symbols.is_none() { + self.symbols = Some(self.frame.resolve_symbols()); + } + } } impl BacktraceSymbol { @@ -368,11 +379,10 @@ impl BacktraceSymbol { impl fmt::Debug for Backtrace { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let full = fmt.alternate(); - let (frames, style) = if full { - (&self.frames[..], PrintFmt::Full) + let style = if fmt.alternate() { + PrintFmt::Full } else { - (&self.frames[self.actual_start_index..], PrintFmt::Short) + PrintFmt::Short }; // When printing paths we try to strip the cwd if it exists, otherwise @@ -383,7 +393,7 @@ impl fmt::Debug for Backtrace { let mut print_path = move |fmt: &mut fmt::Formatter<'_>, path: crate::BytesOrWideString<'_>| { let path = path.into_path_buf(); - if !full { + if style == PrintFmt::Full { if let Ok(cwd) = &cwd { if let Ok(suffix) = path.strip_prefix(cwd) { return fmt::Display::fmt(&suffix.display(), fmt); @@ -395,7 +405,7 @@ impl fmt::Debug for Backtrace { let mut f = BacktraceFmt::new(fmt, style, &mut print_path); f.add_context()?; - for frame in frames { + for frame in &self.frames { f.frame().backtrace_frame(frame)?; } f.finish()?; From 3acbb65294098c7ef97b79e59e2af996334b7f45 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 9 Mar 2024 01:04:00 -0500 Subject: [PATCH 36/42] Apply clippy::uninlined_format_args fixes (rust-lang/backtrace-rs#486) * Apply clippy::uninlined_format_args fix * Addressed feedback and added a few more * also did one minor spelling change in a print statement in `current-exe-mismatch.rs` --- README.md | 2 +- build.rs | 7 ++----- crates/cpp_smoke_test/tests/smoke.rs | 4 ++-- crates/debuglink/src/main.rs | 4 ++-- crates/macos_frames_test/tests/main.rs | 3 +-- examples/raw.rs | 2 +- src/capture.rs | 4 ++-- src/print.rs | 10 +++++----- src/print/fuchsia.rs | 2 +- src/symbolize/mod.rs | 2 +- tests/accuracy/main.rs | 4 ++-- tests/current-exe-mismatch.rs | 4 ++-- tests/long_fn_name.rs | 2 +- tests/skip_inner_frames.rs | 4 ++-- tests/smoke.rs | 12 ++++++------ 15 files changed, 31 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index f090d0ca0..cebbad6d9 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ fn main() { // do_some_work(); - println!("{:?}", bt); + println!("{bt:?}"); } ``` diff --git a/build.rs b/build.rs index ed4e07a85..2a3da849f 100644 --- a/build.rs +++ b/build.rs @@ -28,10 +28,7 @@ fn build_android() { let expansion = match cc::Build::new().file(&android_api_c).try_expand() { Ok(result) => result, Err(e) => { - eprintln!( - "warning: android version detection failed while running C compiler: {}", - e - ); + eprintln!("warning: android version detection failed while running C compiler: {e}"); return; } }; @@ -39,7 +36,7 @@ fn build_android() { Ok(s) => s, Err(_) => return, }; - eprintln!("expanded android version detection:\n{}", expansion); + eprintln!("expanded android version detection:\n{expansion}"); let i = match expansion.find(MARKER) { Some(i) => i, None => return, diff --git a/crates/cpp_smoke_test/tests/smoke.rs b/crates/cpp_smoke_test/tests/smoke.rs index b9aef47f5..a303562f9 100644 --- a/crates/cpp_smoke_test/tests/smoke.rs +++ b/crates/cpp_smoke_test/tests/smoke.rs @@ -49,13 +49,13 @@ fn smoke_test_cpp() { .take(2) .collect(); - println!("actual names = {:#?}", names); + println!("actual names = {names:#?}"); let expected = [ "void space::templated_trampoline(void (*)())", "cpp_trampoline", ]; - println!("expected names = {:#?}", expected); + println!("expected names = {expected:#?}"); assert_eq!(names.len(), expected.len()); for (actual, expected) in names.iter().zip(expected.iter()) { diff --git a/crates/debuglink/src/main.rs b/crates/debuglink/src/main.rs index 99265ae9a..b0f4063d7 100644 --- a/crates/debuglink/src/main.rs +++ b/crates/debuglink/src/main.rs @@ -9,7 +9,7 @@ fn main() { let expect = std::path::Path::new(&crate_dir).join("src/main.rs"); let bt = backtrace::Backtrace::new(); - println!("{:?}", bt); + println!("{bt:?}"); let mut found_main = false; @@ -20,7 +20,7 @@ fn main() { } if let Some(name) = symbols[0].name() { - let name = format!("{:#}", name); + let name = format!("{name:#}"); if name == "debuglink::main" { found_main = true; let filename = symbols[0].filename().unwrap(); diff --git a/crates/macos_frames_test/tests/main.rs b/crates/macos_frames_test/tests/main.rs index f0e905b24..5d74bdcc3 100644 --- a/crates/macos_frames_test/tests/main.rs +++ b/crates/macos_frames_test/tests/main.rs @@ -15,8 +15,7 @@ fn backtrace_no_dsym() { let executable_name = dsym_path.file_name().unwrap().to_str().unwrap().to_string(); assert!(dsym_path.pop()); // Pop executable dsym_path.push(format!( - "{}.dSYM/Contents/Resources/DWARF/{0}", - executable_name + "{executable_name}.dSYM/Contents/Resources/DWARF/{executable_name}" )); let _ = fs::OpenOptions::new() .read(false) diff --git a/examples/raw.rs b/examples/raw.rs index d96a127a2..95e17dbd5 100644 --- a/examples/raw.rs +++ b/examples/raw.rs @@ -33,7 +33,7 @@ fn print() { } if let Some(name) = symbol.name() { - print!(" - {}", name); + print!(" - {name}"); } else { print!(" - "); } diff --git a/src/capture.rs b/src/capture.rs index 4cb398068..73d94dc4b 100644 --- a/src/capture.rs +++ b/src/capture.rs @@ -174,9 +174,9 @@ impl Backtrace { /// use backtrace::Backtrace; /// /// let mut current_backtrace = Backtrace::new_unresolved(); - /// println!("{:?}", current_backtrace); // no symbol names + /// println!("{current_backtrace:?}"); // no symbol names /// current_backtrace.resolve(); - /// println!("{:?}", current_backtrace); // symbol names now present + /// println!("{current_backtrace:?}"); // symbol names now present /// ``` /// /// # Required features diff --git a/src/print.rs b/src/print.rs index de8569182..6e38a0f13 100644 --- a/src/print.rs +++ b/src/print.rs @@ -239,7 +239,7 @@ impl BacktraceFrameFmt<'_, '_, '_> { if self.symbol_index == 0 { write!(self.fmt.fmt, "{:4}: ", self.fmt.frame_index)?; if let PrintFmt::Full = self.fmt.format { - write!(self.fmt.fmt, "{:1$?} - ", frame_ip, HEX_WIDTH)?; + write!(self.fmt.fmt, "{frame_ip:HEX_WIDTH$?} - ")?; } } else { write!(self.fmt.fmt, " ")?; @@ -252,8 +252,8 @@ impl BacktraceFrameFmt<'_, '_, '_> { // more information if we're a full backtrace. Here we also handle // symbols which don't have a name, match (symbol_name, &self.fmt.format) { - (Some(name), PrintFmt::Short) => write!(self.fmt.fmt, "{:#}", name)?, - (Some(name), PrintFmt::Full) => write!(self.fmt.fmt, "{}", name)?, + (Some(name), PrintFmt::Short) => write!(self.fmt.fmt, "{name:#}")?, + (Some(name), PrintFmt::Full) => write!(self.fmt.fmt, "{name}")?, (None, _) | (_, PrintFmt::__Nonexhaustive) => write!(self.fmt.fmt, "")?, } self.fmt.fmt.write_str("\n")?; @@ -282,11 +282,11 @@ impl BacktraceFrameFmt<'_, '_, '_> { // Delegate to our internal callback to print the filename and then // print out the line number. (self.fmt.print_path)(self.fmt.fmt, file)?; - write!(self.fmt.fmt, ":{}", line)?; + write!(self.fmt.fmt, ":{line}")?; // Add column number, if available. if let Some(colno) = colno { - write!(self.fmt.fmt, ":{}", colno)?; + write!(self.fmt.fmt, ":{colno}")?; } write!(self.fmt.fmt, "\n")?; diff --git a/src/print/fuchsia.rs b/src/print/fuchsia.rs index 9bae98dd0..5a5aae530 100644 --- a/src/print/fuchsia.rs +++ b/src/print/fuchsia.rs @@ -312,7 +312,7 @@ struct HexSlice<'a> { impl fmt::Display for HexSlice<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { for byte in self.bytes { - write!(f, "{:02x}", byte)?; + write!(f, "{byte:02x}")?; } Ok(()) } diff --git a/src/symbolize/mod.rs b/src/symbolize/mod.rs index f8571d7be..b01fe86d6 100644 --- a/src/symbolize/mod.rs +++ b/src/symbolize/mod.rs @@ -421,7 +421,7 @@ cfg_if::cfg_if! { // it outwards. if let Some(ref cpp) = self.cpp_demangled.0 { let mut s = String::new(); - if write!(s, "{}", cpp).is_ok() { + if write!(s, "{cpp}").is_ok() { return s.fmt(f) } } diff --git a/tests/accuracy/main.rs b/tests/accuracy/main.rs index 79b2d3797..568acab84 100644 --- a/tests/accuracy/main.rs +++ b/tests/accuracy/main.rs @@ -96,9 +96,9 @@ fn verify(filelines: &[Pos]) { println!("-----------------------------------"); println!("looking for:"); for (file, line) in filelines.iter().rev() { - println!("\t{}:{}", file, line); + println!("\t{file}:{line}"); } - println!("found:\n{:?}", trace); + println!("found:\n{trace:?}"); let mut symbols = trace.frames().iter().flat_map(|frame| frame.symbols()); let mut iter = filelines.iter().rev(); while let Some((file, line)) = iter.next() { diff --git a/tests/current-exe-mismatch.rs b/tests/current-exe-mismatch.rs index e305b6677..e119adfcb 100644 --- a/tests/current-exe-mismatch.rs +++ b/tests/current-exe-mismatch.rs @@ -20,7 +20,7 @@ fn main() { println!("test result: ignored"); } Err(EarlyExit::IoError(e)) => { - println!("{} parent encoutered IoError: {:?}", file!(), e); + println!("{} parent encountered IoError: {:?}", file!(), e); panic!(); } } @@ -74,7 +74,7 @@ fn parent() -> Result<(), EarlyExit> { fn child() -> Result<(), EarlyExit> { let bt = backtrace::Backtrace::new(); - println!("{:?}", bt); + println!("{bt:?}"); let mut found_my_name = false; diff --git a/tests/long_fn_name.rs b/tests/long_fn_name.rs index fa4cfda15..4a03825b6 100644 --- a/tests/long_fn_name.rs +++ b/tests/long_fn_name.rs @@ -25,7 +25,7 @@ fn test_long_fn_name() { // It's actually longer since it also includes `::`, `<>` and the // name of the current module let bt = S::>>>>>>>>>::new(); - println!("{:?}", bt); + println!("{bt:?}"); let mut found_long_name_frame = false; diff --git a/tests/skip_inner_frames.rs b/tests/skip_inner_frames.rs index 60bba35e6..4a370fbc1 100644 --- a/tests/skip_inner_frames.rs +++ b/tests/skip_inner_frames.rs @@ -18,7 +18,7 @@ fn backtrace_new_unresolved_should_start_with_call_site_trace() { } let mut b = Backtrace::new_unresolved(); b.resolve(); - println!("{:?}", b); + println!("{b:?}"); assert!(!b.frames().is_empty()); @@ -34,7 +34,7 @@ fn backtrace_new_should_start_with_call_site_trace() { return; } let b = Backtrace::new(); - println!("{:?}", b); + println!("{b:?}"); assert!(!b.frames().is_empty()); diff --git a/tests/smoke.rs b/tests/smoke.rs index a20c2d981..b47f02323 100644 --- a/tests/smoke.rs +++ b/tests/smoke.rs @@ -112,16 +112,16 @@ fn smoke_test_frames() { backtrace::resolve_frame(frame, |sym| { print!("symbol ip:{:?} address:{:?} ", frame.ip(), frame.symbol_address()); if let Some(name) = sym.name() { - print!("name:{} ", name); + print!("name:{name} "); } if let Some(file) = sym.filename() { print!("file:{} ", file.display()); } if let Some(lineno) = sym.lineno() { - print!("lineno:{} ", lineno); + print!("lineno:{lineno} "); } if let Some(colno) = sym.colno() { - print!("colno:{} ", colno); + print!("colno:{colno} "); } println!(); }); @@ -269,12 +269,12 @@ fn sp_smoke_test() { if refs.len() < 5 { recursive_stack_references(refs); - eprintln!("exiting: {}", x); + eprintln!("exiting: {x}"); return; } backtrace::trace(make_trace_closure(refs)); - eprintln!("exiting: {}", x); + eprintln!("exiting: {x}"); } // NB: the following `make_*` functions are pulled out of line, rather than @@ -296,7 +296,7 @@ fn sp_smoke_test() { sym.name() .and_then(|name| name.as_str()) .map_or(false, |name| { - eprintln!("name = {}", name); + eprintln!("name = {name}"); name.contains("recursive_stack_references") }) }); From b256de4d54d634ef01f1e4817bfbf6ba7e318817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Onur=20=C3=96zkan?= Date: Sat, 9 Mar 2024 23:04:58 +0300 Subject: [PATCH 37/42] ignore clippy lints in `symbolize/gimli/stash.rs` (rust-lang/backtrace-rs#586) Signed-off-by: onur-ozkan --- src/symbolize/gimli/stash.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/symbolize/gimli/stash.rs b/src/symbolize/gimli/stash.rs index 792f9a6f8..5ec06e240 100644 --- a/src/symbolize/gimli/stash.rs +++ b/src/symbolize/gimli/stash.rs @@ -1,3 +1,4 @@ +#![allow(clippy::all)] // only used on Linux right now, so allow dead code elsewhere #![cfg_attr(not(target_os = "linux"), allow(dead_code))] From a53ae08a080c50e117c2ee9330e67fd8436e437b Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sat, 9 Mar 2024 20:46:54 +0000 Subject: [PATCH 38/42] Revert dbghelp 32-bit to known good version --- src/backtrace/dbghelp32.rs | 257 +++++++++++++++++++++ src/backtrace/{dbghelp.rs => dbghelp64.rs} | 0 src/backtrace/mod.rs | 10 +- 3 files changed, 266 insertions(+), 1 deletion(-) create mode 100644 src/backtrace/dbghelp32.rs rename src/backtrace/{dbghelp.rs => dbghelp64.rs} (100%) diff --git a/src/backtrace/dbghelp32.rs b/src/backtrace/dbghelp32.rs new file mode 100644 index 000000000..ba0f05f3b --- /dev/null +++ b/src/backtrace/dbghelp32.rs @@ -0,0 +1,257 @@ +//! Backtrace strategy for MSVC platforms. +//! +//! This module contains the ability to generate a backtrace on MSVC using one +//! of two possible methods. The `StackWalkEx` function is primarily used if +//! possible, but not all systems have that. Failing that the `StackWalk64` +//! function is used instead. Note that `StackWalkEx` is favored because it +//! handles debuginfo internally and returns inline frame information. +//! +//! Note that all dbghelp support is loaded dynamically, see `src/dbghelp.rs` +//! for more information about that. + +#![allow(bad_style)] + +use super::super::{dbghelp, windows::*}; +use core::ffi::c_void; +use core::mem; + +#[derive(Clone, Copy)] +pub enum StackFrame { + New(STACKFRAME_EX), + Old(STACKFRAME64), +} + +#[derive(Clone, Copy)] +pub struct Frame { + pub(crate) stack_frame: StackFrame, + base_address: *mut c_void, +} + +// we're just sending around raw pointers and reading them, never interpreting +// them so this should be safe to both send and share across threads. +unsafe impl Send for Frame {} +unsafe impl Sync for Frame {} + +impl Frame { + pub fn ip(&self) -> *mut c_void { + self.addr_pc().Offset as *mut _ + } + + pub fn sp(&self) -> *mut c_void { + self.addr_stack().Offset as *mut _ + } + + pub fn symbol_address(&self) -> *mut c_void { + self.ip() + } + + pub fn module_base_address(&self) -> Option<*mut c_void> { + Some(self.base_address) + } + + fn addr_pc(&self) -> &ADDRESS64 { + match self.stack_frame { + StackFrame::New(ref new) => &new.AddrPC, + StackFrame::Old(ref old) => &old.AddrPC, + } + } + + fn addr_pc_mut(&mut self) -> &mut ADDRESS64 { + match self.stack_frame { + StackFrame::New(ref mut new) => &mut new.AddrPC, + StackFrame::Old(ref mut old) => &mut old.AddrPC, + } + } + + fn addr_frame_mut(&mut self) -> &mut ADDRESS64 { + match self.stack_frame { + StackFrame::New(ref mut new) => &mut new.AddrFrame, + StackFrame::Old(ref mut old) => &mut old.AddrFrame, + } + } + + fn addr_stack(&self) -> &ADDRESS64 { + match self.stack_frame { + StackFrame::New(ref new) => &new.AddrStack, + StackFrame::Old(ref old) => &old.AddrStack, + } + } + + fn addr_stack_mut(&mut self) -> &mut ADDRESS64 { + match self.stack_frame { + StackFrame::New(ref mut new) => &mut new.AddrStack, + StackFrame::Old(ref mut old) => &mut old.AddrStack, + } + } +} + +#[repr(C, align(16))] // required by `CONTEXT`, is a FIXME in winapi right now +struct MyContext(CONTEXT); + +#[inline(always)] +pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { + // Allocate necessary structures for doing the stack walk + let process = GetCurrentProcess(); + let thread = GetCurrentThread(); + + let mut context = mem::zeroed::(); + RtlCaptureContext(&mut context.0); + + // Ensure this process's symbols are initialized + let dbghelp = match dbghelp::init() { + Ok(dbghelp) => dbghelp, + Err(()) => return, // oh well... + }; + + // On x86_64 and ARM64 we opt to not use the default `Sym*` functions from + // dbghelp for getting the function table and module base. Instead we use + // the `RtlLookupFunctionEntry` function in kernel32 which will account for + // JIT compiler frames as well. These should be equivalent, but using + // `Rtl*` allows us to backtrace through JIT frames. + // + // Note that `RtlLookupFunctionEntry` only works for in-process backtraces, + // but that's all we support anyway, so it all lines up well. + cfg_if::cfg_if! { + if #[cfg(target_pointer_width = "64")] { + use core::ptr; + + unsafe extern "system" fn function_table_access(_process: HANDLE, addr: DWORD64) -> PVOID { + let mut base = 0; + RtlLookupFunctionEntry(addr, &mut base, ptr::null_mut()).cast() + } + + unsafe extern "system" fn get_module_base(_process: HANDLE, addr: DWORD64) -> DWORD64 { + let mut base = 0; + RtlLookupFunctionEntry(addr, &mut base, ptr::null_mut()); + base + } + } else { + let function_table_access = dbghelp.SymFunctionTableAccess64(); + let get_module_base = dbghelp.SymGetModuleBase64(); + } + } + + let process_handle = GetCurrentProcess(); + + // Attempt to use `StackWalkEx` if we can, but fall back to `StackWalk64` + // since it's in theory supported on more systems. + match (*dbghelp.dbghelp()).StackWalkEx() { + Some(StackWalkEx) => { + let mut inner: STACKFRAME_EX = mem::zeroed(); + inner.StackFrameSize = mem::size_of::() as DWORD; + let mut frame = super::Frame { + inner: Frame { + stack_frame: StackFrame::New(inner), + base_address: 0 as _, + }, + }; + let image = init_frame(&mut frame.inner, &context.0); + let frame_ptr = match &mut frame.inner.stack_frame { + StackFrame::New(ptr) => ptr as *mut STACKFRAME_EX, + _ => unreachable!(), + }; + + while StackWalkEx( + image as DWORD, + process, + thread, + frame_ptr, + &mut context.0 as *mut CONTEXT as *mut _, + None, + Some(function_table_access), + Some(get_module_base), + None, + 0, + ) == TRUE + { + frame.inner.base_address = get_module_base(process_handle, frame.ip() as _) as _; + + if !cb(&frame) { + break; + } + } + } + None => { + let mut frame = super::Frame { + inner: Frame { + stack_frame: StackFrame::Old(mem::zeroed()), + base_address: 0 as _, + }, + }; + let image = init_frame(&mut frame.inner, &context.0); + let frame_ptr = match &mut frame.inner.stack_frame { + StackFrame::Old(ptr) => ptr as *mut STACKFRAME64, + _ => unreachable!(), + }; + + while dbghelp.StackWalk64()( + image as DWORD, + process, + thread, + frame_ptr, + &mut context.0 as *mut CONTEXT as *mut _, + None, + Some(function_table_access), + Some(get_module_base), + None, + ) == TRUE + { + frame.inner.base_address = get_module_base(process_handle, frame.ip() as _) as _; + + if !cb(&frame) { + break; + } + } + } + } +} + +#[cfg(target_arch = "x86_64")] +fn init_frame(frame: &mut Frame, ctx: &CONTEXT) -> WORD { + frame.addr_pc_mut().Offset = ctx.Rip as u64; + frame.addr_pc_mut().Mode = AddrModeFlat; + frame.addr_stack_mut().Offset = ctx.Rsp as u64; + frame.addr_stack_mut().Mode = AddrModeFlat; + frame.addr_frame_mut().Offset = ctx.Rbp as u64; + frame.addr_frame_mut().Mode = AddrModeFlat; + + IMAGE_FILE_MACHINE_AMD64 +} + +#[cfg(target_arch = "x86")] +fn init_frame(frame: &mut Frame, ctx: &CONTEXT) -> WORD { + frame.addr_pc_mut().Offset = ctx.Eip as u64; + frame.addr_pc_mut().Mode = AddrModeFlat; + frame.addr_stack_mut().Offset = ctx.Esp as u64; + frame.addr_stack_mut().Mode = AddrModeFlat; + frame.addr_frame_mut().Offset = ctx.Ebp as u64; + frame.addr_frame_mut().Mode = AddrModeFlat; + + IMAGE_FILE_MACHINE_I386 +} + +#[cfg(target_arch = "aarch64")] +fn init_frame(frame: &mut Frame, ctx: &CONTEXT) -> WORD { + frame.addr_pc_mut().Offset = ctx.Pc as u64; + frame.addr_pc_mut().Mode = AddrModeFlat; + frame.addr_stack_mut().Offset = ctx.Sp as u64; + frame.addr_stack_mut().Mode = AddrModeFlat; + unsafe { + frame.addr_frame_mut().Offset = ctx.u.s().Fp as u64; + } + frame.addr_frame_mut().Mode = AddrModeFlat; + IMAGE_FILE_MACHINE_ARM64 +} + +#[cfg(target_arch = "arm")] +fn init_frame(frame: &mut Frame, ctx: &CONTEXT) -> WORD { + frame.addr_pc_mut().Offset = ctx.Pc as u64; + frame.addr_pc_mut().Mode = AddrModeFlat; + frame.addr_stack_mut().Offset = ctx.Sp as u64; + frame.addr_stack_mut().Mode = AddrModeFlat; + unsafe { + frame.addr_frame_mut().Offset = ctx.R11 as u64; + } + frame.addr_frame_mut().Mode = AddrModeFlat; + IMAGE_FILE_MACHINE_ARMNT +} diff --git a/src/backtrace/dbghelp.rs b/src/backtrace/dbghelp64.rs similarity index 100% rename from src/backtrace/dbghelp.rs rename to src/backtrace/dbghelp64.rs diff --git a/src/backtrace/mod.rs b/src/backtrace/mod.rs index c872b1c37..df9f536a4 100644 --- a/src/backtrace/mod.rs +++ b/src/backtrace/mod.rs @@ -193,7 +193,15 @@ cfg_if::cfg_if! { use self::libunwind::trace as trace_imp; pub(crate) use self::libunwind::Frame as FrameImp; } else if #[cfg(all(windows, not(target_vendor = "uwp")))] { - mod dbghelp; + cfg_if::cfg_if! { + if #[cfg(any(target_arch = "x86_64", target_arch = "aarch64", target_arch = "arm64ec"))] { + mod dbghelp64; + use dbghelp64 as dbghelp; + } else if #[cfg(any(target_arch = "x86", target_arch = "arm"))] { + mod dbghelp32; + use dbghelp32 as dbghelp; + } + } use self::dbghelp::trace as trace_imp; pub(crate) use self::dbghelp::Frame as FrameImp; } else { From 1e38a0ca54bc14f158f67fa9b8dbf7a469023786 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sat, 9 Mar 2024 20:47:28 +0000 Subject: [PATCH 39/42] Update dbghelp32 for new code --- src/backtrace/dbghelp32.rs | 54 +++++++------------------------------- 1 file changed, 10 insertions(+), 44 deletions(-) diff --git a/src/backtrace/dbghelp32.rs b/src/backtrace/dbghelp32.rs index ba0f05f3b..672bcba20 100644 --- a/src/backtrace/dbghelp32.rs +++ b/src/backtrace/dbghelp32.rs @@ -49,6 +49,14 @@ impl Frame { Some(self.base_address) } + #[cfg(not(target_env = "gnu"))] + pub fn inline_context(&self) -> Option { + match self.stack_frame { + StackFrame::New(ref new) => Some(new.InlineFrameContext), + StackFrame::Old(_) => None, + } + } + fn addr_pc(&self) -> &ADDRESS64 { match self.stack_frame { StackFrame::New(ref new) => &new.AddrPC, @@ -111,25 +119,8 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { // // Note that `RtlLookupFunctionEntry` only works for in-process backtraces, // but that's all we support anyway, so it all lines up well. - cfg_if::cfg_if! { - if #[cfg(target_pointer_width = "64")] { - use core::ptr; - - unsafe extern "system" fn function_table_access(_process: HANDLE, addr: DWORD64) -> PVOID { - let mut base = 0; - RtlLookupFunctionEntry(addr, &mut base, ptr::null_mut()).cast() - } - - unsafe extern "system" fn get_module_base(_process: HANDLE, addr: DWORD64) -> DWORD64 { - let mut base = 0; - RtlLookupFunctionEntry(addr, &mut base, ptr::null_mut()); - base - } - } else { - let function_table_access = dbghelp.SymFunctionTableAccess64(); - let get_module_base = dbghelp.SymGetModuleBase64(); - } - } + let function_table_access = dbghelp.SymFunctionTableAccess64(); + let get_module_base = dbghelp.SymGetModuleBase64(); let process_handle = GetCurrentProcess(); @@ -206,18 +197,6 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { } } -#[cfg(target_arch = "x86_64")] -fn init_frame(frame: &mut Frame, ctx: &CONTEXT) -> WORD { - frame.addr_pc_mut().Offset = ctx.Rip as u64; - frame.addr_pc_mut().Mode = AddrModeFlat; - frame.addr_stack_mut().Offset = ctx.Rsp as u64; - frame.addr_stack_mut().Mode = AddrModeFlat; - frame.addr_frame_mut().Offset = ctx.Rbp as u64; - frame.addr_frame_mut().Mode = AddrModeFlat; - - IMAGE_FILE_MACHINE_AMD64 -} - #[cfg(target_arch = "x86")] fn init_frame(frame: &mut Frame, ctx: &CONTEXT) -> WORD { frame.addr_pc_mut().Offset = ctx.Eip as u64; @@ -230,19 +209,6 @@ fn init_frame(frame: &mut Frame, ctx: &CONTEXT) -> WORD { IMAGE_FILE_MACHINE_I386 } -#[cfg(target_arch = "aarch64")] -fn init_frame(frame: &mut Frame, ctx: &CONTEXT) -> WORD { - frame.addr_pc_mut().Offset = ctx.Pc as u64; - frame.addr_pc_mut().Mode = AddrModeFlat; - frame.addr_stack_mut().Offset = ctx.Sp as u64; - frame.addr_stack_mut().Mode = AddrModeFlat; - unsafe { - frame.addr_frame_mut().Offset = ctx.u.s().Fp as u64; - } - frame.addr_frame_mut().Mode = AddrModeFlat; - IMAGE_FILE_MACHINE_ARM64 -} - #[cfg(target_arch = "arm")] fn init_frame(frame: &mut Frame, ctx: &CONTEXT) -> WORD { frame.addr_pc_mut().Offset = ctx.Pc as u64; From f3a893342ed3df9eaa41b91489ad8d868470ff53 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sat, 9 Mar 2024 20:48:40 +0000 Subject: [PATCH 40/42] Remove 32-bit from dbghelp64 --- src/backtrace/dbghelp64.rs | 146 ------------------------------------- 1 file changed, 146 deletions(-) diff --git a/src/backtrace/dbghelp64.rs b/src/backtrace/dbghelp64.rs index 71058a038..d3ae06dff 100644 --- a/src/backtrace/dbghelp64.rs +++ b/src/backtrace/dbghelp64.rs @@ -86,42 +86,6 @@ impl MyContext { } } -#[cfg(target_arch = "x86")] -impl MyContext { - #[inline(always)] - fn ip(&self) -> DWORD { - self.0.Eip - } - - #[inline(always)] - fn sp(&self) -> DWORD { - self.0.Esp - } - - #[inline(always)] - fn fp(&self) -> DWORD { - self.0.Ebp - } -} - -#[cfg(target_arch = "arm")] -impl MyContext { - #[inline(always)] - fn ip(&self) -> DWORD { - self.0.Pc - } - - #[inline(always)] - fn sp(&self) -> DWORD { - self.0.Sp - } - - #[inline(always)] - fn fp(&self) -> DWORD { - self.0.R11 - } -} - #[cfg(any( target_arch = "x86_64", target_arch = "aarch64", @@ -172,113 +136,3 @@ pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { ); } } - -#[cfg(any(target_arch = "x86", target_arch = "arm"))] -#[inline(always)] -pub unsafe fn trace(cb: &mut dyn FnMut(&super::Frame) -> bool) { - use core::{mem, ptr}; - - // Allocate necessary structures for doing the stack walk - let process = GetCurrentProcess(); - let thread = GetCurrentThread(); - - let mut context = mem::zeroed::(); - RtlCaptureContext(&mut context.0); - - // Ensure this process's symbols are initialized - let dbghelp = match super::super::dbghelp::init() { - Ok(dbghelp) => dbghelp, - Err(()) => return, // oh well... - }; - - let function_table_access = dbghelp.SymFunctionTableAccess64(); - let get_module_base = dbghelp.SymGetModuleBase64(); - - let process_handle = GetCurrentProcess(); - - #[cfg(target_arch = "x86")] - let image = IMAGE_FILE_MACHINE_I386; - #[cfg(target_arch = "arm")] - let image = IMAGE_FILE_MACHINE_ARMNT; - - // Attempt to use `StackWalkEx` if we can, but fall back to `StackWalk64` - // since it's in theory supported on more systems. - match (*dbghelp.dbghelp()).StackWalkEx() { - Some(StackWalkEx) => { - let mut stack_frame_ex: STACKFRAME_EX = mem::zeroed(); - stack_frame_ex.StackFrameSize = mem::size_of::() as DWORD; - stack_frame_ex.AddrPC.Offset = context.ip() as u64; - stack_frame_ex.AddrPC.Mode = AddrModeFlat; - stack_frame_ex.AddrStack.Offset = context.sp() as u64; - stack_frame_ex.AddrStack.Mode = AddrModeFlat; - stack_frame_ex.AddrFrame.Offset = context.fp() as u64; - stack_frame_ex.AddrFrame.Mode = AddrModeFlat; - - while StackWalkEx( - image as DWORD, - process, - thread, - &mut stack_frame_ex, - ptr::addr_of_mut!(context.0) as PVOID, - None, - Some(function_table_access), - Some(get_module_base), - None, - 0, - ) == TRUE - { - let frame = super::Frame { - inner: Frame { - base_address: get_module_base(process_handle, stack_frame_ex.AddrPC.Offset) - as *mut c_void, - ip: stack_frame_ex.AddrPC.Offset as *mut c_void, - sp: stack_frame_ex.AddrStack.Offset as *mut c_void, - #[cfg(not(target_env = "gnu"))] - inline_context: Some(stack_frame_ex.InlineFrameContext), - }, - }; - - if !cb(&frame) { - break; - } - } - } - None => { - let mut stack_frame64: STACKFRAME64 = mem::zeroed(); - stack_frame64.AddrPC.Offset = context.ip() as u64; - stack_frame64.AddrPC.Mode = AddrModeFlat; - stack_frame64.AddrStack.Offset = context.sp() as u64; - stack_frame64.AddrStack.Mode = AddrModeFlat; - stack_frame64.AddrFrame.Offset = context.fp() as u64; - stack_frame64.AddrFrame.Mode = AddrModeFlat; - - while dbghelp.StackWalk64()( - image as DWORD, - process, - thread, - &mut stack_frame64, - ptr::addr_of_mut!(context.0) as PVOID, - None, - Some(function_table_access), - Some(get_module_base), - None, - ) == TRUE - { - let frame = super::Frame { - inner: Frame { - base_address: get_module_base(process_handle, stack_frame64.AddrPC.Offset) - as *mut c_void, - ip: stack_frame64.AddrPC.Offset as *mut c_void, - sp: stack_frame64.AddrStack.Offset as *mut c_void, - #[cfg(not(target_env = "gnu"))] - inline_context: None, - }, - }; - - if !cb(&frame) { - break; - } - } - } - } -} From a4d5b8c25b395176f2b3728e3232f1bd0018aa5a Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Sun, 10 Mar 2024 08:39:41 +0000 Subject: [PATCH 41/42] Update for Win10+ (rust-lang/backtrace-rs#589) Essentially reverts b891125913228468b3fb33f07c3a1b4b3807644e but only for Win10+ --- src/symbolize/dbghelp.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/symbolize/dbghelp.rs b/src/symbolize/dbghelp.rs index 8a7bdf623..7969f9ff2 100644 --- a/src/symbolize/dbghelp.rs +++ b/src/symbolize/dbghelp.rs @@ -72,6 +72,22 @@ impl Symbol<'_> { #[repr(C, align(8))] struct Aligned8(T); +#[cfg(not(target_vendor = "win7"))] +pub unsafe fn resolve(what: ResolveWhat<'_>, cb: &mut dyn FnMut(&super::Symbol)) { + // Ensure this process's symbols are initialized + let dbghelp = match dbghelp::init() { + Ok(dbghelp) => dbghelp, + Err(()) => return, // oh well... + }; + match what { + ResolveWhat::Address(_) => resolve_with_inline(&dbghelp, what.address_or_ip(), None, cb), + ResolveWhat::Frame(frame) => { + resolve_with_inline(&dbghelp, frame.ip(), frame.inner.inline_context(), cb) + } + } +} + +#[cfg(target_vendor = "win7")] pub unsafe fn resolve(what: ResolveWhat<'_>, cb: &mut dyn FnMut(&super::Symbol)) { // Ensure this process's symbols are initialized let dbghelp = match dbghelp::init() { @@ -100,6 +116,7 @@ pub unsafe fn resolve(what: ResolveWhat<'_>, cb: &mut dyn FnMut(&super::Symbol)) /// /// This should work all the way down to Windows XP. The inline context is /// ignored, since this concept was only introduced in dbghelp 6.2+. +#[cfg(target_vendor = "win7")] unsafe fn resolve_legacy( dbghelp: &dbghelp::Init, addr: *mut c_void, From 92c9cdf116aea5c2b41ce2184fb468d43013704f Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sun, 10 Mar 2024 01:06:37 -0800 Subject: [PATCH 42/42] Cut backtrace 0.3.70 --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dc930c70b..88f853ab9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -32,7 +32,7 @@ dependencies = [ [[package]] name = "backtrace" -version = "0.3.69" +version = "0.3.70" dependencies = [ "addr2line", "cc", diff --git a/Cargo.toml b/Cargo.toml index 932310bcf..7fe05d818 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "backtrace" -version = "0.3.69" +version = "0.3.70" authors = ["The Rust Project Developers"] build = "build.rs" license = "MIT OR Apache-2.0"