Support bitfields in type system

Also adds support for parsing bitfields in PDB, DWARF and SVD plugins

WIP: API needs to be considered more, also need to find type related apis that may need to be rethought.

4 files changed, 122 insertions, 224 deletions

diff --git a/plugins/dwarf/dwarf_import/src/types.rs b/plugins/dwarf/dwarf_import/src/types.rs
index 38f0dbc0..f870707b 100644
--- a/plugins/dwarf/dwarf_import/src/types.rs
+++ b/plugins/dwarf/dwarf_import/src/types.rs
@@ -24,7 +24,7 @@ use binaryninja::{
     },
 };
 
-use gimli::{constants, AttributeValue, DebuggingInformationEntry, Dwarf, Operation, Unit};
+use gimli::{constants, AttributeValue, DebuggingInformationEntry, DwAt, Dwarf, Operation, Unit};
 
 use log::{debug, error, warn};
 
@@ -220,10 +220,6 @@ fn do_structure_parse<R: ReaderType>(
                     continue;
                 };
 
-                /*
-                    TODO: apply correct member size when that's supported
-                    let child_type_width = get_size_as_u64(child_entry).unwrap_or_else(|| child_type.width());
-                */
                 if let Ok(Some(raw_struct_offset)) =
                     child_entry.attr(constants::DW_AT_data_member_location)
                 {
@@ -246,36 +242,65 @@ fn do_structure_parse<R: ReaderType>(
                         MemberAccess::NoAccess, // TODO : Resolve actual scopes, if possible
                         MemberScope::NoScope,
                     );
-                } else if let Ok(Some(raw_struct_offset_bits)) =
-                    child_entry.attr(constants::DW_AT_data_bit_offset)
-                {
-                    //TODO: support misaligned offsets when bitwise data structures get in
-                    let Some(struct_offset_bits) = get_attr_as_u64(&raw_struct_offset_bits)
-                        .or_else(|| get_expr_value(unit, raw_struct_offset_bits))
-                    else {
-                        log::warn!(
-                            "Failed to get DW_AT_data_bit_offset for offset {:#x} in unit {:?}",
-                            child_entry.offset().0,
-                            unit.header.offset()
-                        );
-                        continue;
-                    };
-
-                    structure_builder.insert(
-                        &child_type,
-                        &child_name,
-                        struct_offset_bits / 8,
-                        false,
-                        MemberAccess::NoAccess, // TODO : Resolve actual scopes, if possible
-                        MemberScope::NoScope,
-                    );
                 } else {
-                    structure_builder.append(
-                        &child_type,
-                        &child_name,
-                        MemberAccess::NoAccess,
-                        MemberScope::NoScope,
-                    );
+                    let select_value =
+                        |e: &DebuggingInformationEntry<R>, attr: DwAt| -> Option<u64> {
+                            get_attr_as_u64(&e.attr(attr).ok()??)
+                        };
+
+                    // If no byte offset, try the bitfield using DW_AT_bit_offset/DW_AT_data_bit_offset + DW_AT_bit_size
+                    let bit_size = select_value(child_entry, constants::DW_AT_bit_size);
+                    let bit_offset = select_value(child_entry, constants::DW_AT_bit_offset);
+                    let data_bit_offset =
+                        select_value(child_entry, constants::DW_AT_data_bit_offset);
+
+                    match (bit_size, bit_offset, data_bit_offset) {
+                        (Some(bit_size), Some(bit_offset), _) => {
+                            // Heuristic storage unit bits from the member type width (bytes -> bits). Fallback to 8.
+                            let storage_bits = {
+                                let w = child_type.width();
+                                if w > 0 {
+                                    w * 8
+                                } else {
+                                    8
+                                }
+                            };
+
+                            // DW_AT_bit_offset is from the MSB of the storage unit:
+                            // absolute = base_byte_off*8 + storage_bits - (boffs + bit_sz)
+                            // With no base_byte_off available here, treat base as 0.
+                            let total_bit_off = storage_bits.saturating_sub(bit_offset + bit_size);
+
+                            structure_builder.insert_bitwise(
+                                &child_type,
+                                &child_name,
+                                total_bit_off,
+                                Some(bit_size as u8),
+                                false,
+                                MemberAccess::NoAccess,
+                                MemberScope::NoScope,
+                            );
+                        }
+                        (Some(bit_size), None, Some(data_bit_offset)) => {
+                            structure_builder.insert_bitwise(
+                                &child_type,
+                                &child_name,
+                                data_bit_offset,
+                                Some(bit_size as u8),
+                                false,
+                                MemberAccess::NoAccess,
+                                MemberScope::NoScope,
+                            );
+                        }
+                        _ => {
+                            structure_builder.append(
+                                &child_type,
+                                &child_name,
+                                MemberAccess::NoAccess,
+                                MemberScope::NoScope,
+                            );
+                        }
+                    }
                 }
             }
             constants::DW_TAG_inheritance => {
@@ -316,7 +341,6 @@ fn do_structure_parse<R: ReaderType>(
 
     structure_builder.base_structures(&base_structures);
     let finalized_structure = Type::structure(&structure_builder.finalize());
-
     if let Some(full_name) = full_name {
         debug_info_builder.add_type(
             get_uid(dwarf, unit, entry) + 1, // TODO : This is super broke (uid + 1 is not guaranteed to be unique)
diff --git a/plugins/pdb-ng/src/struct_grouper.rs b/plugins/pdb-ng/src/struct_grouper.rs
index ff911afd..fcaf88ef 100644
--- a/plugins/pdb-ng/src/struct_grouper.rs
+++ b/plugins/pdb-ng/src/struct_grouper.rs
@@ -340,8 +340,10 @@ pub fn group_structure(
         .enumerate()
         .map(|(i, member)| MemberSize {
             index: i,
-            offset: member.offset,
-            width: member.ty.contents.width(),
+            offset: member.bitfield_position.unwrap_or(member.offset * 8),
+            width: member
+                .bitfield_size
+                .unwrap_or(member.ty.contents.width() * 8),
         })
         .collect::<Vec<_>>();
 
@@ -359,14 +361,29 @@ pub fn group_structure(
         Err(e) => {
             warn!("{} Could not resolve structure groups: {}", name, e);
             for member in members {
-                structure.insert(
-                    &member.ty,
-                    &member.name,
-                    member.offset,
-                    false,
-                    member.access,
-                    member.scope,
-                );
+                match (member.bitfield_position, member.bitfield_size) {
+                    (Some(bit_pos), bit_width) => {
+                        structure.insert_bitwise(
+                            &member.ty,
+                            &member.name,
+                            bit_pos,
+                            bit_width.map(|w| w as u8),
+                            false,
+                            member.access,
+                            member.scope,
+                        );
+                    }
+                    (None, _) => {
+                        structure.insert(
+                            &member.ty,
+                            &member.name,
+                            member.offset,
+                            false,
+                            member.access,
+                            member.scope,
+                        );
+                    }
+                }
             }
         }
     }
@@ -387,24 +404,38 @@ fn apply_groups(
 
                 // TODO : Fix inner-offset being larger than `member.offset`
 
-                if offset > member.offset {
-                    structure.insert(
-                        &member.ty,
-                        &member.name,
-                        0,
-                        false,
-                        member.access,
-                        member.scope,
-                    );
-                } else {
-                    structure.insert(
-                        &member.ty,
-                        &member.name,
-                        member.offset - offset,
-                        false,
-                        member.access,
-                        member.scope,
-                    );
+                match (member.bitfield_position, member.bitfield_size) {
+                    (Some(bit_pos), bit_width) => {
+                        structure.insert_bitwise(
+                            &member.ty,
+                            &member.name,
+                            bit_pos,
+                            bit_width.map(|w| w as u8),
+                            false,
+                            member.access,
+                            member.scope,
+                        );
+                    }
+                    (None, _) if offset > member.offset => {
+                        structure.insert(
+                            &member.ty,
+                            &member.name,
+                            0,
+                            false,
+                            member.access,
+                            member.scope,
+                        );
+                    }
+                    (None, _) => {
+                        structure.insert(
+                            &member.ty,
+                            &member.name,
+                            member.offset - offset,
+                            false,
+                            member.access,
+                            member.scope,
+                        );
+                    }
                 }
             }
             ResolvedGroup::Struct(inner_offset, children) => {
diff --git a/plugins/pdb-ng/src/type_parser.rs b/plugins/pdb-ng/src/type_parser.rs
index ba7f3cc9..dcb7234b 100644
--- a/plugins/pdb-ng/src/type_parser.rs
+++ b/plugins/pdb-ng/src/type_parser.rs
@@ -848,92 +848,6 @@ impl<'a, S: Source<'a> + 'a> PDBParserInstance<'a, S> {
             None => {}
         }
 
-        // Combine bitfields into structures
-        let mut combined_bitfield_members = vec![];
-        let mut last_bitfield_offset = u64::MAX;
-        let mut last_bitfield_pos = u64::MAX;
-        let mut last_bitfield_idx = 0;
-        let mut bitfield_builder: Option<StructureBuilder> = None;
-
-        fn bitfield_name(offset: u64, idx: u64) -> String {
-            if idx > 0 {
-                format!("__bitfield{:x}_{}", offset, idx)
-            } else {
-                format!("__bitfield{:x}", offset)
-            }
-        }
-
-        for m in members {
-            match (m.bitfield_position, m.bitfield_size) {
-                (Some(pos), Some(_size)) => {
-                    if last_bitfield_offset != m.offset || last_bitfield_pos >= pos {
-                        if let Some(builder) = bitfield_builder.take() {
-                            combined_bitfield_members.push(ParsedMember {
-                                ty: Conf::new(
-                                    Type::structure(builder.finalize().as_ref()),
-                                    MAX_CONFIDENCE,
-                                ),
-                                name: bitfield_name(last_bitfield_offset, last_bitfield_idx),
-                                offset: last_bitfield_offset,
-                                access: MemberAccess::PublicAccess,
-                                scope: MemberScope::NoScope,
-                                bitfield_size: None,
-                                bitfield_position: None,
-                            });
-                        }
-                        let mut new_builder = StructureBuilder::new();
-                        new_builder.structure_type(StructureType::UnionStructureType);
-                        new_builder.width(m.ty.contents.width());
-                        bitfield_builder = Some(new_builder);
-
-                        if last_bitfield_offset != m.offset {
-                            last_bitfield_idx = 0;
-                        } else {
-                            last_bitfield_idx += 1;
-                        }
-                    }
-
-                    last_bitfield_pos = pos;
-                    last_bitfield_offset = m.offset;
-                    bitfield_builder
-                        .as_mut()
-                        .expect("Invariant")
-                        .insert(&m.ty, &m.name, 0, false, m.access, m.scope);
-                }
-                (None, None) => {
-                    if let Some(builder) = bitfield_builder.take() {
-                        combined_bitfield_members.push(ParsedMember {
-                            ty: Conf::new(
-                                Type::structure(builder.finalize().as_ref()),
-                                MAX_CONFIDENCE,
-                            ),
-                            name: bitfield_name(last_bitfield_offset, last_bitfield_idx),
-                            offset: last_bitfield_offset,
-                            access: MemberAccess::PublicAccess,
-                            scope: MemberScope::NoScope,
-                            bitfield_size: None,
-                            bitfield_position: None,
-                        });
-                    }
-                    last_bitfield_offset = u64::MAX;
-                    last_bitfield_pos = u64::MAX;
-                    combined_bitfield_members.push(m);
-                }
-                e => return Err(anyhow!("Unexpected bitfield parameters {:?}", e)),
-            }
-        }
-        if let Some(builder) = bitfield_builder.take() {
-            combined_bitfield_members.push(ParsedMember {
-                ty: Conf::new(Type::structure(builder.finalize().as_ref()), MAX_CONFIDENCE),
-                name: bitfield_name(last_bitfield_offset, last_bitfield_idx),
-                offset: last_bitfield_offset,
-                access: MemberAccess::PublicAccess,
-                scope: MemberScope::NoScope,
-                bitfield_size: None,
-                bitfield_position: None,
-            });
-        }
-        members = combined_bitfield_members;
         group_structure(
             &format!(
                 "`{}`",
diff --git a/plugins/svd/src/mapper.rs b/plugins/svd/src/mapper.rs
index 8846c858..4c49145b 100644
--- a/plugins/svd/src/mapper.rs
+++ b/plugins/svd/src/mapper.rs
@@ -8,7 +8,7 @@ use binaryninja::segment::{SegmentBuilder, SegmentFlags};
 use binaryninja::symbol::{SymbolBuilder, SymbolType};
 use binaryninja::types::{
     BaseStructure, EnumerationBuilder, MemberAccess, MemberScope, NamedTypeReference,
-    NamedTypeReferenceClass, StructureBuilder, StructureMember, StructureType, Type, TypeBuilder,
+    NamedTypeReferenceClass, StructureBuilder, StructureMember, Type, TypeBuilder,
 };
 use std::collections::HashMap;
 use std::num::NonZeroUsize;
@@ -442,53 +442,12 @@ impl DeviceMapper {
 
                 let type_builder = match &register.fields {
                     Some(fields) => {
-                        // Separate bitfields from regular fields.
-                        let (fields, bitfield_items): (Vec<_>, Vec<_>) =
-                            fields.iter().partition(|f| {
-                                byte_aligned(f.bit_range.width) && byte_aligned(f.bit_range.offset)
-                            });
-
                         for field in fields {
                             let field_member = self.field_member(field);
                             let overwrites = true; // TODO: Handle overwrites?
                             register_struct.insert_member(field_member, overwrites);
                         }
 
-                        if self.settings.add_bitfields {
-                            // The bitfield items need to be coalesced to a map of byte offset to vec of fields.
-                            let mut bitfield_map: HashMap<u64, Vec<&Field>> = HashMap::new();
-
-                            // Sort bitfields by their offset
-                            let mut sorted_bitfields = bitfield_items.iter().collect::<Vec<_>>();
-                            sorted_bitfields.sort_by_key(|f| f.bit_range.offset);
-
-                            // Group bitfields by overlapping bit offsets
-                            let mut current_bit_start = 0;
-                            let mut current_bit_end = 0;
-                            for field in sorted_bitfields {
-                                let bit_start = field.bit_range.offset;
-                                let byte_start = bit_start / 8;
-                                let current_byte_start = current_bit_start / 8;
-                                if current_byte_start != byte_start && current_bit_end < bit_start {
-                                    // Make a new bitfield, only if the current field is in a new byte.
-                                    current_bit_start = bit_start;
-                                }
-                                current_bit_end = bit_start + field.bit_range.width;
-                                bitfield_map
-                                    .entry(current_bit_start as u64)
-                                    .or_insert_with(Vec::new)
-                                    .push(field);
-                            }
-
-                            for (bit_start, fields) in bitfield_map {
-                                // Add each bitfield to the structure!
-                                let byte_start = bit_start / 8;
-                                let bitfield_member = self.bitfield_member(byte_start, fields);
-                                let overwrites = true; // TODO: Handle overwrites?
-                                register_struct.insert_member(bitfield_member, overwrites);
-                            }
-                        }
-
                         TypeBuilder::structure(&register_struct.finalize())
                     }
                     None if register.derived_from.is_some() => {
@@ -559,43 +518,14 @@ impl DeviceMapper {
         }
     }
 
-    pub fn bitfield_member(&self, byte_offset: u64, fields: Vec<&Field>) -> StructureMember {
-        let field_ty = self.bitfield_type(byte_offset, fields);
-        // TODO: Create bitfield name from the fields?
-        let field_name = format!("bitfield_0x{:x}", byte_offset);
-        // TODO: This should be like 120 confidence?
-        let conf_field_ty = Conf::new(field_ty, MAX_CONFIDENCE);
-        StructureMember::new(
-            conf_field_ty,
-            field_name,
-            byte_offset,
-            MemberAccess::PublicAccess,
-            MemberScope::NoScope,
-        )
-    }
-
-    pub fn bitfield_type(&self, byte_offset: u64, fields: Vec<&Field>) -> Ref<Type> {
-        let mut union_builder = StructureBuilder::new();
-        union_builder.structure_type(StructureType::UnionStructureType);
-        for field in fields {
-            let mut field_member = self.field_member(field);
-            // Field members are relative to the union member, so we must remove the union member offset
-            // from the field member offset to make it relative to the union member.
-            field_member.offset -= byte_offset;
-            let overwrites = false; // TODO: Handle overwrites?
-            union_builder.insert_member(field_member, overwrites);
-        }
-        Type::structure(&union_builder.finalize())
-    }
-
     pub fn field_member(&self, field: &Field) -> StructureMember {
         let field_ty = self.field_type(field);
         let conf_field_ty = Conf::new(field_ty, MAX_CONFIDENCE);
-        let byte_offset = field.bit_offset() / 8;
-        StructureMember::new(
+        StructureMember::new_bitfield(
             conf_field_ty,
             field.name.to_owned(),
-            byte_offset as u64,
+            field.bit_offset() as u64,
+            field.bit_width() as u8,
             MemberAccess::PublicAccess,
             MemberScope::NoScope,
         )
@@ -633,7 +563,6 @@ impl DeviceMapper {
     }
 
     pub fn single_field_int_type(&self, field: &FieldInfo) -> Ref<Type> {
-        // TODO: Binary Ninja is unable to handle bit fields, so we abuse unions.
         // Get the closest 8-bit aligned integer and use that.
         let width = field.bit_width();
         let byte_aligned_width = byte_width(width);