Remove the read_byte_loop_raw function, because it is unsound.

See #12

Author: Cryptjar

CHANGELOG.md

@@ -8,6 +8,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 
 
+## Unreleased
+
+### Internal Changes
+
+- Removed `read_byte_loop_raw` function, because it is unsound. Thanks to [workingjubilee](https://github.com/workingjubilee) for pointing this out. (https://github.com/Cryptjar/avr-progmem-rs/issues/12)
+- Made the `lpm-asm-loop` crate feature obsolete, because the lpm assembly loop is now the only implementation.
+
+
 
 ## [0.3.2] - 2023-01-22
 [0.3.2]: https://github.com/Cryptjar/avr-progmem-rs/compare/v0.3.1...v0.3.2
@@ -62,7 +70,7 @@ Changes since `v0.2.0`.
 - Deny storing a reference in progmem (i.e. a direct `&T`) via the `progmem` macro, this should catch some common mistakes.
 - Deny storing a `LoadedString` directly in progmem via the `progmem` macro, use the special `string` rule instead.
 
-### Internal changes
+### Internal Changes
 
 - Migrate from `llvm_asm` to `asm`.
 - Use the `addr_of` macro to never ever have a reference to progmem, just raw pointers.
@@ -91,7 +99,7 @@ Changes since `v0.1.5`.
 - Emit a warning when just storing a reference in progmem instead of the actual data via the `progmem` macro.
 - Add an [`PmIter::iter`] method to lazily iterate through an array (loading only one element at a time).
 
-### Internal changes
+### Internal Changes
 
 - Change `ProgMem` from a direct value wrapper into a pointer wrapper, thus no more references into program memory are kept only raw pointers (the accessible `static`s containing the `ProgMem` struct are in RAM, the data is stored in hidden `static`s in progmem).
 - Patch `ufmt` to fix u32 formatting (https://github.com/Cryptjar/avr-progmem-rs/commit/9d351038fc31d769206b29cd7228b35aa457b518)
@@ -138,7 +146,7 @@ Changes since `v0.1.2`.
 
 - Deprecate [`read_slice`] function, because it is based on passing around plain slices (aka a reference) to program memory, which is extremely hazardous, if not **UB**, and thus will not be supported in the future.
 
-### Internal changes
+### Internal Changes
 
 - Pin the Rust toolchain to `nightly-2021-01-07`, because at the time of writing it is the latest Rust version that supports AVR (future version are broken, also see <https://github.com/rust-lang/compiler-builtins/issues/400>).
 - Add Github CI.
@@ -167,7 +175,7 @@ Changes since `v0.1.0`.
 
 - Use the `.progmem.data` section instead of just `.progmem` to keep compatibility with `avr-binutils >= 2.30` by [@Rahix](https://github.com/Rahix) (https://github.com/Cryptjar/avr-progmem-rs/pull/2)
 
-### Internal changes
+### Internal Changes
 
 - Setup a cargo config to target the Arduino Uno by default (instead of the host), and allow to run the examples directly via `cargo run` by [@Rahix](https://github.com/Rahix) (https://github.com/Cryptjar/avr-progmem-rs/pull/2)
 

Cargo.toml

@@ -30,9 +30,11 @@ opt-level = "s"
 
 [features]
 default = ["lpm-asm-loop", "ufmt"]
-# Enables the use of the `lpm r0, Z+` in an assembly loop, instead of the plain
-# `lpm` instruction with a Rust loop.
+
+# Deprecated, the assembly loop is now the only implementation. Enabling
+# (or disabling) this feature makes no difference, anymore.
 lpm-asm-loop = []
+
 # Enables some tweak to ease debugging, should not be use in production
 dev = []
 # Enables unsize utilities, such as wrapper coercing.

src/raw.rs

@@ -125,63 +125,6 @@ pub unsafe fn read_byte(p_addr: *const u8) -> u8 {
 	}
 }
 
-/// Read an array of type `T` from progmem into data array.
-///
-/// This function uses the above byte-wise `read_byte` function instead
-/// of the looped assembly of `read_asm_loop_raw`.
-///
-///
-/// # Safety
-///
-/// This call is analog to `core::ptr::copy(p_addr, out, len as usize)` thus it
-/// has the same basic requirements such as both pointers must be valid for
-/// dereferencing i.e. not dangling and both pointers must
-/// be valid to read or write, respectively, of `len` many elements of type `T`,
-/// i.e. `len * size_of::<T>()` bytes.
-///
-/// Additionally, `p_addr` must be a valid pointer into the program memory
-/// domain. And `out` must be valid point to a writable location in the data
-/// memory.
-///
-/// However alignment is not strictly required for AVR, since the read/write is
-/// done byte-wise.
-///
-#[allow(dead_code)]
-unsafe fn read_byte_loop_raw<T>(p_addr: *const T, out: *mut T, len: u8)
-where
-	T: Sized + Copy,
-{
-	// Convert to byte pointers
-	let p_addr_bytes = p_addr as *const u8;
-	let out_bytes = out as *mut u8;
-
-	// Get size in bytes of T
-	let size_type = size_of::<T>();
-	// Must not exceed 256 byte
-	assert!(size_type <= u8::MAX as usize);
-
-	// Multiply with the given length
-	let size_bytes = size_type * len as usize;
-	// Must still not exceed 256 byte
-	assert!(size_bytes <= u8::MAX as usize);
-	// Now its fine to cast down to u8
-	let size_bytes = size_bytes as u8;
-
-	for i in 0..size_bytes {
-		let i: isize = i.into();
-
-		let value = unsafe {
-			// SAFETY: The caller must ensure that `p_addr` points into the
-			// program domain.
-			read_byte(p_addr_bytes.offset(i))
-		};
-		unsafe {
-			// SAFETY: The caller must ensure that `size_bytes` of `out` are
-			// to write to (data-domain) and are valid for `T`.
-			out_bytes.offset(i).write(value);
-		}
-	}
-}
 
 
 /// Read an array of type `T` from progmem into data array.
@@ -347,20 +290,10 @@ unsafe fn read_value_raw<T>(p_addr: *const T, out: *mut T, len: u8)
 where
 	T: Sized + Copy,
 {
-	cfg_if! {
-		if #[cfg(feature = "lpm-asm-loop")] {
-			unsafe {
-				// SAFETY: The caller must ensuer the validity of the pointers
-				// and their domains.
-				read_asm_loop_raw(p_addr, out, len)
-			}
-		} else {
-			unsafe {
-				// SAFETY: The caller must ensuer the validity of the pointers
-				// and their domains.
-				read_byte_loop_raw(p_addr, out, len)
-			}
-		}
+	unsafe {
+		// SAFETY: The caller must ensure the validity of the pointers
+		// and their domains.
+		read_asm_loop_raw(p_addr, out, len)
 	}
 }