Skip to content

Commit

Permalink
fix(debuginfo): include more pdb symbols (#641)
Browse files Browse the repository at this point in the history
Instead of checking symbol.function, check whether the symbol is contained
in an executable section. This matches what pdb-addr2line does, and is
closer to what dump_syms' handwritten PDB parsing does.

This change allows more symbols to be included. For example, it allows the
following symbols from basic-opt32.pdb to pass the check:

```
4f9d7 _rtinfpopse
4f9d9 _rtinfnpopse
4f9e9 _fFLN
4fa9e _rtinfpop
4faa0 _rtinfnpop
4fabd _ffexpm1
```

https://github.com/mozilla/dump_syms/blob/54f9e6240e34cf17fc7dc60ac4985772e2c20001/test_data/windows/basic-opt32.pdb

These symbols have symbol.function == false, so they were being filtered
out before.

The change also adds over 300 symbols for ntdll.pdb BD298DA990CD4BF9BE5CE4796D7924C61.

The handwritten PDB parsing in dump_syms currently uses this full check:
`symbol.function || symbol.code || is_in_executable_section || is_in_executable_section_contribution`

This seems like overkill; I've tested a few pdbs and haven't found any
for which this additional complexity would make a difference.
  • Loading branch information
mstange committed Jul 29, 2022
1 parent a0360d3 commit 3e47ef3
Showing 1 changed file with 46 additions and 4 deletions.
50 changes: 46 additions & 4 deletions symbolic-debuginfo/src/pdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ use std::sync::Arc;
use elsa::FrozenMap;
use parking_lot::RwLock;
use pdb_addr2line::pdb::{
AddressMap, FallibleIterator, InlineSiteSymbol, LineProgram, MachineType, Module, ModuleInfo,
PdbInternalSectionOffset, ProcedureSymbol, RawString, SeparatedCodeSymbol, SymbolData,
TypeIndex,
AddressMap, FallibleIterator, ImageSectionHeader, InlineSiteSymbol, LineProgram, MachineType,
Module, ModuleInfo, PdbInternalSectionOffset, ProcedureSymbol, RawString, SeparatedCodeSymbol,
SymbolData, TypeIndex,
};
use pdb_addr2line::ModuleProvider;
use smallvec::SmallVec;
Expand Down Expand Up @@ -120,6 +120,7 @@ pub struct PdbObject<'data> {
debug_info: Arc<pdb::DebugInformation<'data>>,
pdb_info: pdb::PDBInformation<'data>,
public_syms: pdb::SymbolTable<'data>,
executable_sections: ExecutableSections,
data: &'data [u8],
}

Expand All @@ -144,13 +145,15 @@ impl<'data> PdbObject<'data> {
let dbi = pdb.debug_information()?;
let pdbi = pdb.pdb_information()?;
let pubi = pdb.global_symbols()?;
let sections = pdb.sections()?;

Ok(PdbObject {
pdb: Arc::new(RwLock::new(pdb)),
debug_info: Arc::new(dbi),
pdb_info: pdbi,
public_syms: pubi,
data,
executable_sections: ExecutableSections::from_sections(&sections),
})
}

Expand Down Expand Up @@ -224,6 +227,7 @@ impl<'data> PdbObject<'data> {
PdbSymbolIterator {
symbols: self.public_syms.iter(),
address_map: self.pdb.write().address_map().ok(),
executable_sections: &self.executable_sections,
}
}

Expand Down Expand Up @@ -383,12 +387,50 @@ pub(crate) fn arch_from_machine(machine: MachineType) -> Arch {
}
}

/// Contains information about which sections are executable.
struct ExecutableSections {
/// For every section header in the PDB, a boolean which indicates whether the "executable"
/// or "execute" flag is set in the section header's characteristics.
is_executable_per_section: Vec<bool>,
}

impl ExecutableSections {
pub fn from_sections(sections: &Option<Vec<ImageSectionHeader>>) -> Self {
Self {
is_executable_per_section: match sections {
Some(sections) => sections
.iter()
.map(|section| section.characteristics)
.map(|char| char.executable() || char.execute())
.collect(),
None => Default::default(),
},
}
}

/// Returns whether the given offset is contained in an executable section.
pub fn contains(&self, offset: &PdbInternalSectionOffset) -> bool {
// offset.section is a one-based index.
if offset.section == 0 {
// No section.
return false;
}

let section_index = (offset.section - 1) as usize;
self.is_executable_per_section
.get(section_index)
.cloned()
.unwrap_or(false)
}
}

/// An iterator over symbols in the PDB file.
///
/// Returned by [`PdbObject::symbols`](struct.PdbObject.html#method.symbols).
pub struct PdbSymbolIterator<'data, 'object> {
symbols: pdb::SymbolIter<'object>,
address_map: Option<AddressMap<'data>>,
executable_sections: &'object ExecutableSections,
}

impl<'data, 'object> Iterator for PdbSymbolIterator<'data, 'object> {
Expand All @@ -399,7 +441,7 @@ impl<'data, 'object> Iterator for PdbSymbolIterator<'data, 'object> {

while let Ok(Some(symbol)) = self.symbols.next() {
if let Ok(SymbolData::Public(public)) = symbol.parse() {
if !public.function {
if !self.executable_sections.contains(&public.offset) {
continue;
}

Expand Down

0 comments on commit 3e47ef3

Please sign in to comment.