Fix deduplication mismatches in vtables leading to upcasting unsoundness We currently have two cases where subtleties in supertraits can trigger disagreements in the vtable layout, e.g. leading to a different vtable layout being accessed at a callsite compared to what was prepared during unsizing. Namely: ### #135315 In this example, we were not normalizing supertraits when preparing vtables. In the example, ``` trait Supertrait<T> { fn _print_numbers(&self, mem: &[usize; 100]) { println!("{mem:?}"); } } impl<T> Supertrait<T> for () {} trait Identity { type Selff; } impl<Selff> Identity for Selff { type Selff = Selff; } trait Middle<T>: Supertrait<()> + Supertrait<T> { fn say_hello(&self, _: &usize) { println!("Hello!"); } } impl<T> Middle<T> for () {} trait Trait: Middle<<() as Identity>::Selff> {} impl Trait for () {} fn main() { (&() as &dyn Trait as &dyn Middle<()>).say_hello(&0); } ``` When we prepare `dyn Trait`, we see a supertrait of `Middle<<() as Identity>::Selff>`, which itself has two supertraits `Supertrait<()>` and `Supertrait<<() as Identity>::Selff>`. These two supertraits are identical, but they are not duplicated because we were using structural equality and *not* considering normalization. This leads to a vtable layout with two trait pointers. When we upcast to `dyn Middle<()>`, those two supertraits are now the same, leading to a vtable layout with only one trait pointer. This leads to an offset error, and we call the wrong method. ### #135316 This one is a bit more interesting, and is the bulk of the changes in this PR. It's a bit similar, except it uses binder equality instead of normalization to make the compiler get confused about two vtable layouts. In the example, ``` trait Supertrait<T> { fn _print_numbers(&self, mem: &[usize; 100]) { println!("{mem:?}"); } } impl<T> Supertrait<T> for () {} trait Trait<T, U>: Supertrait<T> + Supertrait<U> { fn say_hello(&self, _: &usize) { println!("Hello!"); } } impl<T, U> Trait<T, U> for () {} fn main() { (&() as &'static dyn for<'a> Trait<&'static (), &'a ()> as &'static dyn Trait<&'static (), &'static ()>) .say_hello(&0); } ``` When we prepare the vtable for `dyn for<'a> Trait<&'static (), &'a ()>`, we currently consider the PolyTraitRef of the vtable as the key for a supertrait. This leads two two supertraits -- `Supertrait<&'static ()>` and `for<'a> Supertrait<&'a ()>`. However, we can upcast[^up] without offsetting the vtable from `dyn for<'a> Trait<&'static (), &'a ()>` to `dyn Trait<&'static (), &'static ()>`. This is just instantiating the principal trait ref for a specific `'a = 'static`. However, when considering those supertraits, we now have only one distinct supertrait -- `Supertrait<&'static ()>` (which is deduplicated since there are two supertraits with the same substitutions). This leads to similar offsetting issues, leading to the wrong method being called. [^up]: I say upcast but this is a cast that is allowed on stable, since it's not changing the vtable at all, just instantiating the binder of the principal trait ref for some lifetime. The solution here is to recognize that a vtable isn't really meaningfully higher ranked, and to just treat a vtable as corresponding to a `TraitRef` so we can do this deduplication more faithfully. That is to say, the vtable for `dyn for<'a> Tr<'a>` and `dyn Tr<'x>` are always identical, since they both would correspond to a set of free regions on an impl... Do note that `Tr<for<'a> fn(&'a ())>` and `Tr<fn(&'static ())>` are still distinct. ---- There's a bit more that can be cleaned up. In codegen, we can stop using `PolyExistentialTraitRef` basically everywhere. We can also fix SMIR to stop storing `PolyExistentialTraitRef` in its vtable allocations. As for testing, it's difficult to actually turn this into something that can be tested with `rustc_dump_vtable`, since having multiple supertraits that are identical is a recipe for ambiguity errors. Maybe someone else is more creative with getting that attr to work, since the tests I added being run-pass tests is a bit unsatisfying. Miri also doesn't help here, since it doesn't really generate vtables that are offset by an index in the same way as codegen. r? `@lcnr` for the vibe check? Or reassign, idk. Maybe let's talk about whether this makes sense. <sup>(I guess an alternative would also be to not do any deduplication of vtable supertraits (or only a really conservative subset) rather than trying to normalize and deduplicate more faithfully here. Not sure if that works and is sufficient tho.)</sup> cc `@steffahn` -- ty for the minimizations cc `@WaffleLapkin` -- since you're overseeing the feature stabilization :3 Fixes #135315 Fixes #135316 |
||
|---|---|---|
| .github/workflows | ||
| ci | ||
| examples | ||
| josh-sync | ||
| src | ||
| .editorconfig | ||
| .gitattributes | ||
| .gitignore | ||
| .mailmap | ||
| CITATION.cff | ||
| CNAME | ||
| CODE_OF_CONDUCT.md | ||
| LICENSE-APACHE | ||
| LICENSE-MIT | ||
| README.md | ||
| book.toml | ||
| mermaid-init.js | ||
| mermaid.min.js | ||
| rust-version | ||
| triagebot.toml | ||
README.md
This is a collaborative effort to build a guide that explains how rustc works. The aim of the guide is to help new contributors get oriented to rustc, as well as to help more experienced folks in figuring out some new part of the compiler that they haven't worked on before.
You can read the latest version of the guide here.
You may also find the rustdocs for the compiler itself useful. Note that these are not intended as a guide; it's recommended that you search for the docs you're looking for instead of reading them top to bottom.
For documentation on developing the standard library, see
std-dev-guide.
Contributing to the guide
The guide is useful today, but it has a lot of work still to go.
If you'd like to help improve the guide, we'd love to have you! You can find plenty of issues on the issue tracker. Just post a comment on the issue you would like to work on to make sure that we don't accidentally duplicate work. If you think something is missing, please open an issue about it!
In general, if you don't know how the compiler works, that is not a problem! In that case, what we will do is to schedule a bit of time for you to talk with someone who does know the code, or who wants to pair with you and figure it out. Then you can work on writing up what you learned.
In general, when writing about a particular part of the compiler's code, we recommend that you link to the relevant parts of the rustc rustdocs.
Build Instructions
To build a local static HTML site, install mdbook with:
> cargo install mdbook mdbook-linkcheck2 mdbook-toc mdbook-mermaid
and execute the following command in the root of the repository:
> mdbook build --open
The build files are found in the book/html directory.
Link Validations
We use mdbook-linkcheck2 to validate URLs included in our documentation. Link
checking is not run by default locally, though it is in CI. To enable it
locally, set the environment variable ENABLE_LINKCHECK=1 like in the
following example.
$ ENABLE_LINKCHECK=1 mdbook serve
Table of Contents
We use mdbook-toc to auto-generate TOCs for long sections. You can invoke the preprocessor by
including the <!-- toc --> marker at the place where you want the TOC.
Synchronizing josh subtree with rustc
This repository is linked to rust-lang/rust as a josh subtree. You can use the following commands to synchronize the subtree in both directions.
You'll need to install josh-proxy locally via
cargo +stable install josh-proxy --git https://github.com/josh-project/josh --tag r24.10.04
Older versions of josh-proxy may not round trip commits losslessly so it is important to install this exact version.
Pull changes from rust-lang/rust into this repository
- Checkout a new branch that will be used to create a PR into
rust-lang/rustc-dev-guide - Run the pull command
$ cargo run --manifest-path josh-sync/Cargo.toml rustc-pull - Push the branch to your fork and create a PR into
rustc-dev-guide
Push changes from this repository into rust-lang/rust
- Run the push command to create a branch named
<branch-name>in arustcfork under the<gh-username>account$ cargo run --manifest-path josh-sync/Cargo.toml rustc-push <branch-name> <gh-username> - Create a PR from
<branch-name>intorust-lang/rust