M-Labs/zynq-rs
11
0
Fork 16
Code Issues 6 Pull Requests Releases Wiki Activity

libcortex_a9: fix UB in UncachedSlice #121

Merged
sb10q merged 1 commits from srenblad/zynq-rs:fix_uncached into master 2025-02-07 21:24:35 +08:00
Conversation 0 Commits 1 Files Changed 1 +1 -4
srenblad commented 2025-02-07 13:54:57 +08:00
Contributor
Copy Link

Clean up UB and unnecessary asserts in UncachedSlice

  • Added assertion on allocation failure. The global allocator returns a nullptr on allocation error, so check it here.

  • Removed redundant alignment check. align is always a multiple of L1_PAGE_SIZE. If align_of::<T>() > L1_PAGE_SIZE it will still be a multiple (all aligns are powers of 2). start will be a multiple of k * L1_PAGE_SIZE and thus a multiple of L1_PAGE_SIZE. If there is no allocation/layout error, this is always true.

  • Removed UB during "size verification". From the docs, slice.get_unchecked(len) is always UB. This is not a problem at the moment, but results in a data abort in later rustc versions. It is also a redundant assertion: len * size_of::<T>() <= size is established from line 15, and thus (if the allocation is successful) the slice will be contained inside the allocation object (assertion is always true).

Tested all HITL with Kasli-SoC demo variant from master branches: artiq-zynq and artiq. No noticeable performance regressions.

### Clean up UB and unnecessary asserts in `UncachedSlice` + Added assertion on allocation failure. The global allocator returns a `nullptr` on allocation error, so check it here. + Removed redundant alignment check. `align` is always a multiple of `L1_PAGE_SIZE`. If `align_of::<T>() > L1_PAGE_SIZE` it will still be a multiple (all aligns are powers of 2). `start` will be a multiple of `k * L1_PAGE_SIZE` and thus a multiple of `L1_PAGE_SIZE`. If there is no allocation/layout error, this is always true. + Removed UB during "size verification". From the [docs](https://doc.rust-lang.org/std/primitive.slice.html#method.get_unchecked), `slice.get_unchecked(len)` is always UB. This is not a problem at the moment, but results in a data abort in later `rustc` versions. It is also a redundant assertion: `len * size_of::<T>() <= size` is established from line 15, and thus (if the allocation is successful) the slice will be contained inside the allocation object (assertion is always true). Tested all HITL with Kasli-SoC demo variant from master branches: [artiq-zynq](https://git.m-labs.hk/M-Labs/artiq-zynq/commit/4a2b28dcc38b88766364d557d19e4bbefa5580cd) and [artiq](https://github.com/m-labs/artiq/commit/c8d0ab9afe8eab6fec0b22e80f263ab2e359bb95). No noticeable performance regressions.
srenblad added 1 commit 2025-02-07 13:54:57 +08:00
sb10q merged commit af42d9b819 into master 2025-02-07 21:24:35 +08:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/zynq-rs#121
No description provided.
Delete Branch "srenblad/zynq-rs:fix_uncached"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?