-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RegisterBlocks are not #[repr(C)] #13
Comments
@adamgreig might be interested in this one That does seem like a potential issue to me as well. The docs do seem clear that field ordering and alignment are not guaranteed, though go into more details about some of things that repr(Rust) does. It seems like if all Register types are 4 bytes, we are just hitting that luckily it works scenario. I can say the clocking works fine on my evk board as I've tested that, granted that may not indicate if there are other issues. That all said I think repr(C, align(4)) perhaps makes sense here to ensure field order and alignment rules are always obeyed the way we want. Just to make it clearer to ourselves and the compiler, no we really want fields in this memory order and of this alignment. |
Thanks @mciantyre, you're quite right, they should be |
Without repr(C) the fields may be re-ordered and alignment is whatever rustc decides is best. repr(C) enforced field ordering and C alignment rules. Fixes #13
Without repr(C) the fields may be re-ordered and alignment is whatever rustc decides is best. repr(C) enforced field ordering and C alignment rules. Fixes #13
Without repr(C) the fields may be re-ordered and alignment is whatever rustc decides is best. repr(C) enforced field ordering and C alignment rules. Fixes #13
Caveat: I've still not run anything on hardware 😛
I'm studying the RAL API, and I noticed that none of the peripheral
RegisterBlock
s are#[repr(C)]
. Here's a snippet of the CCM peripheral:The block registers are written in order, according to the SVD and the processor reference manual. Its address is
0x400F_C000
.Given the
RWRegister
API, a hypothetical write to the CCSR register could resembleThe write will cast the the memory at
0x400F_C000
to aRegisterBlock
, offset the pointer it by the number of registers between0x400F_C000
and CCSR (expected to be the absolute address0x400F_C00C
), then store0xDEAD_BEEF
at that address.Because the
RegisterBlock
is not#[repr(C)]
, we cannot guarantee it's layout, since Rust does not guarantee struct layout. Therefore, the store of0xDEAD_BEEF
might not necessarily happen on the actual CCSR register, address0x400F_C00C
; it could happen on another memory address between0x400F_C000
and0x400F_C000 + core::mem::size_of::<RegisterBlock>()
I think this is a bug, or could lead to a bug. It applies to all
RegisterBlock
s inimxrt-ral
. It may also apply to thestm32ral
crate, from which we've derived the RAL generation script. A study of thestm32ral
crate also reveals no usage of#[repr(C)]
. A fix could be to specify#[repr(C)]
in the RAL generation script, then re-generate the RAL crate.What do we think? Let me know if I'm missing something that makes this a non-issue!
The text was updated successfully, but these errors were encountered: