Skip to content
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

analyze: memcpy rewrite improvements #1176

Merged
merged 8 commits into from
Dec 3, 2024
Merged

Conversation

spernsteiner
Copy link
Collaborator

This branch has several improvements to the rewriting of memcpy and memset:

  • Support memcpy on Quantity::Single references (&T/&mut T, as opposed to &[T]/&mut [T]). The reference is automatically converted to a 1-element slice. The copy_from_slice call will panic with an out-of-bounds error if the number of elements to copy is not 0 or 1.
  • Support memcpy on Option references. If the source or destination pointer is None, then this will panic if the element count is nonzero, and otherwise will do nothing.
  • Use mem::size_of::<T>() instead of the original size of T for converting memcpy/memset byte counts to element counts.

The size_of change is a bit subtle. In C, if sizeof(struct foo) == 16, it's legal to memcpy an array of struct foo using n * 16 as the byte length (or n * SIZE_OF_FOO, where SIZE_OF_FOO is #define'd to be 16) instead of n * sizeof(struct foo). This presents a problem for us because c2rust-analyze may change the size of foo when rewriting its pointer fields. After rewriting, the n * 16 version computes a byte length based on the original size for struct foo, while the n * sizeof(struct foo) computes it using the rewritten size. For converting the byte length to an element count, we previously used the original size, which works for the n * 16 version but not for n * sizeof(struct foo). This branch changes the element count calculation to divide by the rewritten size instead, which works for n * sizeof(struct foo) but not for n * 16. The hope is that n * sizeof(struct foo) is more common, since it's usually preferred in C for portability reasons.

@spernsteiner spernsteiner requested a review from ahomescu December 2, 2024 19:59
c2rust-analyze/src/rewrite/expr/convert.rs Show resolved Hide resolved
c2rust-analyze/src/rewrite/expr/convert.rs Show resolved Hide resolved
c2rust-analyze/src/rewrite/expr/convert.rs Show resolved Hide resolved
c2rust-analyze/src/rewrite/expr/convert.rs Outdated Show resolved Hide resolved
c2rust-analyze/src/rewrite/expr/convert.rs Show resolved Hide resolved
@spernsteiner spernsteiner force-pushed the analyze-more-memcpy-base branch from ad6c3ea to 8462bd8 Compare December 3, 2024 18:45
@spernsteiner spernsteiner force-pushed the analyze-more-memcpy branch 2 times, most recently from 4808c78 to 801a299 Compare December 3, 2024 18:47
@spernsteiner spernsteiner force-pushed the analyze-more-memcpy-base branch from 8462bd8 to 33465fe Compare December 3, 2024 22:21
@spernsteiner spernsteiner changed the base branch from analyze-more-memcpy-base to master December 3, 2024 22:52
@spernsteiner spernsteiner merged commit a96f324 into master Dec 3, 2024
8 checks passed
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.

2 participants