[DWARF] Avoid reprocessing types repeated in every compilation unit

plugins/dwarf/dwarf_import/src/die_handlers.rs

@@ -12,7 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::dwarfdebuginfo::{DebugInfoBuilder, DebugInfoBuilderContext, TypeUID};
+use crate::dwarfdebuginfo::{
+    DebugInfoBuilder, DebugInfoBuilderContext, TypeUID, UNNAMED_FUNCTION_NAME,
+};
 use crate::types::get_type;
 use crate::{helpers::*, ReaderType};
 
@@ -173,8 +175,10 @@ pub(crate) fn handle_typedef(
     // This will fail in the case where we have a typedef to a type that doesn't exist (failed to parse, incomplete, etc)
     if let Some(entry_type_offset) = entry_type {
         if let Some(t) = debug_info_builder.get_type(entry_type_offset) {
-            let typedef_type = Type::named_type_from_type(typedef_name, &t.get_type());
-            return (Some(typedef_type), typedef_name != t.name);
+            let target = t.get_type();
+            let renames_target = typedef_name != t.name;
+            let typedef_type = debug_info_builder.typedef_placeholder(typedef_name, &target);
+            return (Some(typedef_type), renames_target);
         }
     }
 
@@ -333,11 +337,19 @@ pub(crate) fn handle_function<R: ReaderType>(
     };
 
     // Alias function type in the case that it contains itself
-    let name = debug_info_builder_context
-        .get_name(dwarf, unit, entry)
-        .unwrap_or("_unnamed_func".to_string());
-    let ntr =
-        Type::named_type_from_type(&name, &Type::function(return_type.as_ref(), vec![], false));
+    let (name, ntr) = match debug_info_builder_context.get_name(dwarf, unit, entry) {
+        Some(name) => {
+            let ntr = Type::named_type_from_type(
+                &name,
+                &Type::function(return_type.as_ref(), vec![], false),
+            );
+            (name, ntr)
+        }
+        None => {
+            let ntr = debug_info_builder.unnamed_function_placeholder(return_type.as_ref());
+            (UNNAMED_FUNCTION_NAME.to_string(), ntr)
+        }
+    };
     debug_info_builder.add_type(get_uid(dwarf, unit, entry), name, ntr, false, None);
 
     let mut parameters: Vec<FunctionParameter> = vec![];

plugins/dwarf/dwarf_import/src/dwarfdebuginfo.rs

@@ -25,7 +25,7 @@ use binaryninja::{
     platform::Platform,
     rc::*,
     symbol::SymbolType,
-    types::{FunctionParameter, Type},
+    types::{FunctionParameter, StructureBuilder, StructureType, Type, TypeClass},
     variable::NamedVariableWithType,
 };
 
@@ -101,6 +101,9 @@ impl FunctionInfoBuilder {
 //////////////////////
 // DebugInfoBuilder
 
+// The name given to a subroutine type that has none of its own.
+pub(crate) const UNNAMED_FUNCTION_NAME: &str = "_unnamed_func";
+
 // TODO : Don't make this pub...fix the value thing
 pub(crate) struct DebugType {
     pub name: String,
@@ -218,6 +221,10 @@ pub(crate) struct DebugInfoBuilder {
     types: IndexMap<TypeUID, DebugType>,
     data_variables: HashMap<u64, (Option<String>, TypeUID)>,
     range_data_offsets: iset::IntervalMap<u64, i64>,
+    structure_placeholders: HashMap<(String, StructureType, u64), Ref<Type>>,
+    typedef_placeholders:
+        HashMap<(String, TypeClass, Option<StructureType>, u64, usize), Ref<Type>>,
+    unnamed_function_placeholder: Option<Ref<Type>>,
 }
 
 impl DebugInfoBuilder {
@@ -229,7 +236,70 @@ impl DebugInfoBuilder {
             types: IndexMap::new(),
             data_variables: HashMap::new(),
             range_data_offsets: iset::IntervalMap::new(),
+            structure_placeholders: HashMap::new(),
+            typedef_placeholders: HashMap::new(),
+            unnamed_function_placeholder: None,
+        }
+    }
+
+    // A placeholder named type reference carries nothing but its name and the reference class,
+    // width and alignment that its target contributes, so the same declaration appearing in
+    // another compilation unit produces the same placeholder. Debug info repeats the common
+    // typedefs and structures in every unit that includes their header, so building each one once
+    // saves marshalling the name across the API, building the target, and asking the core for a
+    // reference, every time after the first.
+    pub(crate) fn structure_placeholder(
+        &mut self,
+        name: &str,
+        structure_type: StructureType,
+        size: u64,
+    ) -> Ref<Type> {
+        let key = (name.to_string(), structure_type, size);
+        if let Some(existing) = self.structure_placeholders.get(&key) {
+            return existing.clone();
         }
+
+        let mut structure_builder = StructureBuilder::new();
+        structure_builder
+            .packed(true)
+            .width(size)
+            .structure_type(structure_type);
+        let placeholder =
+            Type::named_type_from_type(name, &Type::structure(&structure_builder.finalize()));
+        self.structure_placeholders.insert(key, placeholder.clone());
+        placeholder
+    }
+
+    pub(crate) fn typedef_placeholder(&mut self, name: &str, target: &Type) -> Ref<Type> {
+        // A structure target gives the reference its class according to whether it is a struct, a
+        // union or a class, so which of those it is has to identify the placeholder as well.
+        let key = (
+            name.to_string(),
+            target.type_class(),
+            target.get_structure().map(|s| s.structure_type()),
+            target.width(),
+            target.alignment(),
+        );
+        if let Some(existing) = self.typedef_placeholders.get(&key) {
+            return existing.clone();
+        }
+
+        let placeholder = Type::named_type_from_type(name, target);
+        self.typedef_placeholders.insert(key, placeholder.clone());
+        placeholder
+    }
+
+    // Function types carry no width or alignment of their own, so every anonymous subroutine
+    // produces the same placeholder whatever it returns.
+    pub(crate) fn unnamed_function_placeholder(&mut self, return_type: &Type) -> Ref<Type> {
+        self.unnamed_function_placeholder
+            .get_or_insert_with(|| {
+                Type::named_type_from_type(
+                    UNNAMED_FUNCTION_NAME,
+                    &Type::function(return_type, vec![], false),
+                )
+            })
+            .clone()
     }
 
     pub(crate) fn set_range_data_offsets(&mut self, offsets: iset::IntervalMap<u64, i64>) {
@@ -566,6 +636,17 @@ impl DebugInfoBuilder {
         }
     }
 
+    // What committing a type actually stores. A typedef contributes its target, because its own
+    // type is the self-referential placeholder that stands in for it while its children are built.
+    fn committed_type(&self, debug_type: &DebugType) -> Option<Ref<Type>> {
+        if debug_type.get_type().get_named_type_reference().is_none() {
+            return Some(debug_type.get_type());
+        }
+
+        let target_uid = debug_type.target_type_uid?;
+        Some(self.get_type(target_uid)?.get_type())
+    }
+
     fn commit_types(&self, debug_info: &mut DebugInfo) {
         let mut type_uids_by_name: HashMap<String, TypeUID> = HashMap::new();
 
@@ -583,6 +664,20 @@ impl DebugInfoBuilder {
                     continue;
                 };
 
+                // This name already describes this definition. Debug info repeats a type in every
+                // compilation unit that includes its header, so committing it again would have the
+                // core walk and re-resolve every named reference in it for no gain.
+                let same_definition = match (
+                    self.committed_type(stored_debug_type),
+                    self.committed_type(debug_type),
+                ) {
+                    (Some(stored), Some(current)) => stored.as_ref() == current.as_ref(),
+                    _ => false,
+                };
+                if same_definition {
+                    continue;
+                }
+
                 let mut skip_adding_type = false;
                 if stored_debug_type.ty != debug_type.ty {
                     // We already stored a type with this name and it's a different type, deconflict the name and try again
@@ -622,6 +717,7 @@ impl DebugInfoBuilder {
                 if let Some(target_uid) = debug_type.target_type_uid {
                     if let Some(target_type) = self.get_type(target_uid) {
                         debug_info.add_type(&debug_type_name, &target_type.get_type(), &[]);
+                        type_uids_by_name.insert(debug_type_name, *debug_type_uid);
                     } else {
                         tracing::error!(
                             "Failed to find typedef {} target for uid {}",
@@ -638,8 +734,8 @@ impl DebugInfoBuilder {
                 }
             } else {
                 debug_info.add_type(&debug_type_name, &debug_type.ty, &[]);
+                type_uids_by_name.insert(debug_type_name, *debug_type_uid);
             }
-            type_uids_by_name.insert(debug_type_name, *debug_type_uid);
         }
     }
 

plugins/dwarf/dwarf_import/src/types.rs

@@ -166,8 +166,7 @@ fn do_structure_parse<R: ReaderType>(
     // This reference type will be used by any children to grab while we're still building this type
     //  it will also be how any other types refer to this struct
     if let Some(full_name) = &full_name {
-        let ntr =
-            Type::named_type_from_type(full_name, &Type::structure(&structure_builder.finalize()));
+        let ntr = debug_info_builder.structure_placeholder(full_name, structure_type, size);
         debug_info_builder.add_type(
             get_uid(dwarf, unit, entry),
             full_name.to_owned(),