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

Safe Transmute: Compute transmutability from rustc_target::abi::Layout #123367

Merged
merged 1 commit into from
Apr 8, 2024

Conversation

jswrenn
Copy link
Member

@jswrenn jswrenn commented Apr 2, 2024

In its first step of computing transmutability, rustc_transmutability constructs a byte-level representation of type layout (Tree). Previously, this representation was computed for ADTs by inspecting the ADT definition and performing our own layout computations. This process was error-prone, verbose, and limited our ability to analyze many types (particularly default-repr types).

In this PR, we instead construct Trees from rustc_target::abi::Layouts. This helps ensure that layout optimizations are reflected our analyses, and increases the kinds of types we can now analyze, including:

  • default repr ADTs
  • transparent unions
  • UnsafeCell-containing types

Overall, this PR expands the expressvity of rustc_transmutability to be much closer to the transmutability analysis performed by miri. Future PRs will work to close the remaining gaps (e.g., support for Box, raw pointers, NonZero*, coroutines, etc.).

r? @compiler-errors

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 2, 2024
Copy link
Member Author

@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.

(Left some comments as review waypoints.)

Copy link
Member Author

Choose a reason for hiding this comment

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

UnsafeCell was, previously, unsupported by virtue of us not implementing support for anything besides repr(C) structs. We didn't have to worry about the presence/absence of UnsafeCell in references, because we simply bailed at the first sight of any UnsafeCell. Now that we support analyzing types with UnsafeCell, we also need to be careful not permit transmutations that would lead to data races. We do so by emitting Freeze bounds when appropriate (made possible by #121840; thanks @oli-obk!).

Copy link
Member Author

Choose a reason for hiding this comment

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

N.B.: This is what the UX of analyzing an invalid UnsafeCell-containing transmute looks like.

Copy link
Member Author

Choose a reason for hiding this comment

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

The meat of the PR is here.

@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

Minor nits

@@ -8,7 +8,7 @@ error[E0277]: `Src` cannot be safely transmuted into `Dst`
--> $DIR/unknown_src_field.rs:19:36
|
LL | assert::is_transmutable::<Src, Dst>();
| ^^^ `Dst` has an unknown layout
| ^^^ analyzing the transmutability of `Dst` is not yet supported.
Copy link
Member

Choose a reason for hiding this comment

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

Remove the . -- it's inconsistent with diagnostics

Copy link
Member Author

Choose a reason for hiding this comment

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

let align = layout.align();
let size = layout.size();
let ty_and_layout = cx.layout_of(*ty)?;
let align = ty_and_layout.align.abi.bytes() as _;
Copy link
Member

Choose a reason for hiding this comment

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

as usize

Copy link
Member Author

Choose a reason for hiding this comment

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

In its first step of computing transmutability, `rustc_transmutability`
constructs a byte-level representation of type layout (`Tree`). Previously, this
representation was computed for ADTs by inspecting the ADT definition and
performing our own layout computations. This process was error-prone, verbose,
and limited our ability to analyze many types (particularly default-repr types).

In this PR, we instead construct `Tree`s from `rustc_target::abi::Layout`s. This
helps ensure that layout optimizations are reflected our analyses, and increases
the kinds of types we can now analyze, including:
- default repr ADTs
- transparent unions
- `UnsafeCell`-containing types

Overall, this PR expands the expressvity of `rustc_transmutability` to be much
closer to the transmutability analysis performed by miri. Future PRs will work
to close the remaining gaps (e.g., support for `Box`, raw pointers, `NonZero*`,
coroutines, etc.).
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Apr 8, 2024

📌 Commit 3aa14e3 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 8, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 8, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#122781 (Fix argument ABI for overaligned structs on ppc64le)
 - rust-lang#123367 (Safe Transmute: Compute transmutability from `rustc_target::abi::Layout`)
 - rust-lang#123518 (Fix `ByMove` coroutine-closure shim (for 2021 precise closure capturing behavior))
 - rust-lang#123547 (bootstrap: remove unused pub fns)
 - rust-lang#123564 (Don't emit divide-by-zero panic paths in `StepBy::len`)
 - rust-lang#123578 (Restore `pred_known_to_hold_modulo_regions`)
 - rust-lang#123591 (Remove unnecessary cast from `LLVMRustGetInstrProfIncrementIntrinsic`)
 - rust-lang#123632 (parser: reduce visibility of unnecessary public `UnmatchedDelim`)
 - rust-lang#123635 (CFI: Fix ICE in KCFI non-associated function pointers)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0e27c99 into rust-lang:master Apr 8, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 8, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 8, 2024
Rollup merge of rust-lang#123367 - jswrenn:layoutify, r=compiler-errors

Safe Transmute: Compute transmutability from `rustc_target::abi::Layout`

In its first step of computing transmutability, `rustc_transmutability` constructs a byte-level representation of type layout (`Tree`). Previously, this representation was computed for ADTs by inspecting the ADT definition and performing our own layout computations. This process was error-prone, verbose, and limited our ability to analyze many types (particularly default-repr types).

In this PR, we instead construct `Tree`s from `rustc_target::abi::Layout`s. This helps ensure that layout optimizations are reflected our analyses, and increases the kinds of types we can now analyze, including:
- default repr ADTs
- transparent unions
- `UnsafeCell`-containing types

Overall, this PR expands the expressvity of `rustc_transmutability` to be much closer to the transmutability analysis performed by miri. Future PRs will work to close the remaining gaps (e.g., support for `Box`, raw pointers, `NonZero*`, coroutines, etc.).

r? `@compiler-errors`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants