DWARF Import : Misc code cleanup, improvements, and enabling by default changes

This includes:
Gracefully handle missing DIE references
Partially revert 7849cda
Misc DWARFv5 Fixes
Speed Improvements
Fix crash on unexpected EOF
Partially revert 0d5fe9ec8963a26361b8bf1af17e029afc87f952 and Correctly initialize DWARF Import plugin

1 files changed, 183 insertions, 99 deletions

diff --git a/rust/examples/dwarf/dwarf_import/src/dwarfdebuginfo.rs b/rust/examples/dwarf/dwarf_import/src/dwarfdebuginfo.rs
index b088d527..429e64e2 100644
--- a/rust/examples/dwarf/dwarf_import/src/dwarfdebuginfo.rs
+++ b/rust/examples/dwarf/dwarf_import/src/dwarfdebuginfo.rs
@@ -15,10 +15,11 @@
 use crate::helpers::{get_uid, resolve_specification, DieReference};
 
 use binaryninja::{
-    binaryview::{BinaryView, BinaryViewBase},
+    binaryview::{BinaryView, BinaryViewBase, BinaryViewExt},
     debuginfo::{DebugFunctionInfo, DebugInfo},
     rc::*,
-    templatesimplifier::simplify_str_to_str,
+    symbol::SymbolType,
+    templatesimplifier::simplify_str_to_fqn,
     types::{Conf, FunctionParameter, Type},
 };
 
@@ -38,16 +39,16 @@ pub(crate) type TypeUID = usize;
 
 // TODO : Function local variables
 #[derive(PartialEq, Eq, Hash)]
-pub struct FunctionInfoBuilder {
-    pub full_name: Option<CString>,
-    pub raw_name: Option<CString>,
-    pub return_type: Option<TypeUID>,
-    pub address: Option<u64>,
-    pub parameters: Vec<Option<(CString, TypeUID)>>,
+pub(crate) struct FunctionInfoBuilder {
+    pub(crate) full_name: Option<CString>,
+    pub(crate) raw_name: Option<CString>,
+    pub(crate) return_type: Option<TypeUID>,
+    pub(crate) address: Option<u64>,
+    pub(crate) parameters: Vec<Option<(CString, TypeUID)>>,
 }
 
 impl FunctionInfoBuilder {
-    pub fn update(
+    pub(crate) fn update(
         &mut self,
         full_name: Option<CString>,
         raw_name: Option<CString>,
@@ -72,12 +73,14 @@ impl FunctionInfoBuilder {
         }
 
         for (i, new_parameter) in parameters.into_iter().enumerate() {
-            if let Some(old_parameter) = self.parameters.get(i) {
-                if old_parameter.is_none() {
-                    self.parameters[i] = new_parameter;
-                }
-            } else {
-                self.parameters.push(new_parameter);
+            match self.parameters.get(i) {
+                Some(None) => self.parameters[i] = new_parameter,
+                Some(Some(_)) => (),
+                // Some(Some((name, _))) if name.as_bytes().is_empty() => {
+                //     self.parameters[i] = new_parameter
+                // }
+                // Some(Some((_, uid))) if *uid == 0 => self.parameters[i] = new_parameter, // TODO : This is a placebo....void types aren't actually UID 0
+                _ => self.parameters.push(new_parameter),
             }
         }
     }
@@ -93,34 +96,90 @@ pub(crate) struct DebugType {
     commit: bool,
 }
 
+pub(crate) struct DebugInfoBuilderContext<R: Reader<Offset = usize>> {
+    dwarf: Dwarf<R>,
+    units: Vec<Unit<R>>,
+    names: HashMap<TypeUID, CString>,
+    default_address_size: usize,
+    pub(crate) total_die_count: usize,
+}
+
+impl<R: Reader<Offset = usize>> DebugInfoBuilderContext<R> {
+    pub(crate) fn new(view: &BinaryView, dwarf: Dwarf<R>) -> Option<Self> {
+        let mut units = vec![];
+        let mut iter = dwarf.units();
+        while let Ok(Some(header)) = iter.next() {
+            if let Ok(unit) = dwarf.unit(header) {
+                units.push(unit);
+            } else {
+                error!("Unable to read DWARF information. File may be malformed or corrupted. Not applying debug info.");
+                return None;
+            }
+        }
+
+        Some(Self {
+            dwarf,
+            units,
+            names: HashMap::new(),
+            default_address_size: view.address_size(),
+            total_die_count: 0,
+        })
+    }
+
+    pub(crate) fn dwarf(&self) -> &Dwarf<R> {
+        &self.dwarf
+    }
+
+    pub(crate) fn units(&self) -> &[Unit<R>] {
+        &self.units
+    }
+
+    pub(crate) fn default_address_size(&self) -> usize {
+        self.default_address_size
+    }
+
+    pub(crate) fn set_name(&mut self, die_uid: TypeUID, name: CString) {
+        assert!(self.names.insert(die_uid, name).is_none());
+    }
+
+    pub(crate) fn get_name(
+        &self,
+        unit: &Unit<R>,
+        entry: &DebuggingInformationEntry<R>,
+    ) -> Option<CString> {
+        match resolve_specification(unit, entry, self) {
+            DieReference::UnitAndOffset((entry_unit, entry_offset)) => self
+                .names
+                .get(&get_uid(
+                    entry_unit,
+                    &entry_unit.entry(entry_offset).unwrap(),
+                ))
+                .cloned(),
+            DieReference::Err => None,
+        }
+    }
+}
+
 // DWARF info is stored and displayed in a tree, but is really a graph
 //  The purpose of this builder is to help resolve those graph edges by mapping partial function
 //  info and types to one DIE's UID (T) before adding the completed info to BN's debug info
-pub struct DebugInfoBuilder {
+pub(crate) struct DebugInfoBuilder {
     functions: Vec<FunctionInfoBuilder>,
     types: HashMap<TypeUID, DebugType>,
     data_variables: HashMap<u64, (Option<CString>, TypeUID)>,
-    names: HashMap<TypeUID, CString>,
-    default_address_size: usize,
 }
 
 impl DebugInfoBuilder {
-    pub fn new(view: &BinaryView) -> Self {
-        DebugInfoBuilder {
+    pub(crate) fn new() -> Self {
+        Self {
             functions: vec![],
             types: HashMap::new(),
             data_variables: HashMap::new(),
-            names: HashMap::new(),
-            default_address_size: view.address_size(),
         }
     }
 
-    pub fn default_address_size(&self) -> usize {
-        self.default_address_size
-    }
-
     #[allow(clippy::too_many_arguments)]
-    pub fn insert_function(
+    pub(crate) fn insert_function(
         &mut self,
         full_name: Option<CString>,
         raw_name: Option<CString>,
@@ -128,9 +187,18 @@ impl DebugInfoBuilder {
         address: Option<u64>,
         parameters: Vec<Option<(CString, TypeUID)>>,
     ) {
-        if let Some(function) = self.functions.iter_mut().find(|func| {
-            (func.raw_name.is_some() && func.raw_name == raw_name)
-                || (func.full_name.is_some() && func.full_name == full_name)
+        // Raw names should be the primary key, but if they don't exist, use the full name
+        // TODO : Consider further falling back on address/architecture
+        if let Some(function) = self
+            .functions
+            .iter_mut()
+            .find(|func| func.raw_name.is_some() && func.raw_name == raw_name)
+        {
+            function.update(full_name, raw_name, return_type, address, parameters);
+        } else if let Some(function) = self.functions.iter_mut().find(|func| {
+            (func.raw_name.is_none() || raw_name.is_none())
+                && func.full_name.is_some()
+                && func.full_name == full_name
         }) {
             function.update(full_name, raw_name, return_type, address, parameters);
         } else {
@@ -144,7 +212,7 @@ impl DebugInfoBuilder {
         }
     }
 
-    pub fn functions(&self) -> &[FunctionInfoBuilder] {
+    pub(crate) fn functions(&self) -> &[FunctionInfoBuilder] {
         &self.functions
     }
 
@@ -152,7 +220,13 @@ impl DebugInfoBuilder {
         self.types.values()
     }
 
-    pub fn add_type(&mut self, type_uid: TypeUID, name: CString, t: Ref<Type>, commit: bool) {
+    pub(crate) fn add_type(
+        &mut self,
+        type_uid: TypeUID,
+        name: CString,
+        t: Ref<Type>,
+        commit: bool,
+    ) {
         if let Some(DebugType {
             name: existing_name,
             t: existing_type,
@@ -165,7 +239,7 @@ impl DebugInfoBuilder {
                 commit,
             },
         ) {
-            if existing_type != t {
+            if existing_type != t && commit {
                 error!("DWARF info contains duplicate type definition. Overwriting type `{}` (named `{:?}`) with `{}` (named `{:?}`)",
                     existing_type,
                     existing_name,
@@ -176,22 +250,27 @@ impl DebugInfoBuilder {
         }
     }
 
-    pub fn remove_type(&mut self, type_uid: TypeUID) {
+    pub(crate) fn remove_type(&mut self, type_uid: TypeUID) {
         self.types.remove(&type_uid);
     }
 
     // TODO : Non-copy?
-    pub fn get_type(&self, type_uid: TypeUID) -> Option<(CString, Ref<Type>)> {
+    pub(crate) fn get_type(&self, type_uid: TypeUID) -> Option<(CString, Ref<Type>)> {
         self.types
             .get(&type_uid)
             .map(|type_ref_ref| (type_ref_ref.name.clone(), type_ref_ref.t.clone()))
     }
 
-    pub fn contains_type(&self, type_uid: TypeUID) -> bool {
+    pub(crate) fn contains_type(&self, type_uid: TypeUID) -> bool {
         self.types.get(&type_uid).is_some()
     }
 
-    pub fn add_data_variable(&mut self, address: u64, name: Option<CString>, type_uid: TypeUID) {
+    pub(crate) fn add_data_variable(
+        &mut self,
+        address: u64,
+        name: Option<CString>,
+        type_uid: TypeUID,
+    ) {
         if let Some((_existing_name, existing_type_uid)) =
             self.data_variables.insert(address, (name, type_uid))
         {
@@ -208,31 +287,6 @@ impl DebugInfoBuilder {
         }
     }
 
-    pub fn set_name(&mut self, die_uid: TypeUID, name: CString) {
-        assert!(self.names.insert(die_uid, name).is_none());
-    }
-
-    pub fn get_name<R: Reader<Offset = usize>>(
-        &self,
-        dwarf: &Dwarf<R>,
-        unit: &Unit<R>,
-        entry: &DebuggingInformationEntry<R>,
-    ) -> Option<CString> {
-        match resolve_specification(dwarf, unit, entry) {
-            DieReference::Offset(entry_offset) => self
-                .names
-                .get(&get_uid(unit, &unit.entry(entry_offset).unwrap()))
-                .cloned(),
-            DieReference::UnitAndOffset((entry_unit, entry_offset)) => self
-                .names
-                .get(&get_uid(
-                    &entry_unit,
-                    &entry_unit.entry(entry_offset).unwrap(),
-                ))
-                .cloned(),
-        }
-    }
-
     fn commit_types(&self, debug_info: &mut DebugInfo) {
         for debug_type in self.types() {
             if debug_type.commit {
@@ -252,57 +306,87 @@ impl DebugInfoBuilder {
         }
     }
 
-    fn commit_functions(&self, debug_info: &mut DebugInfo) {
-        for function in self.functions() {
-            let return_type = match function.return_type {
-                Some(return_type_id) => {
-                    Conf::new(self.get_type(return_type_id).unwrap().1.clone(), 0)
-                }
-                _ => Conf::new(binaryninja::types::Type::void(), 0),
-            };
+    fn get_function_type(&self, function: &FunctionInfoBuilder) -> Ref<Type> {
+        let return_type = match function.return_type {
+            Some(return_type_id) => Conf::new(self.get_type(return_type_id).unwrap().1.clone(), 0),
+            _ => Conf::new(binaryninja::types::Type::void(), 0),
+        };
 
-            let parameters: Vec<FunctionParameter<CString>> = function
-                .parameters
-                .iter()
-                .filter_map(|parameter| match parameter {
-                    Some((name, 0)) => {
-                        Some(FunctionParameter::new(Type::void(), name.clone(), None))
-                    }
-                    Some((name, uid)) => Some(FunctionParameter::new(
-                        self.get_type(*uid).unwrap().1,
-                        name.clone(),
-                        None,
-                    )),
-                    _ => None,
-                })
-                .collect();
+        let parameters: Vec<FunctionParameter<CString>> = function
+            .parameters
+            .iter()
+            .filter_map(|parameter| match parameter {
+                Some((name, 0)) => Some(FunctionParameter::new(Type::void(), name.clone(), None)),
+                Some((name, uid)) => Some(FunctionParameter::new(
+                    self.get_type(*uid).unwrap().1,
+                    name.clone(),
+                    None,
+                )),
+                _ => None,
+            })
+            .collect();
+
+        // TODO : Handle
+        let variable_parameters = false;
 
+        binaryninja::types::Type::function(&return_type, &parameters, variable_parameters)
+    }
+
+    fn commit_functions(&self, debug_info: &mut DebugInfo) {
+        for function in self.functions() {
             // TODO : Handle
             let platform = None;
-            let variable_parameters = false;
             // let calling_convention: Option<Ref<CallingConvention<CoreArchitecture>>> = None;
 
-            let function_type =
-                binaryninja::types::Type::function(&return_type, &parameters, variable_parameters);
-
-            let simplified_full_name = function
-                .full_name
-                .as_ref()
-                .map(|name| simplify_str_to_str(name.as_ref()).as_str().to_owned())
-                .map(|simp| CString::new(simp).unwrap());
-
             debug_info.add_function(DebugFunctionInfo::new(
-                simplified_full_name.clone(),
-                simplified_full_name, // TODO : This should eventually be changed, but the "full_name" should probably be the unsimplified version, and the "short_name" should be the simplified version...currently the symbols view shows the full version, so changing it here too makes it look bad in the UI
+                function.full_name.clone(),
+                function.full_name.clone(), // TODO : This should eventually be changed, but the "full_name" should probably be the unsimplified version, and the "short_name" should be the simplified version...currently the symbols view shows the full version, so changing it here too makes it look bad in the UI
                 function.raw_name.clone(),
-                Some(function_type),
+                Some(self.get_function_type(function)),
                 function.address,
                 platform,
             ));
         }
     }
 
-    pub fn commit_info(&self, debug_info: &mut DebugInfo) {
+    pub(crate) fn post_process(&mut self, bv: &BinaryView, _debug_info: &mut DebugInfo) -> &Self {
+        // TODO : We don't need post-processing is we process correctly the first time....
+        //   When originally resolving names, we need to check:
+        //     If there's already a name from binja that's "more correct" than what we found (has more namespaces)
+        //     If there's no name for the DIE, but there's a linkage name that's resolved in binja to a usable name
+
+        for func in &mut self.functions {
+            // If the function's raw name already exists in the binary...
+            if let Some(raw_name) = &func.raw_name {
+                if let Ok(symbol) = bv.symbol_by_raw_name(raw_name.as_c_str()) {
+                    // Link mangled names without addresses to existing symbols in the binary
+                    if func.address.is_none() && func.raw_name.is_some() {
+                        // DWARF doesn't contain GOT info, so remove any entries there...they will be wrong (relying on Binja's mechanisms for the GOT is good )
+                        if symbol.sym_type() != SymbolType::ImportAddress {
+                            func.address = Some(symbol.address());
+                        }
+                    }
+
+                    if let Some(full_name) = &func.full_name {
+                        let func_full_name = full_name.to_str().unwrap();
+                        let symbol_full_name = symbol.full_name();
+
+                        // If our name has fewer namespaces than the existing name, assume we lost the namespace info
+                        if simplify_str_to_fqn(func_full_name, true).len()
+                            < simplify_str_to_fqn(symbol_full_name.clone(), true).len()
+                        {
+                            func.full_name =
+                                Some(CString::new(symbol_full_name.to_string()).unwrap());
+                        }
+                    }
+                }
+            }
+        }
+
+        self
+    }
+
+    pub(crate) fn commit_info(&self, debug_info: &mut DebugInfo) {
         self.commit_types(debug_info);
         self.commit_data_variables(debug_info);
         self.commit_functions(debug_info);