Fix: firwmare start address assumptions

src/bmp.rs

@@ -3,16 +3,13 @@
 // SPDX-FileContributor: Written by Mikaela Szekely <mikaela.szekely@qyriad.me>
 // SPDX-FileContributor: Modified by Rachel Mant <git@dragonmux.network>
 
-use std::array::TryFromSliceError;
 use std::cell::{Ref, RefCell};
 use std::fmt::{self, Debug, Display, Formatter};
 use std::io::Read;
 use std::thread;
 use std::time::{Duration, Instant};
 
-use clap::ValueEnum;
-use clap::builder::PossibleValue;
-use color_eyre::eyre::{Context, Error, OptionExt, Result, eyre};
+use color_eyre::eyre::{Context, Error, OptionExt, Result};
 use dfu_core::{DfuIo, DfuProtocol, Error as DfuCoreError, State as DfuState};
 use dfu_nusb::{DfuNusb, DfuSync, Error as DfuNusbError};
 use log::{debug, error, trace, warn};
@@ -22,6 +19,9 @@ use nusb::{Device, DeviceInfo, Interface};
 
 pub use crate::bmp_matcher::BmpMatcher;
 use crate::error::ErrorKind;
+use crate::firmware_file::FirmwareFile;
+// XXX: Ideally this shouldn't be pub here, but that's a breaking change.
+pub use crate::firmware_type::FirmwareType;
 use crate::probe_identity::ProbeIdentity;
 use crate::serial::bmd_rsp::BmdRspInterface;
 use crate::serial::gdb_rsp::GdbRspInterface;
@@ -497,23 +497,24 @@ impl BmpDevice
 	///
 	/// `progress` is a callback of the form `fn(just_written: usize)`, for callers to keep track of
 	/// the flashing process.
-	pub fn download<'r, R, P>(
+	pub fn download<P>(
 		&mut self,
-		firmware: &'r R,
-		length: u32,
+		firmware_file: &FirmwareFile,
 		firmware_type: FirmwareType,
 		progress: P,
 	) -> Result<DfuSync>
 	where
-		&'r R: Read,
-		R: ?Sized,
 		P: Fn(usize) + 'static,
 	{
 		if self.mode == DfuOperatingMode::Runtime {
 			self.detach_and_enumerate().wrap_err("detaching device for download")?;
 		}
 
-		let load_address = self.platform.load_address(firmware_type);
+		// Extract the file's load address, and if that isn't a thing, fall back on
+		// more crude methods based on the platform we're downloading to
+		let load_address = firmware_file
+			.load_address()
+			.unwrap_or_else(|| self.platform.load_address(firmware_type));
 
 		let dfu_dev = DfuNusb::open(
 			self.device.take().expect("Must have a valid device handle"),
@@ -534,7 +535,7 @@ impl BmpDevice
 
 		debug!("Load address: 0x{:08x}", load_address);
 
-		match self.try_download(firmware, length, &mut dfu_iface) {
+		match self.try_download(firmware_file.data(), firmware_file.len(), &mut dfu_iface) {
 			Err(error) => {
 				if let Some(DfuNusbError::Dfu(DfuCoreError::StateError(DfuState::DfuError))) =
 					error.downcast_ref::<DfuNusbError>()
@@ -558,7 +559,7 @@ impl BmpDevice
 						.unwrap()
 						.control_out_blocking(request, &[], Duration::from_secs(2))?;
 
-					self.try_download(firmware, length, &mut dfu_iface)
+					self.try_download(firmware_file.data(), firmware_file.len(), &mut dfu_iface)
 				} else {
 					Err(error)
 				}
@@ -631,164 +632,6 @@ impl Display for BmpDevice
 	}
 }
 
-/// Represents a conceptual Vector Table for Armv7 processors.
-pub struct Armv7mVectorTable<'b>
-{
-	bytes: &'b [u8],
-}
-
-impl<'b> Armv7mVectorTable<'b>
-{
-	fn word(&self, index: usize) -> Result<u32, TryFromSliceError>
-	{
-		let start = index * 4;
-		let array: [u8; 4] = self.bytes[(start)..(start + 4)].try_into()?;
-
-		Ok(u32::from_le_bytes(array))
-	}
-
-	/// Construct a conceptual Armv7m Vector Table from a bytes slice.
-	pub fn from_bytes(bytes: &'b [u8]) -> Self
-	{
-		if bytes.len() < (4 * 2) {
-			panic!("Data passed is not long enough for an Armv7m Vector Table!");
-		}
-
-		Self {
-			bytes,
-		}
-	}
-
-	pub fn stack_pointer(&self) -> Result<u32, TryFromSliceError>
-	{
-		self.word(0)
-	}
-
-	pub fn reset_vector(&self) -> Result<u32, TryFromSliceError>
-	{
-		self.word(1)
-	}
-
-	pub fn exception(&self, exception_number: u32) -> Result<u32, TryFromSliceError>
-	{
-		self.word((exception_number + 1) as usize)
-	}
-}
-
-/// Firmware types for the Black Magic Probe.
-#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
-pub enum FirmwareType
-{
-	/// The bootloader. For native probes this is linked at 0x0800_0000
-	Bootloader,
-	/// The main application. For native probes this is linked at 0x0800_2000.
-	Application,
-}
-
-impl FirmwareType
-{
-	/// Detect the kind of firmware from the given binary by examining its reset vector address.
-	///
-	/// This function panics if `firmware.len() < 8`.
-	pub fn detect_from_firmware(platform: BmpPlatform, firmware: &[u8]) -> Result<Self>
-	{
-		let buffer = &firmware[0..(4 * 2)];
-
-		let vector_table = Armv7mVectorTable::from_bytes(buffer);
-		let reset_vector = vector_table
-			.reset_vector()
-			.wrap_err("Firmware file does not seem valid: vector table too short")?;
-
-		debug!("Detected reset vector in firmware file: 0x{:08x}", reset_vector);
-
-		// Sanity check.
-		if (reset_vector & 0x0800_0000) != 0x0800_0000 {
-			return Err(eyre!(
-				"Firmware file does not seem valid: reset vector address seems to be outside of reasonable bounds - \
-				 0x{:08x}",
-				reset_vector
-			));
-		}
-
-		let app_start = platform.load_address(Self::Application);
-
-		if reset_vector > app_start {
-			Ok(Self::Application)
-		} else {
-			Ok(Self::Bootloader)
-		}
-	}
-}
-
-impl Display for FirmwareType
-{
-	fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result
-	{
-		match self {
-			Self::Bootloader => write!(f, "bootloader")?,
-			Self::Application => write!(f, "application")?,
-		};
-
-		Ok(())
-	}
-}
-
-/// Defaults to [`FirmwareType::Application`].
-impl Default for FirmwareType
-{
-	/// Defaults to [`FirmwareType::Application`].
-	fn default() -> Self
-	{
-		FirmwareType::Application
-	}
-}
-
-impl ValueEnum for FirmwareType
-{
-	fn value_variants<'a>() -> &'a [Self]
-	{
-		&[Self::Application, Self::Bootloader]
-	}
-
-	fn to_possible_value(&self) -> Option<PossibleValue>
-	{
-		match self {
-			Self::Bootloader => Some("bootloader".into()),
-			Self::Application => Some("application".into()),
-		}
-	}
-}
-
-/// File formats that Black Magic Probe firmware can be in.
-pub enum FirmwareFormat
-{
-	/// Raw binary format. Made with `objcopy -O binary`. Typical file extension: `.bin`.
-	Binary,
-
-	/// The Unix ELF executable binary format. Typical file extension: `.elf`.
-	Elf,
-
-	/// Intel HEX. Typical file extensions: `.hex`, `.ihex`.
-	IntelHex,
-}
-
-impl FirmwareFormat
-{
-	/// Detect the kind of firmware from its data.
-	///
-	/// Panics if `firmware.len() < 4`.
-	pub fn detect_from_firmware(firmware: &[u8]) -> Self
-	{
-		if &firmware[0..4] == b"\x7fELF" {
-			FirmwareFormat::Elf
-		} else if &firmware[0..1] == b":" {
-			FirmwareFormat::IntelHex
-		} else {
-			FirmwareFormat::Binary
-		}
-	}
-}
-
 /// Waits for a Black Magic Probe to reboot, erroring after a timeout.
 ///
 /// This function takes a port identifier to attempt to keep track of a single physical device