Fix C++ type name collisions in xcpclient

tools/xcpclient/README.md

@@ -50,6 +50,7 @@ The DWARF type information is mapped to A2L objects as follows:
 | pointers as struct or class members | the address value as unsigned integer of the target's pointer size, the pointee is not followed |
 
 Type names which are not valid A2L identifiers (template instantiations such as `TplStruct<float>`) are sanitized to `TplStruct_float_`.
+Colliding C++ type names are qualified with their namespace or enclosing type (for example, `namespace_1.TypeA`).
 The `TYPEDEF_MEASUREMENT`/`TYPEDEF_CHARACTERISTIC` of a struct field is named after the field; if another structure has a field with
 the same name but a different type or metadata, the name is qualified with the structure name (`TplStruct_float_.value`).
 

tools/xcpclient/fixtures/cpp_type_name_collisions.cpp

@@ -0,0 +1,28 @@
+// C++ type-name collision fixture for the xcpclient unit tests in src/elf_reader/mod.rs (mod test).
+// Covers equal-sized and differently-sized types with identical unqualified names in separate namespaces.
+//
+// cpp_type_name_collisions.elf is built from this file without libraries:
+//   arm-none-eabi-g++ -g -gdwarf-5 -O0 -fdebug-prefix-map=$(pwd)=. -nostdlib -nostartfiles -Wl,-e,main \
+//       -Wl,--unresolved-symbols=ignore-all -o cpp_type_name_collisions.elf cpp_type_name_collisions.cpp
+//
+namespace namespace_1 {
+struct TypeA { unsigned int member_1; };
+struct TypeB { unsigned int member_1; unsigned int member_2; };
+}
+
+namespace namespace_2 {
+struct TypeA { unsigned int member_1; unsigned int member_2; };
+struct TypeB { unsigned int member_3; unsigned int member_4; };
+}
+
+namespace_1::TypeA g_namespace_1_type_a;
+namespace_2::TypeA g_namespace_2_type_a;
+namespace_1::TypeB g_namespace_1_type_b;
+namespace_2::TypeB g_namespace_2_type_b;
+
+volatile unsigned int g_collision_sink;
+int main() {
+    g_collision_sink = g_namespace_1_type_a.member_1 + g_namespace_2_type_a.member_2
+                     + g_namespace_1_type_b.member_2 + g_namespace_2_type_b.member_4;
+    return 0;
+}

tools/xcpclient/src/elf_reader/debuginfo/dwarf/mod.rs

@@ -7,7 +7,7 @@
 use indexmap::IndexMap;
 use std::ffi::OsStr;
 use std::ops::Index;
-use std::{collections::HashMap, fs::File};
+use std::{collections::HashMap, collections::HashSet, fs::File};
 
 type SliceType<'a> = EndianSlice<'a, RunTimeEndian>;
 
@@ -301,6 +301,14 @@ impl DebugDataReader<'_> {
     fn collect_debug_data(mut self, unit_idx_limit: usize) -> DebugData {
         let variables = self.load_variables(unit_idx_limit);
         let (types, typenames) = self.load_types(&variables);
+        let ambiguous_type_refs: HashSet<usize> = typenames
+            .values()
+            .filter(|type_refs| type_refs.len() > 1)
+            .flatten()
+            .copied()
+            .collect();
+        let qualified_type_names = self.load_qualified_type_names(&ambiguous_type_refs);
+        let a2l_type_names = make_a2l_type_names(&typenames, &qualified_type_names);
         let varname_list: Vec<&String> = variables.keys().collect();
         let demangled_names = demangle_cpp_varnames(&varname_list);
         let unit_names = std::mem::take(&mut self.unit_names);
@@ -309,6 +317,7 @@ impl DebugDataReader<'_> {
             variables,
             types,
             typenames,
+            a2l_type_names,
             demangled_names,
             unit_names,
             sections: self.sections,
@@ -406,6 +415,37 @@ impl DebugDataReader<'_> {
         variables
     }
 
+    fn load_qualified_type_names(&self, type_refs: &HashSet<usize>) -> HashMap<usize, String> {
+        let mut qualified_type_names = HashMap::new();
+        if type_refs.is_empty() {
+            return qualified_type_names;
+        }
+
+        for (unit, abbreviations) in &self.units.list {
+            let mut entries_cursor = unit.entries(abbreviations);
+            let mut context: Vec<(gimli::DwTag, Option<String>)> = Vec::new();
+            while let Ok(Some(entry)) = entries_cursor.next_dfs() {
+                let depth = entry.depth();
+                context.truncate(depth.saturating_sub(1) as usize);
+                let tag = entry.tag();
+                let type_ref = entry.offset().to_debug_info_offset(unit).map(|offset| offset.0);
+                let entry_name = if is_named_scope(tag) || type_ref.is_some_and(|type_ref| type_refs.contains(&type_ref)) {
+                    get_name_attribute(entry, &self.dwarf, unit).ok()
+                } else {
+                    None
+                };
+
+                if let (Some(type_ref), Some(type_name)) = (type_ref, &entry_name)
+                    && type_refs.contains(&type_ref)
+                {
+                    qualified_type_names.insert(type_ref, make_qualified_type_name(&context, type_name));
+                }
+                context.push((tag, if is_named_scope(tag) { entry_name } else { None }));
+            }
+        }
+        qualified_type_names
+    }
+
     // Return global variable information
     // an entry of the type DW_TAG_variable only describes a global variable if there is a name, a type and an address
     // this function tries to get all three and returns them
@@ -531,6 +571,50 @@ fn get_varinfo_from_context(context: &[(gimli::DwTag, Option<String>)]) -> (Opti
     (function, namespaces)
 }
 
+fn is_named_scope(tag: gimli::DwTag) -> bool {
+    matches!(
+        tag,
+        gimli::constants::DW_TAG_namespace
+            | gimli::constants::DW_TAG_subprogram
+            | gimli::constants::DW_TAG_structure_type
+            | gimli::constants::DW_TAG_class_type
+            | gimli::constants::DW_TAG_union_type
+    )
+}
+
+fn make_qualified_type_name(context: &[(gimli::DwTag, Option<String>)], type_name: &str) -> String {
+    let mut qualified_name = context
+        .iter()
+        .filter(|(tag, _)| is_named_scope(*tag))
+        .filter_map(|(_, name)| name.as_deref())
+        .filter(|name| !name.is_empty())
+        .collect::<Vec<_>>()
+        .join(".");
+    if !qualified_name.is_empty() {
+        qualified_name.push('.');
+    }
+    qualified_name.push_str(type_name);
+    qualified_name
+}
+
+fn make_a2l_type_names(typenames: &HashMap<String, Vec<usize>>, qualified_type_names: &HashMap<usize, String>) -> HashMap<usize, String> {
+    let mut a2l_type_names = HashMap::new();
+    for type_refs in typenames.values() {
+        let mut names = type_refs.iter().filter_map(|type_ref| qualified_type_names.get(type_ref));
+        let Some(first_name) = names.next() else {
+            continue;
+        };
+        if names.any(|name| name != first_name) {
+            for type_ref in type_refs {
+                if let Some(name) = qualified_type_names.get(type_ref) {
+                    a2l_type_names.insert(*type_ref, name.clone());
+                }
+            }
+        }
+    }
+    a2l_type_names
+}
+
 fn demangle_cpp_varnames(input: &[&String]) -> HashMap<String, String> {
     let mut demangled_symbols = HashMap::<String, String>::new();
     let demangle_opts = cpp_demangle::DemangleOptions::new().no_params().no_return_type();
@@ -590,6 +674,33 @@ mod test {
     // C++ type test fixture, see fixtures/cpp_types.cpp
     static ELF_FILE_NAMES: [&str; 1] = [concat!(env!("CARGO_MANIFEST_DIR"), "/fixtures/cpp_types.elf")];
 
+    #[test]
+    fn test_make_qualified_type_name() {
+        let context = vec![
+            (gimli::constants::DW_TAG_compile_unit, None),
+            (gimli::constants::DW_TAG_namespace, Some("namespace_1".to_string())),
+            (gimli::constants::DW_TAG_class_type, Some("Controller".to_string())),
+        ];
+
+        assert_eq!(make_qualified_type_name(&context, "TypeA"), "namespace_1.Controller.TypeA");
+    }
+
+    #[test]
+    fn test_make_a2l_type_names() {
+        let typenames = HashMap::from([("TypeA".to_string(), vec![1, 2]), ("TypeB".to_string(), vec![3, 4])]);
+        let qualified_type_names = HashMap::from([
+            (1, "namespace_1.TypeA".to_string()),
+            (2, "namespace_2.TypeA".to_string()),
+            (3, "common.TypeB".to_string()),
+            (4, "common.TypeB".to_string()),
+        ]);
+        let a2l_type_names = make_a2l_type_names(&typenames, &qualified_type_names);
+        assert_eq!(a2l_type_names.get(&1).map(String::as_str), Some("namespace_1.TypeA"));
+        assert_eq!(a2l_type_names.get(&2).map(String::as_str), Some("namespace_2.TypeA"));
+        assert!(!a2l_type_names.contains_key(&3));
+        assert!(!a2l_type_names.contains_key(&4));
+    }
+
     #[test]
     fn test_load_data() {
         for filename in ELF_FILE_NAMES {

tools/xcpclient/src/elf_reader/debuginfo/mod.rs

@@ -96,6 +96,7 @@ pub(crate) struct DebugData {
     pub(crate) variables: IndexMap<String, Vec<VarInfo>>, // variable name -> list of VarInfo for instances with that name
     pub(crate) types: HashMap<usize, TypeInfo>,           // type reference -> TypeInfo
     pub(crate) typenames: HashMap<String, Vec<usize>>,    // type name -> list of type references
+    pub(crate) a2l_type_names: HashMap<usize, String>,    // type reference -> qualified A2L name, only for ambiguous type names
     pub(crate) demangled_names: HashMap<String, String>,  // mangled name -> demangled name
     pub(crate) unit_names: Vec<Option<String>>,           // list of compilation unit names by unit index
     pub(crate) sections: HashMap<String, (u64, u64)>,     // section name -> (start, end)
@@ -131,6 +132,12 @@ impl DebugData {
         Some(file_name.replace('.', "_"))
     }
 
+    /// Return the shortest unambiguous A2L name for a DWARF type.
+    pub(crate) fn get_a2l_type_name<'a>(&'a self, type_info: &'a TypeInfo) -> Option<&'a str> {
+        let type_name = type_info.name.as_deref()?;
+        Some(self.a2l_type_names.get(&type_info.dbginfo_offset).map_or(type_name, String::as_str))
+    }
+
     // Get the address of the XCP event descriptor memory section
     pub(crate) fn get_event_section_addr(&self) -> u64 {
         // Find section 'xcp_evts'

tools/xcpclient/src/elf_reader/mod.rs

@@ -112,12 +112,12 @@ impl ElfReader {
             DbgDataType::Float => McValueType::Float32Ieee,
             DbgDataType::Double => McValueType::Float64Ieee,
             DbgDataType::Struct { size, members, .. } => {
-                if let Some(type_name) = &type_info.name {
+                if let Some(type_name) = self.debug_data.get_a2l_type_name(type_info) {
                     // Register a typedef for the struct/class type (no-op if it already exists).
                     // The identifier is sanitized once (e.g. "TplStruct<short unsigned int>" -> "TplStruct_short_unsigned_int_")
                     // and used for the typedef, its fields and the McValueType::TypeDef reference.
                     // Inherited members of structs and classes are already flattened into `members` by the DWARF reader.
-                    let type_id = McIdentifier::from(type_name.clone());
+                    let type_id = McIdentifier::from(type_name.to_string());
                     if let Err(e) = self.register_struct(reg, object_type, type_id, *size as usize, members) {
                         error!("Failed to register typedef '{}' for struct/class type '{}': {}", type_id, type_name, e);
                     }
@@ -1184,6 +1184,8 @@ mod test {
 
     // C++ type test fixture, see fixtures/cpp_types.cpp (GCC 12.3 arm-none-eabi, DWARF 5)
     const CPP_TYPES_ELF: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/fixtures/cpp_types.elf");
+    // C++ type-name collision fixture, see fixtures/cpp_type_name_collisions.cpp
+    const CPP_TYPE_NAME_COLLISIONS_ELF: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/fixtures/cpp_type_name_collisions.elf");
 
     fn load_cpp_types() -> Registry {
         let elf_reader = ElfReader::new(CPP_TYPES_ELF, 0, usize::MAX).expect("failed to load fixtures/cpp_types.elf");
@@ -1273,6 +1275,34 @@ mod test {
         assert_eq!(inner_tpl.offset, 8);
     }
 
+    // Identically named types in different namespaces get distinct typedefs and matching instance references
+    #[test]
+    fn test_register_namespaced_types_with_colliding_names() {
+        let elf_reader = ElfReader::new(CPP_TYPE_NAME_COLLISIONS_ELF, 0, usize::MAX)
+            .expect("failed to load fixtures/cpp_type_name_collisions.elf");
+        let mut reg = Registry::new();
+        elf_reader.register_variables(&mut reg, false, 0, usize::MAX, "", "").expect("register_variables failed");
+
+        for (instance_name, expected_type_name) in [
+            ("g_namespace_1_type_a", "namespace_1.TypeA"),
+            ("g_namespace_2_type_a", "namespace_2.TypeA"),
+            ("g_namespace_1_type_b", "namespace_1.TypeB"),
+            ("g_namespace_2_type_b", "namespace_2.TypeB"),
+        ] {
+            let instance = reg
+                .instance_list
+                .get_instance(instance_name, McObjectType::Measurement, None)
+                .unwrap_or_else(|| panic!("instance '{instance_name}' not registered"));
+            assert_eq!(instance.dim_type.value_type, McValueType::new_typedef(expected_type_name), "{instance_name}");
+        }
+
+        assert_eq!(reg.typedef_list.len(), 4);
+        assert!(reg.typedef_list.find_typedef("namespace_1.TypeA").unwrap().find_field("member_1").is_some());
+        assert!(reg.typedef_list.find_typedef("namespace_2.TypeA").unwrap().find_field("member_2").is_some());
+        assert!(reg.typedef_list.find_typedef("namespace_1.TypeB").unwrap().find_field("member_2").is_some());
+        assert!(reg.typedef_list.find_typedef("namespace_2.TypeB").unwrap().find_field("member_3").is_some());
+    }
+
     // Build an ElfReader from hand-made debug data containing only event definition (evt__) and trigger (trg__) marker variables
     fn elf_reader_with_markers(markers: &[(&str, u64, &str)], event_section: Option<(u64, u64)>) -> ElfReader {
         use std::collections::HashMap;
@@ -1295,6 +1325,7 @@ mod test {
                 variables,
                 types: HashMap::new(),
                 typenames: HashMap::new(),
+                a2l_type_names: HashMap::new(),
                 demangled_names: HashMap::new(),
                 unit_names: vec![Some("main.c".to_string())],
                 sections,