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

Specify bit validity and padding of some types #1392

Merged
merged 14 commits into from
Sep 9, 2023

Conversation

joshlf
Copy link
Contributor

@joshlf joshlf commented Aug 11, 2023

Specify the bit validity and padding of the primitive numeric types, bool, char, and pointer and reference types.

Closes #1291

joshlf added 2 commits August 10, 2023 21:54
Specify the bit validity and padding of the primitive numeric 
types, bool, char, and pointer and reference types.

Closes rust-lang#1291
src/type-layout.md Outdated Show resolved Hide resolved
@scottmcm scottmcm self-assigned this Aug 14, 2023
src/type-layout.md Outdated Show resolved Hide resolved
src/type-layout.md Outdated Show resolved Hide resolved
src/type-layout.md Outdated Show resolved Hide resolved
Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

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

LGTM! I'm confident this is covered by the FCP in rust-lang/unsafe-code-guidelines#439.

@joshlf
Copy link
Contributor Author

joshlf commented Aug 21, 2023

Thanks for all of the discussion on this, @RalfJung!

@joshlf
Copy link
Contributor Author

joshlf commented Aug 24, 2023

Is merging this blocked on anything?

@RalfJung
Copy link
Member

I don't think so, but review capacity on reference PRs is very low.

@joshlf
Copy link
Contributor Author

joshlf commented Aug 24, 2023

Oh does this require more than just your LGTM?

@RalfJung
Copy link
Member

With the reference I am never sure.^^ @ehuss is our main editor here.

@ehuss
Copy link
Contributor

ehuss commented Aug 28, 2023

Can you say why this is placed in the type-layout chapter? In general, that chapter is focused on the size and alignment of types, and not what constitutes a valid type. Valid bit patterns for types are generally documented in their corresponding type pages (like bool specifies that only 0 and 1 are valid bytes).

Comment on lines 63 to 64
pointer type may not be a valid `u8` (the semantics of transmuting a reference or
pointer to a non-pointer type is currently undecided).
Copy link
Contributor

Choose a reason for hiding this comment

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

This "may" seems confusing to me. When is it valid or not valid?

Copy link
Member

Choose a reason for hiding this comment

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

Those details are undecided as of now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see, thanks!

@joshlf
Copy link
Contributor Author

joshlf commented Aug 28, 2023

Can you say why this is placed in the type-layout chapter? In general, that chapter is focused on the size and alignment of types, and not what constitutes and valid type. Valid bit patterns for types are generally documented in their corresponding type pages (like bool specifies that only 0 and 1 are valid bytes).

I can do that, it'd just require somewhat more text since the second paragraph (about going from T to [u8; size_of::<T>()]) would need to be duplicated on the pages for bools, chars, and numeric types. If that's your preference, I'm happy to do that.

@ehuss
Copy link
Contributor

ehuss commented Aug 28, 2023

Yea, I'm fine with duplicating it. For the bool case, that could also be worded a little different since "every byte" is a little confusing since there is only one byte. Thanks!

@joshlf
Copy link
Contributor Author

joshlf commented Aug 28, 2023

Where do you think this text should go? On the page for pointer types?

A byte at any offset in a reference or pointer type may not be a valid u8 (the semantics of transmuting a reference or pointer to a non-pointer type is currently undecided).

Given that it would be lacking the context of the paragraph it's currently in, maybe we could rewrite it to something like the following, in its own section:

Bit validity

Despite pointers and references being similar to usizes in the machine code emitted on most platforms, the semantics of transmuting a reference or pointer type to a non-pointer type is currently undecided. Thus, it may not be valid to transmute a pointer or reference type, P, to a [u8; size_of::<P>()].

@joshlf
Copy link
Contributor Author

joshlf commented Aug 28, 2023

Okay, I updated to this text in pointer.md, so the PR is ready for review. Happy to edit this as requested.

Bit validity

Despite pointers and references being similar to usizes in the machine code emitted on most platforms, the semantics of transmuting a reference or pointer type to a non-pointer type is currently undecided. Thus, it may not be valid to transmute a pointer or reference type, P, to a [u8; size_of::<P>()].

@joshlf
Copy link
Contributor Author

joshlf commented Aug 29, 2023

One follow-up thought inspired by google/zerocopy#294: @RalfJung and @ehuss, would you be on board with me also adding the following to pointer.md? If this is at all controversial, I'll just follow up in a separate PR to avoid holding this one up.

It is always sound to produce a thin null pointer (i.e., for T: Sized and P = *const T or P = *mut T, transmute::<_, P>([0u8; size_of::<P>()])). Note that this can be done without unsafe code using the ptr::null or ptr::null_mut functions.

@RalfJung
Copy link
Member

RalfJung commented Sep 7, 2023 via email

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

@ehuss ehuss added this pull request to the merge queue Sep 9, 2023
Merged via the queue into rust-lang:master with commit ee7c676 Sep 9, 2023
1 check passed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 11, 2023
Update books

## rust-lang/edition-guide

1 commits in 2751bdcef125468ea2ee006c11992cd1405aebe5..34fca48ed284525b2f124bf93c51af36d6685492
2023-09-06 20:34:00 UTC to 2023-09-06 20:34:00 UTC

- Update Rust 2018 "Path and module system changes" for Rust 1.72 (rust-lang/edition-guide#285)

## rust-lang/nomicon

2 commits in 388750b081c0893c275044d37203f97709e058ba..e3f3af69dce71cd37a785bccb7e58449197d940c
2023-09-11 15:57:05 UTC to 2023-09-11 15:55:35 UTC

- specify which integer overflows we mean (rust-lang/nomicon#419)
- remove 'fail to call destructors' from okay-list (rust-lang/nomicon#420)

## rust-lang/reference

4 commits in d43038932adeb16ada80e206d4c073d851298101..ee7c676fd6e287459cb407337652412c990686c0
2023-09-09 20:08:06 UTC to 2023-08-16 16:59:33 UTC

- Specify bit validity and padding of some types (rust-lang/reference#1392)
- implementations.md typo fix (rust-lang/reference#1399)
- Update section on default layout for `repr(Rust)` (rust-lang/reference#1396)
- conditional-compilation.md: Mention the "none" target_os value (rust-lang/reference#1395)

## rust-lang/rust-by-example

4 commits in 07e0df2f006e59d171c6bf3cafa9d61dbeb520d8..c954202c1e1720cba5628f99543cc01188c7d6fc
2023-08-22 18:49:29 UTC to 2023-08-22 18:46:56 UTC

- Improve transparency of 5_i32 versus 5i32 (rust-lang/rust-by-example#1707)
- Removed redundant comma (rust-lang/rust-by-example#1735)
- Fixed link to Functions (rust-lang/rust-by-example#1734)
- Pedantic `'static` lifetime corrections (rust-lang/rust-by-example#1732)

## rust-lang/rustc-dev-guide

25 commits in b123ab4..08bb147
2023-09-11 10:36:36 UTC to 2023-08-18 21:13:31 UTC

- make link more pleasant to eye too (rust-lang/rustc-dev-guide#1778)
- The current playground link used in the page of MIR shows a optimized… (rust-lang/rustc-dev-guide#1789)
- Add section about building an optimized version of `rustc` (rust-lang/rustc-dev-guide#1787)
- Set max line length in `.editorconfig` to 100 (rust-lang/rustc-dev-guide#1788)
- Update minor how-to-build-and-run.md spelling mistake (rust-lang/rustc-dev-guide#1785)
- add sections in 'using git' (rust-lang#1675) (rust-lang/rustc-dev-guide#1784)
- link std-dev-guide from landing page (rust-lang#1699) (rust-lang/rustc-dev-guide#1783)
- Reword sentence about using `./x` over `./x.py` (rust-lang/rustc-dev-guide#1782)
- remove (excessive) indentation (rust-lang/rustc-dev-guide#1781)
- coverage tests have moved, twice (rust-lang/rustc-dev-guide#1780)
- remove extraneous word (rust-lang/rustc-dev-guide#1779)
- llvm updates (rust-lang/rustc-dev-guide#1761)
- make link more pleasant to eye (rust-lang/rustc-dev-guide#1777)
- date-check: test suites/classes using "revisions" (rust-lang/rustc-dev-guide#1738)
- share link target (rust-lang/rustc-dev-guide#1740)
- indicate full hierarchy of config option (rust-lang/rustc-dev-guide#1776)
- remove stray word (rust-lang/rustc-dev-guide#1773)
- it is lower-case (rust-lang/rustc-dev-guide#1772)
- Suggest enabling patch-binaries-for-nix in `shell.nix` (rust-lang/rustc-dev-guide#1774)
- Add additional licensing concerns to docs (rust-lang/rustc-dev-guide#1775)
- Fix broken MD link format (rust-lang/rustc-dev-guide#1771)
- update internal terminology: Substs -> GenericArgs (rust-lang/rustc-dev-guide#1769)
- Update suggested.md : missing word (rust-lang/rustc-dev-guide#1770)
- Update outdated doc for types (rust-lang/rustc-dev-guide#1768)
- Add dropck documentation (rust-lang/rustc-dev-guide#1767)
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 11, 2023
Rollup merge of rust-lang#115761 - rustbot:docs-update, r=ehuss

Update books

## rust-lang/edition-guide

1 commits in 2751bdcef125468ea2ee006c11992cd1405aebe5..34fca48ed284525b2f124bf93c51af36d6685492
2023-09-06 20:34:00 UTC to 2023-09-06 20:34:00 UTC

- Update Rust 2018 "Path and module system changes" for Rust 1.72 (rust-lang/edition-guide#285)

## rust-lang/nomicon

2 commits in 388750b081c0893c275044d37203f97709e058ba..e3f3af69dce71cd37a785bccb7e58449197d940c
2023-09-11 15:57:05 UTC to 2023-09-11 15:55:35 UTC

- specify which integer overflows we mean (rust-lang/nomicon#419)
- remove 'fail to call destructors' from okay-list (rust-lang/nomicon#420)

## rust-lang/reference

4 commits in d43038932adeb16ada80e206d4c073d851298101..ee7c676fd6e287459cb407337652412c990686c0
2023-09-09 20:08:06 UTC to 2023-08-16 16:59:33 UTC

- Specify bit validity and padding of some types (rust-lang/reference#1392)
- implementations.md typo fix (rust-lang/reference#1399)
- Update section on default layout for `repr(Rust)` (rust-lang/reference#1396)
- conditional-compilation.md: Mention the "none" target_os value (rust-lang/reference#1395)

## rust-lang/rust-by-example

4 commits in 07e0df2f006e59d171c6bf3cafa9d61dbeb520d8..c954202c1e1720cba5628f99543cc01188c7d6fc
2023-08-22 18:49:29 UTC to 2023-08-22 18:46:56 UTC

- Improve transparency of 5_i32 versus 5i32 (rust-lang/rust-by-example#1707)
- Removed redundant comma (rust-lang/rust-by-example#1735)
- Fixed link to Functions (rust-lang/rust-by-example#1734)
- Pedantic `'static` lifetime corrections (rust-lang/rust-by-example#1732)

## rust-lang/rustc-dev-guide

25 commits in b123ab4..08bb147
2023-09-11 10:36:36 UTC to 2023-08-18 21:13:31 UTC

- make link more pleasant to eye too (rust-lang/rustc-dev-guide#1778)
- The current playground link used in the page of MIR shows a optimized… (rust-lang/rustc-dev-guide#1789)
- Add section about building an optimized version of `rustc` (rust-lang/rustc-dev-guide#1787)
- Set max line length in `.editorconfig` to 100 (rust-lang/rustc-dev-guide#1788)
- Update minor how-to-build-and-run.md spelling mistake (rust-lang/rustc-dev-guide#1785)
- add sections in 'using git' (rust-lang#1675) (rust-lang/rustc-dev-guide#1784)
- link std-dev-guide from landing page (rust-lang#1699) (rust-lang/rustc-dev-guide#1783)
- Reword sentence about using `./x` over `./x.py` (rust-lang/rustc-dev-guide#1782)
- remove (excessive) indentation (rust-lang/rustc-dev-guide#1781)
- coverage tests have moved, twice (rust-lang/rustc-dev-guide#1780)
- remove extraneous word (rust-lang/rustc-dev-guide#1779)
- llvm updates (rust-lang/rustc-dev-guide#1761)
- make link more pleasant to eye (rust-lang/rustc-dev-guide#1777)
- date-check: test suites/classes using "revisions" (rust-lang/rustc-dev-guide#1738)
- share link target (rust-lang/rustc-dev-guide#1740)
- indicate full hierarchy of config option (rust-lang/rustc-dev-guide#1776)
- remove stray word (rust-lang/rustc-dev-guide#1773)
- it is lower-case (rust-lang/rustc-dev-guide#1772)
- Suggest enabling patch-binaries-for-nix in `shell.nix` (rust-lang/rustc-dev-guide#1774)
- Add additional licensing concerns to docs (rust-lang/rustc-dev-guide#1775)
- Fix broken MD link format (rust-lang/rustc-dev-guide#1771)
- update internal terminology: Substs -> GenericArgs (rust-lang/rustc-dev-guide#1769)
- Update suggested.md : missing word (rust-lang/rustc-dev-guide#1770)
- Update outdated doc for types (rust-lang/rustc-dev-guide#1768)
- Add dropck documentation (rust-lang/rustc-dev-guide#1767)
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Sep 12, 2023
Update books

## rust-lang/edition-guide

1 commits in 2751bdcef125468ea2ee006c11992cd1405aebe5..34fca48ed284525b2f124bf93c51af36d6685492
2023-09-06 20:34:00 UTC to 2023-09-06 20:34:00 UTC

- Update Rust 2018 "Path and module system changes" for Rust 1.72 (rust-lang/edition-guide#285)

## rust-lang/nomicon

2 commits in 388750b081c0893c275044d37203f97709e058ba..e3f3af69dce71cd37a785bccb7e58449197d940c
2023-09-11 15:57:05 UTC to 2023-09-11 15:55:35 UTC

- specify which integer overflows we mean (rust-lang/nomicon#419)
- remove 'fail to call destructors' from okay-list (rust-lang/nomicon#420)

## rust-lang/reference

4 commits in d43038932adeb16ada80e206d4c073d851298101..ee7c676fd6e287459cb407337652412c990686c0
2023-09-09 20:08:06 UTC to 2023-08-16 16:59:33 UTC

- Specify bit validity and padding of some types (rust-lang/reference#1392)
- implementations.md typo fix (rust-lang/reference#1399)
- Update section on default layout for `repr(Rust)` (rust-lang/reference#1396)
- conditional-compilation.md: Mention the "none" target_os value (rust-lang/reference#1395)

## rust-lang/rust-by-example

4 commits in 07e0df2f006e59d171c6bf3cafa9d61dbeb520d8..c954202c1e1720cba5628f99543cc01188c7d6fc
2023-08-22 18:49:29 UTC to 2023-08-22 18:46:56 UTC

- Improve transparency of 5_i32 versus 5i32 (rust-lang/rust-by-example#1707)
- Removed redundant comma (rust-lang/rust-by-example#1735)
- Fixed link to Functions (rust-lang/rust-by-example#1734)
- Pedantic `'static` lifetime corrections (rust-lang/rust-by-example#1732)

## rust-lang/rustc-dev-guide

25 commits in b123ab4754127d822ffb38349ce0fbf561f1b2fd..08bb147d51e815b96e8db7ba4cf870f201c11ff8
2023-09-11 10:36:36 UTC to 2023-08-18 21:13:31 UTC

- make link more pleasant to eye too (rust-lang/rustc-dev-guide#1778)
- The current playground link used in the page of MIR shows a optimized… (rust-lang/rustc-dev-guide#1789)
- Add section about building an optimized version of `rustc` (rust-lang/rustc-dev-guide#1787)
- Set max line length in `.editorconfig` to 100 (rust-lang/rustc-dev-guide#1788)
- Update minor how-to-build-and-run.md spelling mistake (rust-lang/rustc-dev-guide#1785)
- add sections in 'using git' (#1675) (rust-lang/rustc-dev-guide#1784)
- link std-dev-guide from landing page (#1699) (rust-lang/rustc-dev-guide#1783)
- Reword sentence about using `./x` over `./x.py` (rust-lang/rustc-dev-guide#1782)
- remove (excessive) indentation (rust-lang/rustc-dev-guide#1781)
- coverage tests have moved, twice (rust-lang/rustc-dev-guide#1780)
- remove extraneous word (rust-lang/rustc-dev-guide#1779)
- llvm updates (rust-lang/rustc-dev-guide#1761)
- make link more pleasant to eye (rust-lang/rustc-dev-guide#1777)
- date-check: test suites/classes using "revisions" (rust-lang/rustc-dev-guide#1738)
- share link target (rust-lang/rustc-dev-guide#1740)
- indicate full hierarchy of config option (rust-lang/rustc-dev-guide#1776)
- remove stray word (rust-lang/rustc-dev-guide#1773)
- it is lower-case (rust-lang/rustc-dev-guide#1772)
- Suggest enabling patch-binaries-for-nix in `shell.nix` (rust-lang/rustc-dev-guide#1774)
- Add additional licensing concerns to docs (rust-lang/rustc-dev-guide#1775)
- Fix broken MD link format (rust-lang/rustc-dev-guide#1771)
- update internal terminology: Substs -> GenericArgs (rust-lang/rustc-dev-guide#1769)
- Update suggested.md : missing word (rust-lang/rustc-dev-guide#1770)
- Update outdated doc for types (rust-lang/rustc-dev-guide#1768)
- Add dropck documentation (rust-lang/rustc-dev-guide#1767)

Despite pointers and references being similar to `usize`s in the machine code emitted on most platforms,
the semantics of transmuting a reference or pointer type to a non-pointer type is currently undecided.
Thus, it may not be valid to transmute a pointer or reference type, `P`, to a `[u8; size_of::<P>()]`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RalfJung For my own edification, how does this interact with pointer-to-integer as casts? I can imagine a few possibilities (not mutually exclusive):

  • as casts are sound, but transmutations are not necessarily sound (even though you can emulate a transmutation by doing an as cast followed by a transmutation from usize)
  • the first bullet is true and pointer-to-integer as casts probably shouldn't have been allowed in safe Rust, but it's too late

Copy link
Member

@RalfJung RalfJung Sep 14, 2023

Choose a reason for hiding this comment

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

The current working model is that as casts are special:

In contrast:

Here, "transmute" refers to transmute but also any other way to re-interpret bytes at a different type, such as type punning through union field accesses and raw pointer casts.

All of these operations are safe and sound, but as their docs show, they behave quite differently and that can affect the soundness of later code that uses the values they produce.

Note that this is a prototype model, which has some unresolved problems and which is very much not stabilized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay that all makes sense, thanks!

Note that this is a prototype model, which has some unresolved problems and which is very much not stabilized.

I always assume that of everything in this space unless documented otherwise :)

joshlf added a commit to google/zerocopy that referenced this pull request Oct 4, 2023
rust-lang/reference#1392 adds bit validity
guarantees for numeric types. This commit makes use of those guarantees
to provide stronger soundness justifications for some trait impls.
joshlf added a commit to google/zerocopy that referenced this pull request Oct 4, 2023
rust-lang/reference#1392 adds bit validity
guarantees for numeric types. This commit makes use of those guarantees
to provide stronger soundness justifications for some trait impls.

Closes #440
@joshlf joshlf deleted the joshlf-patch-1 branch November 15, 2023 21:31
joshlf added a commit to google/zerocopy that referenced this pull request Nov 15, 2023
rust-lang/reference#1392 adds bit validity
guarantees for numeric types. This commit makes use of those guarantees
to provide stronger soundness justifications for some trait impls.

Closes #440
github-merge-queue bot pushed a commit to google/zerocopy that referenced this pull request Nov 15, 2023
rust-lang/reference#1392 adds bit validity
guarantees for numeric types. This commit makes use of those guarantees
to provide stronger soundness justifications for some trait impls.

Closes #440
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.

Describe bit validity and padding for primitive types
8 participants