Auto merge of #116940 - matthiaskrgr:rollup-25ezp8a, r=matthiaskrgr

bors · bors · commit c104861b7b51 · 2023-10-19T13:25:39.000Z
Rollup of 4 pull requests Successful merges: - #116650 (add some comments and some cleanup around Miri intptrcast) - #116896 (Only check in a single place if a pass is enabled.) - #116906 (Use v0.0.0 in compiler crates) - #116921 (fix(bootstrap) info message show correct path now) r? `@ghost` `@rustbot` modify labels: rollup
diff --git a/Cargo.lock b/Cargo.lock
@@ -3839,7 +3839,7 @@ dependencies = [
 
 [[package]]
 name = "rustc_fluent_macro"
-version = "0.1.0"
+version = "0.0.0"
 dependencies = [
  "annotate-snippets",
  "fluent-bundle",
@@ -3915,7 +3915,7 @@ dependencies = [
 
 [[package]]
 name = "rustc_hir_typeck"
-version = "0.1.0"
+version = "0.0.0"
 dependencies = [
  "rustc_ast",
  "rustc_attr",
@@ -4043,7 +4043,7 @@ dependencies = [
 
 [[package]]
 name = "rustc_lexer"
-version = "0.1.0"
+version = "0.0.0"
 dependencies = [
  "expect-test",
  "unicode-properties",
@@ -4112,7 +4112,7 @@ dependencies = [
 
 [[package]]
 name = "rustc_macros"
-version = "0.1.0"
+version = "0.0.0"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -4595,7 +4595,7 @@ dependencies = [
 
 [[package]]
 name = "rustc_transmute"
-version = "0.1.0"
+version = "0.0.0"
 dependencies = [
  "itertools",
  "rustc_data_structures",
diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs
@@ -1235,6 +1235,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
     /// Turning a "maybe pointer" into a proper pointer (and some information
     /// about where it points), or an absolute address.
+    ///
+    /// The result must be used immediately; it is not allowed to convert
+    /// the returned data back into a `Pointer` and store that in machine state.
+    /// (In fact that's not even possible since `M::ProvenanceExtra` is generic and
+    /// we don't have an operation to turn it back into `M::Provenance`.)
     pub fn ptr_try_get_alloc_id(
         &self,
         ptr: Pointer<Option<M::Provenance>>,
@@ -1253,6 +1258,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     }
 
     /// Turning a "maybe pointer" into a proper pointer (and some information about where it points).
+    ///
+    /// The result must be used immediately; it is not allowed to convert
+    /// the returned data back into a `Pointer` and store that in machine state.
+    /// (In fact that's not even possible since `M::ProvenanceExtra` is generic and
+    /// we don't have an operation to turn it back into `M::Provenance`.)
     #[inline(always)]
     pub fn ptr_get_alloc_id(
         &self,
diff --git a/compiler/rustc_fluent_macro/Cargo.toml b/compiler/rustc_fluent_macro/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "rustc_fluent_macro"
-version = "0.1.0"
+version = "0.0.0"
 edition = "2021"
 
 [lib]
diff --git a/compiler/rustc_hir_typeck/Cargo.toml b/compiler/rustc_hir_typeck/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "rustc_hir_typeck"
-version = "0.1.0"
+version = "0.0.0"
 edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
diff --git a/compiler/rustc_lexer/Cargo.toml b/compiler/rustc_lexer/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "rustc_lexer"
-version = "0.1.0"
+version = "0.0.0"
 license = "MIT OR Apache-2.0"
 edition = "2021"
 
diff --git a/compiler/rustc_macros/Cargo.toml b/compiler/rustc_macros/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "rustc_macros"
-version = "0.1.0"
+version = "0.0.0"
 edition = "2021"
 
 [lib]
diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs
@@ -383,7 +383,7 @@ fn mir_drops_elaborated_and_const_checked(tcx: TyCtxt<'_>, def: LocalDefId) -> &
     let is_fn_like = tcx.def_kind(def).is_fn_like();
     if is_fn_like {
         // Do not compute the mir call graph without said call graph actually being used.
-        if inline::Inline.is_enabled(&tcx.sess) {
+        if pm::should_run_pass(tcx, &inline::Inline) {
             tcx.ensure_with_value().mir_inliner_callees(ty::InstanceDef::Item(def.to_def_id()));
         }
     }
diff --git a/compiler/rustc_mir_transform/src/pass_manager.rs b/compiler/rustc_mir_transform/src/pass_manager.rs
@@ -83,6 +83,25 @@ pub fn run_passes<'tcx>(
     run_passes_inner(tcx, body, passes, phase_change, true);
 }
 
+pub fn should_run_pass<'tcx, P>(tcx: TyCtxt<'tcx>, pass: &P) -> bool
+where
+    P: MirPass<'tcx> + ?Sized,
+{
+    let name = pass.name();
+
+    let overridden_passes = &tcx.sess.opts.unstable_opts.mir_enable_passes;
+    let overridden =
+        overridden_passes.iter().rev().find(|(s, _)| s == &*name).map(|(_name, polarity)| {
+            trace!(
+                pass = %name,
+                "{} as requested by flag",
+                if *polarity { "Running" } else { "Not running" },
+            );
+            *polarity
+        });
+    overridden.unwrap_or_else(|| pass.is_enabled(&tcx.sess))
+}
+
 fn run_passes_inner<'tcx>(
     tcx: TyCtxt<'tcx>,
     body: &mut Body<'tcx>,
@@ -100,19 +119,9 @@ fn run_passes_inner<'tcx>(
         for pass in passes {
             let name = pass.name();
 
-            let overridden = overridden_passes.iter().rev().find(|(s, _)| s == &*name).map(
-                |(_name, polarity)| {
-                    trace!(
-                        pass = %name,
-                        "{} as requested by flag",
-                        if *polarity { "Running" } else { "Not running" },
-                    );
-                    *polarity
-                },
-            );
-            if !overridden.unwrap_or_else(|| pass.is_enabled(&tcx.sess)) {
+            if !should_run_pass(tcx, *pass) {
                 continue;
-            }
+            };
 
             let dump_enabled = pass.is_mir_dump_enabled();
 
diff --git a/compiler/rustc_transmute/Cargo.toml b/compiler/rustc_transmute/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "rustc_transmute"
-version = "0.1.0"
+version = "0.0.0"
 edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
diff --git a/src/bootstrap/src/core/build_steps/setup.rs b/src/bootstrap/src/core/build_steps/setup.rs
@@ -183,7 +183,7 @@ pub fn setup(config: &Config, profile: Profile) {
         eprintln!();
         eprintln!(
             "note: the `tools` profile sets up the `stage2` toolchain (use \
-            `rustup toolchain link 'name' host/build/stage2` to use rustc)"
+            `rustup toolchain link 'name' build/host/stage2` to use rustc)"
         )
     }
 
diff --git a/src/tools/miri/src/intptrcast.rs b/src/tools/miri/src/intptrcast.rs
@@ -119,24 +119,14 @@ impl<'mir, 'tcx> GlobalStateInner {
         Ok(())
     }
 
-    pub fn ptr_from_addr_transmute(
-        _ecx: &MiriInterpCx<'mir, 'tcx>,
-        addr: u64,
-    ) -> Pointer<Option<Provenance>> {
-        trace!("Transmuting {:#x} to a pointer", addr);
-
-        // We consider transmuted pointers to be "invalid" (`None` provenance).
-        Pointer::new(None, Size::from_bytes(addr))
-    }
-
     pub fn ptr_from_addr_cast(
         ecx: &MiriInterpCx<'mir, 'tcx>,
         addr: u64,
     ) -> InterpResult<'tcx, Pointer<Option<Provenance>>> {
         trace!("Casting {:#x} to a pointer", addr);
 
+        // Potentially emit a warning.
         let global_state = ecx.machine.intptrcast.borrow();
-
         match global_state.provenance_mode {
             ProvenanceMode::Default => {
                 // The first time this happens at a particular location, print a warning.
@@ -158,7 +148,12 @@ impl<'mir, 'tcx> GlobalStateInner {
             ProvenanceMode::Permissive => {}
         }
 
-        // This is how wildcard pointers are born.
+        // We do *not* look up the `AllocId` here! This is a `ptr as usize` cast, and it is
+        // completely legal to do a cast and then `wrapping_offset` to another allocation and only
+        // *then* do a memory access. So the allocation that the pointer happens to point to on a
+        // cast is fairly irrelevant. Instead we generate this as a "wildcard" pointer, such that
+        // *every time the pointer is used*, we do an `AllocId` lookup to find the (exposed)
+        // allocation it might be referencing.
         Ok(Pointer::new(Some(Provenance::Wildcard), Size::from_bytes(addr)))
     }
 
@@ -219,22 +214,27 @@ impl<'mir, 'tcx> GlobalStateInner {
         })
     }
 
-    /// Convert a relative (tcx) pointer to an absolute address.
-    pub fn rel_ptr_to_addr(
+    /// Convert a relative (tcx) pointer to a Miri pointer.
+    pub fn ptr_from_rel_ptr(
         ecx: &MiriInterpCx<'mir, 'tcx>,
         ptr: Pointer<AllocId>,
-    ) -> InterpResult<'tcx, u64> {
+        tag: BorTag,
+    ) -> InterpResult<'tcx, Pointer<Provenance>> {
         let (alloc_id, offset) = ptr.into_parts(); // offset is relative (AllocId provenance)
         let base_addr = GlobalStateInner::alloc_base_addr(ecx, alloc_id)?;
 
         // Add offset with the right kind of pointer-overflowing arithmetic.
         let dl = ecx.data_layout();
-        Ok(dl.overflowing_offset(base_addr, offset.bytes()).0)
+        let absolute_addr = dl.overflowing_offset(base_addr, offset.bytes()).0;
+        Ok(Pointer::new(
+            Provenance::Concrete { alloc_id, tag },
+            Size::from_bytes(absolute_addr),
+        ))
     }
 
     /// When a pointer is used for a memory access, this computes where in which allocation the
     /// access is going.
-    pub fn abs_ptr_to_rel(
+    pub fn ptr_get_alloc(
         ecx: &MiriInterpCx<'mir, 'tcx>,
         ptr: Pointer<Provenance>,
     ) -> Option<(AllocId, Size)> {
@@ -252,12 +252,11 @@ impl<'mir, 'tcx> GlobalStateInner {
         let base_addr = GlobalStateInner::alloc_base_addr(ecx, alloc_id).unwrap();
 
         // Wrapping "addr - base_addr"
-        let dl = ecx.data_layout();
         #[allow(clippy::cast_possible_wrap)] // we want to wrap here
         let neg_base_addr = (base_addr as i64).wrapping_neg();
         Some((
             alloc_id,
-            Size::from_bytes(dl.overflowing_signed_offset(addr.bytes(), neg_base_addr).0),
+            Size::from_bytes(ecx.overflowing_signed_offset(addr.bytes(), neg_base_addr).0),
         ))
     }
 
diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs
@@ -168,11 +168,29 @@ impl fmt::Display for MiriMemoryKind {
 /// Pointer provenance.
 #[derive(Clone, Copy)]
 pub enum Provenance {
+    /// For pointers with concrete provenance. we exactly know which allocation they are attached to
+    /// and what their borrow tag is.
     Concrete {
         alloc_id: AllocId,
         /// Borrow Tracker tag.
         tag: BorTag,
     },
+    /// Pointers with wildcard provenance are created on int-to-ptr casts. According to the
+    /// specification, we should at that point angelically "guess" a provenance that will make all
+    /// future uses of this pointer work, if at all possible. Of course such a semantics cannot be
+    /// actually implemented in Miri. So instead, we approximate this, erroring on the side of
+    /// accepting too much code rather than rejecting correct code: a pointer with wildcard
+    /// provenance "acts like" any previously exposed pointer. Each time it is used, we check
+    /// whether *some* exposed pointer could have done what we want to do, and if the answer is yes
+    /// then we allow the access. This allows too much code in two ways:
+    /// - The same wildcard pointer can "take the role" of multiple different exposed pointers on
+    ///   subsequenct memory accesses.
+    /// - In the aliasing model, we don't just have to know the borrow tag of the pointer used for
+    ///   the access, we also have to update the aliasing state -- and that update can be very
+    ///   different depending on which borrow tag we pick! Stacked Borrows has support for this by
+    ///   switching to a stack that is only approximately known, i.e. we overapproximate the effect
+    ///   of using *any* exposed pointer for this access, and only keep information about the borrow
+    ///   stack that would be true with all possible choices.
     Wildcard,
 }
 
@@ -1122,19 +1140,16 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
                 _ => {}
             }
         }
-        let absolute_addr = intptrcast::GlobalStateInner::rel_ptr_to_addr(ecx, ptr)?;
         let tag = if let Some(borrow_tracker) = &ecx.machine.borrow_tracker {
             borrow_tracker.borrow_mut().base_ptr_tag(ptr.provenance, &ecx.machine)
         } else {
             // Value does not matter, SB is disabled
             BorTag::default()
         };
-        Ok(Pointer::new(
-            Provenance::Concrete { alloc_id: ptr.provenance, tag },
-            Size::from_bytes(absolute_addr),
-        ))
+        intptrcast::GlobalStateInner::ptr_from_rel_ptr(ecx, ptr, tag)
     }
 
+    /// Called on `usize as ptr` casts.
     #[inline(always)]
     fn ptr_from_addr_cast(
         ecx: &MiriInterpCx<'mir, 'tcx>,
@@ -1143,6 +1158,9 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
         intptrcast::GlobalStateInner::ptr_from_addr_cast(ecx, addr)
     }
 
+    /// Called on `ptr as usize` casts.
+    /// (Actually computing the resulting `usize` doesn't need machine help,
+    /// that's just `Scalar::try_to_int`.)
     fn expose_ptr(
         ecx: &mut InterpCx<'mir, 'tcx, Self>,
         ptr: Pointer<Self::Provenance>,
@@ -1160,11 +1178,17 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
 
     /// Convert a pointer with provenance into an allocation-offset pair,
     /// or a `None` with an absolute address if that conversion is not possible.
+    ///
+    /// This is called when a pointer is about to be used for memory access,
+    /// an in-bounds check, or anything else that requires knowing which allocation it points to.
+    /// The resulting `AllocId` will just be used for that one step and the forgotten again
+    /// (i.e., we'll never turn the data returned here back into a `Pointer` that might be
+    /// stored in machine state).
     fn ptr_get_alloc(
         ecx: &MiriInterpCx<'mir, 'tcx>,
         ptr: Pointer<Self::Provenance>,
     ) -> Option<(AllocId, Size, Self::ProvenanceExtra)> {
-        let rel = intptrcast::GlobalStateInner::abs_ptr_to_rel(ecx, ptr);
+        let rel = intptrcast::GlobalStateInner::ptr_get_alloc(ecx, ptr);
 
         rel.map(|(alloc_id, size)| {
             let tag = match ptr.provenance {
diff --git a/tests/mir-opt/inline/unit_test.rs b/tests/mir-opt/inline/unit_test.rs
@@ -0,0 +1,19 @@
+// Check that `-Zmir-enable-passes=+Inline` does not ICE because of stolen MIR.
+// unit-test: Inline
+// skip-filecheck
+#![crate_type = "lib"]
+
+// Randomize `def_path_hash` by defining them under a module with different names
+macro_rules! emit {
+    ($($m:ident)*) => {$(
+        pub mod $m {
+            pub fn main() {
+                let func = || 123u8;
+                func();
+            }
+        }
+    )*};
+}
+
+// Increase the chance of triggering the bug
+emit!(m00 m01 m02 m03 m04 m05 m06 m07 m08 m09 m10 m11 m12 m13 m14 m15 m16 m17 m18 m19);

Original file line number	Diff line number	Diff line change
`@@ -383,7 +383,7 @@ fn mir_drops_elaborated_and_const_checked(tcx: TyCtxt<'_>, def: LocalDefId) -> &`
`383`	`383`	`let is_fn_like = tcx.def_kind(def).is_fn_like();`
`384`	`384`	`if is_fn_like {`
`385`	`385`	`// Do not compute the mir call graph without said call graph actually being used.`
`386`		`- if inline::Inline.is_enabled(&tcx.sess) {`
	`386`	`+ if pm::should_run_pass(tcx, &inline::Inline) {`
`387`	`387`	`tcx.ensure_with_value().mir_inliner_callees(ty::InstanceDef::Item(def.to_def_id()));`
`388`	`388`	`}`
`389`	`389`	`}`
Original file line number	Diff line number	Diff line change
`@@ -183,7 +183,7 @@ pub fn setup(config: &Config, profile: Profile) {`
`183`	`183`	`eprintln!();`
`184`	`184`	`eprintln!(`
`185`	`185`	"note: the `tools` profile sets up the `stage2` toolchain (use \
`186`		- `rustup toolchain link 'name' host/build/stage2` to use rustc)"
	`186`	+ `rustup toolchain link 'name' build/host/stage2` to use rustc)"
`187`	`187`	`)`
`188`	`188`	`}`
`189`	`189`