Skip to content

Commit

Permalink
Rollup merge of #134191 - willcrichton:dev, r=RalfJung,lqd
Browse files Browse the repository at this point in the history
Make some types and methods related to Polonius + Miri public

We have a tool, [Aquascope](https://github.com/cognitive-engineering-lab/aquascope/), which uses Polonius and Miri to visualize the compile-time and run-time semantics of a Rust program. Changes in the last few months to both APIs have hidden away details we depend upon. This PR re-exposes some of those details, specifically:

**Polonius:**
- `BorrowSet` and `BorrowData` are added to `rustc_borrowck::consumers`, and their fields are made `pub` instead of `pub(crate)`. We need this to interpret the `BorrowIndex`es generated by Polonius.
- `BorrowSet::build` is now `pub`. We need this because the borrowck API doesn't provide access to the `BorrowSet` constructed during checking.
- `PoloniusRegionVid` is added to `rustc_borrowck::consumers`. We need this because it's also contained in the Polonius facts.

**Miri:**
- `InterpCx::local_to_op` is now a special case of `local_at_frame_to_op`, which allows querying locals in any frame. We need this because we walk the whole stack at each step to collect the state of memory.
- `InterpCx::layout_of_local` is now `pub`. We need this because we need to know the layout of every local at each step.

If these changes go against some design goal for keeping certain types private, please let me know so we can hash out a better solution. Additionally, if there's a better way to document that it's important that certain types stay public, also let me know. For example, `BorrowSet` was previously public but was hidden in 6676cec, breaking our build.

cc ```@RalfJung``` ```@nnethercote``` ```@gavinleroy```
  • Loading branch information
matthiaskrgr authored Dec 14, 2024
2 parents 1c24da6 + 4d5d470 commit fffdb1b
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 18 deletions.
50 changes: 48 additions & 2 deletions compiler/rustc_borrowck/src/borrow_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,25 @@ pub struct BorrowSet<'tcx> {
pub(crate) locals_state_at_exit: LocalsStateAtExit,
}

// These methods are public to support borrowck consumers.
impl<'tcx> BorrowSet<'tcx> {
pub fn location_map(&self) -> &FxIndexMap<Location, BorrowData<'tcx>> {
&self.location_map
}

pub fn activation_map(&self) -> &FxIndexMap<Location, Vec<BorrowIndex>> {
&self.activation_map
}

pub fn local_map(&self) -> &FxIndexMap<mir::Local, FxIndexSet<BorrowIndex>> {
&self.local_map
}

pub fn locals_state_at_exit(&self) -> &LocalsStateAtExit {
&self.locals_state_at_exit
}
}

impl<'tcx> Index<BorrowIndex> for BorrowSet<'tcx> {
type Output = BorrowData<'tcx>;

Expand All @@ -45,7 +64,7 @@ impl<'tcx> Index<BorrowIndex> for BorrowSet<'tcx> {
/// Location where a two-phase borrow is activated, if a borrow
/// is in fact a two-phase borrow.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub(crate) enum TwoPhaseActivation {
pub enum TwoPhaseActivation {
NotTwoPhase,
NotActivated,
ActivatedAt(Location),
Expand All @@ -68,6 +87,33 @@ pub struct BorrowData<'tcx> {
pub(crate) assigned_place: mir::Place<'tcx>,
}

// These methods are public to support borrowck consumers.
impl<'tcx> BorrowData<'tcx> {
pub fn reserve_location(&self) -> Location {
self.reserve_location
}

pub fn activation_location(&self) -> TwoPhaseActivation {
self.activation_location
}

pub fn kind(&self) -> mir::BorrowKind {
self.kind
}

pub fn region(&self) -> RegionVid {
self.region
}

pub fn borrowed_place(&self) -> mir::Place<'tcx> {
self.borrowed_place
}

pub fn assigned_place(&self) -> mir::Place<'tcx> {
self.assigned_place
}
}

impl<'tcx> fmt::Display for BorrowData<'tcx> {
fn fmt(&self, w: &mut fmt::Formatter<'_>) -> fmt::Result {
let kind = match self.kind {
Expand Down Expand Up @@ -120,7 +166,7 @@ impl LocalsStateAtExit {
}

impl<'tcx> BorrowSet<'tcx> {
pub(crate) fn build(
pub fn build(
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
locals_are_invalidated_at_exit: bool,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/consumers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ use rustc_index::{IndexSlice, IndexVec};
use rustc_middle::mir::{Body, Promoted};
use rustc_middle::ty::TyCtxt;

pub use super::borrow_set::{BorrowData, BorrowSet, TwoPhaseActivation};
pub use super::constraints::OutlivesConstraint;
pub use super::dataflow::{BorrowIndex, Borrows, calculate_borrows_out_of_scope_at_location};
pub use super::facts::{AllFacts as PoloniusInput, RustcFacts};
pub use super::facts::{AllFacts as PoloniusInput, PoloniusRegionVid, RustcFacts};
pub use super::location::{LocationTable, RichLocation};
pub use super::nll::PoloniusOutput;
pub use super::place_ext::PlaceExt;
pub use super::places_conflict::{PlaceConflictBias, places_conflict};
pub use super::region_infer::RegionInferenceContext;
use crate::borrow_set::BorrowSet;

/// Options determining the output behavior of [`get_body_with_borrowck_facts`].
///
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,10 +540,14 @@ pub trait Machine<'tcx>: Sized {
interp_ok(ReturnAction::Normal)
}

/// Called immediately after an "immediate" local variable is read
/// Called immediately after an "immediate" local variable is read in a given frame
/// (i.e., this is called for reads that do not end up accessing addressable memory).
#[inline(always)]
fn after_local_read(_ecx: &InterpCx<'tcx, Self>, _local: mir::Local) -> InterpResult<'tcx> {
fn after_local_read(
_ecx: &InterpCx<'tcx, Self>,
_frame: &Frame<'tcx, Self::Provenance, Self::FrameExtra>,
_local: mir::Local,
) -> InterpResult<'tcx> {
interp_ok(())
}

Expand Down
27 changes: 18 additions & 9 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ use rustc_middle::{bug, mir, span_bug, ty};
use tracing::trace;

use super::{
CtfeProvenance, InterpCx, InterpResult, MPlaceTy, Machine, MemPlace, MemPlaceMeta, OffsetMode,
PlaceTy, Pointer, Projectable, Provenance, Scalar, alloc_range, err_ub, from_known_layout,
interp_ok, mir_assign_valid_types, throw_ub,
CtfeProvenance, Frame, InterpCx, InterpResult, MPlaceTy, Machine, MemPlace, MemPlaceMeta,
OffsetMode, PlaceTy, Pointer, Projectable, Provenance, Scalar, alloc_range, err_ub,
from_known_layout, interp_ok, mir_assign_valid_types, throw_ub,
};

/// An `Immediate` represents a single immediate self-contained Rust value.
Expand Down Expand Up @@ -708,23 +708,32 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
interp_ok(str)
}

/// Read from a local of the current frame.
/// Read from a local of the current frame. Convenience method for [`InterpCx::local_at_frame_to_op`].
pub fn local_to_op(
&self,
local: mir::Local,
layout: Option<TyAndLayout<'tcx>>,
) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
self.local_at_frame_to_op(self.frame(), local, layout)
}

/// Read from a local of a given frame.
/// Will not access memory, instead an indirect `Operand` is returned.
///
/// This is public because it is used by [priroda](https://github.com/oli-obk/priroda) to get an
/// OpTy from a local.
pub fn local_to_op(
/// This is public because it is used by [Aquascope](https://github.com/cognitive-engineering-lab/aquascope/)
/// to get an OpTy from a local.
pub fn local_at_frame_to_op(
&self,
frame: &Frame<'tcx, M::Provenance, M::FrameExtra>,
local: mir::Local,
layout: Option<TyAndLayout<'tcx>>,
) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
let frame = self.frame();
let layout = self.layout_of_local(frame, local, layout)?;
let op = *frame.locals[local].access()?;
if matches!(op, Operand::Immediate(_)) {
assert!(!layout.is_unsized());
}
M::after_local_read(self, local)?;
M::after_local_read(self, frame, local)?;
interp_ok(OpTy { op, layout })
}

Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_const_eval/src/interpret/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,8 +584,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
interp_ok(())
}

/// This is public because it is used by [Aquascope](https://github.com/cognitive-engineering-lab/aquascope/)
/// to analyze all the locals in a stack frame.
#[inline(always)]
pub(super) fn layout_of_local(
pub fn layout_of_local(
&self,
frame: &Frame<'tcx, M::Provenance, M::FrameExtra>,
local: mir::Local,
Expand Down
8 changes: 6 additions & 2 deletions src/tools/miri/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1571,8 +1571,12 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
res
}

fn after_local_read(ecx: &InterpCx<'tcx, Self>, local: mir::Local) -> InterpResult<'tcx> {
if let Some(data_race) = &ecx.frame().extra.data_race {
fn after_local_read(
ecx: &InterpCx<'tcx, Self>,
frame: &Frame<'tcx, Provenance, FrameExtra<'tcx>>,
local: mir::Local,
) -> InterpResult<'tcx> {
if let Some(data_race) = &frame.extra.data_race {
data_race.local_read(local, &ecx.machine);
}
interp_ok(())
Expand Down

0 comments on commit fffdb1b

Please sign in to comment.