Skip to content

Commit

Permalink
Auto merge of #116940 - matthiaskrgr:rollup-25ezp8a, r=matthiaskrgr
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bors committed Oct 19, 2023
2 parents 3fbcfd2 + e8544f8 commit c104861
Show file tree
Hide file tree
Showing 13 changed files with 110 additions and 49 deletions.
10 changes: 5 additions & 5 deletions Cargo.lock
Expand Up @@ -3839,7 +3839,7 @@ dependencies = [

[[package]]
name = "rustc_fluent_macro"
version = "0.1.0"
version = "0.0.0"
dependencies = [
"annotate-snippets",
"fluent-bundle",
Expand Down Expand Up @@ -3915,7 +3915,7 @@ dependencies = [

[[package]]
name = "rustc_hir_typeck"
version = "0.1.0"
version = "0.0.0"
dependencies = [
"rustc_ast",
"rustc_attr",
Expand Down Expand Up @@ -4043,7 +4043,7 @@ dependencies = [

[[package]]
name = "rustc_lexer"
version = "0.1.0"
version = "0.0.0"
dependencies = [
"expect-test",
"unicode-properties",
Expand Down Expand Up @@ -4112,7 +4112,7 @@ dependencies = [

[[package]]
name = "rustc_macros"
version = "0.1.0"
version = "0.0.0"
dependencies = [
"proc-macro2",
"quote",
Expand Down Expand Up @@ -4595,7 +4595,7 @@ dependencies = [

[[package]]
name = "rustc_transmute"
version = "0.1.0"
version = "0.0.0"
dependencies = [
"itertools",
"rustc_data_structures",
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_const_eval/src/interpret/memory.rs
Expand Up @@ -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>>,
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion 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]
Expand Down
2 changes: 1 addition & 1 deletion 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
Expand Down
2 changes: 1 addition & 1 deletion 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"

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_macros/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "rustc_macros"
version = "0.1.0"
version = "0.0.0"
edition = "2021"

[lib]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/lib.rs
Expand Up @@ -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()));
}
}
Expand Down
33 changes: 21 additions & 12 deletions compiler/rustc_mir_transform/src/pass_manager.rs
Expand Up @@ -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>,
Expand All @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion 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
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/build_steps/setup.rs
Expand Up @@ -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)"
)
}

Expand Down
37 changes: 18 additions & 19 deletions src/tools/miri/src/intptrcast.rs
Expand Up @@ -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.
Expand All @@ -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)))
}

Expand Down Expand Up @@ -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)> {
Expand All @@ -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),
))
}

Expand Down
36 changes: 30 additions & 6 deletions src/tools/miri/src/machine.rs
Expand Up @@ -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,
}

Expand Down Expand Up @@ -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>,
Expand All @@ -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>,
Expand All @@ -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 {
Expand Down
19 changes: 19 additions & 0 deletions 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);

0 comments on commit c104861

Please sign in to comment.