search

adamgreig / stm32ral

Register access layer in Rust for all STM32 microcontrollers
Apache License 2.0
34 stars 7 forks source link

Ensure alignment when computing reserved registers #17

Closed mciantyre closed 2 years ago

mciantyre commented 2 years ago

If the routine for computing reserved peripheral memory places at least one u32 after a u16 register, the compiler could insert an extra u16 of padding to align the u32 reservation. The extra padding is not considered by the routine. This can result in incorrect register offsets for all registers placed after the reservation.

This PR ensures that we only use larger-than-byte padding when the address is aligned for that primitive. If the address isn't aligned, use the next-largest primitive. Byte array reservations should be the safest approach, but going that route changes all _reserved members of all register blocks, making it harder to see what peripherals were affected by this issue.

When I run make, I'm generating unrelated changes throughout the source tree, so I'm only checking in the script changes and the changes & tests for the TIM2 peripherals. There might be other instances of this issue that this PR does not address.

adamgreig commented 2 years ago

Oh, thanks for reporting this! I guess we could use #[repr(C, packed)] to ensure no padding is added, but since we're already putting in the reserved fields it probably does make more sense to leave it as #[repr(C)] and just ensure no padding ends up required (and hopefully all registers are aligned...).

It might be nicer, having done it this way to see what's affected, to nevertheless go with byte arrays for all padding before the next release, what do you think?

mciantyre commented 2 years ago

As long as large diffs are no matter, I agree that byte array reservations are nicest. Latest commit shows what that might look like.

adamgreig commented 2 years ago

This looks fine to me! I'd suggest you just commit the change to the python script and I'll then regenerate all the actual Rust before release. Probably that little bit of the Python could be made simpler now it's only ever generating u8 arrays, too.