Fix and improve PDB search (#913)

- Fix a bug in PDB search where we treated the absence of the PE-specified file as a hard error
- In the return type of `find_pdb_path()`, distinguish between "no file found" and a search error

src/agent/coverage/src/cache.rs

@@ -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)?;
 

src/agent/coverage/src/pdb.rs

@@ -3,6 +3,7 @@
 
 use std::{
     ffi::CStr,
+    fs,
     path::{Path, PathBuf},
 };
 
@@ -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>> {
     let cv = if let Some(DebugData {
         image_debug_directory: _,
         codeview_pdb70_debug_info: Some(cv),
@@ -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) {
+        if metadata.is_file() {
+            return Ok(Some(cv_filename.to_owned()));
+        }
     }
 
     // If we have one, use the the process handle for an existing debug

src/agent/coverage/src/pe.rs

@@ -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> {

src/agent/debugger/src/dbghelp.rs

@@ -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.
@@ -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,
@@ -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() {
+            // 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> {