Restrict amount of ignored locals.

This commit is contained in:
Camille GILLOT 2023-01-21 10:03:12 +00:00
parent c51fc382bd
commit 65c3c90f3e
8 changed files with 135 additions and 13 deletions

View file

@ -902,6 +902,8 @@ pub enum LocalInfo<'tcx> {
AggregateTemp,
/// A temporary created during the pass `Derefer` to avoid it's retagging
DerefTemp,
/// A temporary created for borrow checking.
FakeBorrow,
}
impl<'tcx> LocalDecl<'tcx> {

View file

@ -140,8 +140,8 @@ pub struct GeneratorSavedTy<'tcx> {
pub ty: Ty<'tcx>,
/// Source info corresponding to the local in the original MIR body.
pub source_info: SourceInfo,
/// Whether the local was introduced as a raw pointer to a static.
pub is_static_ptr: bool,
/// Whether the local should be ignored for trait bound computations.
pub ignore_for_traits: bool,
}
/// The layout of generator state.

View file

@ -625,7 +625,7 @@ impl<'tcx> TyCtxt<'tcx> {
generator_layout
.field_tys
.iter()
.filter(|decl| !decl.is_static_ptr)
.filter(|decl| !decl.ignore_for_traits)
.map(|decl| ty::EarlyBinder(decl.ty))
}

View file

@ -1749,6 +1749,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let fake_borrow_ty = tcx.mk_imm_ref(tcx.lifetimes.re_erased, fake_borrow_deref_ty);
let mut fake_borrow_temp = LocalDecl::new(fake_borrow_ty, temp_span);
fake_borrow_temp.internal = self.local_decls[matched_place.local].internal;
fake_borrow_temp.local_info = Some(Box::new(LocalInfo::FakeBorrow));
let fake_borrow_temp = self.local_decls.push(fake_borrow_temp);
(matched_place, fake_borrow_temp)

View file

@ -879,7 +879,7 @@ fn sanitize_witness<'tcx>(
let mut mismatches = Vec::new();
for fty in &layout.field_tys {
if fty.is_static_ptr {
if fty.ignore_for_traits {
continue;
}
let decl_ty = tcx.normalize_erasing_regions(param_env, fty.ty);
@ -904,6 +904,7 @@ fn sanitize_witness<'tcx>(
}
fn compute_layout<'tcx>(
tcx: TyCtxt<'tcx>,
liveness: LivenessInfo,
body: &Body<'tcx>,
) -> (
@ -923,15 +924,33 @@ fn compute_layout<'tcx>(
let mut locals = IndexVec::<GeneratorSavedLocal, _>::new();
let mut tys = IndexVec::<GeneratorSavedLocal, _>::new();
for (saved_local, local) in saved_locals.iter_enumerated() {
debug!("generator saved local {:?} => {:?}", saved_local, local);
locals.push(local);
let decl = &body.local_decls[local];
let decl = GeneratorSavedTy {
ty: decl.ty,
source_info: decl.source_info,
is_static_ptr: decl.internal,
debug!(?decl);
let ignore_for_traits = if tcx.sess.opts.unstable_opts.drop_tracking_mir {
match decl.local_info {
// Do not include raw pointers created from accessing `static` items, as those could
// well be re-created by another access to the same static.
Some(box LocalInfo::StaticRef { is_thread_local, .. }) => !is_thread_local,
// Fake borrows are only read by fake reads, so do not have any reality in
// post-analysis MIR.
Some(box LocalInfo::FakeBorrow) => true,
_ => false,
}
} else {
// FIXME(#105084) HIR-based drop tracking does not account for all the temporaries that
// MIR building may introduce. This leads to wrongly ignored types, but this is
// necessary for internal consistency and to avoid ICEs.
decl.internal
};
let decl =
GeneratorSavedTy { ty: decl.ty, source_info: decl.source_info, ignore_for_traits };
debug!(?decl);
tys.push(decl);
debug!("generator saved local {:?} => {:?}", saved_local, local);
}
// Leave empty variants for the UNRESUMED, RETURNED, and POISONED states.
@ -1401,7 +1420,7 @@ pub(crate) fn mir_generator_witnesses<'tcx>(
// Extract locals which are live across suspension point into `layout`
// `remap` gives a mapping from local indices onto generator struct indices
// `storage_liveness` tells us which locals have live storage at suspension points
let (_, generator_layout, _) = compute_layout(liveness_info, body);
let (_, generator_layout, _) = compute_layout(tcx, liveness_info, body);
if tcx.sess.opts.unstable_opts.drop_tracking_mir {
check_suspend_tys(tcx, &generator_layout, &body);
@ -1503,7 +1522,7 @@ impl<'tcx> MirPass<'tcx> for StateTransform {
// Extract locals which are live across suspension point into `layout`
// `remap` gives a mapping from local indices onto generator struct indices
// `storage_liveness` tells us which locals have live storage at suspension points
let (remap, layout, storage_liveness) = compute_layout(liveness_info, body);
let (remap, layout, storage_liveness) = compute_layout(tcx, liveness_info, body);
let can_return = can_return(tcx, body, tcx.param_env(body.source.def_id()));
@ -1700,7 +1719,7 @@ fn check_suspend_tys<'tcx>(tcx: TyCtxt<'tcx>, layout: &GeneratorLayout<'tcx>, bo
let decl = &layout.field_tys[local];
debug!(?decl);
if !decl.is_static_ptr && linted_tys.insert(decl.ty) {
if !decl.ignore_for_traits && linted_tys.insert(decl.ty) {
let Some(hir_id) = decl.source_info.scope.lint_root(&body.source_scopes) else { continue };
check_must_not_suspend_ty(

View file

@ -2389,7 +2389,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
for &local in variant {
let decl = &generator_info.field_tys[local];
debug!(?decl);
if ty_matches(ty::Binder::dummy(decl.ty)) && !decl.is_static_ptr {
if ty_matches(ty::Binder::dummy(decl.ty)) && !decl.ignore_for_traits {
interior_or_upvar_span = Some(GeneratorInteriorOrUpvar::Interior(
decl.source_info.span,
Some((None, source_info.span, None, from_awaited_ty)),

View file

@ -0,0 +1,51 @@
error[E0382]: borrow of moved value: `g`
--> $DIR/issue-105084.rs:44:14
|
LL | let mut g = || {
| ----- move occurs because `g` has type `[generator@$DIR/issue-105084.rs:22:17: 22:19]`, which does not implement the `Copy` trait
...
LL | let mut h = copy(g);
| - value moved here
...
LL | Pin::new(&mut g).resume(());
| ^^^^^^ value borrowed here after move
|
note: consider changing this parameter type in function `copy` to borrow instead if owning the value isn't necessary
--> $DIR/issue-105084.rs:17:21
|
LL | fn copy<T: Copy>(x: T) -> T {
| ---- ^ this parameter takes ownership of the value
| |
| in this function
help: consider cloning the value if the performance cost is acceptable
|
LL | let mut h = copy(g.clone());
| ++++++++
error[E0277]: the trait bound `Box<(i32, ())>: Copy` is not satisfied in `[generator@$DIR/issue-105084.rs:22:17: 22:19]`
--> $DIR/issue-105084.rs:38:17
|
LL | let mut g = || {
| -- within this `[generator@$DIR/issue-105084.rs:22:17: 22:19]`
...
LL | let mut h = copy(g);
| ^^^^ within `[generator@$DIR/issue-105084.rs:22:17: 22:19]`, the trait `Copy` is not implemented for `Box<(i32, ())>`
|
note: generator does not implement `Copy` as this value is used across a yield
--> $DIR/issue-105084.rs:28:25
|
LL | let t = box (5, yield);
| --------^^^^^-
| | |
| | yield occurs here, with `box (5, yield)` maybe used later
| has type `Box<(i32, ())>` which does not implement `Copy`
note: required by a bound in `copy`
--> $DIR/issue-105084.rs:17:12
|
LL | fn copy<T: Copy>(x: T) -> T {
| ^^^^ required by this bound in `copy`
error: aborting due to 2 previous errors
Some errors have detailed explanations: E0277, E0382.
For more information about an error, try `rustc --explain E0277`.

View file

@ -0,0 +1,49 @@
// revisions: no_drop_tracking drop_tracking drop_tracking_mir
// [drop_tracking] compile-flags: -Zdrop-tracking
// [drop_tracking_mir] compile-flags: -Zdrop-tracking-mir
// [no_drop_tracking] known-bug: #105084
// [no_drop_tracking] check-pass
// [drop_tracking] known-bug: #105084
// [drop_tracking] check-pass
#![feature(generators)]
#![feature(generator_clone)]
#![feature(generator_trait)]
#![feature(box_syntax)]
use std::ops::Generator;
use std::pin::Pin;
fn copy<T: Copy>(x: T) -> T {
x
}
fn main() {
let mut g = || {
// This is desuraged as 4 stages:
// - allocate a `*mut u8` with `exchange_malloc`;
// - create a Box that is ignored for trait computations;
// - compute fields (and yields);
// - assign to `t`.
let t = box (5, yield);
drop(t);
};
// Allocate the temporary box.
Pin::new(&mut g).resume(());
// The temporary box is in generator locals.
// As it is not taken into account for trait computation,
// the generator is `Copy`.
let mut h = copy(g);
//[drop_tracking_mir]~^ ERROR the trait bound `Box<(i32, ())>: Copy` is not satisfied in
// We now have 2 boxes with the same backing allocation:
// one inside `g` and one inside `h`.
// Proceed and drop `t` in `g`.
Pin::new(&mut g).resume(());
//[drop_tracking_mir]~^ ERROR borrow of moved value: `g`
// Proceed and drop `t` in `h` -> double free!
Pin::new(&mut h).resume(());
}