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.

Sign up for GitHub

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

Jump to bottom

Document soundness requirements around references #8

Closed
joshlf opened this issue Sep 19, 2022 · 0 comments · Fixed by #283
Closed

Document soundness requirements around references #8

joshlf opened this issue Sep 19, 2022 · 0 comments · Fixed by #283
Labels
compatibility-breaking Changes that are (likely to be) breaking

Comments

@joshlf
Copy link
Member

joshlf commented Sep 19, 2022 •
edited

Migrated from https://fxbug.dev/108820

Currently, FromBytes and AsBytes are documented as simply requiring that a type may be converted from an arbitrary sequence of bytes or may be converted to a sequence of bytes (respectively). However, this isn't actually sufficient in practice given the way we use these traits. In particular, we allow converting from &[u8] to &T (where T: FromBytes) and from &T to &[u8] (where T: AsBytes). The UnsafeCell<T> type can be converted from an arbitrary sequence of bytes if T can. However, if we were to implement FromBytes for UnsafeCell<T>, it would make these reference conversions unsound, as code with a &T could perform interior mutability which code with the &[u8] wouldn't know about.

We need to expand the soundness requirements on FromBytes and AsBytes to explicitly mention this reference-safety requirement.

See also #251 for an alternate approach.
@joshlf joshlf added the blocking-next-release This issue should be resolved before we release on crates.io label Oct 15, 2022
This was referenced Aug 8, 2023
Merged
Open
Open
Closed
@joshlf joshlf added compatibility-breaking Changes that are (likely to be) breaking and removed blocking-next-release This issue should be resolved before we release on crates.io labels Aug 12, 2023
@joshlf joshlf mentioned this issue Aug 20, 2023
Open
joshlf added a commit that referenced this issue Aug 25, 2023
@joshlf
Improve trait safety documentation
204b619 
For each of our unsafe traits, clarify who can skip reading the Safety
section.

For each of our unsafe traits (except for `Unaligned`):
- Clarify that it must be sound to construct `&[u8]` and `&T` to the
  same memory region (this addresses #8)
- Clarify that, in order to implement the trait, the type's fields need
  to satisfy the same requirements, but don't actually need to implement
  the trait
- Clarify that, in order to implement the trait, the type must not
  contain any `UnsafeCell`s

Closes #8
@joshlf joshlf mentioned this issue Aug 25, 2023
Merged
joshlf added a commit that referenced this issue Aug 25, 2023
@joshlf
Improve trait safety documentation
93ca281 
For each of our unsafe traits, clarify who can skip reading the Safety
section.

For each of our unsafe traits (except for `Unaligned`):
- Clarify that it must be sound to construct `&[u8]` and `&T` to the
  same memory region (this addresses #8)
- Clarify that, in order to implement the trait, the type's fields need
  to satisfy the same requirements, but don't actually need to implement
  the trait
- Clarify that, in order to implement the trait, the type must not
  contain any `UnsafeCell`s

Closes #8
joshlf added a commit that referenced this issue Aug 28, 2023
@joshlf
Improve trait safety documentation
de6c335 
For each of our unsafe traits, clarify who can skip reading the Safety
section.

For each of our unsafe traits (except for `Unaligned`):
- Clarify that it must be sound to construct `&[u8]` and `&T` to the
  same memory region (this addresses #8)
- Clarify that, in order to implement the trait, the type's fields need
  to satisfy the same requirements, but don't actually need to implement
  the trait
- Clarify that, in order to implement the trait, the type must not
  contain any `UnsafeCell`s

Closes #8
joshlf added a commit that referenced this issue Sep 2, 2023
@joshlf
Improve trait safety documentation (#283)
c979289 
For each of our unsafe traits, clarify who can skip reading the Safety
section.

For each of our unsafe traits (except for `Unaligned`):
- Clarify that it must be sound to construct `&[u8]` and `&T` to the
  same memory region (this addresses #8)
- Clarify that, in order to implement the trait, the type's fields need
  to satisfy the same requirements, but don't actually need to implement
  the trait
- Clarify that, in order to implement the trait, the type must not
  contain any `UnsafeCell`s

Closes #8
joshlf added a commit that referenced this issue Sep 3, 2023
@joshlf
Remove TODO
a87bc52 
This TODO was anchored on #8, which was fixed in #283.
@joshlf joshlf mentioned this issue Sep 3, 2023
Merged
joshlf added a commit that referenced this issue Sep 3, 2023
@joshlf
Remove TODO (#314)
bae7e39 
This TODO was anchored on #8, which was fixed in #283.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compatibility-breaking Changes that are (likely to be) breaking
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Improve trait safety documentation google/zerocopy
1 participant
@joshlf