Avoid sorting predicates by `DefId`
Fixes issue #82920
Even if an item does not change between compilation sessions, it may end
up with a different `DefId`, since inserting/deleting an item affects
the `DefId`s of all subsequent items. Therefore, we use a `DefPathHash`
in the incremental compilation system, which is stable in the face of
changes to unrelated items.
In particular, the query system will consider the inputs to a query to
be unchanged if any `DefId`s in the inputs have their `DefPathHash`es
unchanged. Queries are pure functions, so the query result should be
unchanged if the query inputs are unchanged.
Unfortunately, it's possible to inadvertantly make a query result
incorrectly change across compilations, by relying on the specific value
of a `DefId`. Specifically, if the query result is a slice that gets
sorted by `DefId`, the precise order will depend on how the `DefId`s got
assigned in a particular compilation session. If some definitions end up
with different `DefId`s (but the same `DefPathHash`es) in a subsequent
compilation session, we will end up re-computing a *different* value for
the query, even though the query system expects the result to unchanged
due to the unchanged inputs.
It turns out that we have been sorting the predicates computed during
`astconv` by their `DefId`. These predicates make their way into the
`super_predicates_that_define_assoc_type`, which ends up getting used to
compute the vtables of trait objects. This, re-ordering these predicates
between compilation sessions can lead to undefined behavior at runtime -
the query system will re-use code built with a *differently ordered*
vtable, resulting in the wrong method being invoked at runtime.
This PR avoids sorting by `DefId` in `astconv`, fixing the
miscompilation. However, it's possible that other instances of this
issue exist - they could also be easily introduced in the future.
To fully fix this issue, we should
1. Turn on `-Z incremental-verify-ich` by default. This will cause the
compiler to ICE whenver an 'unchanged' query result changes between
compilation sessions, instead of causing a miscompilation.
2. Remove the `Ord` impls for `CrateNum` and `DefId`. This will make it
difficult to introduce ICEs in the first place.
2021-03-12 22:53:02 +00:00
|
|
|
error[E0277]: `Rc<Foo>` cannot be shared between threads safely
|
2021-10-22 20:49:12 +00:00
|
|
|
--> $DIR/issue-40827.rs:14:7
|
2018-09-30 01:41:49 +00:00
|
|
|
|
|
|
|
|
|
LL | f(Foo(Arc::new(Bar::B(None))));
|
2021-10-22 20:49:12 +00:00
|
|
|
| - ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rc<Foo>` cannot be shared between threads safely
|
|
|
|
|
| |
|
|
|
|
|
| required by a bound introduced by this call
|
2018-09-30 01:41:49 +00:00
|
|
|
|
|
2024-01-29 18:31:02 +00:00
|
|
|
= help: within `Bar`, the trait `Sync` is not implemented for `Rc<Foo>`, which is required by `Foo: Send`
|
2021-03-30 20:37:30 +00:00
|
|
|
note: required because it appears within the type `Bar`
|
|
|
|
|
--> $DIR/issue-40827.rs:6:6
|
|
|
|
|
|
|
|
|
|
|
LL | enum Bar {
|
|
|
|
|
| ^^^
|
2022-08-15 20:31:37 +00:00
|
|
|
= note: required for `Arc<Bar>` to implement `Send`
|
2021-03-30 20:37:30 +00:00
|
|
|
note: required because it appears within the type `Foo`
|
|
|
|
|
--> $DIR/issue-40827.rs:4:8
|
|
|
|
|
|
|
|
|
|
|
LL | struct Foo(Arc<Bar>);
|
|
|
|
|
| ^^^
|
2021-07-31 16:26:55 +00:00
|
|
|
note: required by a bound in `f`
|
|
|
|
|
--> $DIR/issue-40827.rs:11:9
|
|
|
|
|
|
|
|
|
|
|
LL | fn f<T: Send>(_: T) {}
|
|
|
|
|
| ^^^^ required by this bound in `f`
|
2018-09-30 01:41:49 +00:00
|
|
|
|
Avoid sorting predicates by `DefId`
Fixes issue #82920
Even if an item does not change between compilation sessions, it may end
up with a different `DefId`, since inserting/deleting an item affects
the `DefId`s of all subsequent items. Therefore, we use a `DefPathHash`
in the incremental compilation system, which is stable in the face of
changes to unrelated items.
In particular, the query system will consider the inputs to a query to
be unchanged if any `DefId`s in the inputs have their `DefPathHash`es
unchanged. Queries are pure functions, so the query result should be
unchanged if the query inputs are unchanged.
Unfortunately, it's possible to inadvertantly make a query result
incorrectly change across compilations, by relying on the specific value
of a `DefId`. Specifically, if the query result is a slice that gets
sorted by `DefId`, the precise order will depend on how the `DefId`s got
assigned in a particular compilation session. If some definitions end up
with different `DefId`s (but the same `DefPathHash`es) in a subsequent
compilation session, we will end up re-computing a *different* value for
the query, even though the query system expects the result to unchanged
due to the unchanged inputs.
It turns out that we have been sorting the predicates computed during
`astconv` by their `DefId`. These predicates make their way into the
`super_predicates_that_define_assoc_type`, which ends up getting used to
compute the vtables of trait objects. This, re-ordering these predicates
between compilation sessions can lead to undefined behavior at runtime -
the query system will re-use code built with a *differently ordered*
vtable, resulting in the wrong method being invoked at runtime.
This PR avoids sorting by `DefId` in `astconv`, fixing the
miscompilation. However, it's possible that other instances of this
issue exist - they could also be easily introduced in the future.
To fully fix this issue, we should
1. Turn on `-Z incremental-verify-ich` by default. This will cause the
compiler to ICE whenver an 'unchanged' query result changes between
compilation sessions, instead of causing a miscompilation.
2. Remove the `Ord` impls for `CrateNum` and `DefId`. This will make it
difficult to introduce ICEs in the first place.
2021-03-12 22:53:02 +00:00
|
|
|
error[E0277]: `Rc<Foo>` cannot be sent between threads safely
|
2021-10-22 20:49:12 +00:00
|
|
|
--> $DIR/issue-40827.rs:14:7
|
2018-09-30 01:41:49 +00:00
|
|
|
|
|
|
|
|
|
LL | f(Foo(Arc::new(Bar::B(None))));
|
2021-10-22 20:49:12 +00:00
|
|
|
| - ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Rc<Foo>` cannot be sent between threads safely
|
|
|
|
|
| |
|
|
|
|
|
| required by a bound introduced by this call
|
2018-09-30 01:41:49 +00:00
|
|
|
|
|
2024-01-29 18:31:02 +00:00
|
|
|
= help: within `Bar`, the trait `Send` is not implemented for `Rc<Foo>`, which is required by `Foo: Send`
|
2021-03-30 20:37:30 +00:00
|
|
|
note: required because it appears within the type `Bar`
|
|
|
|
|
--> $DIR/issue-40827.rs:6:6
|
|
|
|
|
|
|
|
|
|
|
LL | enum Bar {
|
|
|
|
|
| ^^^
|
2022-08-15 20:31:37 +00:00
|
|
|
= note: required for `Arc<Bar>` to implement `Send`
|
2021-03-30 20:37:30 +00:00
|
|
|
note: required because it appears within the type `Foo`
|
|
|
|
|
--> $DIR/issue-40827.rs:4:8
|
|
|
|
|
|
|
|
|
|
|
LL | struct Foo(Arc<Bar>);
|
|
|
|
|
| ^^^
|
2021-07-31 16:26:55 +00:00
|
|
|
note: required by a bound in `f`
|
|
|
|
|
--> $DIR/issue-40827.rs:11:9
|
|
|
|
|
|
|
|
|
|
|
LL | fn f<T: Send>(_: T) {}
|
|
|
|
|
| ^^^^ required by this bound in `f`
|
2018-09-30 01:41:49 +00:00
|
|
|
|
|
|
|
|
error: aborting due to 2 previous errors
|
|
|
|
|
|
|
|
|
|
For more information about this error, try `rustc --explain E0277`.
|