[SVD] Improvements to loading file

- Improve error messages to include what exactly caused the parsing to fail
- Relaxed the parsing validation to allow for more files to be parsed
- Added a form dialog to improve discoverability of load settings

We still need to relax optional fields failing to parse if they have no child, as that is considered an error, even with no validation.

1 files changed, 112 insertions, 10 deletions

diff --git a/plugins/svd/src/lib.rs b/plugins/svd/src/lib.rs
index 4a611e3d..3a069056 100644
--- a/plugins/svd/src/lib.rs
+++ b/plugins/svd/src/lib.rs
@@ -5,39 +5,139 @@ use crate::mapper::DeviceMapper;
 use crate::settings::LoadSettings;
 use binaryninja::binary_view::{BinaryView, BinaryViewBase, BinaryViewExt};
 use binaryninja::command::Command;
+use binaryninja::interaction::{Form, FormInputField};
 use binaryninja::logger::Logger;
 use binaryninja::workflow::{activity, Activity, AnalysisContext, Workflow};
 use log::LevelFilter;
+use std::path::PathBuf;
+use svd_parser::ValidateLevel;
+
+pub struct LoadFileField;
+
+impl LoadFileField {
+    pub fn field() -> FormInputField {
+        FormInputField::OpenFileName {
+            prompt: "File Path".to_string(),
+            // TODO: This is called extension but is really a filter.
+            extension: Some("*.svd".to_string()),
+            default: None,
+            value: None,
+        }
+    }
+
+    pub fn from_form(form: &Form) -> Option<PathBuf> {
+        let field = form.get_field_with_name("File Path")?;
+        let field_value = field.try_value_string()?;
+        Some(PathBuf::from(field_value))
+    }
+}
+
+pub struct AddCommentsField;
+
+impl AddCommentsField {
+    pub fn field(default: bool) -> FormInputField {
+        FormInputField::Checkbox {
+            prompt: "Add Comments".to_string(),
+            default: Some(default),
+            value: false,
+        }
+    }
+
+    pub fn from_form(form: &Form) -> Option<bool> {
+        let field = form.get_field_with_name("Add Comments")?;
+        let field_value = field.try_value_int()?;
+        match field_value {
+            1 => Some(true),
+            _ => Some(false),
+        }
+    }
+}
+
+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<bool> {
+        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 {
+    pub fn field(default: bool) -> FormInputField {
+        FormInputField::Checkbox {
+            prompt: "Add Memory Regions".to_string(),
+            default: Some(default),
+            value: false,
+        }
+    }
+
+    pub fn from_form(form: &Form) -> Option<bool> {
+        let field = form.get_field_with_name("Add Memory Regions")?;
+        let field_value = field.try_value_int()?;
+        match field_value {
+            1 => Some(true),
+            _ => Some(false),
+        }
+    }
+}
 
 struct LoadSVDFile;
 
 impl Command for LoadSVDFile {
     fn action(&self, view: &BinaryView) {
-        let Some(file) =
-            binaryninja::interaction::get_open_filename_input("Select a .svd file", "*.svd")
-        else {
+        let mut form = Form::new("Load SVD File");
+        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,
+        ));
+        if !form.prompt() {
+            return;
+        }
+        let Some(file_path) = LoadFileField::from_form(&form) else {
             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) {
+        let file_content = match std::fs::read_to_string(&file_path) {
             Ok(content) => content,
             Err(e) => {
-                log::error!("Failed to read file: {}", e);
+                log::error!("Failed to read file: {:?}", e);
                 return;
             }
         };
 
-        match svd_parser::parse(&file_content) {
+        // Disabling validation since vendors are lazy and don't follow the spec.
+        let mut config = svd_parser::Config::default();
+        config.validate_level = ValidateLevel::Disabled;
+        match svd_parser::parse_with_config(&file_content, &config) {
             Ok(device) => {
                 // We have a supported svd device. map it!
-                let load_settings = LoadSettings::from_view_settings(view);
                 let address_size = view.address_size();
                 let mapper = DeviceMapper::new(load_settings, address_size, device);
                 mapper.map_to_view(view);
                 view.update_analysis();
             }
             Err(e) => {
-                log::error!("Failed to parse SVD file: {}", e);
+                log::error!("Failed to parse SVD file: {:?}", e);
             }
         }
     }
@@ -95,14 +195,16 @@ fn plugin_init() -> Result<(), ()> {
                 return;
             }
         };
-        match svd_parser::parse(&file_content) {
+        let mut config = svd_parser::Config::default();
+        config.validate_level = ValidateLevel::Disabled;
+        match svd_parser::parse_with_config(&file_content, &config) {
             Ok(device) => {
                 let address_size = view.address_size();
                 let mapper = DeviceMapper::new(load_settings, address_size, device);
                 mapper.map_to_view(&view);
             }
             Err(e) => {
-                log::error!("Failed to parse SVD file: {}", e);
+                log::error!("Failed to parse SVD file: {:?}", e);
             }
         }
     };