From 0fdc07e197e3d0265452e266d038b089b69da6cb Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 3 Oct 2020 20:57:47 +0000 Subject: [PATCH 01/10] Impl StatementKind::CopyNonOverlapping --- compiler/rustc_codegen_ssa/src/mir/analyze.rs | 6 ++++-- .../rustc_codegen_ssa/src/mir/statement.rs | 15 +++++++++++++ compiler/rustc_middle/src/mir/mod.rs | 17 +++++++++++++++ compiler/rustc_middle/src/mir/visit.rs | 21 +++++++++++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs index 289629d921545..5ad9f46147282 100644 --- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs +++ b/compiler/rustc_codegen_ssa/src/mir/analyze.rs @@ -293,7 +293,8 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> | MutatingUseContext::AsmOutput | MutatingUseContext::Borrow | MutatingUseContext::AddressOf - | MutatingUseContext::Projection, + | MutatingUseContext::Projection + | MutatingUseContext::CopyNonOverlapping, ) | PlaceContext::NonMutatingUse( NonMutatingUseContext::Inspect @@ -301,7 +302,8 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> | NonMutatingUseContext::UniqueBorrow | NonMutatingUseContext::ShallowBorrow | NonMutatingUseContext::AddressOf - | NonMutatingUseContext::Projection, + | NonMutatingUseContext::Projection + | NonMutatingUseContext::CopyNonOverlapping, ) => { self.not_ssa(local); } diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 6f74ba77d4c16..2c90054b6c7f5 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -115,6 +115,21 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { self.codegen_coverage(&mut bx, coverage.clone()); bx } + mir::StatementKind::CopyNonOverlapping(box mir::CopyNonOverlapping { + ref src, + ref dst, + ref size, + }) => { + bx.memcpy( + dst, + todo!(), + src, + todo!(), + size, + todo!(), + ); + bx + } mir::StatementKind::FakeRead(..) | mir::StatementKind::Retag { .. } | mir::StatementKind::AscribeUserType(..) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 42bbc9a0d9552..90111d4080c7e 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1541,6 +1541,11 @@ pub enum StatementKind<'tcx> { /// counter varible at runtime, each time the code region is executed. Coverage(Box), + /// Denotes a call to the intrinsic function copy_overlapping, where `src_dst` denotes the + /// memory being read from and written to(one field to save memory), and size + /// indicates how many bytes are being copied over. + CopyNonOverlapping(Box>), + /// No-op. Useful for deleting instructions without affecting statement indices. Nop, } @@ -1659,6 +1664,11 @@ impl Debug for Statement<'_> { write!(fmt, "Coverage::{:?}", coverage.kind) } } + CopyNonOverlapping(box crate::mir::CopyNonOverlapping { + ref src, + ref dst, + ref size, + }) => write!(fmt, "src {:?} -> dst {:?}, {:?} bytes", src, dst, size), Nop => write!(fmt, "nop"), } } @@ -1670,6 +1680,13 @@ pub struct Coverage { pub code_region: Option, } +#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)] +pub struct CopyNonOverlapping<'tcx> { + pub src: Place<'tcx>, + pub dst: Place<'tcx>, + pub size: Operand<'tcx>, +} + /////////////////////////////////////////////////////////////////////////// // Places diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 464220cf77ede..ad773adcd82ac 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -436,6 +436,23 @@ macro_rules! make_mir_visitor { location ) } + StatementKind::CopyNonOverlapping(box crate::mir::CopyNonOverlapping{ + ref $($mutability)? src, + ref $($mutability)? dst, + ref $($mutability)? size, + }) => { + self.visit_place( + src, + PlaceContext::NonMutatingUse(NonMutatingUseContext::CopyNonOverlapping), + location + ); + self.visit_place( + dst, + PlaceContext::MutatingUse(MutatingUseContext::CopyNonOverlapping), + location + ); + self.visit_operand(size, location) + } StatementKind::Nop => {} } } @@ -1151,6 +1168,8 @@ pub enum NonMutatingUseContext { /// f(&x.y); /// Projection, + /// Source from copy_nonoverlapping. + CopyNonOverlapping, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -1180,6 +1199,8 @@ pub enum MutatingUseContext { Projection, /// Retagging, a "Stacked Borrows" shadow state operation Retag, + /// Memory written to in copy_nonoverlapping. + CopyNonOverlapping, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] From 72c734d001a157e0fb38e1feebf6748189d3e1b9 Mon Sep 17 00:00:00 2001 From: kadmin Date: Mon, 5 Oct 2020 20:13:36 +0000 Subject: [PATCH 02/10] Update fmt and use of memcpy I'm still not totally sure if this is the right way to implement the memcpy, but that portion compiles correctly now. Now to fix the compile errors everywhere else :). --- .../rustc_codegen_ssa/src/mir/statement.rs | 23 +++++++++++-------- compiler/rustc_middle/src/mir/mod.rs | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 2c90054b6c7f5..b507cb0a82312 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -120,15 +120,20 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { ref dst, ref size, }) => { - bx.memcpy( - dst, - todo!(), - src, - todo!(), - size, - todo!(), - ); - bx + let dst_val = self.codegen_place(&mut bx, dst.as_ref()); + let src_val = self.codegen_place(&mut bx, src.as_ref()); + let size_val = self.codegen_operand(&mut bx, size); + let size = size_val.immediate_or_packed_pair(&mut bx); + bx.memcpy( + dst_val.llval, + dst_val.align, + src_val.llval, + src_val.align, + size, + // TODO probably want to have this change based on alignment above? + crate::MemFlags::empty(), + ); + bx } mir::StatementKind::FakeRead(..) | mir::StatementKind::Retag { .. } diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 90111d4080c7e..2db69c27fe86c 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1668,7 +1668,7 @@ impl Debug for Statement<'_> { ref src, ref dst, ref size, - }) => write!(fmt, "src {:?} -> dst {:?}, {:?} bytes", src, dst, size), + }) => write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?},bytes={:?})", src, dst, size), Nop => write!(fmt, "nop"), } } From 89f45ed9f3ce7366ae7153421d637efcb22a45b8 Mon Sep 17 00:00:00 2001 From: kadmin Date: Mon, 5 Oct 2020 22:53:00 +0000 Subject: [PATCH 03/10] Update match branches This updates all places where match branches check on StatementKind or UseContext. This doesn't properly implement them, but adds TODOs where they are, and also adds some best guesses to what they should be in some cases. --- compiler/rustc_codegen_ssa/src/mir/analyze.rs | 6 +- .../rustc_codegen_ssa/src/mir/statement.rs | 20 +- compiler/rustc_middle/src/mir/mod.rs | 6 +- compiler/rustc_middle/src/mir/visit.rs | 10 +- .../src/borrow_check/invalidation.rs | 15 + compiler/rustc_mir/src/borrow_check/mod.rs | 2 + .../src/borrow_check/type_check/mod.rs | 1 + .../rustc_mir/src/dataflow/impls/borrows.rs | 2 + .../src/dataflow/impls/storage_liveness.rs | 2 + .../src/dataflow/move_paths/builder.rs | 2 + compiler/rustc_mir/src/interpret/step.rs | 17 + .../src/transform/check_consts/validation.rs | 1 + .../rustc_mir/src/transform/check_unsafety.rs | 1 + compiler/rustc_mir/src/transform/dest_prop.rs | 1 + compiler/rustc_mir/src/transform/generator.rs | 1 + .../src/transform/instrument_coverage.rs | 1272 +++++++++++++++++ .../src/transform/remove_noop_landing_pads.rs | 1 + compiler/rustc_mir/src/util/spanview.rs | 1 + .../clippy_utils/src/qualify_min_const_fn.rs | 9 +- 19 files changed, 1346 insertions(+), 24 deletions(-) create mode 100644 compiler/rustc_mir/src/transform/instrument_coverage.rs diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs index 5ad9f46147282..289629d921545 100644 --- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs +++ b/compiler/rustc_codegen_ssa/src/mir/analyze.rs @@ -293,8 +293,7 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> | MutatingUseContext::AsmOutput | MutatingUseContext::Borrow | MutatingUseContext::AddressOf - | MutatingUseContext::Projection - | MutatingUseContext::CopyNonOverlapping, + | MutatingUseContext::Projection, ) | PlaceContext::NonMutatingUse( NonMutatingUseContext::Inspect @@ -302,8 +301,7 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> | NonMutatingUseContext::UniqueBorrow | NonMutatingUseContext::ShallowBorrow | NonMutatingUseContext::AddressOf - | NonMutatingUseContext::Projection - | NonMutatingUseContext::CopyNonOverlapping, + | NonMutatingUseContext::Projection, ) => { self.not_ssa(local); } diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index b507cb0a82312..1dafc8cd2c16d 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -120,18 +120,22 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { ref dst, ref size, }) => { - let dst_val = self.codegen_place(&mut bx, dst.as_ref()); - let src_val = self.codegen_place(&mut bx, src.as_ref()); + let dst_val = self.codegen_operand(&mut bx, dst); + let src_val = self.codegen_operand(&mut bx, src); let size_val = self.codegen_operand(&mut bx, size); let size = size_val.immediate_or_packed_pair(&mut bx); + let dst = dst_val.immediate_or_packed_pair(&mut bx); + let src = src_val.immediate_or_packed_pair(&mut bx); + use crate::MemFlags; + let flags = + (!MemFlags::UNALIGNED) & (!MemFlags::VOLATILE) & (!MemFlags::NONTEMPORAL); bx.memcpy( - dst_val.llval, - dst_val.align, - src_val.llval, - src_val.align, + dst, + dst_val.layout.layout.align.pref, + src, + src_val.layout.layout.align.pref, size, - // TODO probably want to have this change based on alignment above? - crate::MemFlags::empty(), + flags, ); bx } diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 2db69c27fe86c..dddda0f611229 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1668,7 +1668,7 @@ impl Debug for Statement<'_> { ref src, ref dst, ref size, - }) => write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?},bytes={:?})", src, dst, size), + }) => write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?}, size={:?})", src, dst, size), Nop => write!(fmt, "nop"), } } @@ -1682,8 +1682,8 @@ pub struct Coverage { #[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)] pub struct CopyNonOverlapping<'tcx> { - pub src: Place<'tcx>, - pub dst: Place<'tcx>, + pub src: Operand<'tcx>, + pub dst: Operand<'tcx>, pub size: Operand<'tcx>, } diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index ad773adcd82ac..7bd4b72cc1240 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -441,14 +441,12 @@ macro_rules! make_mir_visitor { ref $($mutability)? dst, ref $($mutability)? size, }) => { - self.visit_place( + self.visit_operand( src, - PlaceContext::NonMutatingUse(NonMutatingUseContext::CopyNonOverlapping), location ); - self.visit_place( + self.visit_operand( dst, - PlaceContext::MutatingUse(MutatingUseContext::CopyNonOverlapping), location ); self.visit_operand(size, location) @@ -1168,8 +1166,6 @@ pub enum NonMutatingUseContext { /// f(&x.y); /// Projection, - /// Source from copy_nonoverlapping. - CopyNonOverlapping, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -1199,8 +1195,6 @@ pub enum MutatingUseContext { Projection, /// Retagging, a "Stacked Borrows" shadow state operation Retag, - /// Memory written to in copy_nonoverlapping. - CopyNonOverlapping, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] diff --git a/compiler/rustc_mir/src/borrow_check/invalidation.rs b/compiler/rustc_mir/src/borrow_check/invalidation.rs index 1a3ba16585d65..276d9381e32ae 100644 --- a/compiler/rustc_mir/src/borrow_check/invalidation.rs +++ b/compiler/rustc_mir/src/borrow_check/invalidation.rs @@ -92,6 +92,21 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { self.consume_operand(location, input); } } + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + ref src, + ref dst, + ref size, + }) => { + self.consume_operand(location, src); + self.consume_operand(location, dst); + self.consume_operand(location, size); + match dst { + Operand::Move(ref place) | Operand::Copy(ref place) => { + self.mutate_place(location, *place, Deep, JustWrite); + } + _ => {} + } + } StatementKind::Nop | StatementKind::Coverage(..) | StatementKind::AscribeUserType(..) diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index dcf3093baaf41..539319ab9f25f 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -626,6 +626,8 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc self.consume_operand(location, (input, span), flow_state); } } + + StatementKind::CopyNonOverlapping(..) => todo!(), StatementKind::Nop | StatementKind::Coverage(..) | StatementKind::AscribeUserType(..) diff --git a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs index f6bbd3b6283de..74d7fd84c9e72 100644 --- a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs @@ -1520,6 +1520,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ); } } + StatementKind::CopyNonOverlapping(..) => todo!(), StatementKind::FakeRead(..) | StatementKind::StorageLive(..) | StatementKind::StorageDead(..) diff --git a/compiler/rustc_mir/src/dataflow/impls/borrows.rs b/compiler/rustc_mir/src/dataflow/impls/borrows.rs index b149ffa9667a3..ffa02f855c979 100644 --- a/compiler/rustc_mir/src/dataflow/impls/borrows.rs +++ b/compiler/rustc_mir/src/dataflow/impls/borrows.rs @@ -306,6 +306,8 @@ impl<'tcx> dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> { | mir::StatementKind::AscribeUserType(..) | mir::StatementKind::Coverage(..) | mir::StatementKind::Nop => {} + + mir::StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs index 9250cd408479a..e407f394c51ec 100644 --- a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs +++ b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs @@ -150,6 +150,8 @@ impl<'mir, 'tcx> dataflow::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, | StatementKind::Nop | StatementKind::Retag(..) | StatementKind::StorageLive(..) => {} + + StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/dataflow/move_paths/builder.rs b/compiler/rustc_mir/src/dataflow/move_paths/builder.rs index 67c3b043262d5..c14ac74ebadab 100644 --- a/compiler/rustc_mir/src/dataflow/move_paths/builder.rs +++ b/compiler/rustc_mir/src/dataflow/move_paths/builder.rs @@ -319,6 +319,8 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) | StatementKind::Nop => {} + + StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index c22d91fd82a21..5a3fc9f51611d 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -113,6 +113,23 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { M::retag(self, *kind, &dest)?; } + // Call CopyNonOverlapping + CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, size }) => { + let size = self.eval_operand(size, None)?; + + let dst = { + let dst = self.eval_operand(dst, None)?; + dst.assert_mem_place(self) + }; + let src = { + let src = self.eval_operand(src, None)?; + src.assert_mem_place(self) + }; + // Not sure how to convert an MPlaceTy<'_, >::PointerTag> + // to a pointer, or OpTy to a size + self.memory.copy(src, dst, size, /*nonoverlapping*/ true)?; + } + // Statements we do not track. AscribeUserType(..) => {} diff --git a/compiler/rustc_mir/src/transform/check_consts/validation.rs b/compiler/rustc_mir/src/transform/check_consts/validation.rs index dd3e28acf96e3..1ad7b8fbbd5ed 100644 --- a/compiler/rustc_mir/src/transform/check_consts/validation.rs +++ b/compiler/rustc_mir/src/transform/check_consts/validation.rs @@ -808,6 +808,7 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> { | StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => {} } } diff --git a/compiler/rustc_mir/src/transform/check_unsafety.rs b/compiler/rustc_mir/src/transform/check_unsafety.rs index f0472758dfb8e..e7fdd5496cb40 100644 --- a/compiler/rustc_mir/src/transform/check_unsafety.rs +++ b/compiler/rustc_mir/src/transform/check_unsafety.rs @@ -115,6 +115,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { | StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // safe (at least as emitted during MIR construction) } diff --git a/compiler/rustc_mir/src/transform/dest_prop.rs b/compiler/rustc_mir/src/transform/dest_prop.rs index f7568e1d929dd..6656deac967b6 100644 --- a/compiler/rustc_mir/src/transform/dest_prop.rs +++ b/compiler/rustc_mir/src/transform/dest_prop.rs @@ -587,6 +587,7 @@ impl Conflicts<'a> { | StatementKind::FakeRead(..) | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => {} } } diff --git a/compiler/rustc_mir/src/transform/generator.rs b/compiler/rustc_mir/src/transform/generator.rs index 7a1f3d44a5e97..f299b6ecc28dc 100644 --- a/compiler/rustc_mir/src/transform/generator.rs +++ b/compiler/rustc_mir/src/transform/generator.rs @@ -1454,6 +1454,7 @@ impl Visitor<'tcx> for EnsureGeneratorFieldAssignmentsNeverAlias<'_> { | StatementKind::Retag(..) | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => {} } } diff --git a/compiler/rustc_mir/src/transform/instrument_coverage.rs b/compiler/rustc_mir/src/transform/instrument_coverage.rs new file mode 100644 index 0000000000000..7125e1516e9fd --- /dev/null +++ b/compiler/rustc_mir/src/transform/instrument_coverage.rs @@ -0,0 +1,1272 @@ +use crate::transform::MirPass; +use crate::util::pretty; +use crate::util::spanview::{self, SpanViewable}; + +use rustc_data_structures::fingerprint::Fingerprint; +use rustc_data_structures::graph::dominators::Dominators; +use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_data_structures::sync::Lrc; +use rustc_index::bit_set::BitSet; +use rustc_index::vec::IndexVec; +use rustc_middle::hir; +use rustc_middle::hir::map::blocks::FnLikeNode; +use rustc_middle::ich::StableHashingContext; +use rustc_middle::mir; +use rustc_middle::mir::coverage::*; +use rustc_middle::mir::visit::Visitor; +use rustc_middle::mir::{ + AggregateKind, BasicBlock, BasicBlockData, Coverage, CoverageInfo, FakeReadCause, Location, + Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, +}; +use rustc_middle::ty::query::Providers; +use rustc_middle::ty::TyCtxt; +use rustc_span::def_id::DefId; +use rustc_span::source_map::original_sp; +use rustc_span::{BytePos, CharPos, Pos, SourceFile, Span, Symbol, SyntaxContext}; + +use std::cmp::Ordering; + +const ID_SEPARATOR: &str = ","; + +/// Inserts `StatementKind::Coverage` statements that either instrument the binary with injected +/// counters, via intrinsic `llvm.instrprof.increment`, and/or inject metadata used during codegen +/// to construct the coverage map. +pub struct InstrumentCoverage; + +/// The `query` provider for `CoverageInfo`, requested by `codegen_coverage()` (to inject each +/// counter) and `FunctionCoverage::new()` (to extract the coverage map metadata from the MIR). +pub(crate) fn provide(providers: &mut Providers) { + providers.coverageinfo = |tcx, def_id| coverageinfo_from_mir(tcx, def_id); +} + +/// The `num_counters` argument to `llvm.instrprof.increment` is the max counter_id + 1, or in +/// other words, the number of counter value references injected into the MIR (plus 1 for the +/// reserved `ZERO` counter, which uses counter ID `0` when included in an expression). Injected +/// counters have a counter ID from `1..num_counters-1`. +/// +/// `num_expressions` is the number of counter expressions added to the MIR body. +/// +/// Both `num_counters` and `num_expressions` are used to initialize new vectors, during backend +/// code generate, to lookup counters and expressions by simple u32 indexes. +/// +/// MIR optimization may split and duplicate some BasicBlock sequences, or optimize out some code +/// including injected counters. (It is OK if some counters are optimized out, but those counters +/// are still included in the total `num_counters` or `num_expressions`.) Simply counting the +/// calls may not work; but computing the number of counters or expressions by adding `1` to the +/// highest ID (for a given instrumented function) is valid. +struct CoverageVisitor { + info: CoverageInfo, +} + +impl Visitor<'_> for CoverageVisitor { + fn visit_coverage(&mut self, coverage: &Coverage, _location: Location) { + match coverage.kind { + CoverageKind::Counter { id, .. } => { + let counter_id = u32::from(id); + self.info.num_counters = std::cmp::max(self.info.num_counters, counter_id + 1); + } + CoverageKind::Expression { id, .. } => { + let expression_index = u32::MAX - u32::from(id); + self.info.num_expressions = + std::cmp::max(self.info.num_expressions, expression_index + 1); + } + _ => {} + } + } +} + +fn coverageinfo_from_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo { + let mir_body = tcx.optimized_mir(def_id); + + let mut coverage_visitor = + CoverageVisitor { info: CoverageInfo { num_counters: 0, num_expressions: 0 } }; + + coverage_visitor.visit_body(mir_body); + coverage_visitor.info +} + +impl<'tcx> MirPass<'tcx> for InstrumentCoverage { + fn run_pass(&self, tcx: TyCtxt<'tcx>, mir_body: &mut mir::Body<'tcx>) { + // If the InstrumentCoverage pass is called on promoted MIRs, skip them. + // See: https://github.com/rust-lang/rust/pull/73011#discussion_r438317601 + if mir_body.source.promoted.is_some() { + trace!( + "InstrumentCoverage skipped for {:?} (already promoted for Miri evaluation)", + mir_body.source.def_id() + ); + return; + } + + let hir_id = tcx.hir().local_def_id_to_hir_id(mir_body.source.def_id().expect_local()); + let is_fn_like = FnLikeNode::from_node(tcx.hir().get(hir_id)).is_some(); + + // Only instrument functions, methods, and closures (not constants since they are evaluated + // at compile time by Miri). + // FIXME(#73156): Handle source code coverage in const eval + if !is_fn_like { + trace!( + "InstrumentCoverage skipped for {:?} (not an FnLikeNode)", + mir_body.source.def_id(), + ); + return; + } + // FIXME(richkadel): By comparison, the MIR pass `ConstProp` includes associated constants, + // with functions, methods, and closures. I assume Miri is used for associated constants as + // well. If not, we may need to include them here too. + + trace!("InstrumentCoverage starting for {:?}", mir_body.source.def_id()); + Instrumentor::new(&self.name(), tcx, mir_body).inject_counters(); + trace!("InstrumentCoverage starting for {:?}", mir_body.source.def_id()); + } +} + +/// A BasicCoverageBlock (BCB) represents the maximal-length sequence of CFG (MIR) BasicBlocks +/// without conditional branches. +/// +/// The BCB allows coverage analysis to be performed on a simplified projection of the underlying +/// MIR CFG, without altering the original CFG. Note that running the MIR `SimplifyCfg` transform, +/// is not sufficient, and therefore not necessary, since the BCB-based CFG projection is a more +/// aggressive simplification. For example: +/// +/// * The BCB CFG projection ignores (trims) branches not relevant to coverage, such as unwind- +/// related code that is injected by the Rust compiler but has no physical source code to +/// count. This also means a BasicBlock with a `Call` terminator can be merged into its +/// primary successor target block, in the same BCB. +/// * Some BasicBlock terminators support Rust-specific concerns--like borrow-checking--that are +/// not relevant to coverage analysis. `FalseUnwind`, for example, can be treated the same as +/// a `Goto`, and merged with its successor into the same BCB. +/// +/// Each BCB with at least one computed `CoverageSpan` will have no more than one `Counter`. +/// In some cases, a BCB's execution count can be computed by `CounterExpression`. Additional +/// disjoint `CoverageSpan`s in a BCB can also be counted by `CounterExpression` (by adding `ZERO` +/// to the BCB's primary counter or expression). +/// +/// Dominator/dominated relationships (which are fundamental to the coverage analysis algorithm) +/// between two BCBs can be computed using the `mir::Body` `dominators()` with any `BasicBlock` +/// member of each BCB. (For consistency, BCB's use the first `BasicBlock`, also referred to as the +/// `bcb_leader_bb`.) +/// +/// The BCB CFG projection is critical to simplifying the coverage analysis by ensuring graph +/// path-based queries (`is_dominated_by()`, `predecessors`, `successors`, etc.) have branch +/// (control flow) significance. +#[derive(Debug, Clone)] +struct BasicCoverageBlock { + pub blocks: Vec, +} + +impl BasicCoverageBlock { + pub fn leader_bb(&self) -> BasicBlock { + self.blocks[0] + } + + pub fn id(&self) -> String { + format!( + "@{}", + self.blocks + .iter() + .map(|bb| bb.index().to_string()) + .collect::>() + .join(ID_SEPARATOR) + ) + } +} + +struct BasicCoverageBlocks { + vec: IndexVec>, +} + +impl BasicCoverageBlocks { + pub fn from_mir(mir_body: &mir::Body<'tcx>) -> Self { + let mut basic_coverage_blocks = + BasicCoverageBlocks { vec: IndexVec::from_elem_n(None, mir_body.basic_blocks().len()) }; + basic_coverage_blocks.extract_from_mir(mir_body); + basic_coverage_blocks + } + + pub fn iter(&self) -> impl Iterator { + self.vec.iter().filter_map(|option| option.as_ref()) + } + + fn extract_from_mir(&mut self, mir_body: &mir::Body<'tcx>) { + // Traverse the CFG but ignore anything following an `unwind` + let cfg_without_unwind = ShortCircuitPreorder::new(mir_body, |term_kind| { + let mut successors = term_kind.successors(); + match &term_kind { + // SwitchInt successors are never unwind, and all of them should be traversed. + + // NOTE: TerminatorKind::FalseEdge targets from SwitchInt don't appear to be + // helpful in identifying unreachable code. I did test the theory, but the following + // changes were not beneficial. (I assumed that replacing some constants with + // non-deterministic variables might effect which blocks were targeted by a + // `FalseEdge` `imaginary_target`. It did not.) + // + // Also note that, if there is a way to identify BasicBlocks that are part of the + // MIR CFG, but not actually reachable, here are some other things to consider: + // + // Injecting unreachable code regions will probably require computing the set + // difference between the basic blocks found without filtering out unreachable + // blocks, and the basic blocks found with the filter; then computing the + // `CoverageSpans` without the filter; and then injecting `Counter`s or + // `CounterExpression`s for blocks that are not unreachable, or injecting + // `Unreachable` code regions otherwise. This seems straightforward, but not + // trivial. + // + // Alternatively, we might instead want to leave the unreachable blocks in + // (bypass the filter here), and inject the counters. This will result in counter + // values of zero (0) for unreachable code (and, notably, the code will be displayed + // with a red background by `llvm-cov show`). + // + // TerminatorKind::SwitchInt { .. } => { + // let some_imaginary_target = successors.clone().find_map(|&successor| { + // let term = mir_body.basic_blocks()[successor].terminator(); + // if let TerminatorKind::FalseEdge { imaginary_target, .. } = term.kind { + // if mir_body.predecessors()[imaginary_target].len() == 1 { + // return Some(imaginary_target); + // } + // } + // None + // }); + // if let Some(imaginary_target) = some_imaginary_target { + // box successors.filter(move |&&successor| successor != imaginary_target) + // } else { + // box successors + // } + // } + // + // Note this also required changing the closure signature for the + // `ShortCurcuitPreorder` to: + // + // F: Fn(&'tcx TerminatorKind<'tcx>) -> Box + 'a>, + TerminatorKind::SwitchInt { .. } => successors, + + // For all other kinds, return only the first successor, if any, and ignore unwinds + _ => successors.next().into_iter().chain(&[]), + } + }); + + // Walk the CFG using a Preorder traversal, which starts from `START_BLOCK` and follows + // each block terminator's `successors()`. Coverage spans must map to actual source code, + // so compiler generated blocks and paths can be ignored. To that end the CFG traversal + // intentionally omits unwind paths. + let mut blocks = Vec::new(); + for (bb, data) in cfg_without_unwind { + if let Some(last) = blocks.last() { + let predecessors = &mir_body.predecessors()[bb]; + if predecessors.len() > 1 || !predecessors.contains(last) { + // The `bb` has more than one _incoming_ edge, and should start its own + // `BasicCoverageBlock`. (Note, the `blocks` vector does not yet include `bb`; + // it contains a sequence of one or more sequential blocks with no intermediate + // branches in or out. Save these as a new `BasicCoverageBlock` before starting + // the new one.) + self.add_basic_coverage_block(blocks.split_off(0)); + debug!( + " because {}", + if predecessors.len() > 1 { + "predecessors.len() > 1".to_owned() + } else { + format!("bb {} is not in precessors: {:?}", bb.index(), predecessors) + } + ); + } + } + blocks.push(bb); + + let term = data.terminator(); + + match term.kind { + TerminatorKind::Return { .. } + | TerminatorKind::Abort + | TerminatorKind::Assert { .. } + | TerminatorKind::Yield { .. } + | TerminatorKind::SwitchInt { .. } => { + // The `bb` has more than one _outgoing_ edge, or exits the function. Save the + // current sequence of `blocks` gathered to this point, as a new + // `BasicCoverageBlock`. + self.add_basic_coverage_block(blocks.split_off(0)); + debug!(" because term.kind = {:?}", term.kind); + // Note that this condition is based on `TerminatorKind`, even though it + // theoretically boils down to `successors().len() != 1`; that is, either zero + // (e.g., `Return`, `Abort`) or multiple successors (e.g., `SwitchInt`), but + // since the Coverage graph (the BCB CFG projection) ignores things like unwind + // branches (which exist in the `Terminator`s `successors()` list) checking the + // number of successors won't work. + } + TerminatorKind::Goto { .. } + | TerminatorKind::Resume + | TerminatorKind::Unreachable + | TerminatorKind::Drop { .. } + | TerminatorKind::DropAndReplace { .. } + | TerminatorKind::Call { .. } + | TerminatorKind::GeneratorDrop + | TerminatorKind::FalseEdge { .. } + | TerminatorKind::FalseUnwind { .. } + | TerminatorKind::InlineAsm { .. } => {} + } + } + + if !blocks.is_empty() { + // process any remaining blocks into a final `BasicCoverageBlock` + self.add_basic_coverage_block(blocks.split_off(0)); + debug!(" because the end of the CFG was reached while traversing"); + } + } + + fn add_basic_coverage_block(&mut self, blocks: Vec) { + let leader_bb = blocks[0]; + let bcb = BasicCoverageBlock { blocks }; + debug!("adding BCB: {:?}", bcb); + self.vec[leader_bb] = Some(bcb); + } +} + +impl std::ops::Index for BasicCoverageBlocks { + type Output = BasicCoverageBlock; + + fn index(&self, index: BasicBlock) -> &Self::Output { + self.vec[index].as_ref().expect("is_some if BasicBlock is a BasicCoverageBlock leader") + } +} + +#[derive(Debug, Copy, Clone)] +enum CoverageStatement { + Statement(BasicBlock, Span, usize), + Terminator(BasicBlock, Span), +} + +impl CoverageStatement { + pub fn format(&self, tcx: TyCtxt<'tcx>, mir_body: &'a mir::Body<'tcx>) -> String { + match *self { + Self::Statement(bb, span, stmt_index) => { + let stmt = &mir_body.basic_blocks()[bb].statements[stmt_index]; + format!( + "{}: @{}[{}]: {:?}", + spanview::source_range_no_file(tcx, &span), + bb.index(), + stmt_index, + stmt + ) + } + Self::Terminator(bb, span) => { + let term = mir_body.basic_blocks()[bb].terminator(); + format!( + "{}: @{}.{}: {:?}", + spanview::source_range_no_file(tcx, &span), + bb.index(), + term_type(&term.kind), + term.kind + ) + } + } + } + + pub fn span(&self) -> &Span { + match self { + Self::Statement(_, span, _) | Self::Terminator(_, span) => span, + } + } +} + +fn term_type(kind: &TerminatorKind<'tcx>) -> &'static str { + match kind { + TerminatorKind::Goto { .. } => "Goto", + TerminatorKind::SwitchInt { .. } => "SwitchInt", + TerminatorKind::Resume => "Resume", + TerminatorKind::Abort => "Abort", + TerminatorKind::Return => "Return", + TerminatorKind::Unreachable => "Unreachable", + TerminatorKind::Drop { .. } => "Drop", + TerminatorKind::DropAndReplace { .. } => "DropAndReplace", + TerminatorKind::Call { .. } => "Call", + TerminatorKind::Assert { .. } => "Assert", + TerminatorKind::Yield { .. } => "Yield", + TerminatorKind::GeneratorDrop => "GeneratorDrop", + TerminatorKind::FalseEdge { .. } => "FalseEdge", + TerminatorKind::FalseUnwind { .. } => "FalseUnwind", + TerminatorKind::InlineAsm { .. } => "InlineAsm", + } +} + +/// A BCB is deconstructed into one or more `Span`s. Each `Span` maps to a `CoverageSpan` that +/// references the originating BCB and one or more MIR `Statement`s and/or `Terminator`s. +/// Initially, the `Span`s come from the `Statement`s and `Terminator`s, but subsequent +/// transforms can combine adjacent `Span`s and `CoverageSpan` from the same BCB, merging the +/// `CoverageStatement` vectors, and the `Span`s to cover the extent of the combined `Span`s. +/// +/// Note: A `CoverageStatement` merged into another CoverageSpan may come from a `BasicBlock` that +/// is not part of the `CoverageSpan` bcb if the statement was included because it's `Span` matches +/// or is subsumed by the `Span` associated with this `CoverageSpan`, and it's `BasicBlock` +/// `is_dominated_by()` the `BasicBlock`s in this `CoverageSpan`. +#[derive(Debug, Clone)] +struct CoverageSpan { + span: Span, + bcb_leader_bb: BasicBlock, + coverage_statements: Vec, + is_closure: bool, +} + +impl CoverageSpan { + pub fn for_statement( + statement: &Statement<'tcx>, + span: Span, + bcb: &BasicCoverageBlock, + bb: BasicBlock, + stmt_index: usize, + ) -> Self { + let is_closure = match statement.kind { + StatementKind::Assign(box ( + _, + Rvalue::Aggregate(box AggregateKind::Closure(_, _), _), + )) => true, + _ => false, + }; + + Self { + span, + bcb_leader_bb: bcb.leader_bb(), + coverage_statements: vec![CoverageStatement::Statement(bb, span, stmt_index)], + is_closure, + } + } + + pub fn for_terminator(span: Span, bcb: &'a BasicCoverageBlock, bb: BasicBlock) -> Self { + Self { + span, + bcb_leader_bb: bcb.leader_bb(), + coverage_statements: vec![CoverageStatement::Terminator(bb, span)], + is_closure: false, + } + } + + pub fn merge_from(&mut self, mut other: CoverageSpan) { + debug_assert!(self.is_mergeable(&other)); + self.span = self.span.to(other.span); + if other.is_closure { + self.is_closure = true; + } + self.coverage_statements.append(&mut other.coverage_statements); + } + + pub fn cutoff_statements_at(&mut self, cutoff_pos: BytePos) { + self.coverage_statements.retain(|covstmt| covstmt.span().hi() <= cutoff_pos); + if let Some(highest_covstmt) = + self.coverage_statements.iter().max_by_key(|covstmt| covstmt.span().hi()) + { + self.span = self.span.with_hi(highest_covstmt.span().hi()); + } + } + + pub fn is_dominated_by( + &self, + other: &CoverageSpan, + dominators: &Dominators, + ) -> bool { + debug_assert!(!self.is_in_same_bcb(other)); + dominators.is_dominated_by(self.bcb_leader_bb, other.bcb_leader_bb) + } + + pub fn is_mergeable(&self, other: &Self) -> bool { + self.is_in_same_bcb(other) && !(self.is_closure || other.is_closure) + } + + pub fn is_in_same_bcb(&self, other: &Self) -> bool { + self.bcb_leader_bb == other.bcb_leader_bb + } +} + +struct Instrumentor<'a, 'tcx> { + pass_name: &'a str, + tcx: TyCtxt<'tcx>, + mir_body: &'a mut mir::Body<'tcx>, + hir_body: &'tcx rustc_hir::Body<'tcx>, + dominators: Option>, + basic_coverage_blocks: Option, + function_source_hash: Option, + next_counter_id: u32, + num_expressions: u32, +} + +impl<'a, 'tcx> Instrumentor<'a, 'tcx> { + fn new(pass_name: &'a str, tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self { + let hir_body = hir_body(tcx, mir_body.source.def_id()); + Self { + pass_name, + tcx, + mir_body, + hir_body, + dominators: None, + basic_coverage_blocks: None, + function_source_hash: None, + next_counter_id: CounterValueReference::START.as_u32(), + num_expressions: 0, + } + } + + /// Counter IDs start from one and go up. + fn next_counter(&mut self) -> CounterValueReference { + assert!(self.next_counter_id < u32::MAX - self.num_expressions); + let next = self.next_counter_id; + self.next_counter_id += 1; + CounterValueReference::from(next) + } + + /// Expression IDs start from u32::MAX and go down because a CounterExpression can reference + /// (add or subtract counts) of both Counter regions and CounterExpression regions. The counter + /// expression operand IDs must be unique across both types. + fn next_expression(&mut self) -> InjectedExpressionIndex { + assert!(self.next_counter_id < u32::MAX - self.num_expressions); + let next = u32::MAX - self.num_expressions; + self.num_expressions += 1; + InjectedExpressionIndex::from(next) + } + + fn dominators(&self) -> &Dominators { + self.dominators.as_ref().expect("dominators must be initialized before calling") + } + + fn basic_coverage_blocks(&self) -> &BasicCoverageBlocks { + self.basic_coverage_blocks + .as_ref() + .expect("basic_coverage_blocks must be initialized before calling") + } + + fn function_source_hash(&mut self) -> u64 { + match self.function_source_hash { + Some(hash) => hash, + None => { + let hash = hash_mir_source(self.tcx, self.hir_body); + self.function_source_hash.replace(hash); + hash + } + } + } + + fn inject_counters(&mut self) { + let tcx = self.tcx; + let source_map = tcx.sess.source_map(); + let def_id = self.mir_body.source.def_id(); + let mir_body = &self.mir_body; + let body_span = self.body_span(); + let source_file = source_map.lookup_source_file(body_span.lo()); + let file_name = Symbol::intern(&source_file.name.to_string()); + + debug!("instrumenting {:?}, span: {}", def_id, source_map.span_to_string(body_span)); + + self.dominators.replace(mir_body.dominators()); + self.basic_coverage_blocks.replace(BasicCoverageBlocks::from_mir(mir_body)); + + let coverage_spans = self.coverage_spans(); + + let span_viewables = if pretty::dump_enabled(tcx, self.pass_name, def_id) { + Some(self.span_viewables(&coverage_spans)) + } else { + None + }; + + // Inject a counter for each `CoverageSpan`. There can be multiple `CoverageSpan`s for a + // given BCB, but only one actual counter needs to be incremented per BCB. `bb_counters` + // maps each `bcb_leader_bb` to its `Counter`, when injected. Subsequent `CoverageSpan`s + // for a BCB that already has a `Counter` will inject a `CounterExpression` instead, and + // compute its value by adding `ZERO` to the BCB `Counter` value. + let mut bb_counters = IndexVec::from_elem_n(None, mir_body.basic_blocks().len()); + for CoverageSpan { span, bcb_leader_bb: bb, .. } in coverage_spans { + if let Some(&counter_operand) = bb_counters[bb].as_ref() { + let expression = + self.make_expression(counter_operand, Op::Add, ExpressionOperandId::ZERO); + debug!( + "Injecting counter expression {:?} at: {:?}:\n{}\n==========", + expression, + span, + source_map.span_to_snippet(span).expect("Error getting source for span"), + ); + self.inject_statement(file_name, &source_file, expression, span, bb); + } else { + let counter = self.make_counter(); + debug!( + "Injecting counter {:?} at: {:?}:\n{}\n==========", + counter, + span, + source_map.span_to_snippet(span).expect("Error getting source for span"), + ); + let counter_operand = counter.as_operand_id(); + bb_counters[bb] = Some(counter_operand); + self.inject_statement(file_name, &source_file, counter, span, bb); + } + } + + if let Some(span_viewables) = span_viewables { + let mut file = pretty::create_dump_file( + tcx, + "html", + None, + self.pass_name, + &0, + self.mir_body.source, + ) + .expect("Unexpected error creating MIR spanview HTML file"); + let crate_name = tcx.crate_name(def_id.krate); + let item_name = tcx.def_path(def_id).to_filename_friendly_no_crate(); + let title = format!("{}.{} - Coverage Spans", crate_name, item_name); + spanview::write_document(tcx, def_id, span_viewables, &title, &mut file) + .expect("Unexpected IO error dumping coverage spans as HTML"); + } + } + + fn make_counter(&mut self) -> CoverageKind { + CoverageKind::Counter { + function_source_hash: self.function_source_hash(), + id: self.next_counter(), + } + } + + fn make_expression( + &mut self, + lhs: ExpressionOperandId, + op: Op, + rhs: ExpressionOperandId, + ) -> CoverageKind { + CoverageKind::Expression { id: self.next_expression(), lhs, op, rhs } + } + + fn inject_statement( + &mut self, + file_name: Symbol, + source_file: &Lrc, + coverage_kind: CoverageKind, + span: Span, + block: BasicBlock, + ) { + let code_region = make_code_region(file_name, source_file, span); + debug!(" injecting statement {:?} covering {:?}", coverage_kind, code_region); + + let data = &mut self.mir_body[block]; + let source_info = data.terminator().source_info; + let statement = Statement { + source_info, + kind: StatementKind::Coverage(box Coverage { kind: coverage_kind, code_region }), + }; + data.statements.push(statement); + } + + /// Converts the computed `BasicCoverageBlock`s into `SpanViewable`s. + fn span_viewables(&self, coverage_spans: &Vec) -> Vec { + let tcx = self.tcx; + let mir_body = &self.mir_body; + let mut span_viewables = Vec::new(); + for coverage_span in coverage_spans { + let bcb = self.bcb_from_coverage_span(coverage_span); + let CoverageSpan { span, bcb_leader_bb: bb, coverage_statements, .. } = coverage_span; + let id = bcb.id(); + let mut sorted_coverage_statements = coverage_statements.clone(); + sorted_coverage_statements.sort_unstable_by_key(|covstmt| match *covstmt { + CoverageStatement::Statement(bb, _, index) => (bb, index), + CoverageStatement::Terminator(bb, _) => (bb, usize::MAX), + }); + let tooltip = sorted_coverage_statements + .iter() + .map(|covstmt| covstmt.format(tcx, mir_body)) + .collect::>() + .join("\n"); + span_viewables.push(SpanViewable { bb: *bb, span: *span, id, tooltip }); + } + span_viewables + } + + #[inline(always)] + fn bcb_from_coverage_span(&self, coverage_span: &CoverageSpan) -> &BasicCoverageBlock { + &self.basic_coverage_blocks()[coverage_span.bcb_leader_bb] + } + + #[inline(always)] + fn body_span(&self) -> Span { + self.hir_body.value.span + } + + // Generate a set of `CoverageSpan`s from the filtered set of `Statement`s and `Terminator`s of + // the `BasicBlock`(s) in the given `BasicCoverageBlock`. One `CoverageSpan` is generated for + // each `Statement` and `Terminator`. (Note that subsequent stages of coverage analysis will + // merge some `CoverageSpan`s, at which point a `CoverageSpan` may represent multiple + // `Statement`s and/or `Terminator`s.) + fn extract_spans(&self, bcb: &'a BasicCoverageBlock) -> Vec { + let body_span = self.body_span(); + let mir_basic_blocks = self.mir_body.basic_blocks(); + bcb.blocks + .iter() + .map(|bbref| { + let bb = *bbref; + let data = &mir_basic_blocks[bb]; + data.statements + .iter() + .enumerate() + .filter_map(move |(index, statement)| { + filtered_statement_span(statement, body_span).map(|span| { + CoverageSpan::for_statement(statement, span, bcb, bb, index) + }) + }) + .chain( + filtered_terminator_span(data.terminator(), body_span) + .map(|span| CoverageSpan::for_terminator(span, bcb, bb)), + ) + }) + .flatten() + .collect() + } + + /// Generate a minimal set of `CoverageSpan`s, each representing a contiguous code region to be + /// counted. + /// + /// The basic steps are: + /// + /// 1. Extract an initial set of spans from the `Statement`s and `Terminator`s of each + /// `BasicCoverageBlock`. + /// 2. Sort the spans by span.lo() (starting position). Spans that start at the same position + /// are sorted with longer spans before shorter spans; and equal spans are sorted + /// (deterministically) based on "dominator" relationship (if any). + /// 3. Traverse the spans in sorted order to identify spans that can be dropped (for instance, + /// if another span or spans are already counting the same code region), or should be merged + /// into a broader combined span (because it represents a contiguous, non-branching, and + /// uninterrupted region of source code). + /// + /// Closures are exposed in their enclosing functions as `Assign` `Rvalue`s, and since + /// closures have their own MIR, their `Span` in their enclosing function should be left + /// "uncovered". + /// + /// Note the resulting vector of `CoverageSpan`s does may not be fully sorted (and does not need + /// to be). + fn coverage_spans(&self) -> Vec { + let mut initial_spans = + Vec::::with_capacity(self.mir_body.basic_blocks().len() * 2); + for bcb in self.basic_coverage_blocks().iter() { + for coverage_span in self.extract_spans(bcb) { + initial_spans.push(coverage_span); + } + } + + if initial_spans.is_empty() { + // This can happen if, for example, the function is unreachable (contains only a + // `BasicBlock`(s) with an `Unreachable` terminator). + return initial_spans; + } + + initial_spans.sort_unstable_by(|a, b| { + if a.span.lo() == b.span.lo() { + if a.span.hi() == b.span.hi() { + if a.is_in_same_bcb(b) { + Some(Ordering::Equal) + } else { + // Sort equal spans by dominator relationship, in reverse order (so + // dominators always come after the dominated equal spans). When later + // comparing two spans in order, the first will either dominate the second, + // or they will have no dominator relationship. + self.dominators().rank_partial_cmp(b.bcb_leader_bb, a.bcb_leader_bb) + } + } else { + // Sort hi() in reverse order so shorter spans are attempted after longer spans. + // This guarantees that, if a `prev` span overlaps, and is not equal to, a `curr` + // span, the prev span either extends further left of the curr span, or they + // start at the same position and the prev span extends further right of the end + // of the curr span. + b.span.hi().partial_cmp(&a.span.hi()) + } + } else { + a.span.lo().partial_cmp(&b.span.lo()) + } + .unwrap() + }); + + let refinery = CoverageSpanRefinery::from_sorted_spans(initial_spans, self.dominators()); + refinery.to_refined_spans() + } +} + +struct CoverageSpanRefinery<'a> { + sorted_spans_iter: std::vec::IntoIter, + dominators: &'a Dominators, + some_curr: Option, + curr_original_span: Span, + some_prev: Option, + prev_original_span: Span, + pending_dups: Vec, + refined_spans: Vec, +} + +impl<'a> CoverageSpanRefinery<'a> { + fn from_sorted_spans( + sorted_spans: Vec, + dominators: &'a Dominators, + ) -> Self { + let refined_spans = Vec::with_capacity(sorted_spans.len()); + let mut sorted_spans_iter = sorted_spans.into_iter(); + let prev = sorted_spans_iter.next().expect("at least one span"); + let prev_original_span = prev.span; + Self { + sorted_spans_iter, + dominators, + refined_spans, + some_curr: None, + curr_original_span: Span::with_root_ctxt(BytePos(0), BytePos(0)), + some_prev: Some(prev), + prev_original_span, + pending_dups: Vec::new(), + } + } + + /// Iterate through the sorted `CoverageSpan`s, and return the refined list of merged and + /// de-duplicated `CoverageSpan`s. + fn to_refined_spans(mut self) -> Vec { + while self.next_coverage_span() { + if self.curr().is_mergeable(self.prev()) { + debug!(" same bcb (and neither is a closure), merge with prev={:?}", self.prev()); + let prev = self.take_prev(); + self.curr_mut().merge_from(prev); + // Note that curr.span may now differ from curr_original_span + } else if self.prev_ends_before_curr() { + debug!( + " different bcbs and disjoint spans, so keep curr for next iter, and add \ + prev={:?}", + self.prev() + ); + let prev = self.take_prev(); + self.add_refined_span(prev); + } else if self.prev().is_closure { + // drop any equal or overlapping span (`curr`) and keep `prev` to test again in the + // next iter + debug!( + " curr overlaps a closure (prev). Drop curr and keep prev for next iter. \ + prev={:?}", + self.prev() + ); + self.discard_curr(); + } else if self.curr().is_closure { + self.carve_out_span_for_closure(); + } else if self.prev_original_span == self.curr().span { + self.hold_pending_dups_unless_dominated(); + } else { + self.cutoff_prev_at_overlapping_curr(); + } + } + debug!(" AT END, adding last prev={:?}", self.prev()); + let pending_dups = self.pending_dups.split_off(0); + for dup in pending_dups.into_iter() { + debug!(" ...adding at least one pending dup={:?}", dup); + self.add_refined_span(dup); + } + let prev = self.take_prev(); + self.add_refined_span(prev); + + // FIXME(richkadel): Replace some counters with expressions if they can be calculated based + // on branching. (For example, one branch of a SwitchInt can be computed from the counter + // for the CoverageSpan just prior to the SwitchInt minus the sum of the counters of all + // other branches). + + self.to_refined_spans_without_closures() + } + + fn add_refined_span(&mut self, coverage_span: CoverageSpan) { + self.refined_spans.push(coverage_span); + } + + /// Remove `CoverageSpan`s derived from closures, originally added to ensure the coverage + /// regions for the current function leave room for the closure's own coverage regions + /// (injected separately, from the closure's own MIR). + fn to_refined_spans_without_closures(mut self) -> Vec { + self.refined_spans.retain(|covspan| !covspan.is_closure); + self.refined_spans + } + + fn curr(&self) -> &CoverageSpan { + self.some_curr + .as_ref() + .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) + } + + fn curr_mut(&mut self) -> &mut CoverageSpan { + self.some_curr + .as_mut() + .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) + } + + fn prev(&self) -> &CoverageSpan { + self.some_prev + .as_ref() + .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + } + + fn prev_mut(&mut self) -> &mut CoverageSpan { + self.some_prev + .as_mut() + .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + } + + fn take_prev(&mut self) -> CoverageSpan { + self.some_prev.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) + } + + /// If there are `pending_dups` but `prev` is not a matching dup (`prev.span` doesn't match the + /// `pending_dups` spans), then one of the following two things happened during the previous + /// iteration: + /// * the `span` of prev was modified (by `curr_mut().merge_from(prev)`); or + /// * the `span` of prev advanced past the end of the span of pending_dups + /// (`prev().span.hi() <= curr().span.lo()`) + /// In either case, no more spans will match the span of `pending_dups`, so + /// add the `pending_dups` if they don't overlap `curr`, and clear the list. + fn check_pending_dups(&mut self) { + if let Some(dup) = self.pending_dups.last() { + if dup.span != self.prev().span { + debug!( + " SAME spans, but pending_dups are NOT THE SAME, so BCBs matched on \ + previous iteration, or prev started a new disjoint span" + ); + if dup.span.hi() <= self.curr().span.lo() { + let pending_dups = self.pending_dups.split_off(0); + for dup in pending_dups.into_iter() { + debug!(" ...adding at least one pending={:?}", dup); + self.add_refined_span(dup); + } + } else { + self.pending_dups.clear(); + } + } + } + } + + /// Advance `prev` to `curr` (if any), and `curr` to the next `CoverageSpan` in sorted order. + fn next_coverage_span(&mut self) -> bool { + if let Some(curr) = self.some_curr.take() { + self.some_prev = Some(curr); + self.prev_original_span = self.curr_original_span; + } + while let Some(curr) = self.sorted_spans_iter.next() { + debug!("FOR curr={:?}", curr); + if self.prev_starts_after_next(&curr) { + debug!( + " prev.span starts after curr.span, so curr will be dropped (skipping past \ + closure?); prev={:?}", + self.prev() + ); + } else { + // Save a copy of the original span for `curr` in case the `CoverageSpan` is changed + // by `self.curr_mut().merge_from(prev)`. + self.curr_original_span = curr.span; + self.some_curr.replace(curr); + self.check_pending_dups(); + return true; + } + } + false + } + + /// If called, then the next call to `next_coverage_span()` will *not* update `prev` with the + /// `curr` coverage span. + fn discard_curr(&mut self) { + self.some_curr = None; + } + + /// Returns true if the curr span should be skipped because prev has already advanced beyond the + /// end of curr. This can only happen if a prior iteration updated `prev` to skip past a region + /// of code, such as skipping past a closure. + fn prev_starts_after_next(&self, next_curr: &CoverageSpan) -> bool { + self.prev().span.lo() > next_curr.span.lo() + } + + /// Returns true if the curr span starts past the end of the prev span, which means they don't + /// overlap, so we now know the prev can be added to the refined coverage spans. + fn prev_ends_before_curr(&self) -> bool { + self.prev().span.hi() <= self.curr().span.lo() + } + + /// If `prev`s span extends left of the closure (`curr`), carve out the closure's + /// span from `prev`'s span. (The closure's coverage counters will be injected when + /// processing the closure's own MIR.) Add the portion of the span to the left of the + /// closure; and if the span extends to the right of the closure, update `prev` to + /// that portion of the span. For any `pending_dups`, repeat the same process. + fn carve_out_span_for_closure(&mut self) { + let curr_span = self.curr().span; + let left_cutoff = curr_span.lo(); + let right_cutoff = curr_span.hi(); + let has_pre_closure_span = self.prev().span.lo() < right_cutoff; + let has_post_closure_span = self.prev().span.hi() > right_cutoff; + let mut pending_dups = self.pending_dups.split_off(0); + if has_pre_closure_span { + let mut pre_closure = self.prev().clone(); + pre_closure.span = pre_closure.span.with_hi(left_cutoff); + debug!(" prev overlaps a closure. Adding span for pre_closure={:?}", pre_closure); + if !pending_dups.is_empty() { + for mut dup in pending_dups.iter().cloned() { + dup.span = dup.span.with_hi(left_cutoff); + debug!(" ...and at least one pre_closure dup={:?}", dup); + self.add_refined_span(dup); + } + } + self.add_refined_span(pre_closure); + } + if has_post_closure_span { + // Update prev.span to start after the closure (and discard curr) + self.prev_mut().span = self.prev().span.with_lo(right_cutoff); + self.prev_original_span = self.prev().span; + for dup in pending_dups.iter_mut() { + dup.span = dup.span.with_lo(right_cutoff); + } + self.pending_dups.append(&mut pending_dups); + self.discard_curr(); // since self.prev() was already updated + } else { + pending_dups.clear(); + } + } + + /// When two `CoverageSpan`s have the same `Span`, dominated spans can be discarded; but if + /// neither `CoverageSpan` dominates the other, both (or possibly more than two) are held, + /// until their disposition is determined. In this latter case, the `prev` dup is moved into + /// `pending_dups` so the new `curr` dup can be moved to `prev` for the next iteration. + fn hold_pending_dups_unless_dominated(&mut self) { + // equal coverage spans are ordered by dominators before dominated (if any) + debug_assert!(!self.prev().is_dominated_by(self.curr(), self.dominators)); + + if self.curr().is_dominated_by(&self.prev(), self.dominators) { + // If one span dominates the other, assocate the span with the dominator only. + // + // For example: + // match somenum { + // x if x < 1 => { ... } + // }... + // The span for the first `x` is referenced by both the pattern block (every + // time it is evaluated) and the arm code (only when matched). The counter + // will be applied only to the dominator block. + // + // The dominator's (`prev`) execution count may be higher than the dominated + // block's execution count, so drop `curr`. + debug!( + " different bcbs but SAME spans, and prev dominates curr. Drop curr and \ + keep prev for next iter. prev={:?}", + self.prev() + ); + self.discard_curr(); + } else { + // Save `prev` in `pending_dups`. (`curr` will become `prev` in the next iteration.) + // If the `curr` CoverageSpan is later discarded, `pending_dups` can be discarded as + // well; but if `curr` is added to refined_spans, the `pending_dups` will also be added. + debug!( + " different bcbs but SAME spans, and neither dominates, so keep curr for \ + next iter, and, pending upcoming spans (unless overlapping) add prev={:?}", + self.prev() + ); + let prev = self.take_prev(); + self.pending_dups.push(prev); + } + } + + /// `curr` overlaps `prev`. If `prev`s span extends left of `curr`s span, keep _only_ + /// statements that end before `curr.lo()` (if any), and add the portion of the + /// combined span for those statements. Any other statements have overlapping spans + /// that can be ignored because `curr` and/or other upcoming statements/spans inside + /// the overlap area will produce their own counters. This disambiguation process + /// avoids injecting multiple counters for overlapping spans, and the potential for + /// double-counting. + fn cutoff_prev_at_overlapping_curr(&mut self) { + debug!( + " different bcbs, overlapping spans, so ignore/drop pending and only add prev \ + if it has statements that end before curr={:?}", + self.prev() + ); + if self.pending_dups.is_empty() { + let curr_span = self.curr().span; + self.prev_mut().cutoff_statements_at(curr_span.lo()); + if self.prev().coverage_statements.is_empty() { + debug!(" ... no non-overlapping statements to add"); + } else { + debug!(" ... adding modified prev={:?}", self.prev()); + let prev = self.take_prev(); + self.add_refined_span(prev); + } + } else { + // with `pending_dups`, `prev` cannot have any statements that don't overlap + self.pending_dups.clear(); + } + } +} + +fn filtered_statement_span(statement: &'a Statement<'tcx>, body_span: Span) -> Option { + match statement.kind { + // These statements have spans that are often outside the scope of the executed source code + // for their parent `BasicBlock`. + StatementKind::StorageLive(_) + | StatementKind::StorageDead(_) + // Coverage should not be encountered, but don't inject coverage coverage + | StatementKind::Coverage(_) + // Ignore `Nop`s + | StatementKind::Nop => None, + + // FIXME(richkadel): Look into a possible issue assigning the span to a + // FakeReadCause::ForGuardBinding, in this example: + // match somenum { + // x if x < 1 => { ... } + // }... + // The BasicBlock within the match arm code included one of these statements, but the span + // for it covered the `1` in this source. The actual statements have nothing to do with that + // source span: + // FakeRead(ForGuardBinding, _4); + // where `_4` is: + // _4 = &_1; (at the span for the first `x`) + // and `_1` is the `Place` for `somenum`. + // + // The arm code BasicBlock already has its own assignment for `x` itself, `_3 = 1`, and I've + // decided it's reasonable for that span (even though outside the arm code) to be part of + // the counted coverage of the arm code execution, but I can't justify including the literal + // `1` in the arm code. I'm pretty sure that, if the `FakeRead(ForGuardBinding)` has a + // purpose in codegen, it's probably in the right BasicBlock, but if so, the `Statement`s + // `source_info.span` can't be right. + // + // Consider correcting the span assignment, assuming there is a better solution, and see if + // the following pattern can be removed here: + StatementKind::FakeRead(cause, _) if cause == FakeReadCause::ForGuardBinding => None, + + // Retain spans from all other statements + StatementKind::FakeRead(_, _) // Not including `ForGuardBinding` + | StatementKind::CopyNonOverlapping(..) + | StatementKind::Assign(_) + | StatementKind::SetDiscriminant { .. } + | StatementKind::LlvmInlineAsm(_) + | StatementKind::Retag(_, _) + | StatementKind::AscribeUserType(_, _) => { + Some(source_info_span(&statement.source_info, body_span)) + } + } +} + +fn filtered_terminator_span(terminator: &'a Terminator<'tcx>, body_span: Span) -> Option { + match terminator.kind { + // These terminators have spans that don't positively contribute to computing a reasonable + // span of actually executed source code. (For example, SwitchInt terminators extracted from + // an `if condition { block }` has a span that includes the executed block, if true, + // but for coverage, the code region executed, up to *and* through the SwitchInt, + // actually stops before the if's block.) + TerminatorKind::Unreachable // Unreachable blocks are not connected to the CFG + | TerminatorKind::Assert { .. } + | TerminatorKind::Drop { .. } + | TerminatorKind::DropAndReplace { .. } + | TerminatorKind::SwitchInt { .. } + | TerminatorKind::Goto { .. } + // For `FalseEdge`, only the `real` branch is taken, so it is similar to a `Goto`. + | TerminatorKind::FalseEdge { .. } => None, + + // Retain spans from all other terminators + TerminatorKind::Resume + | TerminatorKind::Abort + | TerminatorKind::Return + | TerminatorKind::Call { .. } + | TerminatorKind::Yield { .. } + | TerminatorKind::GeneratorDrop + | TerminatorKind::FalseUnwind { .. } + | TerminatorKind::InlineAsm { .. } => { + Some(source_info_span(&terminator.source_info, body_span)) + } + } +} + +#[inline(always)] +fn source_info_span(source_info: &SourceInfo, body_span: Span) -> Span { + let span = original_sp(source_info.span, body_span).with_ctxt(SyntaxContext::root()); + if body_span.contains(span) { span } else { body_span } +} + +/// Convert the Span into its file name, start line and column, and end line and column +fn make_code_region(file_name: Symbol, source_file: &Lrc, span: Span) -> CodeRegion { + let (start_line, mut start_col) = source_file.lookup_file_pos(span.lo()); + let (end_line, end_col) = if span.hi() == span.lo() { + let (end_line, mut end_col) = (start_line, start_col); + // Extend an empty span by one character so the region will be counted. + let CharPos(char_pos) = start_col; + if char_pos > 0 { + start_col = CharPos(char_pos - 1); + } else { + end_col = CharPos(char_pos + 1); + } + (end_line, end_col) + } else { + source_file.lookup_file_pos(span.hi()) + }; + CodeRegion { + file_name, + start_line: start_line as u32, + start_col: start_col.to_u32() + 1, + end_line: end_line as u32, + end_col: end_col.to_u32() + 1, + } +} + +fn hir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx rustc_hir::Body<'tcx> { + let hir_node = tcx.hir().get_if_local(def_id).expect("expected DefId is local"); + let fn_body_id = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); + tcx.hir().body(fn_body_id) +} + +fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 { + let mut hcx = tcx.create_no_span_stable_hashing_context(); + hash(&mut hcx, &hir_body.value).to_smaller_hash() +} + +fn hash( + hcx: &mut StableHashingContext<'tcx>, + node: &impl HashStable>, +) -> Fingerprint { + let mut stable_hasher = StableHasher::new(); + node.hash_stable(hcx, &mut stable_hasher); + stable_hasher.finish() +} + +pub struct ShortCircuitPreorder< + 'a, + 'tcx, + F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>, +> { + body: &'a mir::Body<'tcx>, + visited: BitSet, + worklist: Vec, + filtered_successors: F, +} + +impl<'a, 'tcx, F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>> + ShortCircuitPreorder<'a, 'tcx, F> +{ + pub fn new( + body: &'a mir::Body<'tcx>, + filtered_successors: F, + ) -> ShortCircuitPreorder<'a, 'tcx, F> { + let worklist = vec![mir::START_BLOCK]; + + ShortCircuitPreorder { + body, + visited: BitSet::new_empty(body.basic_blocks().len()), + worklist, + filtered_successors, + } + } +} + +impl<'a: 'tcx, 'tcx, F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>> Iterator + for ShortCircuitPreorder<'a, 'tcx, F> +{ + type Item = (BasicBlock, &'a BasicBlockData<'tcx>); + + fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> { + while let Some(idx) = self.worklist.pop() { + if !self.visited.insert(idx) { + continue; + } + + let data = &self.body[idx]; + + if let Some(ref term) = data.terminator { + self.worklist.extend((self.filtered_successors)(&term.kind)); + } + + return Some((idx, data)); + } + + None + } + + fn size_hint(&self) -> (usize, Option) { + let size = self.body.basic_blocks().len() - self.visited.count(); + (size, Some(size)) + } +} diff --git a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs index 31e201c3a5bbe..a37f5d4f329f3 100644 --- a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs +++ b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs @@ -39,6 +39,7 @@ impl RemoveNoopLandingPads { | StatementKind::StorageDead(_) | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // These are all nops in a landing pad } diff --git a/compiler/rustc_mir/src/util/spanview.rs b/compiler/rustc_mir/src/util/spanview.rs index d3ef8c64565c6..a9a30e407b4b0 100644 --- a/compiler/rustc_mir/src/util/spanview.rs +++ b/compiler/rustc_mir/src/util/spanview.rs @@ -245,6 +245,7 @@ pub fn statement_kind_name(statement: &Statement<'_>) -> &'static str { Retag(..) => "Retag", AscribeUserType(..) => "AscribeUserType", Coverage(..) => "Coverage", + CopyNonOverlapping(..) => "CopyNonOverlapping", Nop => "Nop", } } diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index 2cb9588e13f98..53935f02a90e4 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -210,7 +210,7 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen StatementKind::Assign(box (place, rval)) => { check_place(tcx, *place, span, body)?; check_rvalue(tcx, body, def_id, rval, span) - }, + } StatementKind::FakeRead(_, place) | // just an assignment @@ -218,6 +218,13 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen StatementKind::LlvmInlineAsm { .. } => Err((span, "cannot use inline assembly in const fn".into())), + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping{ + dst, src, size, + }) => { + check_operand(tcx, dst, span, body)?; + check_operand(tcx, src, span, body)?; + check_operand(tcx, size, span, body) + }, // These are all NOPs StatementKind::StorageLive(_) | StatementKind::StorageDead(_) From 37a6c04718b9e8577b92aff2ab58a5c69a6c822a Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 7 Nov 2020 00:49:55 +0000 Subject: [PATCH 04/10] Update interpret step --- compiler/rustc_mir/src/interpret/step.rs | 14 +++++++++++--- compiler/rustc_mir/src/transform/coverage/spans.rs | 1 + compiler/rustc_mir/src/transform/simplify.rs | 1 + 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index 5a3fc9f51611d..cc5e34fbde5a2 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -119,15 +119,23 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let dst = { let dst = self.eval_operand(dst, None)?; - dst.assert_mem_place(self) + let mplace = *dst.assert_mem_place(self); + match mplace.ptr { + Scalar::Ptr(ptr) => ptr, + _ => panic!(), + } }; let src = { let src = self.eval_operand(src, None)?; - src.assert_mem_place(self) + let mplace = *src.assert_mem_place(self); + match mplace.ptr { + Scalar::Ptr(ptr) => ptr, + _ => panic!(), + } }; // Not sure how to convert an MPlaceTy<'_, >::PointerTag> // to a pointer, or OpTy to a size - self.memory.copy(src, dst, size, /*nonoverlapping*/ true)?; + self.memory.copy(src, dst, size.layout.layout.size, /*nonoverlapping*/ true)?; } // Statements we do not track. diff --git a/compiler/rustc_mir/src/transform/coverage/spans.rs b/compiler/rustc_mir/src/transform/coverage/spans.rs index fd3e782f6df43..e7097ce861902 100644 --- a/compiler/rustc_mir/src/transform/coverage/spans.rs +++ b/compiler/rustc_mir/src/transform/coverage/spans.rs @@ -687,6 +687,7 @@ pub(super) fn filtered_statement_span( // Retain spans from all other statements StatementKind::FakeRead(_, _) // Not including `ForGuardBinding` + | StatementKind::CopyNonOverlapping(..) | StatementKind::Assign(_) | StatementKind::SetDiscriminant { .. } | StatementKind::LlvmInlineAsm(_) diff --git a/compiler/rustc_mir/src/transform/simplify.rs b/compiler/rustc_mir/src/transform/simplify.rs index 85f27428bbbf4..a5764d9bf4e3d 100644 --- a/compiler/rustc_mir/src/transform/simplify.rs +++ b/compiler/rustc_mir/src/transform/simplify.rs @@ -428,6 +428,7 @@ impl Visitor<'_> for UsedLocals { fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { match statement.kind { StatementKind::LlvmInlineAsm(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Retag(..) | StatementKind::Coverage(..) | StatementKind::FakeRead(..) From 982382dc0361aa19e2f7e1e2b02003320d34b502 Mon Sep 17 00:00:00 2001 From: kadmin Date: Tue, 29 Dec 2020 02:00:04 +0000 Subject: [PATCH 05/10] Update cranelift --- compiler/rustc_codegen_cranelift/src/base.rs | 10 + compiler/rustc_codegen_cranelift/src/lib.rs | 1 + .../rustc_codegen_ssa/src/mir/statement.rs | 11 +- compiler/rustc_middle/src/mir/mod.rs | 9 +- compiler/rustc_middle/src/mir/visit.rs | 14 +- .../src/borrow_check/invalidation.rs | 4 +- compiler/rustc_mir/src/interpret/step.rs | 36 +- .../src/transform/instrument_coverage.rs | 1272 ----------------- .../clippy_utils/src/qualify_min_const_fn.rs | 4 +- 9 files changed, 52 insertions(+), 1309 deletions(-) delete mode 100644 compiler/rustc_mir/src/transform/instrument_coverage.rs diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index f2c61c95f4ff2..ba7c82d24c517 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -832,6 +832,16 @@ fn codegen_stmt<'tcx>( } } StatementKind::Coverage { .. } => fx.tcx.sess.fatal("-Zcoverage is unimplemented"), + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + src, + dst, + count, + }) => { + let dst = codegen_operand(fx, dst).load_scalar(fx); + let src = codegen_operand(fx, src).load_scalar(fx); + let count = codegen_operand(fx, count).load_scalar(fx); + fx.bcx.call_memcpy(fx.cx.module.target_config(), dst, src, count); + } } } diff --git a/compiler/rustc_codegen_cranelift/src/lib.rs b/compiler/rustc_codegen_cranelift/src/lib.rs index 8edb883ccb5f9..fae71fef9e676 100644 --- a/compiler/rustc_codegen_cranelift/src/lib.rs +++ b/compiler/rustc_codegen_cranelift/src/lib.rs @@ -11,6 +11,7 @@ #![warn(rust_2018_idioms)] #![warn(unused_lifetimes)] #![warn(unreachable_pub)] +#![feature(box_patterns)] extern crate snap; #[macro_use] diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 1dafc8cd2c16d..774c920ee9666 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -118,23 +118,22 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { mir::StatementKind::CopyNonOverlapping(box mir::CopyNonOverlapping { ref src, ref dst, - ref size, + ref count, }) => { let dst_val = self.codegen_operand(&mut bx, dst); let src_val = self.codegen_operand(&mut bx, src); - let size_val = self.codegen_operand(&mut bx, size); - let size = size_val.immediate_or_packed_pair(&mut bx); + let count_val = self.codegen_operand(&mut bx, count); + let count = count_val.immediate_or_packed_pair(&mut bx); let dst = dst_val.immediate_or_packed_pair(&mut bx); let src = src_val.immediate_or_packed_pair(&mut bx); use crate::MemFlags; - let flags = - (!MemFlags::UNALIGNED) & (!MemFlags::VOLATILE) & (!MemFlags::NONTEMPORAL); + let flags = MemFlags::empty(); bx.memcpy( dst, dst_val.layout.layout.align.pref, src, src_val.layout.layout.align.pref, - size, + count, flags, ); bx diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index dddda0f611229..c046333e68d2b 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1667,8 +1667,10 @@ impl Debug for Statement<'_> { CopyNonOverlapping(box crate::mir::CopyNonOverlapping { ref src, ref dst, - ref size, - }) => write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?}, size={:?})", src, dst, size), + ref count, + }) => { + write!(fmt, "copy_nonoverlapping(src={:?}, dst={:?}, count={:?})", src, dst, count) + } Nop => write!(fmt, "nop"), } } @@ -1684,7 +1686,8 @@ pub struct Coverage { pub struct CopyNonOverlapping<'tcx> { pub src: Operand<'tcx>, pub dst: Operand<'tcx>, - pub size: Operand<'tcx>, + /// Number of elements to copy from src to dest, not bytes. + pub count: Operand<'tcx>, } /////////////////////////////////////////////////////////////////////////// diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 7bd4b72cc1240..4e81612c0b9de 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -439,17 +439,11 @@ macro_rules! make_mir_visitor { StatementKind::CopyNonOverlapping(box crate::mir::CopyNonOverlapping{ ref $($mutability)? src, ref $($mutability)? dst, - ref $($mutability)? size, + ref $($mutability)? count, }) => { - self.visit_operand( - src, - location - ); - self.visit_operand( - dst, - location - ); - self.visit_operand(size, location) + self.visit_operand(src, location); + self.visit_operand(dst, location); + self.visit_operand(count, location) } StatementKind::Nop => {} } diff --git a/compiler/rustc_mir/src/borrow_check/invalidation.rs b/compiler/rustc_mir/src/borrow_check/invalidation.rs index 276d9381e32ae..1f3dfc251e10c 100644 --- a/compiler/rustc_mir/src/borrow_check/invalidation.rs +++ b/compiler/rustc_mir/src/borrow_check/invalidation.rs @@ -95,11 +95,11 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { ref src, ref dst, - ref size, + ref count, }) => { self.consume_operand(location, src); self.consume_operand(location, dst); - self.consume_operand(location, size); + self.consume_operand(location, count); match dst { Operand::Move(ref place) | Operand::Copy(ref place) => { self.mutate_place(location, *place, Deep, JustWrite); diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index cc5e34fbde5a2..89dc93c790090 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -114,28 +114,36 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } // Call CopyNonOverlapping - CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, size }) => { - let size = self.eval_operand(size, None)?; + CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, count }) => { + let (src, size) = { + let src = self.eval_operand(src, None)?; + let size = src.layout.layout.size; + let mplace = *src.assert_mem_place(self); + let ptr = match mplace.ptr { + Scalar::Ptr(ptr) => ptr, + _ => panic!(), + }; + (ptr, size) + }; let dst = { let dst = self.eval_operand(dst, None)?; let mplace = *dst.assert_mem_place(self); match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), + Scalar::Ptr(ptr) => ptr, + _ => panic!(), } }; - let src = { - let src = self.eval_operand(src, None)?; - let mplace = *src.assert_mem_place(self); - match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), - } + + let count = self.eval_operand(count, None)?; + let count = self.read_immediate(count)?.to_scalar()?; + let count = if let Scalar::Int(i) = count { + core::convert::TryFrom::try_from(i).unwrap() + } else { + panic!(); }; - // Not sure how to convert an MPlaceTy<'_, >::PointerTag> - // to a pointer, or OpTy to a size - self.memory.copy(src, dst, size.layout.layout.size, /*nonoverlapping*/ true)?; + + self.memory.copy_repeatedly(src, dst, size, count, /*nonoverlapping*/ true)?; } // Statements we do not track. diff --git a/compiler/rustc_mir/src/transform/instrument_coverage.rs b/compiler/rustc_mir/src/transform/instrument_coverage.rs deleted file mode 100644 index 7125e1516e9fd..0000000000000 --- a/compiler/rustc_mir/src/transform/instrument_coverage.rs +++ /dev/null @@ -1,1272 +0,0 @@ -use crate::transform::MirPass; -use crate::util::pretty; -use crate::util::spanview::{self, SpanViewable}; - -use rustc_data_structures::fingerprint::Fingerprint; -use rustc_data_structures::graph::dominators::Dominators; -use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; -use rustc_data_structures::sync::Lrc; -use rustc_index::bit_set::BitSet; -use rustc_index::vec::IndexVec; -use rustc_middle::hir; -use rustc_middle::hir::map::blocks::FnLikeNode; -use rustc_middle::ich::StableHashingContext; -use rustc_middle::mir; -use rustc_middle::mir::coverage::*; -use rustc_middle::mir::visit::Visitor; -use rustc_middle::mir::{ - AggregateKind, BasicBlock, BasicBlockData, Coverage, CoverageInfo, FakeReadCause, Location, - Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, -}; -use rustc_middle::ty::query::Providers; -use rustc_middle::ty::TyCtxt; -use rustc_span::def_id::DefId; -use rustc_span::source_map::original_sp; -use rustc_span::{BytePos, CharPos, Pos, SourceFile, Span, Symbol, SyntaxContext}; - -use std::cmp::Ordering; - -const ID_SEPARATOR: &str = ","; - -/// Inserts `StatementKind::Coverage` statements that either instrument the binary with injected -/// counters, via intrinsic `llvm.instrprof.increment`, and/or inject metadata used during codegen -/// to construct the coverage map. -pub struct InstrumentCoverage; - -/// The `query` provider for `CoverageInfo`, requested by `codegen_coverage()` (to inject each -/// counter) and `FunctionCoverage::new()` (to extract the coverage map metadata from the MIR). -pub(crate) fn provide(providers: &mut Providers) { - providers.coverageinfo = |tcx, def_id| coverageinfo_from_mir(tcx, def_id); -} - -/// The `num_counters` argument to `llvm.instrprof.increment` is the max counter_id + 1, or in -/// other words, the number of counter value references injected into the MIR (plus 1 for the -/// reserved `ZERO` counter, which uses counter ID `0` when included in an expression). Injected -/// counters have a counter ID from `1..num_counters-1`. -/// -/// `num_expressions` is the number of counter expressions added to the MIR body. -/// -/// Both `num_counters` and `num_expressions` are used to initialize new vectors, during backend -/// code generate, to lookup counters and expressions by simple u32 indexes. -/// -/// MIR optimization may split and duplicate some BasicBlock sequences, or optimize out some code -/// including injected counters. (It is OK if some counters are optimized out, but those counters -/// are still included in the total `num_counters` or `num_expressions`.) Simply counting the -/// calls may not work; but computing the number of counters or expressions by adding `1` to the -/// highest ID (for a given instrumented function) is valid. -struct CoverageVisitor { - info: CoverageInfo, -} - -impl Visitor<'_> for CoverageVisitor { - fn visit_coverage(&mut self, coverage: &Coverage, _location: Location) { - match coverage.kind { - CoverageKind::Counter { id, .. } => { - let counter_id = u32::from(id); - self.info.num_counters = std::cmp::max(self.info.num_counters, counter_id + 1); - } - CoverageKind::Expression { id, .. } => { - let expression_index = u32::MAX - u32::from(id); - self.info.num_expressions = - std::cmp::max(self.info.num_expressions, expression_index + 1); - } - _ => {} - } - } -} - -fn coverageinfo_from_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo { - let mir_body = tcx.optimized_mir(def_id); - - let mut coverage_visitor = - CoverageVisitor { info: CoverageInfo { num_counters: 0, num_expressions: 0 } }; - - coverage_visitor.visit_body(mir_body); - coverage_visitor.info -} - -impl<'tcx> MirPass<'tcx> for InstrumentCoverage { - fn run_pass(&self, tcx: TyCtxt<'tcx>, mir_body: &mut mir::Body<'tcx>) { - // If the InstrumentCoverage pass is called on promoted MIRs, skip them. - // See: https://github.com/rust-lang/rust/pull/73011#discussion_r438317601 - if mir_body.source.promoted.is_some() { - trace!( - "InstrumentCoverage skipped for {:?} (already promoted for Miri evaluation)", - mir_body.source.def_id() - ); - return; - } - - let hir_id = tcx.hir().local_def_id_to_hir_id(mir_body.source.def_id().expect_local()); - let is_fn_like = FnLikeNode::from_node(tcx.hir().get(hir_id)).is_some(); - - // Only instrument functions, methods, and closures (not constants since they are evaluated - // at compile time by Miri). - // FIXME(#73156): Handle source code coverage in const eval - if !is_fn_like { - trace!( - "InstrumentCoverage skipped for {:?} (not an FnLikeNode)", - mir_body.source.def_id(), - ); - return; - } - // FIXME(richkadel): By comparison, the MIR pass `ConstProp` includes associated constants, - // with functions, methods, and closures. I assume Miri is used for associated constants as - // well. If not, we may need to include them here too. - - trace!("InstrumentCoverage starting for {:?}", mir_body.source.def_id()); - Instrumentor::new(&self.name(), tcx, mir_body).inject_counters(); - trace!("InstrumentCoverage starting for {:?}", mir_body.source.def_id()); - } -} - -/// A BasicCoverageBlock (BCB) represents the maximal-length sequence of CFG (MIR) BasicBlocks -/// without conditional branches. -/// -/// The BCB allows coverage analysis to be performed on a simplified projection of the underlying -/// MIR CFG, without altering the original CFG. Note that running the MIR `SimplifyCfg` transform, -/// is not sufficient, and therefore not necessary, since the BCB-based CFG projection is a more -/// aggressive simplification. For example: -/// -/// * The BCB CFG projection ignores (trims) branches not relevant to coverage, such as unwind- -/// related code that is injected by the Rust compiler but has no physical source code to -/// count. This also means a BasicBlock with a `Call` terminator can be merged into its -/// primary successor target block, in the same BCB. -/// * Some BasicBlock terminators support Rust-specific concerns--like borrow-checking--that are -/// not relevant to coverage analysis. `FalseUnwind`, for example, can be treated the same as -/// a `Goto`, and merged with its successor into the same BCB. -/// -/// Each BCB with at least one computed `CoverageSpan` will have no more than one `Counter`. -/// In some cases, a BCB's execution count can be computed by `CounterExpression`. Additional -/// disjoint `CoverageSpan`s in a BCB can also be counted by `CounterExpression` (by adding `ZERO` -/// to the BCB's primary counter or expression). -/// -/// Dominator/dominated relationships (which are fundamental to the coverage analysis algorithm) -/// between two BCBs can be computed using the `mir::Body` `dominators()` with any `BasicBlock` -/// member of each BCB. (For consistency, BCB's use the first `BasicBlock`, also referred to as the -/// `bcb_leader_bb`.) -/// -/// The BCB CFG projection is critical to simplifying the coverage analysis by ensuring graph -/// path-based queries (`is_dominated_by()`, `predecessors`, `successors`, etc.) have branch -/// (control flow) significance. -#[derive(Debug, Clone)] -struct BasicCoverageBlock { - pub blocks: Vec, -} - -impl BasicCoverageBlock { - pub fn leader_bb(&self) -> BasicBlock { - self.blocks[0] - } - - pub fn id(&self) -> String { - format!( - "@{}", - self.blocks - .iter() - .map(|bb| bb.index().to_string()) - .collect::>() - .join(ID_SEPARATOR) - ) - } -} - -struct BasicCoverageBlocks { - vec: IndexVec>, -} - -impl BasicCoverageBlocks { - pub fn from_mir(mir_body: &mir::Body<'tcx>) -> Self { - let mut basic_coverage_blocks = - BasicCoverageBlocks { vec: IndexVec::from_elem_n(None, mir_body.basic_blocks().len()) }; - basic_coverage_blocks.extract_from_mir(mir_body); - basic_coverage_blocks - } - - pub fn iter(&self) -> impl Iterator { - self.vec.iter().filter_map(|option| option.as_ref()) - } - - fn extract_from_mir(&mut self, mir_body: &mir::Body<'tcx>) { - // Traverse the CFG but ignore anything following an `unwind` - let cfg_without_unwind = ShortCircuitPreorder::new(mir_body, |term_kind| { - let mut successors = term_kind.successors(); - match &term_kind { - // SwitchInt successors are never unwind, and all of them should be traversed. - - // NOTE: TerminatorKind::FalseEdge targets from SwitchInt don't appear to be - // helpful in identifying unreachable code. I did test the theory, but the following - // changes were not beneficial. (I assumed that replacing some constants with - // non-deterministic variables might effect which blocks were targeted by a - // `FalseEdge` `imaginary_target`. It did not.) - // - // Also note that, if there is a way to identify BasicBlocks that are part of the - // MIR CFG, but not actually reachable, here are some other things to consider: - // - // Injecting unreachable code regions will probably require computing the set - // difference between the basic blocks found without filtering out unreachable - // blocks, and the basic blocks found with the filter; then computing the - // `CoverageSpans` without the filter; and then injecting `Counter`s or - // `CounterExpression`s for blocks that are not unreachable, or injecting - // `Unreachable` code regions otherwise. This seems straightforward, but not - // trivial. - // - // Alternatively, we might instead want to leave the unreachable blocks in - // (bypass the filter here), and inject the counters. This will result in counter - // values of zero (0) for unreachable code (and, notably, the code will be displayed - // with a red background by `llvm-cov show`). - // - // TerminatorKind::SwitchInt { .. } => { - // let some_imaginary_target = successors.clone().find_map(|&successor| { - // let term = mir_body.basic_blocks()[successor].terminator(); - // if let TerminatorKind::FalseEdge { imaginary_target, .. } = term.kind { - // if mir_body.predecessors()[imaginary_target].len() == 1 { - // return Some(imaginary_target); - // } - // } - // None - // }); - // if let Some(imaginary_target) = some_imaginary_target { - // box successors.filter(move |&&successor| successor != imaginary_target) - // } else { - // box successors - // } - // } - // - // Note this also required changing the closure signature for the - // `ShortCurcuitPreorder` to: - // - // F: Fn(&'tcx TerminatorKind<'tcx>) -> Box + 'a>, - TerminatorKind::SwitchInt { .. } => successors, - - // For all other kinds, return only the first successor, if any, and ignore unwinds - _ => successors.next().into_iter().chain(&[]), - } - }); - - // Walk the CFG using a Preorder traversal, which starts from `START_BLOCK` and follows - // each block terminator's `successors()`. Coverage spans must map to actual source code, - // so compiler generated blocks and paths can be ignored. To that end the CFG traversal - // intentionally omits unwind paths. - let mut blocks = Vec::new(); - for (bb, data) in cfg_without_unwind { - if let Some(last) = blocks.last() { - let predecessors = &mir_body.predecessors()[bb]; - if predecessors.len() > 1 || !predecessors.contains(last) { - // The `bb` has more than one _incoming_ edge, and should start its own - // `BasicCoverageBlock`. (Note, the `blocks` vector does not yet include `bb`; - // it contains a sequence of one or more sequential blocks with no intermediate - // branches in or out. Save these as a new `BasicCoverageBlock` before starting - // the new one.) - self.add_basic_coverage_block(blocks.split_off(0)); - debug!( - " because {}", - if predecessors.len() > 1 { - "predecessors.len() > 1".to_owned() - } else { - format!("bb {} is not in precessors: {:?}", bb.index(), predecessors) - } - ); - } - } - blocks.push(bb); - - let term = data.terminator(); - - match term.kind { - TerminatorKind::Return { .. } - | TerminatorKind::Abort - | TerminatorKind::Assert { .. } - | TerminatorKind::Yield { .. } - | TerminatorKind::SwitchInt { .. } => { - // The `bb` has more than one _outgoing_ edge, or exits the function. Save the - // current sequence of `blocks` gathered to this point, as a new - // `BasicCoverageBlock`. - self.add_basic_coverage_block(blocks.split_off(0)); - debug!(" because term.kind = {:?}", term.kind); - // Note that this condition is based on `TerminatorKind`, even though it - // theoretically boils down to `successors().len() != 1`; that is, either zero - // (e.g., `Return`, `Abort`) or multiple successors (e.g., `SwitchInt`), but - // since the Coverage graph (the BCB CFG projection) ignores things like unwind - // branches (which exist in the `Terminator`s `successors()` list) checking the - // number of successors won't work. - } - TerminatorKind::Goto { .. } - | TerminatorKind::Resume - | TerminatorKind::Unreachable - | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } - | TerminatorKind::Call { .. } - | TerminatorKind::GeneratorDrop - | TerminatorKind::FalseEdge { .. } - | TerminatorKind::FalseUnwind { .. } - | TerminatorKind::InlineAsm { .. } => {} - } - } - - if !blocks.is_empty() { - // process any remaining blocks into a final `BasicCoverageBlock` - self.add_basic_coverage_block(blocks.split_off(0)); - debug!(" because the end of the CFG was reached while traversing"); - } - } - - fn add_basic_coverage_block(&mut self, blocks: Vec) { - let leader_bb = blocks[0]; - let bcb = BasicCoverageBlock { blocks }; - debug!("adding BCB: {:?}", bcb); - self.vec[leader_bb] = Some(bcb); - } -} - -impl std::ops::Index for BasicCoverageBlocks { - type Output = BasicCoverageBlock; - - fn index(&self, index: BasicBlock) -> &Self::Output { - self.vec[index].as_ref().expect("is_some if BasicBlock is a BasicCoverageBlock leader") - } -} - -#[derive(Debug, Copy, Clone)] -enum CoverageStatement { - Statement(BasicBlock, Span, usize), - Terminator(BasicBlock, Span), -} - -impl CoverageStatement { - pub fn format(&self, tcx: TyCtxt<'tcx>, mir_body: &'a mir::Body<'tcx>) -> String { - match *self { - Self::Statement(bb, span, stmt_index) => { - let stmt = &mir_body.basic_blocks()[bb].statements[stmt_index]; - format!( - "{}: @{}[{}]: {:?}", - spanview::source_range_no_file(tcx, &span), - bb.index(), - stmt_index, - stmt - ) - } - Self::Terminator(bb, span) => { - let term = mir_body.basic_blocks()[bb].terminator(); - format!( - "{}: @{}.{}: {:?}", - spanview::source_range_no_file(tcx, &span), - bb.index(), - term_type(&term.kind), - term.kind - ) - } - } - } - - pub fn span(&self) -> &Span { - match self { - Self::Statement(_, span, _) | Self::Terminator(_, span) => span, - } - } -} - -fn term_type(kind: &TerminatorKind<'tcx>) -> &'static str { - match kind { - TerminatorKind::Goto { .. } => "Goto", - TerminatorKind::SwitchInt { .. } => "SwitchInt", - TerminatorKind::Resume => "Resume", - TerminatorKind::Abort => "Abort", - TerminatorKind::Return => "Return", - TerminatorKind::Unreachable => "Unreachable", - TerminatorKind::Drop { .. } => "Drop", - TerminatorKind::DropAndReplace { .. } => "DropAndReplace", - TerminatorKind::Call { .. } => "Call", - TerminatorKind::Assert { .. } => "Assert", - TerminatorKind::Yield { .. } => "Yield", - TerminatorKind::GeneratorDrop => "GeneratorDrop", - TerminatorKind::FalseEdge { .. } => "FalseEdge", - TerminatorKind::FalseUnwind { .. } => "FalseUnwind", - TerminatorKind::InlineAsm { .. } => "InlineAsm", - } -} - -/// A BCB is deconstructed into one or more `Span`s. Each `Span` maps to a `CoverageSpan` that -/// references the originating BCB and one or more MIR `Statement`s and/or `Terminator`s. -/// Initially, the `Span`s come from the `Statement`s and `Terminator`s, but subsequent -/// transforms can combine adjacent `Span`s and `CoverageSpan` from the same BCB, merging the -/// `CoverageStatement` vectors, and the `Span`s to cover the extent of the combined `Span`s. -/// -/// Note: A `CoverageStatement` merged into another CoverageSpan may come from a `BasicBlock` that -/// is not part of the `CoverageSpan` bcb if the statement was included because it's `Span` matches -/// or is subsumed by the `Span` associated with this `CoverageSpan`, and it's `BasicBlock` -/// `is_dominated_by()` the `BasicBlock`s in this `CoverageSpan`. -#[derive(Debug, Clone)] -struct CoverageSpan { - span: Span, - bcb_leader_bb: BasicBlock, - coverage_statements: Vec, - is_closure: bool, -} - -impl CoverageSpan { - pub fn for_statement( - statement: &Statement<'tcx>, - span: Span, - bcb: &BasicCoverageBlock, - bb: BasicBlock, - stmt_index: usize, - ) -> Self { - let is_closure = match statement.kind { - StatementKind::Assign(box ( - _, - Rvalue::Aggregate(box AggregateKind::Closure(_, _), _), - )) => true, - _ => false, - }; - - Self { - span, - bcb_leader_bb: bcb.leader_bb(), - coverage_statements: vec![CoverageStatement::Statement(bb, span, stmt_index)], - is_closure, - } - } - - pub fn for_terminator(span: Span, bcb: &'a BasicCoverageBlock, bb: BasicBlock) -> Self { - Self { - span, - bcb_leader_bb: bcb.leader_bb(), - coverage_statements: vec![CoverageStatement::Terminator(bb, span)], - is_closure: false, - } - } - - pub fn merge_from(&mut self, mut other: CoverageSpan) { - debug_assert!(self.is_mergeable(&other)); - self.span = self.span.to(other.span); - if other.is_closure { - self.is_closure = true; - } - self.coverage_statements.append(&mut other.coverage_statements); - } - - pub fn cutoff_statements_at(&mut self, cutoff_pos: BytePos) { - self.coverage_statements.retain(|covstmt| covstmt.span().hi() <= cutoff_pos); - if let Some(highest_covstmt) = - self.coverage_statements.iter().max_by_key(|covstmt| covstmt.span().hi()) - { - self.span = self.span.with_hi(highest_covstmt.span().hi()); - } - } - - pub fn is_dominated_by( - &self, - other: &CoverageSpan, - dominators: &Dominators, - ) -> bool { - debug_assert!(!self.is_in_same_bcb(other)); - dominators.is_dominated_by(self.bcb_leader_bb, other.bcb_leader_bb) - } - - pub fn is_mergeable(&self, other: &Self) -> bool { - self.is_in_same_bcb(other) && !(self.is_closure || other.is_closure) - } - - pub fn is_in_same_bcb(&self, other: &Self) -> bool { - self.bcb_leader_bb == other.bcb_leader_bb - } -} - -struct Instrumentor<'a, 'tcx> { - pass_name: &'a str, - tcx: TyCtxt<'tcx>, - mir_body: &'a mut mir::Body<'tcx>, - hir_body: &'tcx rustc_hir::Body<'tcx>, - dominators: Option>, - basic_coverage_blocks: Option, - function_source_hash: Option, - next_counter_id: u32, - num_expressions: u32, -} - -impl<'a, 'tcx> Instrumentor<'a, 'tcx> { - fn new(pass_name: &'a str, tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self { - let hir_body = hir_body(tcx, mir_body.source.def_id()); - Self { - pass_name, - tcx, - mir_body, - hir_body, - dominators: None, - basic_coverage_blocks: None, - function_source_hash: None, - next_counter_id: CounterValueReference::START.as_u32(), - num_expressions: 0, - } - } - - /// Counter IDs start from one and go up. - fn next_counter(&mut self) -> CounterValueReference { - assert!(self.next_counter_id < u32::MAX - self.num_expressions); - let next = self.next_counter_id; - self.next_counter_id += 1; - CounterValueReference::from(next) - } - - /// Expression IDs start from u32::MAX and go down because a CounterExpression can reference - /// (add or subtract counts) of both Counter regions and CounterExpression regions. The counter - /// expression operand IDs must be unique across both types. - fn next_expression(&mut self) -> InjectedExpressionIndex { - assert!(self.next_counter_id < u32::MAX - self.num_expressions); - let next = u32::MAX - self.num_expressions; - self.num_expressions += 1; - InjectedExpressionIndex::from(next) - } - - fn dominators(&self) -> &Dominators { - self.dominators.as_ref().expect("dominators must be initialized before calling") - } - - fn basic_coverage_blocks(&self) -> &BasicCoverageBlocks { - self.basic_coverage_blocks - .as_ref() - .expect("basic_coverage_blocks must be initialized before calling") - } - - fn function_source_hash(&mut self) -> u64 { - match self.function_source_hash { - Some(hash) => hash, - None => { - let hash = hash_mir_source(self.tcx, self.hir_body); - self.function_source_hash.replace(hash); - hash - } - } - } - - fn inject_counters(&mut self) { - let tcx = self.tcx; - let source_map = tcx.sess.source_map(); - let def_id = self.mir_body.source.def_id(); - let mir_body = &self.mir_body; - let body_span = self.body_span(); - let source_file = source_map.lookup_source_file(body_span.lo()); - let file_name = Symbol::intern(&source_file.name.to_string()); - - debug!("instrumenting {:?}, span: {}", def_id, source_map.span_to_string(body_span)); - - self.dominators.replace(mir_body.dominators()); - self.basic_coverage_blocks.replace(BasicCoverageBlocks::from_mir(mir_body)); - - let coverage_spans = self.coverage_spans(); - - let span_viewables = if pretty::dump_enabled(tcx, self.pass_name, def_id) { - Some(self.span_viewables(&coverage_spans)) - } else { - None - }; - - // Inject a counter for each `CoverageSpan`. There can be multiple `CoverageSpan`s for a - // given BCB, but only one actual counter needs to be incremented per BCB. `bb_counters` - // maps each `bcb_leader_bb` to its `Counter`, when injected. Subsequent `CoverageSpan`s - // for a BCB that already has a `Counter` will inject a `CounterExpression` instead, and - // compute its value by adding `ZERO` to the BCB `Counter` value. - let mut bb_counters = IndexVec::from_elem_n(None, mir_body.basic_blocks().len()); - for CoverageSpan { span, bcb_leader_bb: bb, .. } in coverage_spans { - if let Some(&counter_operand) = bb_counters[bb].as_ref() { - let expression = - self.make_expression(counter_operand, Op::Add, ExpressionOperandId::ZERO); - debug!( - "Injecting counter expression {:?} at: {:?}:\n{}\n==========", - expression, - span, - source_map.span_to_snippet(span).expect("Error getting source for span"), - ); - self.inject_statement(file_name, &source_file, expression, span, bb); - } else { - let counter = self.make_counter(); - debug!( - "Injecting counter {:?} at: {:?}:\n{}\n==========", - counter, - span, - source_map.span_to_snippet(span).expect("Error getting source for span"), - ); - let counter_operand = counter.as_operand_id(); - bb_counters[bb] = Some(counter_operand); - self.inject_statement(file_name, &source_file, counter, span, bb); - } - } - - if let Some(span_viewables) = span_viewables { - let mut file = pretty::create_dump_file( - tcx, - "html", - None, - self.pass_name, - &0, - self.mir_body.source, - ) - .expect("Unexpected error creating MIR spanview HTML file"); - let crate_name = tcx.crate_name(def_id.krate); - let item_name = tcx.def_path(def_id).to_filename_friendly_no_crate(); - let title = format!("{}.{} - Coverage Spans", crate_name, item_name); - spanview::write_document(tcx, def_id, span_viewables, &title, &mut file) - .expect("Unexpected IO error dumping coverage spans as HTML"); - } - } - - fn make_counter(&mut self) -> CoverageKind { - CoverageKind::Counter { - function_source_hash: self.function_source_hash(), - id: self.next_counter(), - } - } - - fn make_expression( - &mut self, - lhs: ExpressionOperandId, - op: Op, - rhs: ExpressionOperandId, - ) -> CoverageKind { - CoverageKind::Expression { id: self.next_expression(), lhs, op, rhs } - } - - fn inject_statement( - &mut self, - file_name: Symbol, - source_file: &Lrc, - coverage_kind: CoverageKind, - span: Span, - block: BasicBlock, - ) { - let code_region = make_code_region(file_name, source_file, span); - debug!(" injecting statement {:?} covering {:?}", coverage_kind, code_region); - - let data = &mut self.mir_body[block]; - let source_info = data.terminator().source_info; - let statement = Statement { - source_info, - kind: StatementKind::Coverage(box Coverage { kind: coverage_kind, code_region }), - }; - data.statements.push(statement); - } - - /// Converts the computed `BasicCoverageBlock`s into `SpanViewable`s. - fn span_viewables(&self, coverage_spans: &Vec) -> Vec { - let tcx = self.tcx; - let mir_body = &self.mir_body; - let mut span_viewables = Vec::new(); - for coverage_span in coverage_spans { - let bcb = self.bcb_from_coverage_span(coverage_span); - let CoverageSpan { span, bcb_leader_bb: bb, coverage_statements, .. } = coverage_span; - let id = bcb.id(); - let mut sorted_coverage_statements = coverage_statements.clone(); - sorted_coverage_statements.sort_unstable_by_key(|covstmt| match *covstmt { - CoverageStatement::Statement(bb, _, index) => (bb, index), - CoverageStatement::Terminator(bb, _) => (bb, usize::MAX), - }); - let tooltip = sorted_coverage_statements - .iter() - .map(|covstmt| covstmt.format(tcx, mir_body)) - .collect::>() - .join("\n"); - span_viewables.push(SpanViewable { bb: *bb, span: *span, id, tooltip }); - } - span_viewables - } - - #[inline(always)] - fn bcb_from_coverage_span(&self, coverage_span: &CoverageSpan) -> &BasicCoverageBlock { - &self.basic_coverage_blocks()[coverage_span.bcb_leader_bb] - } - - #[inline(always)] - fn body_span(&self) -> Span { - self.hir_body.value.span - } - - // Generate a set of `CoverageSpan`s from the filtered set of `Statement`s and `Terminator`s of - // the `BasicBlock`(s) in the given `BasicCoverageBlock`. One `CoverageSpan` is generated for - // each `Statement` and `Terminator`. (Note that subsequent stages of coverage analysis will - // merge some `CoverageSpan`s, at which point a `CoverageSpan` may represent multiple - // `Statement`s and/or `Terminator`s.) - fn extract_spans(&self, bcb: &'a BasicCoverageBlock) -> Vec { - let body_span = self.body_span(); - let mir_basic_blocks = self.mir_body.basic_blocks(); - bcb.blocks - .iter() - .map(|bbref| { - let bb = *bbref; - let data = &mir_basic_blocks[bb]; - data.statements - .iter() - .enumerate() - .filter_map(move |(index, statement)| { - filtered_statement_span(statement, body_span).map(|span| { - CoverageSpan::for_statement(statement, span, bcb, bb, index) - }) - }) - .chain( - filtered_terminator_span(data.terminator(), body_span) - .map(|span| CoverageSpan::for_terminator(span, bcb, bb)), - ) - }) - .flatten() - .collect() - } - - /// Generate a minimal set of `CoverageSpan`s, each representing a contiguous code region to be - /// counted. - /// - /// The basic steps are: - /// - /// 1. Extract an initial set of spans from the `Statement`s and `Terminator`s of each - /// `BasicCoverageBlock`. - /// 2. Sort the spans by span.lo() (starting position). Spans that start at the same position - /// are sorted with longer spans before shorter spans; and equal spans are sorted - /// (deterministically) based on "dominator" relationship (if any). - /// 3. Traverse the spans in sorted order to identify spans that can be dropped (for instance, - /// if another span or spans are already counting the same code region), or should be merged - /// into a broader combined span (because it represents a contiguous, non-branching, and - /// uninterrupted region of source code). - /// - /// Closures are exposed in their enclosing functions as `Assign` `Rvalue`s, and since - /// closures have their own MIR, their `Span` in their enclosing function should be left - /// "uncovered". - /// - /// Note the resulting vector of `CoverageSpan`s does may not be fully sorted (and does not need - /// to be). - fn coverage_spans(&self) -> Vec { - let mut initial_spans = - Vec::::with_capacity(self.mir_body.basic_blocks().len() * 2); - for bcb in self.basic_coverage_blocks().iter() { - for coverage_span in self.extract_spans(bcb) { - initial_spans.push(coverage_span); - } - } - - if initial_spans.is_empty() { - // This can happen if, for example, the function is unreachable (contains only a - // `BasicBlock`(s) with an `Unreachable` terminator). - return initial_spans; - } - - initial_spans.sort_unstable_by(|a, b| { - if a.span.lo() == b.span.lo() { - if a.span.hi() == b.span.hi() { - if a.is_in_same_bcb(b) { - Some(Ordering::Equal) - } else { - // Sort equal spans by dominator relationship, in reverse order (so - // dominators always come after the dominated equal spans). When later - // comparing two spans in order, the first will either dominate the second, - // or they will have no dominator relationship. - self.dominators().rank_partial_cmp(b.bcb_leader_bb, a.bcb_leader_bb) - } - } else { - // Sort hi() in reverse order so shorter spans are attempted after longer spans. - // This guarantees that, if a `prev` span overlaps, and is not equal to, a `curr` - // span, the prev span either extends further left of the curr span, or they - // start at the same position and the prev span extends further right of the end - // of the curr span. - b.span.hi().partial_cmp(&a.span.hi()) - } - } else { - a.span.lo().partial_cmp(&b.span.lo()) - } - .unwrap() - }); - - let refinery = CoverageSpanRefinery::from_sorted_spans(initial_spans, self.dominators()); - refinery.to_refined_spans() - } -} - -struct CoverageSpanRefinery<'a> { - sorted_spans_iter: std::vec::IntoIter, - dominators: &'a Dominators, - some_curr: Option, - curr_original_span: Span, - some_prev: Option, - prev_original_span: Span, - pending_dups: Vec, - refined_spans: Vec, -} - -impl<'a> CoverageSpanRefinery<'a> { - fn from_sorted_spans( - sorted_spans: Vec, - dominators: &'a Dominators, - ) -> Self { - let refined_spans = Vec::with_capacity(sorted_spans.len()); - let mut sorted_spans_iter = sorted_spans.into_iter(); - let prev = sorted_spans_iter.next().expect("at least one span"); - let prev_original_span = prev.span; - Self { - sorted_spans_iter, - dominators, - refined_spans, - some_curr: None, - curr_original_span: Span::with_root_ctxt(BytePos(0), BytePos(0)), - some_prev: Some(prev), - prev_original_span, - pending_dups: Vec::new(), - } - } - - /// Iterate through the sorted `CoverageSpan`s, and return the refined list of merged and - /// de-duplicated `CoverageSpan`s. - fn to_refined_spans(mut self) -> Vec { - while self.next_coverage_span() { - if self.curr().is_mergeable(self.prev()) { - debug!(" same bcb (and neither is a closure), merge with prev={:?}", self.prev()); - let prev = self.take_prev(); - self.curr_mut().merge_from(prev); - // Note that curr.span may now differ from curr_original_span - } else if self.prev_ends_before_curr() { - debug!( - " different bcbs and disjoint spans, so keep curr for next iter, and add \ - prev={:?}", - self.prev() - ); - let prev = self.take_prev(); - self.add_refined_span(prev); - } else if self.prev().is_closure { - // drop any equal or overlapping span (`curr`) and keep `prev` to test again in the - // next iter - debug!( - " curr overlaps a closure (prev). Drop curr and keep prev for next iter. \ - prev={:?}", - self.prev() - ); - self.discard_curr(); - } else if self.curr().is_closure { - self.carve_out_span_for_closure(); - } else if self.prev_original_span == self.curr().span { - self.hold_pending_dups_unless_dominated(); - } else { - self.cutoff_prev_at_overlapping_curr(); - } - } - debug!(" AT END, adding last prev={:?}", self.prev()); - let pending_dups = self.pending_dups.split_off(0); - for dup in pending_dups.into_iter() { - debug!(" ...adding at least one pending dup={:?}", dup); - self.add_refined_span(dup); - } - let prev = self.take_prev(); - self.add_refined_span(prev); - - // FIXME(richkadel): Replace some counters with expressions if they can be calculated based - // on branching. (For example, one branch of a SwitchInt can be computed from the counter - // for the CoverageSpan just prior to the SwitchInt minus the sum of the counters of all - // other branches). - - self.to_refined_spans_without_closures() - } - - fn add_refined_span(&mut self, coverage_span: CoverageSpan) { - self.refined_spans.push(coverage_span); - } - - /// Remove `CoverageSpan`s derived from closures, originally added to ensure the coverage - /// regions for the current function leave room for the closure's own coverage regions - /// (injected separately, from the closure's own MIR). - fn to_refined_spans_without_closures(mut self) -> Vec { - self.refined_spans.retain(|covspan| !covspan.is_closure); - self.refined_spans - } - - fn curr(&self) -> &CoverageSpan { - self.some_curr - .as_ref() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) - } - - fn curr_mut(&mut self) -> &mut CoverageSpan { - self.some_curr - .as_mut() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr")) - } - - fn prev(&self) -> &CoverageSpan { - self.some_prev - .as_ref() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) - } - - fn prev_mut(&mut self) -> &mut CoverageSpan { - self.some_prev - .as_mut() - .unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) - } - - fn take_prev(&mut self) -> CoverageSpan { - self.some_prev.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev")) - } - - /// If there are `pending_dups` but `prev` is not a matching dup (`prev.span` doesn't match the - /// `pending_dups` spans), then one of the following two things happened during the previous - /// iteration: - /// * the `span` of prev was modified (by `curr_mut().merge_from(prev)`); or - /// * the `span` of prev advanced past the end of the span of pending_dups - /// (`prev().span.hi() <= curr().span.lo()`) - /// In either case, no more spans will match the span of `pending_dups`, so - /// add the `pending_dups` if they don't overlap `curr`, and clear the list. - fn check_pending_dups(&mut self) { - if let Some(dup) = self.pending_dups.last() { - if dup.span != self.prev().span { - debug!( - " SAME spans, but pending_dups are NOT THE SAME, so BCBs matched on \ - previous iteration, or prev started a new disjoint span" - ); - if dup.span.hi() <= self.curr().span.lo() { - let pending_dups = self.pending_dups.split_off(0); - for dup in pending_dups.into_iter() { - debug!(" ...adding at least one pending={:?}", dup); - self.add_refined_span(dup); - } - } else { - self.pending_dups.clear(); - } - } - } - } - - /// Advance `prev` to `curr` (if any), and `curr` to the next `CoverageSpan` in sorted order. - fn next_coverage_span(&mut self) -> bool { - if let Some(curr) = self.some_curr.take() { - self.some_prev = Some(curr); - self.prev_original_span = self.curr_original_span; - } - while let Some(curr) = self.sorted_spans_iter.next() { - debug!("FOR curr={:?}", curr); - if self.prev_starts_after_next(&curr) { - debug!( - " prev.span starts after curr.span, so curr will be dropped (skipping past \ - closure?); prev={:?}", - self.prev() - ); - } else { - // Save a copy of the original span for `curr` in case the `CoverageSpan` is changed - // by `self.curr_mut().merge_from(prev)`. - self.curr_original_span = curr.span; - self.some_curr.replace(curr); - self.check_pending_dups(); - return true; - } - } - false - } - - /// If called, then the next call to `next_coverage_span()` will *not* update `prev` with the - /// `curr` coverage span. - fn discard_curr(&mut self) { - self.some_curr = None; - } - - /// Returns true if the curr span should be skipped because prev has already advanced beyond the - /// end of curr. This can only happen if a prior iteration updated `prev` to skip past a region - /// of code, such as skipping past a closure. - fn prev_starts_after_next(&self, next_curr: &CoverageSpan) -> bool { - self.prev().span.lo() > next_curr.span.lo() - } - - /// Returns true if the curr span starts past the end of the prev span, which means they don't - /// overlap, so we now know the prev can be added to the refined coverage spans. - fn prev_ends_before_curr(&self) -> bool { - self.prev().span.hi() <= self.curr().span.lo() - } - - /// If `prev`s span extends left of the closure (`curr`), carve out the closure's - /// span from `prev`'s span. (The closure's coverage counters will be injected when - /// processing the closure's own MIR.) Add the portion of the span to the left of the - /// closure; and if the span extends to the right of the closure, update `prev` to - /// that portion of the span. For any `pending_dups`, repeat the same process. - fn carve_out_span_for_closure(&mut self) { - let curr_span = self.curr().span; - let left_cutoff = curr_span.lo(); - let right_cutoff = curr_span.hi(); - let has_pre_closure_span = self.prev().span.lo() < right_cutoff; - let has_post_closure_span = self.prev().span.hi() > right_cutoff; - let mut pending_dups = self.pending_dups.split_off(0); - if has_pre_closure_span { - let mut pre_closure = self.prev().clone(); - pre_closure.span = pre_closure.span.with_hi(left_cutoff); - debug!(" prev overlaps a closure. Adding span for pre_closure={:?}", pre_closure); - if !pending_dups.is_empty() { - for mut dup in pending_dups.iter().cloned() { - dup.span = dup.span.with_hi(left_cutoff); - debug!(" ...and at least one pre_closure dup={:?}", dup); - self.add_refined_span(dup); - } - } - self.add_refined_span(pre_closure); - } - if has_post_closure_span { - // Update prev.span to start after the closure (and discard curr) - self.prev_mut().span = self.prev().span.with_lo(right_cutoff); - self.prev_original_span = self.prev().span; - for dup in pending_dups.iter_mut() { - dup.span = dup.span.with_lo(right_cutoff); - } - self.pending_dups.append(&mut pending_dups); - self.discard_curr(); // since self.prev() was already updated - } else { - pending_dups.clear(); - } - } - - /// When two `CoverageSpan`s have the same `Span`, dominated spans can be discarded; but if - /// neither `CoverageSpan` dominates the other, both (or possibly more than two) are held, - /// until their disposition is determined. In this latter case, the `prev` dup is moved into - /// `pending_dups` so the new `curr` dup can be moved to `prev` for the next iteration. - fn hold_pending_dups_unless_dominated(&mut self) { - // equal coverage spans are ordered by dominators before dominated (if any) - debug_assert!(!self.prev().is_dominated_by(self.curr(), self.dominators)); - - if self.curr().is_dominated_by(&self.prev(), self.dominators) { - // If one span dominates the other, assocate the span with the dominator only. - // - // For example: - // match somenum { - // x if x < 1 => { ... } - // }... - // The span for the first `x` is referenced by both the pattern block (every - // time it is evaluated) and the arm code (only when matched). The counter - // will be applied only to the dominator block. - // - // The dominator's (`prev`) execution count may be higher than the dominated - // block's execution count, so drop `curr`. - debug!( - " different bcbs but SAME spans, and prev dominates curr. Drop curr and \ - keep prev for next iter. prev={:?}", - self.prev() - ); - self.discard_curr(); - } else { - // Save `prev` in `pending_dups`. (`curr` will become `prev` in the next iteration.) - // If the `curr` CoverageSpan is later discarded, `pending_dups` can be discarded as - // well; but if `curr` is added to refined_spans, the `pending_dups` will also be added. - debug!( - " different bcbs but SAME spans, and neither dominates, so keep curr for \ - next iter, and, pending upcoming spans (unless overlapping) add prev={:?}", - self.prev() - ); - let prev = self.take_prev(); - self.pending_dups.push(prev); - } - } - - /// `curr` overlaps `prev`. If `prev`s span extends left of `curr`s span, keep _only_ - /// statements that end before `curr.lo()` (if any), and add the portion of the - /// combined span for those statements. Any other statements have overlapping spans - /// that can be ignored because `curr` and/or other upcoming statements/spans inside - /// the overlap area will produce their own counters. This disambiguation process - /// avoids injecting multiple counters for overlapping spans, and the potential for - /// double-counting. - fn cutoff_prev_at_overlapping_curr(&mut self) { - debug!( - " different bcbs, overlapping spans, so ignore/drop pending and only add prev \ - if it has statements that end before curr={:?}", - self.prev() - ); - if self.pending_dups.is_empty() { - let curr_span = self.curr().span; - self.prev_mut().cutoff_statements_at(curr_span.lo()); - if self.prev().coverage_statements.is_empty() { - debug!(" ... no non-overlapping statements to add"); - } else { - debug!(" ... adding modified prev={:?}", self.prev()); - let prev = self.take_prev(); - self.add_refined_span(prev); - } - } else { - // with `pending_dups`, `prev` cannot have any statements that don't overlap - self.pending_dups.clear(); - } - } -} - -fn filtered_statement_span(statement: &'a Statement<'tcx>, body_span: Span) -> Option { - match statement.kind { - // These statements have spans that are often outside the scope of the executed source code - // for their parent `BasicBlock`. - StatementKind::StorageLive(_) - | StatementKind::StorageDead(_) - // Coverage should not be encountered, but don't inject coverage coverage - | StatementKind::Coverage(_) - // Ignore `Nop`s - | StatementKind::Nop => None, - - // FIXME(richkadel): Look into a possible issue assigning the span to a - // FakeReadCause::ForGuardBinding, in this example: - // match somenum { - // x if x < 1 => { ... } - // }... - // The BasicBlock within the match arm code included one of these statements, but the span - // for it covered the `1` in this source. The actual statements have nothing to do with that - // source span: - // FakeRead(ForGuardBinding, _4); - // where `_4` is: - // _4 = &_1; (at the span for the first `x`) - // and `_1` is the `Place` for `somenum`. - // - // The arm code BasicBlock already has its own assignment for `x` itself, `_3 = 1`, and I've - // decided it's reasonable for that span (even though outside the arm code) to be part of - // the counted coverage of the arm code execution, but I can't justify including the literal - // `1` in the arm code. I'm pretty sure that, if the `FakeRead(ForGuardBinding)` has a - // purpose in codegen, it's probably in the right BasicBlock, but if so, the `Statement`s - // `source_info.span` can't be right. - // - // Consider correcting the span assignment, assuming there is a better solution, and see if - // the following pattern can be removed here: - StatementKind::FakeRead(cause, _) if cause == FakeReadCause::ForGuardBinding => None, - - // Retain spans from all other statements - StatementKind::FakeRead(_, _) // Not including `ForGuardBinding` - | StatementKind::CopyNonOverlapping(..) - | StatementKind::Assign(_) - | StatementKind::SetDiscriminant { .. } - | StatementKind::LlvmInlineAsm(_) - | StatementKind::Retag(_, _) - | StatementKind::AscribeUserType(_, _) => { - Some(source_info_span(&statement.source_info, body_span)) - } - } -} - -fn filtered_terminator_span(terminator: &'a Terminator<'tcx>, body_span: Span) -> Option { - match terminator.kind { - // These terminators have spans that don't positively contribute to computing a reasonable - // span of actually executed source code. (For example, SwitchInt terminators extracted from - // an `if condition { block }` has a span that includes the executed block, if true, - // but for coverage, the code region executed, up to *and* through the SwitchInt, - // actually stops before the if's block.) - TerminatorKind::Unreachable // Unreachable blocks are not connected to the CFG - | TerminatorKind::Assert { .. } - | TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } - | TerminatorKind::SwitchInt { .. } - | TerminatorKind::Goto { .. } - // For `FalseEdge`, only the `real` branch is taken, so it is similar to a `Goto`. - | TerminatorKind::FalseEdge { .. } => None, - - // Retain spans from all other terminators - TerminatorKind::Resume - | TerminatorKind::Abort - | TerminatorKind::Return - | TerminatorKind::Call { .. } - | TerminatorKind::Yield { .. } - | TerminatorKind::GeneratorDrop - | TerminatorKind::FalseUnwind { .. } - | TerminatorKind::InlineAsm { .. } => { - Some(source_info_span(&terminator.source_info, body_span)) - } - } -} - -#[inline(always)] -fn source_info_span(source_info: &SourceInfo, body_span: Span) -> Span { - let span = original_sp(source_info.span, body_span).with_ctxt(SyntaxContext::root()); - if body_span.contains(span) { span } else { body_span } -} - -/// Convert the Span into its file name, start line and column, and end line and column -fn make_code_region(file_name: Symbol, source_file: &Lrc, span: Span) -> CodeRegion { - let (start_line, mut start_col) = source_file.lookup_file_pos(span.lo()); - let (end_line, end_col) = if span.hi() == span.lo() { - let (end_line, mut end_col) = (start_line, start_col); - // Extend an empty span by one character so the region will be counted. - let CharPos(char_pos) = start_col; - if char_pos > 0 { - start_col = CharPos(char_pos - 1); - } else { - end_col = CharPos(char_pos + 1); - } - (end_line, end_col) - } else { - source_file.lookup_file_pos(span.hi()) - }; - CodeRegion { - file_name, - start_line: start_line as u32, - start_col: start_col.to_u32() + 1, - end_line: end_line as u32, - end_col: end_col.to_u32() + 1, - } -} - -fn hir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx rustc_hir::Body<'tcx> { - let hir_node = tcx.hir().get_if_local(def_id).expect("expected DefId is local"); - let fn_body_id = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); - tcx.hir().body(fn_body_id) -} - -fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 { - let mut hcx = tcx.create_no_span_stable_hashing_context(); - hash(&mut hcx, &hir_body.value).to_smaller_hash() -} - -fn hash( - hcx: &mut StableHashingContext<'tcx>, - node: &impl HashStable>, -) -> Fingerprint { - let mut stable_hasher = StableHasher::new(); - node.hash_stable(hcx, &mut stable_hasher); - stable_hasher.finish() -} - -pub struct ShortCircuitPreorder< - 'a, - 'tcx, - F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>, -> { - body: &'a mir::Body<'tcx>, - visited: BitSet, - worklist: Vec, - filtered_successors: F, -} - -impl<'a, 'tcx, F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>> - ShortCircuitPreorder<'a, 'tcx, F> -{ - pub fn new( - body: &'a mir::Body<'tcx>, - filtered_successors: F, - ) -> ShortCircuitPreorder<'a, 'tcx, F> { - let worklist = vec![mir::START_BLOCK]; - - ShortCircuitPreorder { - body, - visited: BitSet::new_empty(body.basic_blocks().len()), - worklist, - filtered_successors, - } - } -} - -impl<'a: 'tcx, 'tcx, F: Fn(&'tcx TerminatorKind<'tcx>) -> mir::Successors<'tcx>> Iterator - for ShortCircuitPreorder<'a, 'tcx, F> -{ - type Item = (BasicBlock, &'a BasicBlockData<'tcx>); - - fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> { - while let Some(idx) = self.worklist.pop() { - if !self.visited.insert(idx) { - continue; - } - - let data = &self.body[idx]; - - if let Some(ref term) = data.terminator { - self.worklist.extend((self.filtered_successors)(&term.kind)); - } - - return Some((idx, data)); - } - - None - } - - fn size_hint(&self) -> (usize, Option) { - let size = self.body.basic_blocks().len() - self.visited.count(); - (size, Some(size)) - } -} diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index 53935f02a90e4..0678d8253d590 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -219,11 +219,11 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen StatementKind::LlvmInlineAsm { .. } => Err((span, "cannot use inline assembly in const fn".into())), StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping{ - dst, src, size, + dst, src, count, }) => { check_operand(tcx, dst, span, body)?; check_operand(tcx, src, span, body)?; - check_operand(tcx, size, span, body) + check_operand(tcx, count, span, body) }, // These are all NOPs StatementKind::StorageLive(_) From 049045b100f2b7f5fbc36ecd36418dec1f6853cb Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 23 Jan 2021 00:23:13 +0000 Subject: [PATCH 06/10] Replace todos with impls Changed to various implementations, copying the style of prior function calls in places I was unsure of. Also one minor style nit. --- .../rustc_codegen_ssa/src/mir/statement.rs | 3 +- .../src/borrow_check/invalidation.rs | 6 ---- compiler/rustc_mir/src/borrow_check/mod.rs | 10 +++++- .../src/borrow_check/type_check/mod.rs | 36 ++++++++++++++++++- 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 774c920ee9666..054273262f79c 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -126,8 +126,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let count = count_val.immediate_or_packed_pair(&mut bx); let dst = dst_val.immediate_or_packed_pair(&mut bx); let src = src_val.immediate_or_packed_pair(&mut bx); - use crate::MemFlags; - let flags = MemFlags::empty(); + let flags = crate::MemFlags::empty(); bx.memcpy( dst, dst_val.layout.layout.align.pref, diff --git a/compiler/rustc_mir/src/borrow_check/invalidation.rs b/compiler/rustc_mir/src/borrow_check/invalidation.rs index 1f3dfc251e10c..17c4f3c649460 100644 --- a/compiler/rustc_mir/src/borrow_check/invalidation.rs +++ b/compiler/rustc_mir/src/borrow_check/invalidation.rs @@ -100,12 +100,6 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { self.consume_operand(location, src); self.consume_operand(location, dst); self.consume_operand(location, count); - match dst { - Operand::Move(ref place) | Operand::Copy(ref place) => { - self.mutate_place(location, *place, Deep, JustWrite); - } - _ => {} - } } StatementKind::Nop | StatementKind::Coverage(..) diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index 539319ab9f25f..037abb04f56d2 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -627,7 +627,15 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc } } - StatementKind::CopyNonOverlapping(..) => todo!(), + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + src, + dst, + count, + }) => { + self.consume_operand(location, (src, span), flow_state); + self.consume_operand(location, (dst, span), flow_state); + self.consume_operand(location, (count, span), flow_state); + } StatementKind::Nop | StatementKind::Coverage(..) | StatementKind::AscribeUserType(..) diff --git a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs index 74d7fd84c9e72..58db2752c9845 100644 --- a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs @@ -1520,7 +1520,41 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { ); } } - StatementKind::CopyNonOverlapping(..) => todo!(), + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + ref src, + ref dst, + ref count, + }) => { + let op_src_ty = self.normalize(src.ty(body, self.tcx()), location); + let op_dst_ty = self.normalize(dst.ty(body, self.tcx()), location); + // since CopyNonOverlapping is parametrized by 1 type, + // we only need to check that they are equal and not keep an extra parameter. + if let Err(terr) = self.eq_types( + op_src_ty, + op_dst_ty, + location.to_locations(), + ConstraintCategory::Internal, + ) { + span_mirbug!( + self, + stmt, + "bad arg ({:?} != {:?}): {:?}", + op_src_ty, + op_dst_ty, + terr + ); + } + + let op_cnt_ty = self.normalize(count.ty(body, self.tcx()), location); + if let Err(terr) = self.eq_types( + op_cnt_ty, + tcx.types.usize, + location.to_locations(), + ConstraintCategory::Internal, + ) { + span_mirbug!(self, stmt, "bad arg ({:?} != usize): {:?}", op_cnt_ty, terr); + } + } StatementKind::FakeRead(..) | StatementKind::StorageLive(..) | StatementKind::StorageDead(..) From 845e4b5962aa84fcfc0b8a6b1e4b9e32725547ef Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 23 Jan 2021 02:28:08 +0000 Subject: [PATCH 07/10] Change CopyNonOverlapping::codegen_ssa Fixes copy_non_overlapping codegen_ssa to properly handle pointees, and use bytes instead of elem count --- .../rustc_codegen_ssa/src/mir/statement.rs | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 054273262f79c..e11539c14760c 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -122,19 +122,21 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { }) => { let dst_val = self.codegen_operand(&mut bx, dst); let src_val = self.codegen_operand(&mut bx, src); - let count_val = self.codegen_operand(&mut bx, count); - let count = count_val.immediate_or_packed_pair(&mut bx); - let dst = dst_val.immediate_or_packed_pair(&mut bx); - let src = src_val.immediate_or_packed_pair(&mut bx); - let flags = crate::MemFlags::empty(); - bx.memcpy( - dst, - dst_val.layout.layout.align.pref, - src, - src_val.layout.layout.align.pref, - count, - flags, - ); + let count = self.codegen_operand(&mut bx, count).immediate(); + let get_val_align = |oper_ref: crate::mir::OperandRef<'_, _>| match oper_ref.val { + OperandValue::Ref(val, _, align) => (val, align), + _ => unreachable!(), + }; + let pointee_layout = dst_val + .layout + .pointee_info_at(&mut bx, rustc_target::abi::Size::ZERO) + .expect("Expected pointer"); + let elem_size = bx.const_u64(pointee_layout.size.bytes()); + let byte_count = bx.mul(count, elem_size); + + let (dst, dst_align) = get_val_align(dst_val); + let (src, src_align) = get_val_align(src_val); + bx.memcpy(dst, dst_align, src, src_align, byte_count, crate::MemFlags::empty()); bx } mir::StatementKind::FakeRead(..) From d4ae9ff82664a1d7473e32d59819c208efce48c7 Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 23 Jan 2021 03:55:41 +0000 Subject: [PATCH 08/10] Build StKind::CopyOverlapping This replaces where it was previously being constructed in intrinsics, with direct construction of the Statement. --- compiler/rustc_codegen_cranelift/src/base.rs | 15 +- compiler/rustc_codegen_ssa/src/lib.rs | 1 + compiler/rustc_codegen_ssa/src/mir/block.rs | 130 ++++++++++-------- .../rustc_codegen_ssa/src/mir/intrinsic.rs | 13 +- .../rustc_codegen_ssa/src/mir/statement.rs | 14 +- 5 files changed, 98 insertions(+), 75 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index ba7c82d24c517..8b5ae9e0541ad 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -837,10 +837,21 @@ fn codegen_stmt<'tcx>( dst, count, }) => { - let dst = codegen_operand(fx, dst).load_scalar(fx); + let dst = codegen_operand(fx, dst); + let pointee = dst + .layout() + .pointee_info_at(fx, rustc_target::abi::Size::ZERO) + .expect("Expected pointer"); + let dst = dst.load_scalar(fx); let src = codegen_operand(fx, src).load_scalar(fx); let count = codegen_operand(fx, count).load_scalar(fx); - fx.bcx.call_memcpy(fx.cx.module.target_config(), dst, src, count); + let elem_size: u64 = pointee.size.bytes(); + let bytes = if elem_size != 1 { + fx.bcx.ins().imul_imm(count, elem_size as i64) + } else { + count + }; + fx.bcx.call_memcpy(fx.cx.module.target_config(), dst, src, bytes); } } } diff --git a/compiler/rustc_codegen_ssa/src/lib.rs b/compiler/rustc_codegen_ssa/src/lib.rs index 0307117e1c8b2..2c2330409fd70 100644 --- a/compiler/rustc_codegen_ssa/src/lib.rs +++ b/compiler/rustc_codegen_ssa/src/lib.rs @@ -9,6 +9,7 @@ #![feature(or_patterns)] #![feature(associated_type_bounds)] #![recursion_limit = "256"] +#![feature(box_syntax)] //! This crate contains codegen code that is used by all codegen backends (LLVM and others). //! The backend-agnostic functions of this crate use functions defined in various traits that diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 9ce9066980066..1150d4d734870 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -641,67 +641,89 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { return; } - if intrinsic.is_some() && intrinsic != Some(sym::drop_in_place) { - let intrinsic = intrinsic.unwrap(); - let dest = match ret_dest { - _ if fn_abi.ret.is_indirect() => llargs[0], - ReturnDest::Nothing => { - bx.const_undef(bx.type_ptr_to(bx.arg_memory_ty(&fn_abi.ret))) - } - ReturnDest::IndirectOperand(dst, _) | ReturnDest::Store(dst) => dst.llval, - ReturnDest::DirectOperand(_) => { - bug!("Cannot use direct operand with an intrinsic call") - } - }; + match intrinsic { + None | Some(sym::drop_in_place) => {} + Some(sym::copy_nonoverlapping) => { + bx = self.codegen_statement( + bx, + &rustc_middle::mir::Statement { + source_info: rustc_middle::mir::SourceInfo::outermost(span), + kind: rustc_middle::mir::StatementKind::CopyNonOverlapping( + box rustc_middle::mir::CopyNonOverlapping { + src: args[0].clone(), + dst: args[1].clone(), + count: args[2].clone(), + }, + ), + }, + ); + helper.funclet_br(self, &mut bx, destination.unwrap().1); + return; + } + Some(intrinsic) => { + let dest = match ret_dest { + _ if fn_abi.ret.is_indirect() => llargs[0], + ReturnDest::Nothing => { + bx.const_undef(bx.type_ptr_to(bx.arg_memory_ty(&fn_abi.ret))) + } + ReturnDest::IndirectOperand(dst, _) | ReturnDest::Store(dst) => dst.llval, + ReturnDest::DirectOperand(_) => { + bug!("Cannot use direct operand with an intrinsic call") + } + }; - let args: Vec<_> = args - .iter() - .enumerate() - .map(|(i, arg)| { - // The indices passed to simd_shuffle* in the - // third argument must be constant. This is - // checked by const-qualification, which also - // promotes any complex rvalues to constants. - if i == 2 && intrinsic.as_str().starts_with("simd_shuffle") { - if let mir::Operand::Constant(constant) = arg { - let c = self.eval_mir_constant(constant); - let (llval, ty) = self.simd_shuffle_indices( - &bx, - constant.span, - constant.literal.ty, - c, - ); - return OperandRef { val: Immediate(llval), layout: bx.layout_of(ty) }; - } else { - span_bug!(span, "shuffle indices must be constant"); + let args: Vec<_> = args + .iter() + .enumerate() + .map(|(i, arg)| { + // The indices passed to simd_shuffle* in the + // third argument must be constant. This is + // checked by const-qualification, which also + // promotes any complex rvalues to constants. + if i == 2 && intrinsic.as_str().starts_with("simd_shuffle") { + if let mir::Operand::Constant(constant) = arg { + let c = self.eval_mir_constant(constant); + let (llval, ty) = self.simd_shuffle_indices( + &bx, + constant.span, + constant.literal.ty, + c, + ); + return OperandRef { + val: Immediate(llval), + layout: bx.layout_of(ty), + }; + } else { + span_bug!(span, "shuffle indices must be constant"); + } } - } - self.codegen_operand(&mut bx, arg) - }) - .collect(); + self.codegen_operand(&mut bx, arg) + }) + .collect(); + + self.codegen_intrinsic_call( + &mut bx, + *instance.as_ref().unwrap(), + &fn_abi, + &args, + dest, + span, + ); - Self::codegen_intrinsic_call( - &mut bx, - *instance.as_ref().unwrap(), - &fn_abi, - &args, - dest, - span, - ); + if let ReturnDest::IndirectOperand(dst, _) = ret_dest { + self.store_return(&mut bx, ret_dest, &fn_abi.ret, dst.llval); + } - if let ReturnDest::IndirectOperand(dst, _) = ret_dest { - self.store_return(&mut bx, ret_dest, &fn_abi.ret, dst.llval); - } + if let Some((_, target)) = *destination { + helper.maybe_sideeffect(self.mir, &mut bx, &[target]); + helper.funclet_br(self, &mut bx, target); + } else { + bx.unreachable(); + } - if let Some((_, target)) = *destination { - helper.maybe_sideeffect(self.mir, &mut bx, &[target]); - helper.funclet_br(self, &mut bx, target); - } else { - bx.unreachable(); + return; } - - return; } // Split the rust-call tupled arguments off. diff --git a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs index 80e3ed75b8585..00fc5b6606151 100644 --- a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs +++ b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs @@ -49,6 +49,7 @@ fn memset_intrinsic<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { pub fn codegen_intrinsic_call( + &self, bx: &mut Bx, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, @@ -127,16 +128,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } sym::copy_nonoverlapping => { - copy_intrinsic( - bx, - false, - false, - substs.type_at(0), - args[1].immediate(), - args[0].immediate(), - args[2].immediate(), - ); - return; + // handled explicitly in compiler/rustc_codegen_ssa/src/mir/block.rs + unreachable!(); } sym::copy => { copy_intrinsic( diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index e11539c14760c..5523e5f2e8604 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -123,20 +123,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let dst_val = self.codegen_operand(&mut bx, dst); let src_val = self.codegen_operand(&mut bx, src); let count = self.codegen_operand(&mut bx, count).immediate(); - let get_val_align = |oper_ref: crate::mir::OperandRef<'_, _>| match oper_ref.val { - OperandValue::Ref(val, _, align) => (val, align), - _ => unreachable!(), - }; let pointee_layout = dst_val .layout .pointee_info_at(&mut bx, rustc_target::abi::Size::ZERO) .expect("Expected pointer"); - let elem_size = bx.const_u64(pointee_layout.size.bytes()); - let byte_count = bx.mul(count, elem_size); + let bytes = bx.mul(count, bx.const_usize(pointee_layout.size.bytes())); - let (dst, dst_align) = get_val_align(dst_val); - let (src, src_align) = get_val_align(src_val); - bx.memcpy(dst, dst_align, src, src_align, byte_count, crate::MemFlags::empty()); + let align = pointee_layout.align; + let dst = dst_val.immediate(); + let src = src_val.immediate(); + bx.memcpy(dst, align, src, align, bytes, crate::MemFlags::empty()); bx } mir::StatementKind::FakeRead(..) From 217ff6b7ea5ca80b01ee1436914a061ed190d8a8 Mon Sep 17 00:00:00 2001 From: kadmin Date: Sat, 23 Jan 2021 08:57:04 +0000 Subject: [PATCH 09/10] Switch to changing cp_non_overlap in tform It was suggested to lower this in MIR instead of ssa, so do that instead. --- compiler/rustc_codegen_ssa/src/mir/block.rs | 20 +------ .../rustc_codegen_ssa/src/mir/intrinsic.rs | 6 -- compiler/rustc_middle/src/mir/mod.rs | 2 +- compiler/rustc_mir/src/borrow_check/mod.rs | 6 ++ .../rustc_mir/src/interpret/intrinsics.rs | 12 ++-- compiler/rustc_mir/src/interpret/step.rs | 56 ++++++++++--------- .../rustc_mir/src/transform/check_unsafety.rs | 2 +- .../src/transform/lower_intrinsics.rs | 21 +++++++ .../src/transform/remove_noop_landing_pads.rs | 2 +- .../clippy_utils/src/qualify_min_const_fn.rs | 2 +- 10 files changed, 67 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 1150d4d734870..e148ed7ad3bce 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -643,23 +643,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { match intrinsic { None | Some(sym::drop_in_place) => {} - Some(sym::copy_nonoverlapping) => { - bx = self.codegen_statement( - bx, - &rustc_middle::mir::Statement { - source_info: rustc_middle::mir::SourceInfo::outermost(span), - kind: rustc_middle::mir::StatementKind::CopyNonOverlapping( - box rustc_middle::mir::CopyNonOverlapping { - src: args[0].clone(), - dst: args[1].clone(), - count: args[2].clone(), - }, - ), - }, - ); - helper.funclet_br(self, &mut bx, destination.unwrap().1); - return; - } + Some(sym::copy_nonoverlapping) => unreachable!(), Some(intrinsic) => { let dest = match ret_dest { _ if fn_abi.ret.is_indirect() => llargs[0], @@ -702,7 +686,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { }) .collect(); - self.codegen_intrinsic_call( + Self::codegen_intrinsic_call( &mut bx, *instance.as_ref().unwrap(), &fn_abi, diff --git a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs index 00fc5b6606151..8502309b90e5a 100644 --- a/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs +++ b/compiler/rustc_codegen_ssa/src/mir/intrinsic.rs @@ -49,7 +49,6 @@ fn memset_intrinsic<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { pub fn codegen_intrinsic_call( - &self, bx: &mut Bx, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, @@ -126,11 +125,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let offset = args[1].immediate(); bx.gep(ptr, &[offset]) } - - sym::copy_nonoverlapping => { - // handled explicitly in compiler/rustc_codegen_ssa/src/mir/block.rs - unreachable!(); - } sym::copy => { copy_intrinsic( bx, diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index c046333e68d2b..f6952667494db 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1682,7 +1682,7 @@ pub struct Coverage { pub code_region: Option, } -#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)] +#[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable)] pub struct CopyNonOverlapping<'tcx> { pub src: Operand<'tcx>, pub dst: Operand<'tcx>, diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index 037abb04f56d2..8683318e4a7b5 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -628,13 +628,19 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc } StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + .. + /* src, dst, count, + */ }) => { + unreachable!() + /* self.consume_operand(location, (src, span), flow_state); self.consume_operand(location, (dst, span), flow_state); self.consume_operand(location, (count, span), flow_state); + */ } StatementKind::Nop | StatementKind::Coverage(..) diff --git a/compiler/rustc_mir/src/interpret/intrinsics.rs b/compiler/rustc_mir/src/interpret/intrinsics.rs index d36b3a7d9b56e..b0a2d906519ac 100644 --- a/compiler/rustc_mir/src/interpret/intrinsics.rs +++ b/compiler/rustc_mir/src/interpret/intrinsics.rs @@ -323,7 +323,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let result = Scalar::from_uint(truncated_bits, layout.size); self.write_scalar(result, dest)?; } - sym::copy | sym::copy_nonoverlapping => { + sym::copy_nonoverlapping => { + self.copy_nonoverlapping(args[0], args[1], args[2])?; + } + sym::copy => { let elem_ty = instance.substs.type_at(0); let elem_layout = self.layout_of(elem_ty)?; let count = self.read_scalar(&args[2])?.to_machine_usize(self)?; @@ -338,12 +341,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let dest = self.memory.check_ptr_access(dest, size, elem_align)?; if let (Some(src), Some(dest)) = (src, dest) { - self.memory.copy( - src, - dest, - size, - intrinsic_name == sym::copy_nonoverlapping, - )?; + self.memory.copy(src, dest, size, false)?; } } sym::offset => { diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index 89dc93c790090..1c2bc57c99a83 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -2,6 +2,7 @@ //! //! The main entry point is the `step` method. +use crate::interpret::OpTy; use rustc_middle::mir; use rustc_middle::mir::interpret::{InterpResult, Scalar}; use rustc_target::abi::LayoutOf; @@ -115,35 +116,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Call CopyNonOverlapping CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { dst, src, count }) => { - let (src, size) = { - let src = self.eval_operand(src, None)?; - let size = src.layout.layout.size; - let mplace = *src.assert_mem_place(self); - let ptr = match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), - }; - (ptr, size) - }; - - let dst = { - let dst = self.eval_operand(dst, None)?; - let mplace = *dst.assert_mem_place(self); - match mplace.ptr { - Scalar::Ptr(ptr) => ptr, - _ => panic!(), - } - }; - let count = self.eval_operand(count, None)?; - let count = self.read_immediate(count)?.to_scalar()?; - let count = if let Scalar::Int(i) = count { - core::convert::TryFrom::try_from(i).unwrap() - } else { - panic!(); - }; - self.memory.copy_repeatedly(src, dst, size, count, /*nonoverlapping*/ true)?; + let src = self.eval_operand(src, None)?; + let dst = self.eval_operand(dst, None)?; + self.copy_nonoverlapping(src, dst, count)?; } // Statements we do not track. @@ -173,6 +150,31 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } + pub(crate) fn copy_nonoverlapping( + &mut self, + src: OpTy<'tcx, >::PointerTag>, + dst: OpTy<'tcx, >::PointerTag>, + count: OpTy<'tcx, >::PointerTag>, + ) -> InterpResult<'tcx> { + let count = self.read_scalar(&count)?.to_machine_usize(self)?; + let layout = self.layout_of(src.layout.ty.builtin_deref(true).unwrap().ty)?; + let (size, align) = (layout.size, layout.align.abi); + let src = + self.memory.check_ptr_access(self.read_scalar(&src)?.check_init()?, size, align)?; + + let dst = + self.memory.check_ptr_access(self.read_scalar(&dst)?.check_init()?, size, align)?; + + let size = size.checked_mul(count, self).ok_or_else(|| { + err_ub_format!("overflow computing total size of `copy_nonoverlapping`") + })?; + + if let (Some(src), Some(dst)) = (src, dst) { + self.memory.copy(src, dst, size, /*nonoverlapping*/ true)?; + } + Ok(()) + } + /// Evaluate an assignment statement. /// /// There is no separate `eval_rvalue` function. Instead, the code for handling each rvalue diff --git a/compiler/rustc_mir/src/transform/check_unsafety.rs b/compiler/rustc_mir/src/transform/check_unsafety.rs index e7fdd5496cb40..33848bc130581 100644 --- a/compiler/rustc_mir/src/transform/check_unsafety.rs +++ b/compiler/rustc_mir/src/transform/check_unsafety.rs @@ -115,7 +115,6 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { | StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) - | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // safe (at least as emitted during MIR construction) } @@ -124,6 +123,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { UnsafetyViolationKind::General, UnsafetyViolationDetails::UseOfInlineAssembly, ), + StatementKind::CopyNonOverlapping(..) => unreachable!(), } self.super_statement(statement, location); } diff --git a/compiler/rustc_mir/src/transform/lower_intrinsics.rs b/compiler/rustc_mir/src/transform/lower_intrinsics.rs index 177b00b00da36..d6a7336061608 100644 --- a/compiler/rustc_mir/src/transform/lower_intrinsics.rs +++ b/compiler/rustc_mir/src/transform/lower_intrinsics.rs @@ -40,6 +40,27 @@ impl<'tcx> MirPass<'tcx> for LowerIntrinsics { terminator.kind = TerminatorKind::Goto { target }; } } + sym::copy_nonoverlapping => { + let target = destination.unwrap().1; + let mut args = args.drain(..); + block.statements.push(Statement { + source_info: terminator.source_info, + kind: StatementKind::CopyNonOverlapping( + box rustc_middle::mir::CopyNonOverlapping { + src: args.next().unwrap(), + dst: args.next().unwrap(), + count: args.next().unwrap(), + }, + ), + }); + assert_eq!( + args.next(), + None, + "Extra argument for copy_non_overlapping intrinsic" + ); + drop(args); + terminator.kind = TerminatorKind::Goto { target }; + } sym::wrapping_add | sym::wrapping_sub | sym::wrapping_mul => { if let Some((destination, target)) = *destination { let lhs; diff --git a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs index a37f5d4f329f3..5347846a4b334 100644 --- a/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs +++ b/compiler/rustc_mir/src/transform/remove_noop_landing_pads.rs @@ -39,7 +39,6 @@ impl RemoveNoopLandingPads { | StatementKind::StorageDead(_) | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) - | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => { // These are all nops in a landing pad } @@ -56,6 +55,7 @@ impl RemoveNoopLandingPads { StatementKind::Assign { .. } | StatementKind::SetDiscriminant { .. } | StatementKind::LlvmInlineAsm { .. } + | StatementKind::CopyNonOverlapping(..) | StatementKind::Retag { .. } => { return false; } diff --git a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs index 0678d8253d590..1391f7505e27c 100644 --- a/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs +++ b/src/tools/clippy/clippy_utils/src/qualify_min_const_fn.rs @@ -224,7 +224,7 @@ fn check_statement(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, def_id: DefId, statemen check_operand(tcx, dst, span, body)?; check_operand(tcx, src, span, body)?; check_operand(tcx, count, span, body) - }, + } // These are all NOPs StatementKind::StorageLive(_) | StatementKind::StorageDead(_) From 4bceb294f419066e98cab9a953a43ddeaea5494a Mon Sep 17 00:00:00 2001 From: kadmin Date: Fri, 26 Feb 2021 16:42:51 +0000 Subject: [PATCH 10/10] Clean up todos Also add some span_bugs where it is unreachable --- compiler/rustc_codegen_cranelift/src/lib.rs | 1 - compiler/rustc_mir/src/borrow_check/mod.rs | 15 ++----- .../src/borrow_check/type_check/mod.rs | 39 +++------------- .../rustc_mir/src/dataflow/impls/borrows.rs | 3 +- .../src/dataflow/impls/storage_liveness.rs | 3 +- .../src/dataflow/move_paths/builder.rs | 3 +- .../rustc_mir/src/interpret/intrinsics.rs | 20 +-------- compiler/rustc_mir/src/interpret/step.rs | 13 +++--- compiler/rustc_mir/src/transform/validate.rs | 44 ++++++++++++++++++- 9 files changed, 63 insertions(+), 78 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/lib.rs b/compiler/rustc_codegen_cranelift/src/lib.rs index fae71fef9e676..8edb883ccb5f9 100644 --- a/compiler/rustc_codegen_cranelift/src/lib.rs +++ b/compiler/rustc_codegen_cranelift/src/lib.rs @@ -11,7 +11,6 @@ #![warn(rust_2018_idioms)] #![warn(unused_lifetimes)] #![warn(unreachable_pub)] -#![feature(box_patterns)] extern crate snap; #[macro_use] diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs index 8683318e4a7b5..5b8bb7257e230 100644 --- a/compiler/rustc_mir/src/borrow_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/mod.rs @@ -629,18 +629,11 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { .. - /* - src, - dst, - count, - */ }) => { - unreachable!() - /* - self.consume_operand(location, (src, span), flow_state); - self.consume_operand(location, (dst, span), flow_state); - self.consume_operand(location, (count, span), flow_state); - */ + span_bug!( + span, + "Unexpected CopyNonOverlapping, should only appear after lower_intrinsics", + ) } StatementKind::Nop | StatementKind::Coverage(..) diff --git a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs index 58db2752c9845..ab7e75bf4f10c 100644 --- a/compiler/rustc_mir/src/borrow_check/type_check/mod.rs +++ b/compiler/rustc_mir/src/borrow_check/type_check/mod.rs @@ -1521,40 +1521,11 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { } } StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { - ref src, - ref dst, - ref count, - }) => { - let op_src_ty = self.normalize(src.ty(body, self.tcx()), location); - let op_dst_ty = self.normalize(dst.ty(body, self.tcx()), location); - // since CopyNonOverlapping is parametrized by 1 type, - // we only need to check that they are equal and not keep an extra parameter. - if let Err(terr) = self.eq_types( - op_src_ty, - op_dst_ty, - location.to_locations(), - ConstraintCategory::Internal, - ) { - span_mirbug!( - self, - stmt, - "bad arg ({:?} != {:?}): {:?}", - op_src_ty, - op_dst_ty, - terr - ); - } - - let op_cnt_ty = self.normalize(count.ty(body, self.tcx()), location); - if let Err(terr) = self.eq_types( - op_cnt_ty, - tcx.types.usize, - location.to_locations(), - ConstraintCategory::Internal, - ) { - span_mirbug!(self, stmt, "bad arg ({:?} != usize): {:?}", op_cnt_ty, terr); - } - } + .. + }) => span_bug!( + stmt.source_info.span, + "Unexpected StatementKind::CopyNonOverlapping, should only appear after lowering_intrinsics", + ), StatementKind::FakeRead(..) | StatementKind::StorageLive(..) | StatementKind::StorageDead(..) diff --git a/compiler/rustc_mir/src/dataflow/impls/borrows.rs b/compiler/rustc_mir/src/dataflow/impls/borrows.rs index ffa02f855c979..f24d0f0266d9f 100644 --- a/compiler/rustc_mir/src/dataflow/impls/borrows.rs +++ b/compiler/rustc_mir/src/dataflow/impls/borrows.rs @@ -305,9 +305,8 @@ impl<'tcx> dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> { | mir::StatementKind::Retag { .. } | mir::StatementKind::AscribeUserType(..) | mir::StatementKind::Coverage(..) + | mir::StatementKind::CopyNonOverlapping(..) | mir::StatementKind::Nop => {} - - mir::StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs index e407f394c51ec..792664597fd9a 100644 --- a/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs +++ b/compiler/rustc_mir/src/dataflow/impls/storage_liveness.rs @@ -149,9 +149,8 @@ impl<'mir, 'tcx> dataflow::GenKillAnalysis<'tcx> for MaybeRequiresStorage<'mir, | StatementKind::FakeRead(..) | StatementKind::Nop | StatementKind::Retag(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::StorageLive(..) => {} - - StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/dataflow/move_paths/builder.rs b/compiler/rustc_mir/src/dataflow/move_paths/builder.rs index c14ac74ebadab..1ddd81e779b15 100644 --- a/compiler/rustc_mir/src/dataflow/move_paths/builder.rs +++ b/compiler/rustc_mir/src/dataflow/move_paths/builder.rs @@ -318,9 +318,8 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { StatementKind::Retag { .. } | StatementKind::AscribeUserType(..) | StatementKind::Coverage(..) + | StatementKind::CopyNonOverlapping(..) | StatementKind::Nop => {} - - StatementKind::CopyNonOverlapping(..) => todo!(), } } diff --git a/compiler/rustc_mir/src/interpret/intrinsics.rs b/compiler/rustc_mir/src/interpret/intrinsics.rs index b0a2d906519ac..25c3c2c632d81 100644 --- a/compiler/rustc_mir/src/interpret/intrinsics.rs +++ b/compiler/rustc_mir/src/interpret/intrinsics.rs @@ -323,26 +323,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let result = Scalar::from_uint(truncated_bits, layout.size); self.write_scalar(result, dest)?; } - sym::copy_nonoverlapping => { - self.copy_nonoverlapping(args[0], args[1], args[2])?; - } sym::copy => { - let elem_ty = instance.substs.type_at(0); - let elem_layout = self.layout_of(elem_ty)?; - let count = self.read_scalar(&args[2])?.to_machine_usize(self)?; - let elem_align = elem_layout.align.abi; - - let size = elem_layout.size.checked_mul(count, self).ok_or_else(|| { - err_ub_format!("overflow computing total size of `{}`", intrinsic_name) - })?; - let src = self.read_scalar(&args[0])?.check_init()?; - let src = self.memory.check_ptr_access(src, size, elem_align)?; - let dest = self.read_scalar(&args[1])?.check_init()?; - let dest = self.memory.check_ptr_access(dest, size, elem_align)?; - - if let (Some(src), Some(dest)) = (src, dest) { - self.memory.copy(src, dest, size, false)?; - } + self.copy(&args[0], &args[1], &args[2], /*nonoverlapping*/ false)?; } sym::offset => { let ptr = self.read_scalar(&args[0])?.check_init()?; diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index 1c2bc57c99a83..0f365eaa41dde 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -120,7 +120,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let src = self.eval_operand(src, None)?; let dst = self.eval_operand(dst, None)?; - self.copy_nonoverlapping(src, dst, count)?; + self.copy(&src, &dst, &count, /* nonoverlapping */ true)?; } // Statements we do not track. @@ -150,11 +150,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } - pub(crate) fn copy_nonoverlapping( + pub(crate) fn copy( &mut self, - src: OpTy<'tcx, >::PointerTag>, - dst: OpTy<'tcx, >::PointerTag>, - count: OpTy<'tcx, >::PointerTag>, + src: &OpTy<'tcx, >::PointerTag>, + dst: &OpTy<'tcx, >::PointerTag>, + count: &OpTy<'tcx, >::PointerTag>, + nonoverlapping: bool, ) -> InterpResult<'tcx> { let count = self.read_scalar(&count)?.to_machine_usize(self)?; let layout = self.layout_of(src.layout.ty.builtin_deref(true).unwrap().ty)?; @@ -170,7 +171,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { })?; if let (Some(src), Some(dst)) = (src, dst) { - self.memory.copy(src, dst, size, /*nonoverlapping*/ true)?; + self.memory.copy(src, dst, size, nonoverlapping)?; } Ok(()) } diff --git a/compiler/rustc_mir/src/transform/validate.rs b/compiler/rustc_mir/src/transform/validate.rs index 29b90bff210bd..d009b0b1b2384 100644 --- a/compiler/rustc_mir/src/transform/validate.rs +++ b/compiler/rustc_mir/src/transform/validate.rs @@ -294,7 +294,49 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { ); } } - _ => {} + StatementKind::CopyNonOverlapping(box rustc_middle::mir::CopyNonOverlapping { + ref src, + ref dst, + ref count, + }) => { + let src_ty = src.ty(&self.body.local_decls, self.tcx); + let op_src_ty = if let Some(src_deref) = src_ty.builtin_deref(true) { + src_deref.ty + } else { + self.fail( + location, + format!("Expected src to be ptr in copy_nonoverlapping, got: {}", src_ty), + ); + return; + }; + let dst_ty = dst.ty(&self.body.local_decls, self.tcx); + let op_dst_ty = if let Some(dst_deref) = dst_ty.builtin_deref(true) { + dst_deref.ty + } else { + self.fail( + location, + format!("Expected dst to be ptr in copy_nonoverlapping, got: {}", dst_ty), + ); + return; + }; + // since CopyNonOverlapping is parametrized by 1 type, + // we only need to check that they are equal and not keep an extra parameter. + if op_src_ty != op_dst_ty { + self.fail(location, format!("bad arg ({:?} != {:?})", op_src_ty, op_dst_ty)); + } + + let op_cnt_ty = count.ty(&self.body.local_decls, self.tcx); + if op_cnt_ty != self.tcx.types.usize { + self.fail(location, format!("bad arg ({:?} != usize)", op_cnt_ty)) + } + } + StatementKind::SetDiscriminant { .. } + | StatementKind::StorageLive(..) + | StatementKind::StorageDead(..) + | StatementKind::LlvmInlineAsm(..) + | StatementKind::Retag(_, _) + | StatementKind::Coverage(_) + | StatementKind::Nop => {} } self.super_statement(statement, location);