From 8c0c59e937bfa95551f35e62fafb7d6614f7e886 Mon Sep 17 00:00:00 2001 From: Mason Reed Date: Sat, 28 Mar 2026 15:07:27 -0700 Subject: [PATCH 1/2] [SVD Import] Remove "add bitfield" setting We unconditionally add them now that bitfields are represented correctly in the type system --- plugins/svd/src/lib.rs | 23 ----------------------- plugins/svd/src/settings.rs | 19 ------------------- 2 files changed, 42 deletions(-) diff --git a/plugins/svd/src/lib.rs b/plugins/svd/src/lib.rs index 6ada01f9fb..1eb84a5f1c 100644 --- a/plugins/svd/src/lib.rs +++ b/plugins/svd/src/lib.rs @@ -51,27 +51,6 @@ impl AddCommentsField { } } -pub struct AddBitfieldsField; - -impl AddBitfieldsField { - pub fn field(default: bool) -> FormInputField { - FormInputField::Checkbox { - prompt: "Add Bitfields".to_string(), - default: Some(default), - value: false, - } - } - - pub fn from_form(form: &Form) -> Option { - let field = form.get_field_with_name("Add Bitfields")?; - let field_value = field.try_value_int()?; - match field_value { - 1 => Some(true), - _ => Some(false), - } - } -} - pub struct AddMemoryRegionsField; impl AddMemoryRegionsField { @@ -101,7 +80,6 @@ impl Command for LoadSVDFile { let mut load_settings = LoadSettings::from_view_settings(view); form.add_field(LoadFileField::field()); form.add_field(AddCommentsField::field(load_settings.add_comments)); - form.add_field(AddBitfieldsField::field(load_settings.add_bitfields)); form.add_field(AddMemoryRegionsField::field( load_settings.add_backing_regions, )); @@ -112,7 +90,6 @@ impl Command for LoadSVDFile { return; }; load_settings.add_comments = AddCommentsField::from_form(&form).unwrap_or(true); - load_settings.add_bitfields = AddBitfieldsField::from_form(&form).unwrap_or(true); load_settings.add_backing_regions = AddMemoryRegionsField::from_form(&form).unwrap_or(true); let file_content = match std::fs::read_to_string(&file_path) { diff --git a/plugins/svd/src/settings.rs b/plugins/svd/src/settings.rs index fb2f680c96..7f57f0bfe4 100644 --- a/plugins/svd/src/settings.rs +++ b/plugins/svd/src/settings.rs @@ -6,7 +6,6 @@ use std::path::PathBuf; #[derive(Debug, Clone)] pub struct LoadSettings { pub add_backing_regions: bool, - pub add_bitfields: bool, pub add_comments: bool, pub auto_load_file: Option, } @@ -14,8 +13,6 @@ pub struct LoadSettings { impl LoadSettings { pub const ADD_BACKING_REGIONS_DEFAULT: bool = true; pub const ADD_BACKING_REGIONS_SETTING: &'static str = "analysis.svd.addBackingRegions"; - pub const ADD_BITFIELDS_DEFAULT: bool = true; - pub const ADD_BITFIELDS_SETTING: &'static str = "analysis.svd.addBitfields"; pub const ADD_COMMENTS_DEFAULT: bool = true; pub const ADD_COMMENTS_SETTING: &'static str = "analysis.svd.addComments"; pub const AUTO_LOAD_FILE_DEFAULT: &'static str = ""; @@ -35,17 +32,6 @@ impl LoadSettings { &add_backing_region_props.to_string(), ); - let add_bitfields_props = json!({ - "title" : "Add Bitfields", - "type" : "boolean", - "default" : Self::ADD_BITFIELDS_DEFAULT, - "description" : "Whether to add bitfields. Bitfields are not supported by Binary Ninja, so this is a workaround using unions.", - }); - bn_settings.register_setting_json( - Self::ADD_BITFIELDS_SETTING, - &add_bitfields_props.to_string(), - ); - let add_comments_props = json!({ "title" : "Add Comments", "type" : "boolean", @@ -73,10 +59,6 @@ impl LoadSettings { load_settings.add_backing_regions = settings.get_bool_with_opts(Self::ADD_BACKING_REGIONS_SETTING, &mut query_opts); } - if settings.contains(Self::ADD_BITFIELDS_SETTING) { - load_settings.add_bitfields = - settings.get_bool_with_opts(Self::ADD_BITFIELDS_SETTING, &mut query_opts); - } if settings.contains(Self::ADD_COMMENTS_SETTING) { load_settings.add_comments = settings.get_bool_with_opts(Self::ADD_COMMENTS_SETTING, &mut query_opts); @@ -97,7 +79,6 @@ impl Default for LoadSettings { fn default() -> Self { Self { add_backing_regions: Self::ADD_BACKING_REGIONS_DEFAULT, - add_bitfields: Self::ADD_BITFIELDS_DEFAULT, add_comments: Self::ADD_COMMENTS_DEFAULT, auto_load_file: None, } From 87934d70bb95c984da05852e683c671c5b7b5c0e Mon Sep 17 00:00:00 2001 From: Mason Reed Date: Fri, 27 Mar 2026 13:05:02 -0700 Subject: [PATCH 2/2] [SVD Import] Fix misc mapper issues - Fix unordered register fields causing spurious __offset fields to be rendered for the structure - Fix registers overlayed onto an alternate register not being treated as a union Fixes https://github.com/Vector35/binaryninja-api/issues/7918 --- plugins/svd/src/mapper.rs | 60 +++++++++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/plugins/svd/src/mapper.rs b/plugins/svd/src/mapper.rs index 8f4abe47b3..1ba7f61c34 100644 --- a/plugins/svd/src/mapper.rs +++ b/plugins/svd/src/mapper.rs @@ -8,9 +8,9 @@ use binaryninja::segment::{SegmentBuilder, SegmentFlags}; use binaryninja::symbol::{SymbolBuilder, SymbolType}; use binaryninja::types::{ BaseStructure, EnumerationBuilder, MemberAccess, MemberScope, NamedTypeReference, - NamedTypeReferenceClass, StructureBuilder, StructureMember, Type, TypeBuilder, + NamedTypeReferenceClass, StructureBuilder, StructureMember, StructureType, Type, TypeBuilder, }; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::num::NonZeroUsize; use svd_parser::svd::{ Access, AddressBlock, AddressBlockUsage, DataType, Device, EnumeratedValues, Field, FieldInfo, @@ -373,22 +373,55 @@ impl DeviceMapper { peripheral_struct.width(address_block.size as u64); if let Some(register_clusters) = &peripheral.registers { + // Collect registers by offset so we can handle overlapping registers by creating a union. + let mut registers_by_offset: BTreeMap> = BTreeMap::new(); for register_cluster in register_clusters { match register_cluster { RegisterCluster::Register(register) => { - let mut register_member = self.register_member(register); - // TODO: If we want registers to be relative to the peripheral than we must - // TODO: assert that we create the peripheral type at offset 0 from the base address. - // Make the register member relative to the address block, not the peripheral. - register_member.offset -= address_block.offset as u64; - let overwrite = false; // TODO: Handle overwrites? - peripheral_struct.insert_member(register_member, overwrite); + registers_by_offset + .entry(register.address_offset as u64) + .or_default() + .push(register); } RegisterCluster::Cluster(_cluster) => { // TODO: Support clusters } } } + + for (offset, registers) in registers_by_offset { + match registers.as_slice() { + [register] => { + // We only have one register at this offset, just insert it. + let mut register_member = self.register_member(register); + register_member.offset -= address_block.offset as u64; + peripheral_struct.insert_member(register_member, false); + } + _ => { + // We have multiple registers at the same offset, create a union of them. + // This happens typically when there is some mode field that changes the + // behavior of the register region. + // NOTE: Typically overlapping registers are specified with an alternate register. + let mut union_builder = StructureBuilder::new(); + union_builder.structure_type(StructureType::UnionStructureType); + for register in registers { + let mut register_member = self.register_member(register); + register_member.offset = 0; + union_builder.insert_member(register_member, false); + } + + let union_ty = Type::structure(&union_builder.finalize()); + let union_member = StructureMember::new( + Conf::new(union_ty, MAX_CONFIDENCE), + "".to_string(), + offset - address_block.offset as u64, + MemberAccess::PublicAccess, + MemberScope::NoScope, + ); + peripheral_struct.insert_member(union_member, false); + } + } + } } Type::structure(&peripheral_struct.finalize()) @@ -440,10 +473,13 @@ impl DeviceMapper { register_struct.base_structures(&[base_struct]); } - let type_builder = match ®ister.fields { - Some(fields) => { + let type_builder = match register.fields.clone() { + Some(mut fields) => { + // Order the fields by offset so that rendering of the structure will not + // insert "offset" fields, which happens when fields are unordered. + fields.sort_by(|a, b| a.bit_range.offset.cmp(&b.bit_range.offset)); for field in fields { - let field_member = self.field_member(field); + let field_member = self.field_member(&field); let overwrites = true; // TODO: Handle overwrites? register_struct.insert_member(field_member, overwrites); }