From 92aab3c5df2efa6b764fd60e74fd42d75efa0532 Mon Sep 17 00:00:00 2001 From: Mason Reed Date: Mon, 31 Aug 2026 18:53:39 -0700 Subject: [PATCH 1/4] Revert "[PDB Import] Only adjust sp-relative locals on x86_64" This reverts commit 84d66dc7e6244e6ef78ebb3a2aa24b57a6ed4345. --- plugins/pdb-ng/src/symbol_parser.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/plugins/pdb-ng/src/symbol_parser.rs b/plugins/pdb-ng/src/symbol_parser.rs index 3306324b3..37b19a812 100644 --- a/plugins/pdb-ng/src/symbol_parser.rs +++ b/plugins/pdb-ng/src/symbol_parser.rs @@ -1714,21 +1714,15 @@ 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 - }; + // Adjust RSP-relative locations to RSP_entry-relative before param detection let adjusted_storage: Vec = storage .iter() .map(|loc| { if loc.stack_relative { ParsedLocation { location: Variable { - storage: loc.location.storage - frame_adjustment, + storage: loc.location.storage + - data.frame_byte_count as i64, ..loc.location }, stack_relative: false, From d1c391de765ae6fb3dfcfbb4a6ba2c894afdf59a Mon Sep 17 00:00:00 2001 From: Mason Reed Date: Mon, 31 Aug 2026 18:53:39 -0700 Subject: [PATCH 2/4] Revert "[PDB Import] Collect locals and params from blocks contained in a FrameProcedure" This reverts commit 71b1c61391c0b32dd19053bfcaf51bbd67f0a642. --- plugins/pdb-ng/src/symbol_parser.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/plugins/pdb-ng/src/symbol_parser.rs b/plugins/pdb-ng/src/symbol_parser.rs index 37b19a812..672fb70dd 100644 --- a/plugins/pdb-ng/src/symbol_parser.rs +++ b/plugins/pdb-ng/src/symbol_parser.rs @@ -1702,10 +1702,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, From 7aa1be1e89587d84ef97c3c0d84e808f595035d0 Mon Sep 17 00:00:00 2001 From: Mason Reed Date: Mon, 31 Aug 2026 18:53:39 -0700 Subject: [PATCH 3/4] Revert "[PDB Import] Improve recovery of sp-based locals" This reverts commit 0498fe8cc36075829deeebb919a57150d1ff9a44. --- plugins/pdb-ng/src/symbol_parser.rs | 70 +++-------------------------- 1 file changed, 5 insertions(+), 65 deletions(-) diff --git a/plugins/pdb-ng/src/symbol_parser.rs b/plugins/pdb-ng/src/symbol_parser.rs index 672fb70dd..d5fabeb44 100644 --- a/plugins/pdb-ng/src/symbol_parser.rs +++ b/plugins/pdb-ng/src/symbol_parser.rs @@ -1587,15 +1587,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> { data: &DefRangeFramePointerRelativeSymbol, ) -> Result> { 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( @@ -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( @@ -1635,31 +1619,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> { data: &DefRangeRegisterRelativeSymbol, ) -> Result> { 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( @@ -1711,29 +1671,9 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> { is_param, .. })) => { - // Adjust RSP-relative locations to RSP_entry-relative before param detection - let adjusted_storage: Vec = storage - .iter() - .map(|loc| { - if loc.stack_relative { - ParsedLocation { - location: Variable { - storage: loc.location.storage - - data.frame_byte_count as i64, - ..loc.location - }, - stack_relative: false, - ..*loc - } - } else { - *loc - } - }) - .collect(); - // 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() { + for loc in storage.iter() { match loc.location { Variable { ty: VariableSourceType::RegisterVariableSourceType, @@ -1764,7 +1704,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> { let var = ParsedVariable { name: name.clone(), type_: type_.clone(), - storage: adjusted_storage, + storage: storage.clone(), is_param: really_is_param, }; if really_is_param { From 34de8db0231e5ed5eecbe1ea09d4d27967cdcc99 Mon Sep 17 00:00:00 2001 From: Mason Reed Date: Mon, 31 Aug 2026 18:53:39 -0700 Subject: [PATCH 4/4] Revert "[PDB Import] Initial support for loading locals" This reverts commit 8e7535e92fbb4305232eb19405f1d738437dcf4b. --- plugins/pdb-ng/src/parser.rs | 28 ++------------- plugins/pdb-ng/src/symbol_parser.rs | 56 ++++++++++++++++++++--------- 2 files changed, 42 insertions(+), 42 deletions(-) diff --git a/plugins/pdb-ng/src/parser.rs b/plugins/pdb-ng/src/parser.rs index e24ae838e..e6517ae22 100644 --- a/plugins/pdb-ng/src/parser.rs +++ b/plugins/pdb-ng/src/parser.rs @@ -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. @@ -252,7 +252,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> { address, name, type_, - locals, + locals: _, .. }) => { self.log(|| { @@ -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![], //TODO: local variables )); } _ => {} diff --git a/plugins/pdb-ng/src/symbol_parser.rs b/plugins/pdb-ng/src/symbol_parser.rs index d5fabeb44..a0d09d9ca 100644 --- a/plugins/pdb-ng/src/symbol_parser.rs +++ b/plugins/pdb-ng/src/symbol_parser.rs @@ -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, @@ -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"))?; @@ -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); } @@ -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( @@ -1671,10 +1671,12 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> { is_param, .. })) => { + let new_storage = storage.iter().map(|&var| var.location).collect::>(); + // 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 storage.iter() { - match loc.location { + for loc in &new_storage { + match loc { Variable { ty: VariableSourceType::RegisterVariableSourceType, .. @@ -1684,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 @@ -1695,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: storage.clone(), - 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(_)) => {