Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Fix and improve PDB search #913

Merged
merged 3 commits into from
May 24, 2021
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
4 changes: 3 additions & 1 deletion src/agent/coverage/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ impl ModuleInfo {
let pe = goblin::pe::PE::parse(&data)?;
let module = ModuleIndex::index_pe(path.clone(), &pe);

let pdb_path = crate::pdb::find_pdb_path(path.as_ref(), &pe, handle)?;
let pdb_path = crate::pdb::find_pdb_path(path.as_ref(), &pe, handle)?
.ok_or_else(|| anyhow::format_err!("could not find PDB for module: {}", path))?;

let pdb = std::fs::File::open(&pdb_path)?;
let mut pdb = pdb::PDB::open(pdb)?;

Expand Down
13 changes: 10 additions & 3 deletions src/agent/coverage/src/pdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use std::{
ffi::CStr,
fs,
path::{Path, PathBuf},
};

Expand All @@ -23,7 +24,11 @@ use winapi::um::{dbghelp::SYMOPT_EXACT_SYMBOLS, winnt::HANDLE};
// See: https://docs.microsoft.com/en-us/windows/win32/api/dbghelp/nf-dbghelp-syminitializew
const PSEUDO_HANDLE: HANDLE = -2i64 as _;

pub fn find_pdb_path(pe_path: &Path, pe: &PE, target_handle: Option<HANDLE>) -> Result<PathBuf> {
pub fn find_pdb_path(
pe_path: &Path,
pe: &PE,
target_handle: Option<HANDLE>,
) -> Result<Option<PathBuf>> {
ranweiler marked this conversation as resolved.
Show resolved Hide resolved
let cv = if let Some(DebugData {
image_debug_directory: _,
codeview_pdb70_debug_info: Some(cv),
Expand All @@ -41,8 +46,10 @@ pub fn find_pdb_path(pe_path: &Path, pe: &PE, target_handle: Option<HANDLE>) ->
let cv_filename = Path::new(cv_filename);

// If the PE-specified PDB file exists on disk, use that.
if std::fs::metadata(&cv_filename)?.is_file() {
return Ok(cv_filename.to_owned());
if let Ok(metadata) = fs::metadata(&cv_filename) {
ranweiler marked this conversation as resolved.
Show resolved Hide resolved
if metadata.is_file() {
return Ok(Some(cv_filename.to_owned()));
}
}

// If we have one, use the the process handle for an existing debug
Expand Down
11 changes: 7 additions & 4 deletions src/agent/coverage/src/pe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,13 @@ pub fn process_module(
let pdb_path = crate::pdb::find_pdb_path(pe_path.as_ref(), pe, target_handle)
.with_context(|| format!("searching for PDB for PE: {}", pe_path.as_ref().display()))?;

log::info!("found PDB: {}", pdb_path.display());

process_pdb(data, pe, functions_only, &pdb_path)
.with_context(|| format!("processing PDB: {}", pdb_path.display()))
if let Some(pdb_path) = pdb_path {
log::info!("found PDB: {}", pdb_path.display());
process_pdb(data, pe, functions_only, &pdb_path)
.with_context(|| format!("processing PDB: {}", pdb_path.display()))
} else {
anyhow::bail!("PDB not found for PE: {}", pe_path.as_ref().display())
}
}

fn process_pdb(data: &[u8], pe: &PE, functions_only: bool, pdb_path: &Path) -> Result<FixedBitSet> {
Expand Down
24 changes: 14 additions & 10 deletions src/agent/debugger/src/dbghelp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ impl DebugHelpGuard {
file_name: impl AsRef<Path>,
pdb_signature: u32,
pdb_age: u32,
) -> Result<PathBuf> {
) -> Result<Option<PathBuf>> {
let file_name = win_util::string::to_wstring(file_name);

// Must be at least `MAX_PATH` characters in length.
Expand All @@ -726,7 +726,7 @@ impl DebugHelpGuard {
// Assert that we are passing a DWORD signature in `id`.
let flags = SSRVOPT_DWORD;

check_winapi(|| unsafe {
let result = check_winapi(|| unsafe {
SymFindFileInPathW(
process_handle,
search_path,
Expand All @@ -739,16 +739,20 @@ impl DebugHelpGuard {
None,
std::ptr::null_mut(),
)
})?;
});

// Safety: `found_file_data` must contain at least one NUL byte.
//
// We zero-initialize `found_file_data`, and assume that `SymFindFileInPathW`
// only succeeds if it wrote a NUL-terminated wide string.
let found_file =
unsafe { win_util::string::os_string_from_wide_ptr(found_file_data.as_ptr()) };
if result.is_ok() {
ranweiler marked this conversation as resolved.
Show resolved Hide resolved
// Safety: `found_file_data` must contain at least one NUL byte.
//
// We zero-initialize `found_file_data`, and assume that `SymFindFileInPathW`
// only succeeds if it wrote a NUL-terminated wide string.
let found_file =
unsafe { win_util::string::os_string_from_wide_ptr(found_file_data.as_ptr()) };

Ok(found_file.into())
Ok(Some(found_file.into()))
} else {
Ok(None)
}
}

pub fn sym_get_search_path(&self, process_handle: HANDLE) -> Result<OsString> {
Expand Down