Skip to content

Fix PDB import applying stack based variables without handling stack re-use #8497

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.

Sign up for GitHub

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
emesare merged 4 commits into from
Sep 2, 2026
+45 −114
Merged

Fix PDB import applying stack based variables without handling stack re-use #8497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
92aab3c
Revert "[PDB Import] Only adjust sp-relative locals on x86_64"
emesare Sep 1, 2026
d1c391d
Revert "[PDB Import] Collect locals and params from blocks contained …
emesare Sep 1, 2026
7aa1be1
Revert "[PDB Import] Improve recovery of sp-based locals"
emesare Sep 1, 2026
34de8db
Revert "[PDB Import] Initial support for loading locals"
emesare Sep 1, 2026
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
28 changes: 3 additions & 25 deletions plugins/pdb-ng/src/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use binaryninja::types::{
EnumerationBuilder, NamedTypeReference, NamedTypeReferenceClass, StructureBuilder,
StructureType, Type, TypeClass,
};
use binaryninja::variable::{NamedDataVariableWithType, NamedVariableWithType};
use binaryninja::variable::NamedDataVariableWithType;

/// Megastruct for all the parsing
/// Certain fields are only used by specific files, as marked below.
Expand Down Expand Up @@ -252,7 +252,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
address,
name,
type_,
locals,
locals: _,
..
}) => {
self.log(|| {
Expand All @@ -276,29 +276,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
Some(address),
Some(self.platform.clone()),
vec![], // TODO : Components
locals
.iter()
.filter_map(|v| {
let Some(var_type) = &v.type_ else {
return None;
};
if v.storage.len() != 1 {
// TODO: how should we handle variables with multiple storage locations?
return None;
}

let mut var_loc = v.storage[0].location;
if v.storage[0].base_relative {
var_loc.storage -= self.arch.address_size() as i64;
}
Some(NamedVariableWithType {
variable: var_loc,
ty: var_type.clone(),
name: v.name.clone(),
auto_defined: false,
})
})
.collect::<Vec<_>>(),
vec![], //TODO: local variables
));
}
_ => {}
Expand Down
131 changes: 42 additions & 89 deletions plugins/pdb-ng/src/symbol_parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub struct ParsedVariable {
pub struct ParsedLocation {
/// Location information
pub location: Variable,
/// Is the storage location relative to the base pointer?
/// Is the storage location relative to the base pointer? See [ParsedProcedureInfo.frame_offset]
pub base_relative: bool,
/// Is the storage location relative to the stack pointer?
pub stack_relative: bool,
Expand Down Expand Up @@ -943,7 +943,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
// We need both of these to exist (not sure why they wouldn't)
let (raw_type, fancy_type) = match (raw_type, fancy_type) {
(Some(raw), Some(fancy)) => (raw, fancy),
_ => return Ok((fancier_type, locals)),
_ => return Ok((fancier_type, vec![])),
};

let raw_params = raw_type.contents.parameters().ok_or(anyhow!("no params"))?;
Expand Down Expand Up @@ -1004,7 +1004,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
// enough parameter variables declared as parameters, the remaining parameters are
// the first however many locals. If you don't have enough of those, idk??
if expected_param_count > (parsed_params.len() + parsed_locals.len()) {
return Ok((fancier_type, locals));
return Ok((fancier_type, vec![]));
}
parsed_params.extend(parsed_locals);
}
Expand Down Expand Up @@ -1075,7 +1075,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
self.log(|| format!("Fancy type: {:#x?}", fancy_type));
self.log(|| format!("Result type: {:#x?}", fancier_type));

Ok((Some(fancier_type), locals))
Ok((Some(fancier_type), vec![]))
}

fn handle_procedure_symbol(
Expand Down Expand Up @@ -1587,15 +1587,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
data: &DefRangeFramePointerRelativeSymbol,
) -> Result<Option<ParsedSymbol>> {
self.log(|| format!("Got DefRangeFramePointerRelative symbol: {:?}", data));
Ok(Some(ParsedSymbol::Location(ParsedLocation {
location: Variable {
ty: VariableSourceType::StackVariableSourceType,
index: 0,
storage: data.offset as i64,
},
base_relative: true,
stack_relative: false,
})))
Ok(None)
}

fn handle_def_range_frame_pointer_relative_full_scope_symbol(
Expand All @@ -1609,15 +1601,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
data
)
});
Ok(Some(ParsedSymbol::Location(ParsedLocation {
location: Variable {
ty: VariableSourceType::StackVariableSourceType,
index: 0,
storage: data.offset as i64,
},
base_relative: true,
stack_relative: false,
})))
Ok(None)
}

fn handle_def_range_sub_field_register_symbol(
Expand All @@ -1635,31 +1619,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
data: &DefRangeRegisterRelativeSymbol,
) -> Result<Option<ParsedSymbol>> {
self.log(|| format!("Got DefRangeRegisterRelative symbol: {:?}", data));
match self.lookup_register(data.base_register) {
Some(X86(X86Register::EBP)) | Some(AMD64(AMD64Register::RBP)) => {
Ok(Some(ParsedSymbol::Location(ParsedLocation {
location: Variable {
ty: VariableSourceType::StackVariableSourceType,
index: 0,
storage: data.offset_base_pointer as i64,
},
base_relative: true,
stack_relative: false,
})))
}
Some(X86(X86Register::ESP)) | Some(AMD64(AMD64Register::RSP)) => {
Ok(Some(ParsedSymbol::Location(ParsedLocation {
location: Variable {
ty: VariableSourceType::StackVariableSourceType,
index: 0,
storage: data.offset_base_pointer as i64,
},
base_relative: false,
stack_relative: true,
})))
}
_ => Ok(None),
}
Ok(None)
}

fn handle_base_pointer_relative_symbol(
Expand Down Expand Up @@ -1702,10 +1662,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
let mut locals = vec![];
let mut seen_offsets = HashSet::new();

for child in self.walk_children(index) {
if child == index {
continue;
}
for child in self.symbol_children(index) {
match self.lookup_symbol(&child) {
Some(ParsedSymbol::LocalVariable(ParsedVariable {
name,
Expand All @@ -1714,36 +1671,12 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
is_param,
..
})) => {
// Adjust RSP-relative locations to RSP_entry-relative before param detection.
// Only for x86_64, because REGREL32 RSP offsets are measured after the prolog >:(
// On x86, offsets are already relative to the entry stack pointer
let frame_adjustment = if self.arch.address_size() == 8 {
data.frame_byte_count as i64
} else {
0
};
let adjusted_storage: Vec<ParsedLocation> = storage
.iter()
.map(|loc| {
if loc.stack_relative {
ParsedLocation {
location: Variable {
storage: loc.location.storage - frame_adjustment,
..loc.location
},
stack_relative: false,
..*loc
}
} else {
*loc
}
})
.collect();
let new_storage = storage.iter().map(|&var| var.location).collect::<Vec<_>>();

// See if the parameter really is a parameter. Sometimes they don't say they are
let mut really_is_param = *is_param;
for loc in adjusted_storage.iter() {
match loc.location {
for loc in &new_storage {
match loc {
Variable {
ty: VariableSourceType::RegisterVariableSourceType,
..
Expand All @@ -1753,9 +1686,9 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
}
Variable {
ty: VariableSourceType::StackVariableSourceType,
storage: offset,
storage,
..
} if offset >= 0 => {
} if *storage >= 0 => {
// Sometimes you can get two locals at the same offset, both rbp+(x > 0)
// I'm guessing from looking at dumps from dia2dump that only the first
// one is considered a parameter, although there are times that I see
Expand All @@ -1764,22 +1697,42 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
// and only one would be useful anyway.
// Regardless of the mess, Binja can only handle one parameter per slot
// so we're just going to use the first one.
really_is_param = seen_offsets.insert(offset);
really_is_param = seen_offsets.insert(*storage);
}
_ => {}
}
}

let var = ParsedVariable {
name: name.clone(),
type_: type_.clone(),
storage: adjusted_storage,
is_param: really_is_param,
};
if really_is_param {
params.push(var);
params.push(ParsedVariable {
name: name.clone(),
type_: type_.clone(),
storage: new_storage
.into_iter()
.map(|loc| ParsedLocation {
location: loc,
// This has been handled now
base_relative: false,
stack_relative: false,
})
.collect(),
is_param: really_is_param,
});
} else {
locals.push(var);
locals.push(ParsedVariable {
name: name.clone(),
type_: type_.clone(),
storage: new_storage
.into_iter()
.map(|loc| ParsedLocation {
location: loc,
// This has been handled now
base_relative: false,
stack_relative: false,
})
.collect(),
is_param: really_is_param,
});
}
}
Some(ParsedSymbol::Data(_)) => {
Expand Down
Loading