Skip to content

[#1481] byte-wise atomic wrapper to prevent UB#1577

Open
FerdinandSpitzschnueffler wants to merge 23 commits intoeclipse-iceoryx:mainfrom
ekxide:iox2-1481-atomic-memcpy
Open

[#1481] byte-wise atomic wrapper to prevent UB#1577
FerdinandSpitzschnueffler wants to merge 23 commits intoeclipse-iceoryx:mainfrom
ekxide:iox2-1481-atomic-memcpy

Conversation

@FerdinandSpitzschnueffler
Copy link
Copy Markdown
Contributor

Notes for Reviewer

To handle padding bytes in the wrapper ByteAtomic, the new trait AtomicCopy was introduced. It basically provides a method to apply a callback to each offset-size pair of every field of a type.

Pre-Review Checklist for the PR Author

  • Add sensible notes for the reviewer
  • PR title is short, expressive and meaningful
  • Consider switching the PR to a draft (Convert to draft)
    • as draft PR, the CI will be skipped for pushes
  • Relevant issues are linked in the References section
  • Branch follows the naming format (iox2-123-introduce-posix-ipc-example)
  • Commits messages are according to this guideline
    • Commit messages have the issue ID ([#123] Add posix ipc example)
    • Keep in mind to use the same email that was used to sign the Eclipse Contributor Agreement
  • Tests follow the best practice for testing
  • Changelog updated in the unreleased section including API breaking changes

PR Reviewer Reminders

  • Commits are properly organized and messages are according to the guideline
  • Unit tests have been written for new behavior
  • Public API is documented
  • PR title describes the changes

References

Closes #1481

Rename __for_each_field, add documentation, replace std::thread/barrier
in tests and add miri related todo, remove unnecessary dependencies
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes to this file were made because the tests didn't compile for reasons other than those stated in the test names.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

❌ Patch coverage is 49.36709% with 80 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.59%. Comparing base (dc1eea2) to head (fbbb874).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
iceoryx2-bb/derive-macros/src/lib.rs 0.00% 58 Missing ⚠️
iceoryx2-bb/elementary-traits/src/atomic_copy.rs 52.17% 22 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1577      +/-   ##
==========================================
- Coverage   77.70%   77.59%   -0.11%     
==========================================
  Files         419      421       +2     
  Lines       40284    40442     +158     
  Branches     1297     1297              
==========================================
+ Hits        31302    31381      +79     
- Misses       7928     8007      +79     
  Partials     1054     1054              
Flag Coverage Δ
CPP 68.90% <ø> (ø)
Rust 77.27% <49.36%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
iceoryx2-bb/container/src/byte_atomic.rs 100.00% <100.00%> (ø)
iceoryx2-bb/container/src/string/static_string.rs 85.55% <100.00%> (+1.03%) ⬆️
...ceoryx2-bb/elementary-traits/src/zero_copy_send.rs 75.00% <ø> (ø)
iceoryx2-bb/elementary-traits/src/atomic_copy.rs 52.17% <52.17%> (ø)
iceoryx2-bb/derive-macros/src/lib.rs 0.00% <0.00%> (ø)

... and 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

/// * See Safety section of [`crate::zero_copy_send::ZeroCopySend`].
/// * Implementations of this trait must ensure that the offset and size of each field are
/// calculated correctly. Otherwise, undefined behavior may occur.
pub unsafe trait AtomicCopy: ZeroCopySend + Copy + 'static {
Copy link
Copy Markdown
Contributor

@elfenpiff elfenpiff Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think ZeroCopySend is strictly speaking not needed for AtomicCopy. First of all, it does not have any dependencies to ZeroCopySend or things that trait provides and AtomicCopy does not require #[repr(C)]. It could be used also in just one process and the member ordering can be anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement a byte-wise atomic wrapper to prevent UB

2 participants