From cb2c7bb833f439c224457442c83c3dfcba1709d6 Mon Sep 17 00:00:00 2001 From: Jakob Degen Date: Wed, 21 Dec 2022 10:30:38 -0800 Subject: [PATCH 1/2] Clarify that raw retags are not permitted in Mir --- compiler/rustc_const_eval/src/transform/validate.rs | 9 ++++++--- compiler/rustc_middle/src/mir/syntax.rs | 7 +++++-- .../src/build/custom/parse/instruction.rs | 3 --- library/core/src/intrinsics/mir.rs | 1 - .../building/custom/references.immut_ref.built.after.mir | 7 +++---- .../building/custom/references.mut_ref.built.after.mir | 7 +++---- src/test/mir-opt/building/custom/references.rs | 2 -- 7 files changed, 17 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index bb897b95b2c53..eb247a4cebe4b 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -9,8 +9,8 @@ use rustc_middle::mir::visit::{PlaceContext, Visitor}; use rustc_middle::mir::{ traversal, AggregateKind, BasicBlock, BinOp, Body, BorrowKind, CastKind, CopyNonOverlapping, Local, Location, MirPass, MirPhase, NonDivergingIntrinsic, Operand, Place, PlaceElem, PlaceRef, - ProjectionElem, RuntimePhase, Rvalue, SourceScope, Statement, StatementKind, Terminator, - TerminatorKind, UnOp, START_BLOCK, + ProjectionElem, RetagKind, RuntimePhase, Rvalue, SourceScope, Statement, StatementKind, + Terminator, TerminatorKind, UnOp, START_BLOCK, }; use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeVisitable}; use rustc_mir_dataflow::impls::MaybeStorageLive; @@ -667,10 +667,13 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { self.fail(location, "`Deinit`is not allowed until deaggregation"); } } - StatementKind::Retag(_, _) => { + StatementKind::Retag(kind, _) => { // FIXME(JakobDegen) The validator should check that `self.mir_phase < // DropsLowered`. However, this causes ICEs with generation of drop shims, which // seem to fail to set their `MirPhase` correctly. + if *kind == RetagKind::Raw { + self.fail(location, "explicit `RetagKind::Raw` is forbidden"); + } } StatementKind::StorageLive(..) | StatementKind::StorageDead(..) diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 99e59c770d754..bbb8148f5bc16 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -320,8 +320,11 @@ pub enum StatementKind<'tcx> { /// for /// more details. /// - /// For code that is not specific to stacked borrows, you should consider retags to read - /// and modify the place in an opaque way. + /// For code that is not specific to stacked borrows, you should consider retags to read and + /// modify the place in an opaque way. + /// + /// Explicit `RetagKind::Raw` is not permitted - it is implicit as a part of + /// `Rvalue::AddressOf`. Retag(RetagKind, Box>), /// Encodes a user's type ascription. These need to be preserved diff --git a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs index 7c39a93a8eb9e..dca4906c07de5 100644 --- a/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs +++ b/compiler/rustc_mir_build/src/build/custom/parse/instruction.rs @@ -15,9 +15,6 @@ impl<'tcx, 'body> ParseCtxt<'tcx, 'body> { @call("mir_retag", args) => { Ok(StatementKind::Retag(RetagKind::Default, Box::new(self.parse_place(args[0])?))) }, - @call("mir_retag_raw", args) => { - Ok(StatementKind::Retag(RetagKind::Raw, Box::new(self.parse_place(args[0])?))) - }, @call("mir_set_discriminant", args) => { let place = self.parse_place(args[0])?; let var = self.parse_integer_literal(args[1])? as u32; diff --git a/library/core/src/intrinsics/mir.rs b/library/core/src/intrinsics/mir.rs index e08a15571fcbe..9e7099ddfd1e1 100644 --- a/library/core/src/intrinsics/mir.rs +++ b/library/core/src/intrinsics/mir.rs @@ -259,7 +259,6 @@ define!("mir_drop", fn Drop(place: T, goto: BasicBlock)); define!("mir_drop_and_replace", fn DropAndReplace(place: T, value: T, goto: BasicBlock)); define!("mir_call", fn Call(place: T, goto: BasicBlock, call: T)); define!("mir_retag", fn Retag(place: T)); -define!("mir_retag_raw", fn RetagRaw(place: T)); define!("mir_move", fn Move(place: T) -> T); define!("mir_static", fn Static(s: T) -> &'static T); define!("mir_static_mut", fn StaticMut(s: T) -> *mut T); diff --git a/src/test/mir-opt/building/custom/references.immut_ref.built.after.mir b/src/test/mir-opt/building/custom/references.immut_ref.built.after.mir index 4d38d45c0f479..f5ee112623541 100644 --- a/src/test/mir-opt/building/custom/references.immut_ref.built.after.mir +++ b/src/test/mir-opt/building/custom/references.immut_ref.built.after.mir @@ -6,9 +6,8 @@ fn immut_ref(_1: &i32) -> &i32 { bb0: { _2 = &raw const (*_1); // scope 0 at $DIR/references.rs:+5:13: +5:29 - Retag([raw] _2); // scope 0 at $DIR/references.rs:+6:13: +6:24 - _0 = &(*_2); // scope 0 at $DIR/references.rs:+7:13: +7:23 - Retag(_0); // scope 0 at $DIR/references.rs:+8:13: +8:23 - return; // scope 0 at $DIR/references.rs:+9:13: +9:21 + _0 = &(*_2); // scope 0 at $DIR/references.rs:+6:13: +6:23 + Retag(_0); // scope 0 at $DIR/references.rs:+7:13: +7:23 + return; // scope 0 at $DIR/references.rs:+8:13: +8:21 } } diff --git a/src/test/mir-opt/building/custom/references.mut_ref.built.after.mir b/src/test/mir-opt/building/custom/references.mut_ref.built.after.mir index 01bc8a9cd3580..8e2ffc33b1a0a 100644 --- a/src/test/mir-opt/building/custom/references.mut_ref.built.after.mir +++ b/src/test/mir-opt/building/custom/references.mut_ref.built.after.mir @@ -6,9 +6,8 @@ fn mut_ref(_1: &mut i32) -> &mut i32 { bb0: { _2 = &raw mut (*_1); // scope 0 at $DIR/references.rs:+5:13: +5:33 - Retag([raw] _2); // scope 0 at $DIR/references.rs:+6:13: +6:24 - _0 = &mut (*_2); // scope 0 at $DIR/references.rs:+7:13: +7:26 - Retag(_0); // scope 0 at $DIR/references.rs:+8:13: +8:23 - return; // scope 0 at $DIR/references.rs:+9:13: +9:21 + _0 = &mut (*_2); // scope 0 at $DIR/references.rs:+6:13: +6:26 + Retag(_0); // scope 0 at $DIR/references.rs:+7:13: +7:23 + return; // scope 0 at $DIR/references.rs:+8:13: +8:21 } } diff --git a/src/test/mir-opt/building/custom/references.rs b/src/test/mir-opt/building/custom/references.rs index c93f6ec624b35..a1c896de04cf0 100644 --- a/src/test/mir-opt/building/custom/references.rs +++ b/src/test/mir-opt/building/custom/references.rs @@ -12,7 +12,6 @@ pub fn mut_ref(x: &mut i32) -> &mut i32 { { t = addr_of_mut!(*x); - RetagRaw(t); RET = &mut *t; Retag(RET); Return() @@ -28,7 +27,6 @@ pub fn immut_ref(x: &i32) -> &i32 { { t = addr_of!(*x); - RetagRaw(t); RET = & *t; Retag(RET); Return() From 7c4c62047504aff1e17d4b2686116fb4de04797c Mon Sep 17 00:00:00 2001 From: Jakob Degen Date: Wed, 21 Dec 2022 11:46:13 -0800 Subject: [PATCH 2/2] Forbid `RetagKind::TwoPhase` as well --- compiler/rustc_const_eval/src/transform/validate.rs | 4 ++-- compiler/rustc_middle/src/mir/syntax.rs | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index eb247a4cebe4b..94e1b95a0eb3c 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -671,8 +671,8 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { // FIXME(JakobDegen) The validator should check that `self.mir_phase < // DropsLowered`. However, this causes ICEs with generation of drop shims, which // seem to fail to set their `MirPhase` correctly. - if *kind == RetagKind::Raw { - self.fail(location, "explicit `RetagKind::Raw` is forbidden"); + if *kind == RetagKind::Raw || *kind == RetagKind::TwoPhase { + self.fail(location, format!("explicit `{:?}` is forbidden", kind)); } } StatementKind::StorageLive(..) diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index bbb8148f5bc16..bb03359b138f3 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -323,8 +323,7 @@ pub enum StatementKind<'tcx> { /// For code that is not specific to stacked borrows, you should consider retags to read and /// modify the place in an opaque way. /// - /// Explicit `RetagKind::Raw` is not permitted - it is implicit as a part of - /// `Rvalue::AddressOf`. + /// Only `RetagKind::Default` and `RetagKind::FnEntry` are permitted. Retag(RetagKind, Box>), /// Encodes a user's type ascription. These need to be preserved