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

Document that writes to padding are not preserved #1660

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

joshlf
Copy link
Member

@joshlf joshlf commented Sep 15, 2024

Document that writes to padding bytes are not preserved when an object is moved or copied.

Makes progress on #1648

@joshlf joshlf requested a review from jswrenn September 15, 2024 14:44
@joshlf
Copy link
Member Author

joshlf commented Sep 15, 2024

@saethlin Following up on our conversation at RustConf, does the wording in this warning accurately describe the behavior wrt padding bytes?

@joshlf joshlf force-pushed the padding-in-typed-copies branch 2 times, most recently from ad9b0bc to a01ce83 Compare September 15, 2024 15:20
@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.01%. Comparing base (f998114) to head (afb1b76).
Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1660   +/-   ##
=======================================
  Coverage   88.00%   88.01%           
=======================================
  Files          16       16           
  Lines        5801     5805    +4     
=======================================
+ Hits         5105     5109    +4     
  Misses        696      696           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/lib.rs Outdated
@@ -1953,6 +1953,15 @@ fn swap<T, U>((t, u): (T, U)) -> (U, T) {
/// overhead. This is useful whenever memory is known to be in a zeroed state,
/// such memory returned from some allocation routines.
///
/// # Warning: Padding bytes
///
/// Note that, when an object is moved or copied, only the non-padding bytes of
Copy link

@saethlin saethlin Sep 15, 2024

Choose a reason for hiding this comment

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

Rust does not have objects. We have allocations and typed or untyped copies. Untyped copies copy all bytes in the specified range, and currently the only way you can do this is ptr::copy and ptr::copy_nonoverlapping.

Typed copies are every other copy, which includes (some of these may be surprising to users) assignment, reads/writes through pointers, function arguments, and function return values.

Typed copies are not guaranteed to preserve bytes which are padding in all variants, of the type for the copy (the allocation doesn't have a type). That's probably what this comment means to identify.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, "typed copy" is what I'm referring to. Do you know if that term is defined anywhere in the Reference or stdlib docs? A quick Google doesn't turn anything up.

Choose a reason for hiding this comment

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

The docs for ptr::copy contain a probably-unhelpful mention of "untyped".

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you know of any internal docs/discussions that define the term? Or is this just one of those "compiler people use these terms and know what they mean when they talk to each other and they weren't expecting you to look inside the compiler" things?

Choose a reason for hiding this comment

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

@rust-lang/opsem do we have documentation for a typed copy?

Choose a reason for hiding this comment

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

Ugh of course that doesn't work.
@CAD97 @digama0 @JakobDegen @RalfJung do we have documentation for what a typed copy is?

Choose a reason for hiding this comment

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

I don't know of any proper docs for this, no. That's clearly something we should fix. Any volunteers? :)

One question is where these docs would even go. The primary syntax for a typed copy is a = b;, so maybe this should be here?

Copy link
Member Author

Choose a reason for hiding this comment

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

s/object/value, but otherwise I'm gonna merge this and we can nitpick the wording later once docs have landed to Rust that we can cite.

Document that writes to padding bytes are not preserved when an object
is moved or copied.

Makes progress on #1648
Copy link
Collaborator

@jswrenn jswrenn left a comment

Choose a reason for hiding this comment

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

Approved, contingent on this change.

src/lib.rs Outdated
@@ -1953,6 +1953,15 @@ fn swap<T, U>((t, u): (T, U)) -> (U, T) {
/// overhead. This is useful whenever memory is known to be in a zeroed state,
/// such memory returned from some allocation routines.
///
/// # Warning: Padding bytes
///
/// Note that, when an object is moved or copied, only the non-padding bytes of
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// Note that, when an object is moved or copied, only the non-padding bytes of
/// Note that, when an value is moved or copied, only the non-padding bytes of

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@joshlf joshlf added this pull request to the merge queue Sep 16, 2024
Merged via the queue into main with commit d2ba8cd Sep 16, 2024
80 checks passed
@joshlf joshlf deleted the padding-in-typed-copies branch September 16, 2024 21:11
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.

5 participants