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 3306324b3..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( @@ -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( @@ -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, @@ -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 = 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::>(); // 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, .. @@ -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 @@ -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(_)) => {