[PDB] Remove `QualifiedName` state

Apart of fixes for https://github.com/Vector35/binaryninja-api/issues/7827

This also makes vtable type names refer to base class vtable type name, this was the original behavior, but it should be revisited later.

3 files changed, 58 insertions, 51 deletions

diff --git a/plugins/pdb-ng/src/parser.rs b/plugins/pdb-ng/src/parser.rs
index 2ca8e209..defdedd6 100644
--- a/plugins/pdb-ng/src/parser.rs
+++ b/plugins/pdb-ng/src/parser.rs
@@ -32,8 +32,8 @@ use binaryninja::platform::Platform;
 use binaryninja::rc::Ref;
 use binaryninja::settings::{QueryOptions, Settings};
 use binaryninja::types::{
-    EnumerationBuilder, NamedTypeReference, NamedTypeReferenceClass, QualifiedName,
-    StructureBuilder, StructureType, Type, TypeClass,
+    EnumerationBuilder, NamedTypeReference, NamedTypeReferenceClass, StructureBuilder,
+    StructureType, Type, TypeClass,
 };
 use binaryninja::variable::NamedDataVariableWithType;
 
@@ -69,13 +69,13 @@ pub struct PDBParserInstance<'a, S: Source<'a> + 'a> {
     /// TypeIndex -> ParsedType enum used during parsing
     pub(crate) indexed_types: BTreeMap<TypeIndex, ParsedType>,
     /// QName -> Binja Type for finished types
-    pub(crate) named_types: BTreeMap<QualifiedName, Ref<Type>>,
+    pub(crate) named_types: BTreeMap<String, Ref<Type>>,
     /// Raw (mangled) name -> TypeIndex for resolving forward references
     pub(crate) full_type_indices: BTreeMap<String, TypeIndex>,
     /// Stack of types we're currently parsing
     pub(crate) type_stack: Vec<TypeIndex>,
     /// Stack of parent types we're parsing nested types inside of
-    pub(crate) namespace_stack: QualifiedName,
+    pub(crate) namespace_stack: Vec<String>,
     /// Type Index -> Does it return on the stack
     pub(crate) type_default_returnable: BTreeMap<TypeIndex, bool>,
 
@@ -291,9 +291,9 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
     fn collect_name(
         &self,
         name: &NamedTypeReference,
-        unknown_names: &mut HashMap<QualifiedName, NamedTypeReferenceClass>,
+        unknown_names: &mut HashMap<String, NamedTypeReferenceClass>,
     ) {
-        let used_name = name.name();
+        let used_name = name.name().to_string();
         if let Some(&found) = unknown_names.get(&used_name) {
             if found != name.class() {
                 // Interesting case, not sure we care
@@ -314,7 +314,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
     fn collect_names(
         &self,
         ty: &Type,
-        unknown_names: &mut HashMap<QualifiedName, NamedTypeReferenceClass>,
+        unknown_names: &mut HashMap<String, NamedTypeReferenceClass>,
     ) {
         match ty.type_class() {
             TypeClass::StructureTypeClass => {
@@ -366,7 +366,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
             .bv
             .types()
             .iter()
-            .map(|qnat| qnat.name)
+            .map(|qnat| qnat.name.to_string())
             .collect::<HashSet<_>>();
 
         for ty in &self.named_types {
diff --git a/plugins/pdb-ng/src/symbol_parser.rs b/plugins/pdb-ng/src/symbol_parser.rs
index 7e3e5f6e..182e88fc 100644
--- a/plugins/pdb-ng/src/symbol_parser.rs
+++ b/plugins/pdb-ng/src/symbol_parser.rs
@@ -1849,8 +1849,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
             for (search_name, search_types) in name_to_type.iter() {
                 if last_name.contains(search_name) {
                     for search_type in search_types {
-                        let qualified_search_type = QualifiedName::from(search_type);
-                        if let Some(ty) = self.named_types.get(&qualified_search_type) {
+                        if let Some(ty) = self.named_types.get(search_type) {
                             // Fallback in case we don't find a specific one
                             t = Some(Conf::new(
                                 Type::named_type_from_type(search_type, ty.as_ref()),
@@ -1865,8 +1864,8 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
                                     self.make_lengthy_type(ty, self.bv.start() + rva.0 as u64)?
                                 {
                                     // See if we have a type with this length
-                                    let lengthy_name: QualifiedName =
-                                        format!("${}$_extraBytes_{}", search_type, length).into();
+                                    let lengthy_name =
+                                        format!("${}$_extraBytes_{}", search_type, length);
 
                                     if let Some(ty) = self.named_types.get(&lengthy_name) {
                                         // Wow!
@@ -2008,7 +2007,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
 /// Parse a vtable symbol name and return the parts for the vtable type.
 /// Takes the last element of (e.g., `"ClassName::`vftable'{for `BaseClass'}"`)
 /// and converts it to a vtable type name (e.g., `["ClassName", "BaseClass", "VTable"]`).
-fn parse_vtable_type_name(name: &QualifiedName) -> Option<QualifiedName> {
+fn parse_vtable_type_name(name: &QualifiedName) -> Option<String> {
     let (last, qualified_name_base) = name.items.split_last()?;
     let (class_name, rest) = last.split_once("::`vftable'")?;
 
@@ -2026,11 +2025,12 @@ fn parse_vtable_type_name(name: &QualifiedName) -> Option<QualifiedName> {
         .and_then(|(_, base_start)| base_start.split_once("'}"))
         .map(|(base, _)| base)
     {
-        parts.push(clean(base_class));
+        parts = vec![clean(base_class)];
     }
 
-    parts.push("VTable".to_string());
-    Some(QualifiedName::new(parts))
+    // We want only the last class name referenced in "parts", this is because the type we construct
+    // for the vtable will want to use the existing base vt defined in the binary view.
+    Some(format!("{}::VTable", parts.join("::")))
 }
 
 #[cfg(test)]
@@ -2043,7 +2043,7 @@ mod tests {
         // Simple vtables
         ("Base::`vftable'", &["Base", "VTable"]),
         // Multiple inheritance with {for}
-        ("MultiDerived::`vftable'{for `InterfaceA'}", &["MultiDerived", "InterfaceA", "VTable"]),
+        ("MultiDerived::`vftable'{for `InterfaceA'}", &["InterfaceA", "VTable"]),
         // Simple templates
         ("Container<int32_t>::`vftable'", &["Container<int32_t>", "VTable"]),
         ("Pair<int32_t,char>::`vftable'", &["Pair<int32_t,char>", "VTable"]),
@@ -2053,8 +2053,8 @@ mod tests {
         // Nested class inside template
         ("complex::HasNested<int32_t>::Nested::`vftable'", &["complex", "HasNested<int32_t>", "Nested", "VTable"]),
         // Template with {for} clause
-        ("TemplateMultiDerived<int32_t>::`vftable'{for `TemplateInterfaceA<int32_t>'}", &["TemplateMultiDerived<int32_t>", "TemplateInterfaceA<int32_t>", "VTable"]),
-        ("TemplateDiamond<int32_t>::`vftable'{for `TemplateLeftVirtual<int32_t>'}", &["TemplateDiamond<int32_t>", "TemplateLeftVirtual<int32_t>", "VTable"]),
+        ("TemplateMultiDerived<int32_t>::`vftable'{for `TemplateInterfaceA<int32_t>'}", &["TemplateInterfaceA<int32_t>", "VTable"]),
+        ("TemplateDiamond<int32_t>::`vftable'{for `TemplateLeftVirtual<int32_t>'}", &["TemplateLeftVirtual<int32_t>", "VTable"]),
         // Nested templates - note: "class " prefix gets stripped
         ("Wrapper<class Container<int32_t> >::`vftable'", &["Wrapper<Container<int32_t> >", "VTable"]),
         // Class/struct/enum keyword removal
@@ -2068,8 +2068,12 @@ mod tests {
         for (input, expected) in VTABLE_TEST_CASES {
             let name = QualifiedName::new(vec![input.to_string()]);
             let result = parse_vtable_type_name(&name);
-            let expected_qn = QualifiedName::new(expected.into_iter().cloned());
-            assert_eq!(result, Some(expected_qn), "Failed for input: {}", input);
+            assert_eq!(
+                result,
+                Some(expected.join("::")),
+                "Failed for input: {}",
+                input
+            );
         }
     }
 }
diff --git a/plugins/pdb-ng/src/type_parser.rs b/plugins/pdb-ng/src/type_parser.rs
index dcb7234b..101daa73 100644
--- a/plugins/pdb-ng/src/type_parser.rs
+++ b/plugins/pdb-ng/src/type_parser.rs
@@ -26,8 +26,8 @@ use binaryninja::platform::Platform;
 use binaryninja::rc::Ref;
 use binaryninja::types::{
     BaseStructure, EnumerationBuilder, EnumerationMember, FunctionParameter, MemberAccess,
-    MemberScope, NamedTypeReference, NamedTypeReferenceClass, QualifiedName, StructureBuilder,
-    StructureMember, StructureType, Type, TypeBuilder, TypeClass,
+    MemberScope, NamedTypeReference, NamedTypeReferenceClass, StructureBuilder, StructureMember,
+    StructureType, Type, TypeBuilder, TypeClass,
 };
 use log::warn;
 use pdb::Error::UnimplementedTypeKind;
@@ -136,7 +136,7 @@ pub struct ParsedMemberFunction {
 #[derive(Debug, Clone)]
 pub struct VirtualBaseClass {
     /// Base class name
-    pub base_name: QualifiedName,
+    pub base_name: String,
     /// Base class type
     pub base_type: Ref<Type>,
     /// Offset in this class where the base's fields are located
@@ -153,7 +153,7 @@ pub enum ParsedType {
     /// No info other than type data
     Bare(Ref<Type>),
     /// Named fully parsed class/enum/union/etc type
-    Named(QualifiedName, Ref<Type>),
+    Named(String, Ref<Type>),
     /// Function procedure
     Procedure(ParsedProcedureType),
     /// Bitfield entries
@@ -163,7 +163,7 @@ pub enum ParsedType {
     /// One member in a structure/union
     Member(ParsedMember),
     /// Base class name and offset details
-    BaseClass(QualifiedName, StructureMember),
+    BaseClass(String, StructureMember),
     /// One member in an enumeration
     Enumerate(EnumerationMember),
     /// List of arguments to a function
@@ -352,7 +352,8 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
                     let name = ty
                         .get_named_type_reference()
                         .ok_or(anyhow!("expected ntr"))?
-                        .name();
+                        .name()
+                        .to_string();
                     if Self::is_name_anonymous(&name) {
                         continue;
                     }
@@ -405,9 +406,8 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
 
         // Cleanup a couple builtin names
         for &name in BUILTIN_NAMES {
-            let builtin_qualified_name = QualifiedName::from(name);
-            if self.named_types.contains_key(&builtin_qualified_name) {
-                self.named_types.remove(&builtin_qualified_name);
+            if self.named_types.contains_key(name) {
+                self.named_types.remove(name);
                 self.log(|| format!("Remove builtin type {}", name));
             }
         }
@@ -742,7 +742,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
         self.log(|| format!("Got Class type: {:x?}", data));
 
         let raw_class_name = data.name.to_string();
-        let class_name = QualifiedName::from(raw_class_name);
+        let class_name = raw_class_name.to_string();
 
         self.log(|| format!("Named: {}", class_name));
 
@@ -776,7 +776,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
         structure.packed(data.properties.packed());
 
         if let Some(fields) = data.fields {
-            self.namespace_stack.push(class_name.to_string());
+            self.namespace_stack.push(class_name.clone());
             let success = self.parse_structure_fields(&mut structure, fields, finder);
             self.namespace_stack.pop();
             let _ = success?;
@@ -852,7 +852,6 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
             &format!(
                 "`{}`",
                 self.namespace_stack
-                    .items
                     .last()
                     .ok_or_else(|| anyhow!("Expected class in ns stack"))?
             ),
@@ -925,7 +924,6 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
                     warn!(
                         "Class `{}` uses virtual inheritance. Type information may be inaccurate.",
                         self.namespace_stack
-                            .items
                             .last()
                             .ok_or_else(|| anyhow!("Expected class in ns stack"))?
                     );
@@ -938,7 +936,6 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
             warn!(
                 "Class `{}` has multiple base classes. Type information may be inaccurate.",
                 self.namespace_stack
-                    .items
                     .last()
                     .ok_or_else(|| anyhow!("Expected class in ns stack"))?
             );
@@ -957,8 +954,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
             for base_class in &base_classes {
                 match base_class {
                     ParsedType::BaseClass(base_name, _base_type) => {
-                        let mut vt_base_name = base_name.clone();
-                        vt_base_name.items.push("VTable".to_string());
+                        let vt_base_name = format!("{}::VTable", base_name);
 
                         match self.named_types.get(&vt_base_name) {
                             Some(vt_base_type)
@@ -1034,8 +1030,8 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
                 return Err(anyhow!("Expected class in ns stack"));
             }
 
-            let mut vt_name = self.namespace_stack.clone();
-            vt_name.items.push("VTable".to_string());
+            let class_name = self.namespace_stack.last().cloned().unwrap_or_default();
+            let vt_name = format!("{}::VTable", class_name);
             self.named_types.insert(vt_name.clone(), vt_type.clone());
 
             let vt_pointer = Type::pointer(
@@ -1268,9 +1264,12 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
     ) -> Result<Option<Box<ParsedType>>> {
         self.log(|| format!("Got Nested type: {:x?}", data));
         let mut class_name_ns = self.namespace_stack.clone();
-        class_name_ns.push(data.name.to_string().into());
+        class_name_ns.push(data.name.to_string().to_string());
         let ty = self.type_index_to_bare(data.nested_type, finder, false)?;
-        Ok(Some(Box::new(ParsedType::Named(class_name_ns, ty))))
+        Ok(Some(Box::new(ParsedType::Named(
+            class_name_ns.join("::"),
+            ty,
+        ))))
     }
 
     fn handle_base_class_type(
@@ -1289,7 +1288,8 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
                 let name = t
                     .get_named_type_reference()
                     .ok_or(anyhow!("Expected NTR to have NTR"))?
-                    .name();
+                    .name()
+                    .to_string();
                 (name, t.clone())
             }
             e => return Err(anyhow!("Unexpected base class type: {:x?}", e)),
@@ -1297,7 +1297,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
 
         // Try to resolve the full base type
         let resolved_type = match self.try_type_index_to_bare(data.base_class, finder, true)? {
-            Some(ty) => Type::named_type_from_type(member_name.clone(), ty.as_ref()),
+            Some(ty) => Type::named_type_from_type(&member_name, ty.as_ref()),
             None => t.clone(),
         };
 
@@ -1313,7 +1313,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
             member_name.clone(),
             StructureMember::new(
                 Conf::new(resolved_type, MAX_CONFIDENCE),
-                member_name.to_string(),
+                member_name,
                 base_offset as u64,
                 access,
                 scope,
@@ -1334,7 +1334,8 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
                 let name = t
                     .get_named_type_reference()
                     .ok_or(anyhow!("Expected NTR to have NTR"))?
-                    .name();
+                    .name()
+                    .to_string();
                 (name, t.clone())
             }
             e => return Err(anyhow!("Unexpected base class type: {:x?}", e)),
@@ -1523,7 +1524,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
         self.log(|| format!("Got Enumeration type: {:x?}", data));
 
         let raw_enum_name = data.name.to_string();
-        let enum_name = QualifiedName::from(raw_enum_name);
+        let enum_name = raw_enum_name.to_string();
         self.log(|| format!("Named: {}", enum_name));
 
         if data.properties.forward_reference() {
@@ -1651,7 +1652,7 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
         self.log(|| format!("Got Union type: {:x?}", data));
 
         let raw_union_name = data.name.to_string();
-        let union_name = QualifiedName::from(raw_union_name);
+        let union_name = raw_union_name.to_string();
         self.log(|| format!("Named: {}", union_name));
 
         if data.properties.forward_reference() {
@@ -1908,7 +1909,8 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
                 let name = type_
                     .get_named_type_reference()
                     .ok_or(anyhow!("expected ntr"))?
-                    .name();
+                    .name()
+                    .to_string();
                 if let Some(full_ntr) = self.named_types.get(&name) {
                     type_ = Type::named_type_from_type(name, full_ntr.as_ref());
                 }
@@ -1955,7 +1957,8 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
             let name = type_
                 .get_named_type_reference()
                 .ok_or(anyhow!("expected ntr"))?
-                .name();
+                .name()
+                .to_string();
             if Self::is_name_anonymous(&name) {
                 if let Some(inner) = inner.as_ref() {
                     type_ = inner.clone();
@@ -1992,8 +1995,8 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
     }
 
     /// Is this name one of the stupid microsoft unnamed type names
-    fn is_name_anonymous(name: &QualifiedName) -> bool {
-        match name.items.last() {
+    fn is_name_anonymous(name: &String) -> bool {
+        match name.split("::").last() {
             Some(item) if item == "<anonymous-tag>" => true,
             Some(item) if item.contains("<unnamed-") => true,
             _ => false,