Skip to content

Reduce panics in dbghelp #608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ verify-winapi = [
'winapi/tlhelp32',
'winapi/winbase',
'winapi/winnt',
'winapi/winnls',
'winapi/stringapiset',
]

[[example]]
Expand Down
27 changes: 15 additions & 12 deletions src/dbghelp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,16 +376,21 @@ pub fn init() -> Result<Init, ()> {
DBGHELP.ensure_open()?;

static mut INITIALIZED: bool = false;
if INITIALIZED {
return Ok(ret);
if !INITIALIZED {
set_optional_options();
INITIALIZED = true;
}

let orig = DBGHELP.SymGetOptions().unwrap()();
Ok(ret)
}
}
fn set_optional_options() -> Option<()> {
unsafe {
let orig = DBGHELP.SymGetOptions()?();

// Ensure that the `SYMOPT_DEFERRED_LOADS` flag is set, because
// according to MSVC's own docs about this: "This is the fastest, most
// efficient way to use the symbol handler.", so let's do that!
DBGHELP.SymSetOptions().unwrap()(orig | SYMOPT_DEFERRED_LOADS);
DBGHELP.SymSetOptions()?(orig | SYMOPT_DEFERRED_LOADS);

// Actually initialize symbols with MSVC. Note that this can fail, but we
// ignore it. There's not a ton of prior art for this per se, but LLVM
Expand All @@ -399,7 +404,7 @@ pub fn init() -> Result<Init, ()> {
// the time, but now that it's using this crate it means that someone will
// get to initialization first and the other will pick up that
// initialization.
DBGHELP.SymInitializeW().unwrap()(GetCurrentProcess(), ptr::null_mut(), TRUE);
DBGHELP.SymInitializeW()?(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`.
Expand All @@ -413,7 +418,7 @@ pub fn init() -> Result<Init, ()> {
search_path_buf.resize(1024, 0);

// Prefill the buffer with the current search path.
if DBGHELP.SymGetSearchPathW().unwrap()(
if DBGHELP.SymGetSearchPathW()?(
GetCurrentProcess(),
search_path_buf.as_mut_ptr(),
search_path_buf.len() as _,
Expand All @@ -433,7 +438,7 @@ pub fn init() -> Result<Init, ()> {
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()(
DBGHELP.EnumerateLoadedModulesW64()?(
GetCurrentProcess(),
Some(enum_loaded_modules_callback),
((&mut search_path) as *mut SearchPath) as *mut c_void,
Expand All @@ -442,11 +447,9 @@ pub fn init() -> Result<Init, ()> {
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)
DBGHELP.SymSetSearchPathW()?(GetCurrentProcess(), new_search_path.as_ptr());
}
Some(())
}

struct SearchPath {
Expand Down
66 changes: 32 additions & 34 deletions src/symbolize/dbghelp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

use super::super::{dbghelp, windows::*};
use super::{BytesOrWideString, ResolveWhat, SymbolName};
use core::char;
use core::ffi::c_void;
use core::marker;
use core::mem;
Expand Down Expand Up @@ -91,7 +90,7 @@ pub unsafe fn resolve(what: ResolveWhat<'_>, cb: &mut dyn FnMut(&super::Symbol))
ResolveWhat::Frame(frame) => {
resolve_with_inline(&dbghelp, frame.ip(), frame.inner.inline_context(), cb)
}
}
};
}

#[cfg(target_vendor = "win7")]
Expand All @@ -116,7 +115,7 @@ pub unsafe fn resolve(what: ResolveWhat<'_>, cb: &mut dyn FnMut(&super::Symbol))
ResolveWhat::Frame(frame) => {
resolve_inner(&dbghelp, frame.ip(), frame.inner.inline_context(), cb)
}
}
};
}

/// Resolve the address using the legacy dbghelp API.
Expand Down Expand Up @@ -147,22 +146,28 @@ unsafe fn resolve_with_inline(
addr: *mut c_void,
inline_context: Option<DWORD>,
cb: &mut dyn FnMut(&super::Symbol),
) {
) -> Option<()> {
let current_process = GetCurrentProcess();
// Ensure we have the functions we need. Return if any aren't found.
let SymFromInlineContextW = (*dbghelp.dbghelp()).SymFromInlineContextW()?;
let SymGetLineFromInlineContextW = (*dbghelp.dbghelp()).SymGetLineFromInlineContextW()?;

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 SymAddrIncludeInlineTrace = (*dbghelp.dbghelp()).SymAddrIncludeInlineTrace()?;
let SymQueryInlineTrace = (*dbghelp.dbghelp()).SymQueryInlineTrace()?;

let mut inlined_frame_count = 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()(
&& SymQueryInlineTrace(
current_process,
addr,
0,
Expand All @@ -184,22 +189,14 @@ unsafe fn resolve_with_inline(

for inline_context in inline_context..last_inline_context {
do_resolve(
|info| {
dbghelp.SymFromInlineContextW()(current_process, addr, inline_context, &mut 0, info)
},
|info| SymFromInlineContextW(current_process, addr, inline_context, &mut 0, info),
|line| {
dbghelp.SymGetLineFromInlineContextW()(
current_process,
addr,
inline_context,
0,
&mut 0,
line,
)
SymGetLineFromInlineContextW(current_process, addr, inline_context, 0, &mut 0, line)
},
cb,
);
}
Some(())
}

unsafe fn do_resolve(
Expand All @@ -225,26 +222,27 @@ unsafe fn do_resolve(
// 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().cast::<u16>();
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
// all other platforms
let mut name_len = 0;
let mut name_buffer = [0; 256];
{
let mut remaining = &mut name_buffer[..];
for c in char::decode_utf16(name.iter().cloned()) {
let c = c.unwrap_or(char::REPLACEMENT_CHARACTER);
let len = c.len_utf8();
if len < remaining.len() {
c.encode_utf8(remaining);
let tmp = remaining;
remaining = &mut tmp[len..];
name_len += len;
} else {
break;
}
}
let mut name_buffer = [0_u8; 256];
let mut name_len = WideCharToMultiByte(
CP_UTF8,
0,
name_ptr,
name_len as i32,
name_buffer.as_mut_ptr().cast::<i8>(),
name_buffer.len() as i32,
core::ptr::null_mut(),
core::ptr::null_mut(),
) as usize;
if name_len == 0 {
// If the returned length is zero that means the buffer wasn't big enough.
// However, the buffer will be filled with as much as will fit.
name_len = name_buffer.len();
} else if name_len > name_buffer.len() {
// This can't happen.
return;
}
let name = ptr::addr_of!(name_buffer[..name_len]);

Expand Down
13 changes: 13 additions & 0 deletions src/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ cfg_if::cfg_if! {
pub use winapi::um::tlhelp32::*;
pub use winapi::um::winbase::*;
pub use winapi::um::winnt::*;
pub use winapi::um::winnls::*;
pub use winapi::um::stringapiset::*;

// Work around winapi not having this function on aarch64.
#[cfg(target_arch = "aarch64")]
Expand Down Expand Up @@ -379,6 +381,7 @@ ffi! {
pub const INVALID_HANDLE_VALUE: HANDLE = -1isize as HANDLE;
pub const MAX_MODULE_NAME32: usize = 255;
pub const MAX_PATH: usize = 260;
pub const CP_UTF8: u32 = 65001;

pub type DWORD = u32;
pub type PDWORD = *mut u32;
Expand Down Expand Up @@ -456,6 +459,16 @@ ffi! {
lpme: LPMODULEENTRY32W,
) -> BOOL;
pub fn lstrlenW(lpstring: PCWSTR) -> i32;
pub fn WideCharToMultiByte(
codepage: u32,
dwflags: u32,
lpwidecharstr: PCWSTR,
cchwidechar: i32,
lpmultibytestr: *mut i8,
cbmultibyte: i32,
lpdefaultchar: *const i8,
lpuseddefaultchar: *mut BOOL
) -> i32;
}
}

Expand Down