From 146a8b511966dec6b4e58f5ef01132664b906f32 Mon Sep 17 00:00:00 2001 From: Joe Ranweiler Date: Mon, 3 May 2021 10:16:00 -0700 Subject: [PATCH] Move PDB functions into own module (#843) This does not implement any logic changes, but reorganizes code for easier re-use. --- src/agent/coverage/src/lib.rs | 3 + src/agent/coverage/src/pdb.rs | 114 +++++++++++++++++++++++++++++ src/agent/coverage/src/pe.rs | 133 ++-------------------------------- 3 files changed, 125 insertions(+), 125 deletions(-) create mode 100644 src/agent/coverage/src/pdb.rs diff --git a/src/agent/coverage/src/lib.rs b/src/agent/coverage/src/lib.rs index d882b41e4..ec513331f 100644 --- a/src/agent/coverage/src/lib.rs +++ b/src/agent/coverage/src/lib.rs @@ -6,6 +6,9 @@ #[cfg(target_os = "windows")] mod intel; +#[cfg(target_os = "windows")] +pub mod pdb; + #[cfg(target_os = "windows")] pub mod pe; diff --git a/src/agent/coverage/src/pdb.rs b/src/agent/coverage/src/pdb.rs new file mode 100644 index 000000000..1edecba22 --- /dev/null +++ b/src/agent/coverage/src/pdb.rs @@ -0,0 +1,114 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use std::{ + ffi::CStr, + path::{Path, PathBuf}, +}; + +use anyhow::Result; +use debugger::dbghelp::DebugHelpGuard; +use goblin::pe::{debug::DebugData, PE}; +use winapi::um::{dbghelp::SYMOPT_EXACT_SYMBOLS, winnt::HANDLE}; + +// This is a fallback pseudo-handle used for interacting with dbghelp. +// +// We want to avoid `(HANDLE) -1`, because that pseudo-handle is reserved for +// the current process. Reusing it is documented as causing unexpected dbghelp +// behavior when debugging other processes (which we typically will be). +// +// By picking some other very large value, we avoid collisions with handles that +// are concretely either table indices or virtual addresses. +// +// 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) -> Result { + let cv = if let Some(DebugData { + image_debug_directory: _, + codeview_pdb70_debug_info: Some(cv), + }) = pe.debug_data + { + cv + } else { + anyhow::bail!("PE missing Codeview PDB debug info: {}", pe_path.display(),) + }; + + let cv_filename = CStr::from_bytes_with_nul(cv.filename)?.to_str()?; + + // This field is named `filename`, but it may be an absolute path. + // The callee `find_pdb_file_in_path()` handles either. + 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 we have one, use the the process handle for an existing debug + let handle = target_handle.unwrap_or(PSEUDO_HANDLE); + + let dbghelp = debugger::dbghelp::lock()?; + + // If a target handle was provided, we assume the caller initialized the + // dbghelp symbol handler, and will clean up after itself. + // + // Otherwise, initialize a symbol handler with our own pseudo-path, and use + // a drop guard to ensure we don't leak resources. + let _cleanup = if target_handle.is_some() { + None + } else { + dbghelp.sym_initialize(handle)?; + Some(DbgHelpCleanupGuard::new(&dbghelp, handle)) + }; + + // Enable signature and age checking. + let options = dbghelp.sym_get_options(); + dbghelp.sym_set_options(options | SYMOPT_EXACT_SYMBOLS); + + let mut search_path = dbghelp.sym_get_search_path(handle)?; + + log::debug!("initial search path = {:?}", search_path); + + // Try to add the directory of the PE to the PDB search path. + // + // This may be redundant, and should always succeed. + if let Some(pe_dir) = pe_path.parent() { + log::debug!("pushing PE dir to search path = {:?}", pe_dir.display()); + + search_path.push(";"); + search_path.push(pe_dir); + } else { + log::warn!("PE path has no parent dir: {}", pe_path.display()); + } + + dbghelp.sym_set_search_path(handle, search_path)?; + + let pdb_path = + dbghelp.find_pdb_file_in_path(handle, cv_filename, cv.codeview_signature, cv.age)?; + + Ok(pdb_path) +} + +/// On drop, deallocates all resources associated with its process handle. +struct DbgHelpCleanupGuard<'d> { + dbghelp: &'d DebugHelpGuard, + process_handle: HANDLE, +} + +impl<'d> DbgHelpCleanupGuard<'d> { + pub fn new(dbghelp: &'d DebugHelpGuard, process_handle: HANDLE) -> Self { + Self { + dbghelp, + process_handle, + } + } +} + +impl<'d> Drop for DbgHelpCleanupGuard<'d> { + fn drop(&mut self) { + if let Err(err) = self.dbghelp.sym_cleanup(self.process_handle) { + log::error!("error cleaning up symbol handler: {:?}", err); + } + } +} diff --git a/src/agent/coverage/src/pe.rs b/src/agent/coverage/src/pe.rs index 74c3d563a..37fa5fac6 100644 --- a/src/agent/coverage/src/pe.rs +++ b/src/agent/coverage/src/pe.rs @@ -3,27 +3,16 @@ #![allow(clippy::manual_swap)] -use std::{ - ffi::CStr, - fs::File, - path::{Path, PathBuf}, -}; +use std::{fs::File, path::Path}; use anyhow::{Context, Result}; -use debugger::dbghelp::DebugHelpGuard; use fixedbitset::FixedBitSet; -use goblin::pe::{ - debug::{CodeviewPDB70DebugInfo, DebugData}, - PE, -}; +use goblin::pe::PE; use memmap2::Mmap; use pdb::{ AddressMap, FallibleIterator, PdbInternalSectionOffset, ProcedureSymbol, TypeIndex, PDB, }; -use winapi::um::{ - dbghelp::SYMOPT_EXACT_SYMBOLS, - winnt::{HANDLE, IMAGE_FILE_MACHINE_AMD64, IMAGE_FILE_MACHINE_I386}, -}; +use winapi::um::winnt::{HANDLE, IMAGE_FILE_MACHINE_AMD64, IMAGE_FILE_MACHINE_I386}; use crate::intel; @@ -235,23 +224,13 @@ pub fn process_module( functions_only: bool, target_handle: Option, ) -> Result { - if let Some(DebugData { - image_debug_directory: _, - codeview_pdb70_debug_info: Some(cv), - }) = pe.debug_data - { - let pdb_path = find_pdb_path(pe_path.as_ref(), &cv, target_handle) - .with_context(|| format!("searching for PDB for PE: {}", pe_path.as_ref().display()))?; - log::info!("found PDB: {}", pdb_path.display()); + 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()))?; - return process_pdb(data, pe, functions_only, &pdb_path) - .with_context(|| format!("processing PDB: {}", pdb_path.display())); - } + log::info!("found PDB: {}", pdb_path.display()); - anyhow::bail!( - "PE missing Codeview PDB debug info: {}", - pe_path.as_ref().display() - ) + process_pdb(data, pe, functions_only, &pdb_path) + .with_context(|| format!("processing PDB: {}", pdb_path.display())) } fn process_pdb(data: &[u8], pe: &PE, functions_only: bool, pdb_path: &Path) -> Result { @@ -291,102 +270,6 @@ fn process_pdb(data: &[u8], pe: &PE, functions_only: bool, pdb_path: &Path) -> R Ok(blocks) } -// This is a fallback pseudo-handle used for interacting with dbghelp. -// -// We want to avoid `(HANDLE) -1`, because that pseudo-handle is reserved for -// the current process. Reusing it is documented as causing unexpected dbghelp -// behavior when debugging other processes (which we typically will be). -// -// By picking some other very large value, we avoid collisions with handles that -// are concretely either table indices or virtual addresses. -// -// See: https://docs.microsoft.com/en-us/windows/win32/api/dbghelp/nf-dbghelp-syminitializew -const PSEUDO_HANDLE: HANDLE = -2i64 as _; - -fn find_pdb_path( - pe_path: &Path, - cv: &CodeviewPDB70DebugInfo, - target_handle: Option, -) -> Result { - let cv_filename = CStr::from_bytes_with_nul(cv.filename)?.to_str()?; - - // This field is named `filename`, but it may be an absolute path. - // The callee `find_pdb_file_in_path()` handles either. - 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 we have one, use the the process handle for an existing debug - let handle = target_handle.unwrap_or(PSEUDO_HANDLE); - - let dbghelp = debugger::dbghelp::lock()?; - - // If a target handle was provided, we assume the caller initialized the - // dbghelp symbol handler, and will clean up after itself. - // - // Otherwise, initialize a symbol handler with our own pseudo-path, and use - // a drop guard to ensure we don't leak resources. - let _cleanup = if target_handle.is_some() { - None - } else { - dbghelp.sym_initialize(handle)?; - Some(DbgHelpCleanupGuard::new(&dbghelp, handle)) - }; - - // Enable signature and age checking. - let options = dbghelp.sym_get_options(); - dbghelp.sym_set_options(options | SYMOPT_EXACT_SYMBOLS); - - let mut search_path = dbghelp.sym_get_search_path(handle)?; - - log::debug!("initial search path = {:?}", search_path); - - // Try to add the directory of the PE to the PDB search path. - // - // This may be redundant, and should always succeed. - if let Some(pe_dir) = pe_path.parent() { - log::debug!("pushing PE dir to search path = {:?}", pe_dir.display()); - - search_path.push(";"); - search_path.push(pe_dir); - } else { - log::warn!("PE path has no parent dir: {}", pe_path.display()); - } - - dbghelp.sym_set_search_path(handle, search_path)?; - - let pdb_path = - dbghelp.find_pdb_file_in_path(handle, cv_filename, cv.codeview_signature, cv.age)?; - - Ok(pdb_path) -} - -/// On drop, deallocates all resources associated with its process handle. -struct DbgHelpCleanupGuard<'d> { - dbghelp: &'d DebugHelpGuard, - process_handle: HANDLE, -} - -impl<'d> DbgHelpCleanupGuard<'d> { - pub fn new(dbghelp: &'d DebugHelpGuard, process_handle: HANDLE) -> Self { - Self { - dbghelp, - process_handle, - } - } -} - -impl<'d> Drop for DbgHelpCleanupGuard<'d> { - fn drop(&mut self) { - if let Err(err) = self.dbghelp.sym_cleanup(self.process_handle) { - log::error!("error cleaning up symbol handler: {:?}", err); - } - } -} - pub fn process_image( path: impl AsRef, functions_only: bool,