fix array drop glue: properly turn raw ptr into reference

[RalfJung]

Discovered while working on #53424: The generated drop glue uses an assignment ptr = cur where ptr is a reference and cur a raw pointer. This is not well-formed MIR.

Do we have MIR sanity checks that run on the drop glue and should have caught this?

r? @eddyb

[eddyb]

cc @arielb1

@bors r+

[bors]

📌 Commit 0447b50 has been approved by eddyb

[arielb1]

I'm not sure how legitimate it is to call drop on an &mut T (maybe it is with your semantics?). If that is a problem, I would prefer the type to be *mut T everywhere.

[RalfJung]

I do not understand -- drop takes &mut...

[RalfJung]

Also, I couldn't actually figure out how ptr is used later. Not sure if I have been looking at the wrong places or if the comments showing the MIR syntax being generated are just missing? #53532 does not help either.

[matthewjasper]

ptr is used here:

rust/src/librustc_mir/util/elaborate_drops.rs

Lines 632 to 636 in 1558ae7

	self.elaborator.patch().patch_terminator(drop_block, TerminatorKind::Drop {
	location: ptr.clone().deref(),
	target: loop_block,
	unwind: unwind.into_option()
	});

The mir that you want to look at is probably the one generated here:

rust/src/librustc_mir/shim.rs

Lines 160 to 236 in 6a3db03

	fn build_drop_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
	def_id: DefId,
	ty: Option<Ty<'tcx>>)
	-> Mir<'tcx>
	{
	debug!("build_drop_shim(def_id={:?}, ty={:?})", def_id, ty);
	// Check if this is a generator, if so, return the drop glue for it
	if let Some(&ty::TyS { sty: ty::TyGenerator(gen_def_id, substs, _), .. }) = ty {
	let mir = &**tcx.optimized_mir(gen_def_id).generator_drop.as_ref().unwrap();
	return mir.subst(tcx, substs.substs);
	}
	let substs = if let Some(ty) = ty {
	tcx.intern_substs(&[ty.into()])
	} else {
	Substs::identity_for_item(tcx, def_id)
	};
	let sig = tcx.fn_sig(def_id).subst(tcx, substs);
	let sig = tcx.erase_late_bound_regions(&sig);
	let span = tcx.def_span(def_id);
	let source_info = SourceInfo { span, scope: OUTERMOST_SOURCE_SCOPE };
	let return_block = BasicBlock::new(1);
	let mut blocks = IndexVec::new();
	let block = |blocks: &mut IndexVec<_, _>, kind| {
	blocks.push(BasicBlockData {
	statements: vec![],
	terminator: Some(Terminator { source_info, kind }),
	is_cleanup: false
	})
	};
	block(&mut blocks, TerminatorKind::Goto { target: return_block });
	block(&mut blocks, TerminatorKind::Return);
	let mut mir = Mir::new(
	blocks,
	IndexVec::from_elem_n(
	SourceScopeData { span: span, parent_scope: None }, 1
	),
	ClearCrossCrate::Clear,
	IndexVec::new(),
	None,
	local_decls_for_sig(&sig, span),
	sig.inputs().len(),
	vec![],
	span
	);
	if let Some(..) = ty {
	let patch = {
	let param_env = tcx.param_env(def_id).with_reveal_all();
	let mut elaborator = DropShimElaborator {
	mir: &mir,
	patch: MirPatch::new(&mir),
	tcx,
	param_env
	};
	let dropee = Place::Local(Local::new(1+0)).deref();
	let resume_block = elaborator.patch.resume_block();
	elaborate_drops::elaborate_drop(
	&mut elaborator,
	source_info,
	&dropee,
	(),
	return_block,
	elaborate_drops::Unwind::To(resume_block),
	START_BLOCK
	);
	elaborator.patch
	};
	patch.apply(&mut mir);
	}
	mir
	}

[RalfJung]

Ah so it moves on to the Drop terminator, which takes any place -- so we could just as well deref a raw pointer?

EDIT: OTOH, I do not see why. If the reference is valid enough for Drop to be called, it better be valid enough for &mut.

[bors]

⌛ Testing commit 0447b50 with merge c24f27c...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing c24f27c to master...