From 5ecda78d107b5c97ff568799e64018d29fa2d038 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Tue, 24 Mar 2020 11:13:20 -0700 Subject: [PATCH 1/6] WIP - Add LLVM coverage instrumentation --- src/librustc_ast_lowering/lib.rs | 184 +++++++++++++++++++++++++++++++ src/librustc_session/options.rs | 2 + 2 files changed, 186 insertions(+) diff --git a/src/librustc_ast_lowering/lib.rs b/src/librustc_ast_lowering/lib.rs index 68cb1f8585fc7..4d529ce77ce47 100644 --- a/src/librustc_ast_lowering/lib.rs +++ b/src/librustc_ast_lowering/lib.rs @@ -40,6 +40,7 @@ use rustc_ast::ast; use rustc_ast::ast::*; use rustc_ast::attr; use rustc_ast::node_id::NodeMap; +use rustc_ast::ptr::P; use rustc_ast::token::{self, Nonterminal, Token}; use rustc_ast::tokenstream::{TokenStream, TokenTree}; use rustc_ast::visit::{self, AssocCtxt, Visitor}; @@ -69,6 +70,7 @@ use log::{debug, trace}; use smallvec::{smallvec, SmallVec}; use std::collections::BTreeMap; use std::mem; +use std::sync::Mutex; macro_rules! arena_vec { ($this:expr; $($x:expr),*) => ({ @@ -171,6 +173,9 @@ struct LoweringContext<'a, 'hir: 'a> { allow_try_trait: Option>, allow_gen_future: Option>, + + /// `true` if `rustc` was invoked with an option to inject code coverage instrumentation. + is_instrumenting_for_coverage: bool, } pub trait Resolver { @@ -300,6 +305,7 @@ pub fn lower_crate<'a, 'hir>( in_scope_lifetimes: Vec::new(), allow_try_trait: Some([sym::try_trait][..].into()), allow_gen_future: Some([sym::gen_future][..].into()), + is_instrumenting_for_coverage: false, } .lower_crate(krate) } @@ -394,6 +400,148 @@ impl Visitor<'_> for ImplTraitTypeIdVisitor<'_> { } } +struct CoverageRegion { + region: Span, + counter_hash: u128, +} + +static mut COVERAGE_REGIONS: Option>> = None; + +impl CoverageRegion { + + /// Generates a unique coverage region identifier to associate with + /// a counter. The counter increment statement is injected into the + /// code at the start of a coverage region. + // TODO(richkadel): This function will need additional arguments and/or context + // data from which to generate the hash values. + fn generate_hash(region: &Span) -> u128 { + // THIS IS NOT THREAD SAFE, BUT WILL BE REPLACED WITH HASH FUNCTION ANYWAY. + // Seems like lazy_static is not used in the compiler at all. + static mut NEXT_COUNTER_ID: Option> = None; + let counter_hash = { + let counter_id = unsafe { + &match NEXT_COUNTER_ID.as_ref() { + Some(counter_id) => counter_id, + None => { + NEXT_COUNTER_ID = Some(Mutex::new(0)); + NEXT_COUNTER_ID.as_ref().unwrap() + } + } + }; + let mut locked_counter_id = counter_id.lock().unwrap(); + *locked_counter_id += 1; + *locked_counter_id + }; + + let coverage_regions = unsafe { + &match COVERAGE_REGIONS.as_ref() { + Some(coverage_regions) => coverage_regions, + None => { + COVERAGE_REGIONS = Some(Mutex::new(vec![])); + COVERAGE_REGIONS.as_ref().unwrap() + } + } + }; + let mut locked_coverage_regions = coverage_regions.lock().unwrap(); + locked_coverage_regions.push(CoverageRegion { + region: region.clone(), + counter_hash, + }); + + // return the counter hash value + counter_hash + } + + pub fn write_coverage_regions(/* filename param? */) { + unsafe { + if let Some(coverage_regions) = COVERAGE_REGIONS.as_ref() { + let locked_coverage_regions = coverage_regions.lock().unwrap(); + for coverage in locked_coverage_regions.iter() { + println!("{}: {:?}", coverage.counter_hash, coverage.region); + } + } + } + } +} + +struct CoverageInjector<'tcx, 'lowering, 'hir> { + lctx: &'tcx mut LoweringContext<'lowering, 'hir>, + span: Span, +} + +impl<'tcx, 'lowering, 'hir> CoverageInjector<'tcx, 'lowering, 'hir> { + fn at(lctx: &'tcx mut LoweringContext<'lowering, 'hir>, before: &Span) -> CoverageInjector<'tcx, 'lowering, 'hir> { + CoverageInjector { + lctx, + span: before.shrink_to_lo(), + } + } + + fn next_ast_node_id(&mut self) -> NodeId { + self.lctx.resolver.next_node_id() + } + + fn span(&self) -> Span { + self.span.clone() + } + + fn expr(&mut self, kind: ExprKind) -> P { + P(Expr { + kind, + span: self.span(), + attrs: AttrVec::new(), + id: self.next_ast_node_id(), + }) + } + + fn path_segment(&mut self, string: &str) -> PathSegment { + PathSegment { + ident: Ident::from_str_and_span(string, self.span()), + id: self.next_ast_node_id(), + args: None, + } + } + + fn coverage_count_fn_path(&mut self) -> P { + let path = Path { + span: self.span(), + segments: vec![ + self.path_segment("coverage"), + self.path_segment("count"), + ], + }; + self.expr(ExprKind::Path(None, path)) + } + + fn coverage_counter_hash_lit(&mut self, counter_hash: u128) -> P { + let token = token::Lit::new(token::Integer, sym::integer(counter_hash), /*suffix=*/None); + let kind = LitKind::Int( + counter_hash, + LitIntType::Unsigned(UintTy::U128), // TODO: this should not be necessary (should be "Unsuffixed" per JSON) + ); + let lit = Lit { token, kind, span: self.span() }; + self.expr(ExprKind::Lit(lit)) + } + + fn call(&mut self, fn_path: P, args: Vec>) -> P { + self.expr(ExprKind::Call(fn_path, args)) + } + + fn counter_stmt(&mut self, coverage_span: &Span) -> Stmt { + let counter_hash = CoverageRegion::generate_hash(coverage_span); + let coverage_count_fn = self.coverage_count_fn_path(); + let args = vec![ self.coverage_counter_hash_lit(counter_hash) ]; + let call = self.call(coverage_count_fn, args); + + Stmt { + id: self.next_ast_node_id(), + span: self.span(), + kind: StmtKind::Semi(call) + } + } +} + + impl<'a, 'hir> LoweringContext<'a, 'hir> { fn lower_crate(mut self, c: &Crate) -> hir::Crate<'hir> { /// Full-crate AST visitor that inserts into a fresh @@ -521,12 +669,21 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } } + self.is_instrumenting_for_coverage = match self.sess.opts.debugging_opts.instrument_coverage { + Some(opt) => opt, + None => false, + }; + self.lower_node_id(CRATE_NODE_ID); debug_assert!(self.node_id_to_hir_id[CRATE_NODE_ID] == hir::CRATE_HIR_ID); visit::walk_crate(&mut MiscCollector { lctx: &mut self, hir_id_owner: None }, c); visit::walk_crate(&mut item::ItemLowerer { lctx: &mut self }, c); + if self.is_instrumenting_for_coverage { + CoverageRegion::write_coverage_regions(); + } + let module = self.lower_mod(&c.module); let attrs = self.lower_attrs(&c.attrs); let body_ids = body_ids(&self.bodies); @@ -2208,6 +2365,33 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let mut stmts = vec![]; let mut expr: Option<&'hir _> = None; + // THIS COVERAGE CODE MAY NEED TO BE IN A DIFFERENT LOCATION? BUT SEEMS LIKE IT SHOULD + // WORK FOR BASIC BLOCKS THAT DON'T `break`, `continue`, or `return` in any nested + // branches. For those, we need to add additional counters beyond the code that might be + // skipped. + if self.is_instrumenting_for_coverage && ! b.stmts.is_empty() { + let inject_site = &b.stmts[0].span; + let mut coverage_injector = CoverageInjector::at(self, inject_site); + let counter_stmt = coverage_injector.counter_stmt(&b.span); + stmts.extend(self.lower_stmt(&counter_stmt)); + // TODO(richkadel): The span should stop at the first occurrence of an early return + // (if any), and if there is one, a new counter should be inserted just after the branch + // that ended in the early return, and a new span should start from just after that + // injected counter, and ending at the end of the block, or at another early return if + // there is another. + + // ALSO! There are language constructs that have statement spans that don't require + // braces if only one statement, in which case, they PROBABLY don't hit "Block", and + // therefore, I need to insert the counters in other parts of the AST as well, while + // also virtually inserting the curly braces: + // * closures: |...| stmt -> |...| { coverage::counter(n); stmt } + // * match arms: match variant { pat => stmt, -> match variant { pat => coverage::counter(n); stmt } + // any others? + + // There may be existing checks of similar types. See for instance "targeted_by_break" + // in this file. + } + for (index, stmt) in b.stmts.iter().enumerate() { if index == b.stmts.len() - 1 { if let StmtKind::Expr(ref e) = stmt.kind { diff --git a/src/librustc_session/options.rs b/src/librustc_session/options.rs index a1ecf4e8528be..2b8d7fd3e4edd 100644 --- a/src/librustc_session/options.rs +++ b/src/librustc_session/options.rs @@ -968,4 +968,6 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "link the `.rlink` file generated by `-Z no-link`"), new_llvm_pass_manager: Option = (None, parse_opt_bool, [TRACKED], "use new LLVM pass manager"), + instrument_coverage: Option = (None, parse_opt_bool, [TRACKED], + "inject code coverage counters and export a counter-to-coverage-span map"), } From 7ff63c293e00448106e6bfa1070f991d6e3c8763 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Tue, 24 Mar 2020 11:13:20 -0700 Subject: [PATCH 2/6] WIP - Add LLVM coverage instrumentation --- Cargo.lock | 12 ++ src/librustc_coverage/Cargo.toml | 16 +++ src/librustc_coverage/coverage.rs | 224 ++++++++++++++++++++++++++++++ src/librustc_coverage/lib.rs | 5 + src/librustc_driver/lib.rs | 3 + src/librustc_interface/Cargo.toml | 1 + src/librustc_interface/queries.rs | 62 ++++++++- src/librustc_session/session.rs | 1 + 8 files changed, 322 insertions(+), 2 deletions(-) create mode 100644 src/librustc_coverage/Cargo.toml create mode 100644 src/librustc_coverage/coverage.rs create mode 100644 src/librustc_coverage/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 22a06151353ba..a8f39d5b8ab22 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3613,6 +3613,17 @@ dependencies = [ "tempfile", ] +[[package]] +name = "rustc_coverage" +version = "0.0.0" +dependencies = [ + "log", + "rustc", + "rustc_ast", + "rustc_resolve", + "rustc_span", +] + [[package]] name = "rustc_data_structures" version = "0.0.0" @@ -3798,6 +3809,7 @@ dependencies = [ "rustc_builtin_macros", "rustc_codegen_llvm", "rustc_codegen_ssa", + "rustc_coverage", "rustc_data_structures", "rustc_errors", "rustc_expand", diff --git a/src/librustc_coverage/Cargo.toml b/src/librustc_coverage/Cargo.toml new file mode 100644 index 0000000000000..6a1b06a2b96ff --- /dev/null +++ b/src/librustc_coverage/Cargo.toml @@ -0,0 +1,16 @@ +[package] +authors = ["The Rust Project Developers"] +name = "rustc_coverage" +version = "0.0.0" +edition = "2018" + +[lib] +name = "rustc_coverage" +path = "lib.rs" + +[dependencies] +log = "0.4" +rustc = { path = "../librustc" } +rustc_resolve = { path = "../librustc_resolve" } +rustc_ast = { path = "../librustc_ast" } +rustc_span = { path = "../librustc_span" } \ No newline at end of file diff --git a/src/librustc_coverage/coverage.rs b/src/librustc_coverage/coverage.rs new file mode 100644 index 0000000000000..fca64905308b5 --- /dev/null +++ b/src/librustc_coverage/coverage.rs @@ -0,0 +1,224 @@ +//! Injects code coverage instrumentation into the AST. + +use rustc::util::common::ErrorReported; +use rustc_ast::ast::*; +use rustc_ast::ptr::P; +use rustc_ast::token; +use rustc_ast::mut_visit::{self, MutVisitor}; +use rustc_resolve::Resolver; +use rustc_span::symbol::sym; +use rustc_span::Span; + +use log::{debug, trace}; +use std::ops::DerefMut; +use std::sync::Mutex; + +pub type Result = std::result::Result; + +struct CoverageRegion { + region: Span, + counter_hash: u128, +} + +static mut COVERAGE_REGIONS: Option>> = None; + +impl CoverageRegion { + + /// Generates a unique coverage region identifier to associate with + /// a counter. The counter increment statement is injected into the + /// code at the start of a coverage region. + // TODO(richkadel): This function will need additional arguments and/or context + // data from which to generate the hash values. + fn generate_hash(region: &Span) -> u128 { + // THIS IS NOT THREAD SAFE, BUT WILL BE REPLACED WITH HASH FUNCTION ANYWAY. + // Seems like lazy_static is not used in the compiler at all. + static mut NEXT_COUNTER_ID: Option> = None; + let counter_hash = { + let counter_id = unsafe { + &match NEXT_COUNTER_ID.as_ref() { + Some(counter_id) => counter_id, + None => { + NEXT_COUNTER_ID = Some(Mutex::new(0)); + NEXT_COUNTER_ID.as_ref().unwrap() + } + } + }; + let mut locked_counter_id = counter_id.lock().unwrap(); + *locked_counter_id += 1; + *locked_counter_id + }; + + let coverage_regions = unsafe { + &match COVERAGE_REGIONS.as_ref() { + Some(coverage_regions) => coverage_regions, + None => { + COVERAGE_REGIONS = Some(Mutex::new(vec![])); + COVERAGE_REGIONS.as_ref().unwrap() + } + } + }; + let mut locked_coverage_regions = coverage_regions.lock().unwrap(); + locked_coverage_regions.push(CoverageRegion { + region: region.clone(), + counter_hash, + }); + + // return the counter hash value + counter_hash + } + + pub fn write_coverage_regions(/* filename param? */) { + unsafe { + if let Some(coverage_regions) = COVERAGE_REGIONS.as_ref() { + let locked_coverage_regions = coverage_regions.lock().unwrap(); + for coverage in locked_coverage_regions.iter() { + println!("{}: {:?}", coverage.counter_hash, coverage.region); + } + } + } + } +} + +struct CoverageInjector<'res,'internal> { + resolver: &'res mut Resolver<'internal>, + span: Span, +} + +impl CoverageInjector<'_,'_> { + fn at<'res,'internal>(resolver: &'res mut Resolver<'internal>, span: Span) -> CoverageInjector<'res,'internal> { + CoverageInjector { + resolver, + span, + } + } + + fn next_ast_node_id(&mut self) -> NodeId { + self.resolver.next_node_id() + } + + fn span(&self) -> Span { + self.span.clone() + } + + fn expr(&mut self, kind: ExprKind) -> P { + P(Expr { + kind, + span: self.span(), + attrs: AttrVec::new(), + id: self.next_ast_node_id(), + }) + } + + fn path_segment(&mut self, string: &str) -> PathSegment { + PathSegment { + ident: Ident::from_str_and_span(string, self.span()), + id: self.next_ast_node_id(), + args: None, + } + } + + fn coverage_count_fn_path(&mut self) -> P { + let path = Path { + span: self.span(), + segments: vec![ + self.path_segment("coverage"), + self.path_segment("count"), + ], + }; + self.expr(ExprKind::Path(None, path)) + } + + fn coverage_counter_hash_lit(&mut self, counter_hash: u128) -> P { + let token = token::Lit::new(token::Integer, sym::integer(counter_hash), /*suffix=*/None); + let kind = LitKind::Int( + counter_hash, + LitIntType::Unsigned(UintTy::U128), // TODO: this should not be necessary (should be "Unsuffixed" per JSON) + ); + let lit = Lit { token, kind, span: self.span() }; + self.expr(ExprKind::Lit(lit)) + } + + fn call(&mut self, fn_path: P, args: Vec>) -> P { + self.expr(ExprKind::Call(fn_path, args)) + } + + fn counter_stmt(&mut self, coverage_span: &Span) -> Stmt { + let counter_hash = CoverageRegion::generate_hash(coverage_span); + let coverage_count_fn = self.coverage_count_fn_path(); + let args = vec![ self.coverage_counter_hash_lit(counter_hash) ]; + let call = self.call(coverage_count_fn, args); + + Stmt { + id: self.next_ast_node_id(), + span: self.span(), + kind: StmtKind::Semi(call) + } + } +} + +struct CoverageVisitor<'res,'internal> { + resolver: &'res mut Resolver<'internal>, +} + +impl CoverageVisitor<'_,'_> { + + fn instrument_block(&mut self, block: &mut Block) { + trace!("instrument_block: {:?}", block); + let _ = self.resolver; + if let Some(last) = block.stmts.last() { + let inject_site = if last.is_expr() { + last.span.shrink_to_lo() + } else { + last.span.shrink_to_hi() + }; + + let mut coverage_injector = CoverageInjector::at(&mut self.resolver, inject_site); + let counter_stmt = coverage_injector.counter_stmt(&block.span); + if last.is_expr() { + block.stmts.insert(block.stmts.len()-1, counter_stmt); + } else { + block.stmts.push(counter_stmt); + } + + // TODO(richkadel): The span should stop at the first occurrence of an early return + // (if any), and if there is one, a new counter should be inserted just after the branch + // that ended in the early return (or panic?), and a new span should start from just after that + // injected counter, and ending at the end of the block, or at another early return if + // there is another. + + // FIRST DEMO VERSION SHOULD WORK FOR BASIC BLOCKS + // THAT DON'T `break`, `continue`, `return`, or panic in any nested + // branches. For those, we need to add additional counters beyond the code that might be + // skipped. + // + // IMPORTANT! BREAK UP BLOCKS THAT MAY PANIC, SIMILAR TO EARLY RETURN, OR INJECT COUNTER + // AFTER THE LAST `Semi` Stmt, AND NOTE PANICS IN SOME OTHER WAY? + + // There may be existing checks of similar types. See for instance "targeted_by_break" + // in librustc_ast_lowering/src/lib.rs + } + } +} + +impl MutVisitor for CoverageVisitor<'_,'_> { + + fn visit_block(&mut self, block: &mut P) { + self.instrument_block(block.deref_mut()); + mut_visit::noop_visit_block(block, self); + } + + // TODO(richkadel): visit match arm expr (if not block, wrap expr in block and inject counter) + // ALSO! There are language constructs that have statement spans that don't require + // braces if only one statement, in which case, they PROBABLY don't hit "Block", and + // therefore, I need to insert the counters in other parts of the AST as well, while + // also virtually inserting the curly braces: + // * closures: |...| stmt -> |...| { coverage::counter(n); stmt } + // * match arms: match variant { pat => stmt, ...} -> match variant { pat => { coverage::counter(n); stmt } ... } + // any others? +} + +pub fn instrument<'res>(krate: &mut Crate, resolver: &'res mut Resolver<'_>) { //, resolver: &'a mut Resolver<'a>) { + debug!("Calling instrument for {:?}", krate); + mut_visit::noop_visit_crate(krate, &mut CoverageVisitor { resolver } ); // , &mut CoverageVisitor::new(resolver)); + CoverageRegion::write_coverage_regions(); +} \ No newline at end of file diff --git a/src/librustc_coverage/lib.rs b/src/librustc_coverage/lib.rs new file mode 100644 index 0000000000000..8d0c6d2a05fc1 --- /dev/null +++ b/src/librustc_coverage/lib.rs @@ -0,0 +1,5 @@ +#![doc(html_root_url = "https://doc.rust-lang.org/nightly/")] +#![feature(nll)] +#![recursion_limit = "256"] + +pub mod coverage; \ No newline at end of file diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index 9097a72f36f49..c3780a5be4b47 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -46,6 +46,7 @@ use rustc_session::{early_error, early_warn}; use rustc_span::source_map::{FileLoader, FileName}; use rustc_span::symbol::sym; +use log::debug; use std::borrow::Cow; use std::cmp::max; use std::default::Default; @@ -337,7 +338,9 @@ pub fn run_compiler( } } + debug!("before death"); queries.expansion()?; + debug!("AFTER DEATH (we don't get here?)"); if callbacks.after_expansion(compiler, queries) == Compilation::Stop { return early_exit(); } diff --git a/src/librustc_interface/Cargo.toml b/src/librustc_interface/Cargo.toml index 2e055ff183f2c..ede5ed3932284 100644 --- a/src/librustc_interface/Cargo.toml +++ b/src/librustc_interface/Cargo.toml @@ -38,6 +38,7 @@ rustc_mir_build = { path = "../librustc_mir_build" } rustc_passes = { path = "../librustc_passes" } rustc_typeck = { path = "../librustc_typeck" } rustc_lint = { path = "../librustc_lint" } +rustc_coverage = { path = "../librustc_coverage" } rustc_errors = { path = "../librustc_errors" } rustc_plugin_impl = { path = "../librustc_plugin_impl" } rustc_privacy = { path = "../librustc_privacy" } diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index b0eeb57173fa3..600ce202d19cd 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -8,6 +8,7 @@ use rustc::ty::{GlobalCtxt, ResolverOutputs, TyCtxt}; use rustc::util::common::ErrorReported; use rustc_ast::{self, ast}; use rustc_codegen_ssa::traits::CodegenBackend; +use rustc_coverage::coverage; use rustc_data_structures::sync::{Lrc, Once, WorkerLocal}; use rustc_hir::def_id::LOCAL_CRATE; use rustc_hir::Crate; @@ -16,6 +17,8 @@ use rustc_lint::LintStore; use rustc_session::config::{OutputFilenames, OutputType}; use rustc_session::{output::find_crate_name, Session}; use rustc_span::symbol::sym; + +use log::debug; use std::any::Any; use std::cell::{Ref, RefCell, RefMut}; use std::mem; @@ -75,6 +78,7 @@ pub struct Queries<'tcx> { crate_name: Query, register_plugins: Query<(ast::Crate, Lrc)>, expansion: Query<(ast::Crate, Steal>>, Lrc)>, + instrument: Query<(ast::Crate, Steal>>, Lrc)>, dep_graph: Query, lower_to_hir: Query<(&'tcx Crate<'tcx>, Steal)>, prepare_outputs: Query, @@ -94,6 +98,7 @@ impl<'tcx> Queries<'tcx> { crate_name: Default::default(), register_plugins: Default::default(), expansion: Default::default(), + instrument: Default::default(), dep_graph: Default::default(), lower_to_hir: Default::default(), prepare_outputs: Default::default(), @@ -110,7 +115,9 @@ impl<'tcx> Queries<'tcx> { } pub fn dep_graph_future(&self) -> Result<&Query>> { + debug!("query dep_graph_future"); self.dep_graph_future.compute(|| { + debug!("compute query dep_graph_future"); Ok(self .session() .opts @@ -120,7 +127,9 @@ impl<'tcx> Queries<'tcx> { } pub fn parse(&self) -> Result<&Query> { + debug!("query parse"); self.parse.compute(|| { + debug!("compute query parse"); passes::parse(self.session(), &self.compiler.input).map_err(|mut parse_error| { parse_error.emit(); ErrorReported @@ -129,7 +138,9 @@ impl<'tcx> Queries<'tcx> { } pub fn register_plugins(&self) -> Result<&Query<(ast::Crate, Lrc)>> { + debug!("query register_plugins"); self.register_plugins.compute(|| { + debug!("compute query register_plugins"); let crate_name = self.crate_name()?.peek().clone(); let krate = self.parse()?.take(); @@ -154,7 +165,9 @@ impl<'tcx> Queries<'tcx> { } pub fn crate_name(&self) -> Result<&Query> { + debug!("query crate_name"); self.crate_name.compute(|| { + debug!("compute query crate_name"); Ok(match self.compiler.crate_name { Some(ref crate_name) => crate_name.clone(), None => { @@ -169,10 +182,13 @@ impl<'tcx> Queries<'tcx> { pub fn expansion( &self, ) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { + debug!("query expansion"); self.expansion.compute(|| { + debug!("compute query expansion"); let crate_name = self.crate_name()?.peek().clone(); let (krate, lint_store) = self.register_plugins()?.take(); let _timer = self.session().timer("configure_and_expand"); +debug!("just before death"); passes::configure_and_expand( self.session().clone(), lint_store.clone(), @@ -181,13 +197,16 @@ impl<'tcx> Queries<'tcx> { &crate_name, ) .map(|(krate, resolver)| { +debug!("NEVER GET HERE"); (krate, Steal::new(Rc::new(RefCell::new(resolver))), lint_store) }) }) } pub fn dep_graph(&self) -> Result<&Query> { + debug!("query dep_graph"); self.dep_graph.compute(|| { + debug!("compute query dep_graph"); Ok(match self.dep_graph_future()?.take() { None => DepGraph::new_disabled(), Some(future) => { @@ -206,10 +225,42 @@ impl<'tcx> Queries<'tcx> { }) } + /// Instrumentation is optional (typically included based on `rustc` option). The + /// `instrument` pass/query depends on (and implies) `expansion`. To ensure the instrumentation + /// pass is executed (if requested), replace queries to `expansion` with queries to + /// `instrument`. The query results are the same. If instrumentation is not requested, + /// `expansion` will be queried and returned. If instrumentation is requested, `instrument` + /// will query `expansion`, inject instrumentation code (modifying the crate and updating the + /// `next_node_id()` in the resolver), and then return the query result from `expansion`, with + /// the changes to crate and resolver. + pub fn instrument(&'tcx self) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { + debug!("query instrument (depends on expansion), will instrument coverage if requested"); + let expansion = self.expansion(); + let instrument_coverage = match self.session().opts.debugging_opts.instrument_coverage { + Some(opt) => opt, + None => false, + }; + if instrument_coverage { + self.instrument.compute(|| { + debug!("compute query instrument (depends on expansion), will instrument coverage if requested"); + let (mut krate, boxed_resolver, lint_store) = expansion?.take(); + let resolver = boxed_resolver.steal(); + resolver.borrow_mut().access(|resolver| { + coverage::instrument(&mut krate, resolver) + }); + Ok((krate, Steal::new(resolver), lint_store)) + }) + } else { + expansion + } + } + pub fn lower_to_hir(&'tcx self) -> Result<&Query<(&'tcx Crate<'tcx>, Steal)>> { + debug!("query lower_to_hir"); self.lower_to_hir.compute(|| { - let expansion_result = self.expansion()?; - let peeked = expansion_result.peek(); + debug!("compute query lower_to_hir"); + let instrument_result = self.instrument()?; + let peeked = instrument_result.peek(); let krate = &peeked.0; let resolver = peeked.1.steal(); let lint_store = &peeked.2; @@ -229,7 +280,9 @@ impl<'tcx> Queries<'tcx> { } pub fn prepare_outputs(&self) -> Result<&Query> { + debug!("query prepare_outputs"); self.prepare_outputs.compute(|| { + debug!("compute query prepare_outputs"); let expansion_result = self.expansion()?; let (krate, boxed_resolver, _) = &*expansion_result.peek(); let crate_name = self.crate_name()?; @@ -245,7 +298,9 @@ impl<'tcx> Queries<'tcx> { } pub fn global_ctxt(&'tcx self) -> Result<&Query>> { + debug!("query global_ctxt"); self.global_ctxt.compute(|| { + debug!("compute query global_ctxt"); let crate_name = self.crate_name()?.peek().clone(); let outputs = self.prepare_outputs()?.peek().clone(); let lint_store = self.expansion()?.peek().2.clone(); @@ -268,7 +323,9 @@ impl<'tcx> Queries<'tcx> { } pub fn ongoing_codegen(&'tcx self) -> Result<&Query>> { + debug!("query ongoing_codegen"); self.ongoing_codegen.compute(|| { + debug!("compute query ongoing_codegen"); let outputs = self.prepare_outputs()?; self.global_ctxt()?.peek_mut().enter(|tcx| { tcx.analysis(LOCAL_CRATE).ok(); @@ -356,6 +413,7 @@ pub struct Linker { impl Linker { pub fn link(self) -> Result<()> { + debug!("Linker::link"); let codegen_results = self.codegen_backend.join_codegen(self.ongoing_codegen, &self.sess, &self.dep_graph)?; let prof = self.sess.prof.clone(); diff --git a/src/librustc_session/session.rs b/src/librustc_session/session.rs index 80f59aff69137..adc6532028237 100644 --- a/src/librustc_session/session.rs +++ b/src/librustc_session/session.rs @@ -440,6 +440,7 @@ impl Session { } pub fn init_features(&self, features: rustc_feature::Features) { + debug!("ABOUT TO PANIC?") self.features.set(features); } From 1b452bb5d73a9db0771e66a01ca066c07d079577 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Tue, 24 Mar 2020 11:13:20 -0700 Subject: [PATCH 3/6] WIP - Add LLVM coverage instrumentation https://github.com/rust-lang/rust/issues/34701 Prototype coverage implementation with partial support for the various branch types to be covered. cargo -vv rustc -- -Z instrument-coverage With the `-Z instrument-coverage` flag, the compiler injects calls to a new std::coverage module, adding counters at various points in the code. At the end of the compile, a list of coverage regions (`Span`s) and corresponding coverage retion IDs are printed to stdout. When executing the program, the counters increment statically, and upon normal exit, the counters are printed to stdout with the same corresponding coverage region IDs. The stdout results will be replaced by output files in a format compatible with LLVM coverage reports. Currently counts the following coverage regions: * All blocks ending in a semicolon or expression * break, return, and yield statements, with or without a value expression * continue statment Not yet supported are: * match pattern arms without a block * closures without a block * expressions ending in '?' (that return on Result::Err) * lazy boolean right-hand-side expressions that may not be invoked --- Cargo.lock | 1 + src/librustc_ast_lowering/lib.rs | 184 --------------------- src/librustc_coverage/Cargo.toml | 3 +- src/librustc_coverage/coverage.rs | 257 +++++++++++++++++++----------- src/librustc_coverage/lib.rs | 2 +- src/librustc_driver/lib.rs | 3 - src/librustc_interface/passes.rs | 65 ++++++-- src/librustc_interface/queries.rs | 109 ++++++------- src/librustc_session/session.rs | 1 - src/libstd/coverage.rs | 81 ++++++++++ src/libstd/lib.rs | 1 + 11 files changed, 349 insertions(+), 358 deletions(-) create mode 100644 src/libstd/coverage.rs diff --git a/Cargo.lock b/Cargo.lock index a8f39d5b8ab22..8e511999d56e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3622,6 +3622,7 @@ dependencies = [ "rustc_ast", "rustc_resolve", "rustc_span", + "smallvec 1.0.0", ] [[package]] diff --git a/src/librustc_ast_lowering/lib.rs b/src/librustc_ast_lowering/lib.rs index 4d529ce77ce47..68cb1f8585fc7 100644 --- a/src/librustc_ast_lowering/lib.rs +++ b/src/librustc_ast_lowering/lib.rs @@ -40,7 +40,6 @@ use rustc_ast::ast; use rustc_ast::ast::*; use rustc_ast::attr; use rustc_ast::node_id::NodeMap; -use rustc_ast::ptr::P; use rustc_ast::token::{self, Nonterminal, Token}; use rustc_ast::tokenstream::{TokenStream, TokenTree}; use rustc_ast::visit::{self, AssocCtxt, Visitor}; @@ -70,7 +69,6 @@ use log::{debug, trace}; use smallvec::{smallvec, SmallVec}; use std::collections::BTreeMap; use std::mem; -use std::sync::Mutex; macro_rules! arena_vec { ($this:expr; $($x:expr),*) => ({ @@ -173,9 +171,6 @@ struct LoweringContext<'a, 'hir: 'a> { allow_try_trait: Option>, allow_gen_future: Option>, - - /// `true` if `rustc` was invoked with an option to inject code coverage instrumentation. - is_instrumenting_for_coverage: bool, } pub trait Resolver { @@ -305,7 +300,6 @@ pub fn lower_crate<'a, 'hir>( in_scope_lifetimes: Vec::new(), allow_try_trait: Some([sym::try_trait][..].into()), allow_gen_future: Some([sym::gen_future][..].into()), - is_instrumenting_for_coverage: false, } .lower_crate(krate) } @@ -400,148 +394,6 @@ impl Visitor<'_> for ImplTraitTypeIdVisitor<'_> { } } -struct CoverageRegion { - region: Span, - counter_hash: u128, -} - -static mut COVERAGE_REGIONS: Option>> = None; - -impl CoverageRegion { - - /// Generates a unique coverage region identifier to associate with - /// a counter. The counter increment statement is injected into the - /// code at the start of a coverage region. - // TODO(richkadel): This function will need additional arguments and/or context - // data from which to generate the hash values. - fn generate_hash(region: &Span) -> u128 { - // THIS IS NOT THREAD SAFE, BUT WILL BE REPLACED WITH HASH FUNCTION ANYWAY. - // Seems like lazy_static is not used in the compiler at all. - static mut NEXT_COUNTER_ID: Option> = None; - let counter_hash = { - let counter_id = unsafe { - &match NEXT_COUNTER_ID.as_ref() { - Some(counter_id) => counter_id, - None => { - NEXT_COUNTER_ID = Some(Mutex::new(0)); - NEXT_COUNTER_ID.as_ref().unwrap() - } - } - }; - let mut locked_counter_id = counter_id.lock().unwrap(); - *locked_counter_id += 1; - *locked_counter_id - }; - - let coverage_regions = unsafe { - &match COVERAGE_REGIONS.as_ref() { - Some(coverage_regions) => coverage_regions, - None => { - COVERAGE_REGIONS = Some(Mutex::new(vec![])); - COVERAGE_REGIONS.as_ref().unwrap() - } - } - }; - let mut locked_coverage_regions = coverage_regions.lock().unwrap(); - locked_coverage_regions.push(CoverageRegion { - region: region.clone(), - counter_hash, - }); - - // return the counter hash value - counter_hash - } - - pub fn write_coverage_regions(/* filename param? */) { - unsafe { - if let Some(coverage_regions) = COVERAGE_REGIONS.as_ref() { - let locked_coverage_regions = coverage_regions.lock().unwrap(); - for coverage in locked_coverage_regions.iter() { - println!("{}: {:?}", coverage.counter_hash, coverage.region); - } - } - } - } -} - -struct CoverageInjector<'tcx, 'lowering, 'hir> { - lctx: &'tcx mut LoweringContext<'lowering, 'hir>, - span: Span, -} - -impl<'tcx, 'lowering, 'hir> CoverageInjector<'tcx, 'lowering, 'hir> { - fn at(lctx: &'tcx mut LoweringContext<'lowering, 'hir>, before: &Span) -> CoverageInjector<'tcx, 'lowering, 'hir> { - CoverageInjector { - lctx, - span: before.shrink_to_lo(), - } - } - - fn next_ast_node_id(&mut self) -> NodeId { - self.lctx.resolver.next_node_id() - } - - fn span(&self) -> Span { - self.span.clone() - } - - fn expr(&mut self, kind: ExprKind) -> P { - P(Expr { - kind, - span: self.span(), - attrs: AttrVec::new(), - id: self.next_ast_node_id(), - }) - } - - fn path_segment(&mut self, string: &str) -> PathSegment { - PathSegment { - ident: Ident::from_str_and_span(string, self.span()), - id: self.next_ast_node_id(), - args: None, - } - } - - fn coverage_count_fn_path(&mut self) -> P { - let path = Path { - span: self.span(), - segments: vec![ - self.path_segment("coverage"), - self.path_segment("count"), - ], - }; - self.expr(ExprKind::Path(None, path)) - } - - fn coverage_counter_hash_lit(&mut self, counter_hash: u128) -> P { - let token = token::Lit::new(token::Integer, sym::integer(counter_hash), /*suffix=*/None); - let kind = LitKind::Int( - counter_hash, - LitIntType::Unsigned(UintTy::U128), // TODO: this should not be necessary (should be "Unsuffixed" per JSON) - ); - let lit = Lit { token, kind, span: self.span() }; - self.expr(ExprKind::Lit(lit)) - } - - fn call(&mut self, fn_path: P, args: Vec>) -> P { - self.expr(ExprKind::Call(fn_path, args)) - } - - fn counter_stmt(&mut self, coverage_span: &Span) -> Stmt { - let counter_hash = CoverageRegion::generate_hash(coverage_span); - let coverage_count_fn = self.coverage_count_fn_path(); - let args = vec![ self.coverage_counter_hash_lit(counter_hash) ]; - let call = self.call(coverage_count_fn, args); - - Stmt { - id: self.next_ast_node_id(), - span: self.span(), - kind: StmtKind::Semi(call) - } - } -} - - impl<'a, 'hir> LoweringContext<'a, 'hir> { fn lower_crate(mut self, c: &Crate) -> hir::Crate<'hir> { /// Full-crate AST visitor that inserts into a fresh @@ -669,21 +521,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } } - self.is_instrumenting_for_coverage = match self.sess.opts.debugging_opts.instrument_coverage { - Some(opt) => opt, - None => false, - }; - self.lower_node_id(CRATE_NODE_ID); debug_assert!(self.node_id_to_hir_id[CRATE_NODE_ID] == hir::CRATE_HIR_ID); visit::walk_crate(&mut MiscCollector { lctx: &mut self, hir_id_owner: None }, c); visit::walk_crate(&mut item::ItemLowerer { lctx: &mut self }, c); - if self.is_instrumenting_for_coverage { - CoverageRegion::write_coverage_regions(); - } - let module = self.lower_mod(&c.module); let attrs = self.lower_attrs(&c.attrs); let body_ids = body_ids(&self.bodies); @@ -2365,33 +2208,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let mut stmts = vec![]; let mut expr: Option<&'hir _> = None; - // THIS COVERAGE CODE MAY NEED TO BE IN A DIFFERENT LOCATION? BUT SEEMS LIKE IT SHOULD - // WORK FOR BASIC BLOCKS THAT DON'T `break`, `continue`, or `return` in any nested - // branches. For those, we need to add additional counters beyond the code that might be - // skipped. - if self.is_instrumenting_for_coverage && ! b.stmts.is_empty() { - let inject_site = &b.stmts[0].span; - let mut coverage_injector = CoverageInjector::at(self, inject_site); - let counter_stmt = coverage_injector.counter_stmt(&b.span); - stmts.extend(self.lower_stmt(&counter_stmt)); - // TODO(richkadel): The span should stop at the first occurrence of an early return - // (if any), and if there is one, a new counter should be inserted just after the branch - // that ended in the early return, and a new span should start from just after that - // injected counter, and ending at the end of the block, or at another early return if - // there is another. - - // ALSO! There are language constructs that have statement spans that don't require - // braces if only one statement, in which case, they PROBABLY don't hit "Block", and - // therefore, I need to insert the counters in other parts of the AST as well, while - // also virtually inserting the curly braces: - // * closures: |...| stmt -> |...| { coverage::counter(n); stmt } - // * match arms: match variant { pat => stmt, -> match variant { pat => coverage::counter(n); stmt } - // any others? - - // There may be existing checks of similar types. See for instance "targeted_by_break" - // in this file. - } - for (index, stmt) in b.stmts.iter().enumerate() { if index == b.stmts.len() - 1 { if let StmtKind::Expr(ref e) = stmt.kind { diff --git a/src/librustc_coverage/Cargo.toml b/src/librustc_coverage/Cargo.toml index 6a1b06a2b96ff..9f5989a23e00d 100644 --- a/src/librustc_coverage/Cargo.toml +++ b/src/librustc_coverage/Cargo.toml @@ -13,4 +13,5 @@ log = "0.4" rustc = { path = "../librustc" } rustc_resolve = { path = "../librustc_resolve" } rustc_ast = { path = "../librustc_ast" } -rustc_span = { path = "../librustc_span" } \ No newline at end of file +rustc_span = { path = "../librustc_span" } +smallvec = { version = "1.0", features = ["union", "may_dangle"] } \ No newline at end of file diff --git a/src/librustc_coverage/coverage.rs b/src/librustc_coverage/coverage.rs index fca64905308b5..5eca70f46212a 100644 --- a/src/librustc_coverage/coverage.rs +++ b/src/librustc_coverage/coverage.rs @@ -2,14 +2,16 @@ use rustc::util::common::ErrorReported; use rustc_ast::ast::*; +use rustc_ast::mut_visit::{self, MutVisitor}; use rustc_ast::ptr::P; use rustc_ast::token; -use rustc_ast::mut_visit::{self, MutVisitor}; use rustc_resolve::Resolver; use rustc_span::symbol::sym; +use rustc_span::symbol::Symbol; use rustc_span::Span; +use smallvec::SmallVec; -use log::{debug, trace}; +use log::trace; use std::ops::DerefMut; use std::sync::Mutex; @@ -23,11 +25,10 @@ struct CoverageRegion { static mut COVERAGE_REGIONS: Option>> = None; impl CoverageRegion { - /// Generates a unique coverage region identifier to associate with /// a counter. The counter increment statement is injected into the /// code at the start of a coverage region. - // TODO(richkadel): This function will need additional arguments and/or context + // FIXME(richkadel): This function will need additional arguments and/or context // data from which to generate the hash values. fn generate_hash(region: &Span) -> u128 { // THIS IS NOT THREAD SAFE, BUT WILL BE REPLACED WITH HASH FUNCTION ANYWAY. @@ -58,10 +59,7 @@ impl CoverageRegion { } }; let mut locked_coverage_regions = coverage_regions.lock().unwrap(); - locked_coverage_regions.push(CoverageRegion { - region: region.clone(), - counter_hash, - }); + locked_coverage_regions.push(CoverageRegion { region: region.clone(), counter_hash }); // return the counter hash value counter_hash @@ -79,146 +77,217 @@ impl CoverageRegion { } } -struct CoverageInjector<'res,'internal> { +struct CoverageInjector<'res, 'internal> { resolver: &'res mut Resolver<'internal>, span: Span, } -impl CoverageInjector<'_,'_> { - fn at<'res,'internal>(resolver: &'res mut Resolver<'internal>, span: Span) -> CoverageInjector<'res,'internal> { - CoverageInjector { - resolver, - span, - } +impl CoverageInjector<'_, '_> { + fn at<'res, 'internal>( + resolver: &'res mut Resolver<'internal>, + span: Span, + ) -> CoverageInjector<'res, 'internal> { + CoverageInjector { resolver, span } } fn next_ast_node_id(&mut self) -> NodeId { self.resolver.next_node_id() } - fn span(&self) -> Span { - self.span.clone() - } - - fn expr(&mut self, kind: ExprKind) -> P { - P(Expr { - kind, - span: self.span(), - attrs: AttrVec::new(), - id: self.next_ast_node_id(), - }) + fn expr(&mut self, kind: ExprKind, span: Span) -> P { + P(Expr { kind, span, attrs: AttrVec::new(), id: self.next_ast_node_id() }) } fn path_segment(&mut self, string: &str) -> PathSegment { PathSegment { - ident: Ident::from_str_and_span(string, self.span()), + ident: Ident::from_str_and_span(string, self.span.shrink_to_lo()), id: self.next_ast_node_id(), args: None, } } - fn coverage_count_fn_path(&mut self) -> P { + fn coverage_count_fn_path(&mut self, print_coverage_report: bool) -> P { + let fn_name = if print_coverage_report { + "count_and_report" + } else { + "count" + }; let path = Path { - span: self.span(), - segments: vec![ - self.path_segment("coverage"), - self.path_segment("count"), - ], + span: self.span.shrink_to_lo(), + segments: vec![self.path_segment("std"), self.path_segment("coverage"), self.path_segment(fn_name)], }; - self.expr(ExprKind::Path(None, path)) + self.expr(ExprKind::Path(None, path), self.span.shrink_to_lo()) } fn coverage_counter_hash_lit(&mut self, counter_hash: u128) -> P { - let token = token::Lit::new(token::Integer, sym::integer(counter_hash), /*suffix=*/None); - let kind = LitKind::Int( - counter_hash, - LitIntType::Unsigned(UintTy::U128), // TODO: this should not be necessary (should be "Unsuffixed" per JSON) - ); - let lit = Lit { token, kind, span: self.span() }; - self.expr(ExprKind::Lit(lit)) + let token = + token::Lit::new(token::Integer, sym::integer(counter_hash), /*suffix=*/ None); + let kind = LitKind::Int(counter_hash, LitIntType::Unsuffixed); + let lit = Lit { token, kind, span: self.span.shrink_to_lo() }; + self.expr(ExprKind::Lit(lit), self.span.shrink_to_lo()) } fn call(&mut self, fn_path: P, args: Vec>) -> P { - self.expr(ExprKind::Call(fn_path, args)) + self.expr(ExprKind::Call(fn_path, args), self.span.clone()) } +} - fn counter_stmt(&mut self, coverage_span: &Span) -> Stmt { - let counter_hash = CoverageRegion::generate_hash(coverage_span); - let coverage_count_fn = self.coverage_count_fn_path(); - let args = vec![ self.coverage_counter_hash_lit(counter_hash) ]; - let call = self.call(coverage_count_fn, args); +struct CoverageVisitor<'res, 'internal> { + resolver: &'res mut Resolver<'internal>, + function_stack: Vec, + main_block_id: Option, +} - Stmt { - id: self.next_ast_node_id(), - span: self.span(), - kind: StmtKind::Semi(call) +impl CoverageVisitor<'_, '_> { + + fn new<'res, 'internal>(resolver: &'res mut Resolver<'internal>) -> CoverageVisitor<'res, 'internal> { + CoverageVisitor { + resolver, + function_stack: vec![], + main_block_id: None, } } -} -struct CoverageVisitor<'res,'internal> { - resolver: &'res mut Resolver<'internal>, -} + fn next_ast_node_id(&mut self) -> NodeId { + self.resolver.next_node_id() + } -impl CoverageVisitor<'_,'_> { + fn is_visiting_main(&self) -> bool { + if let Some(current_fn) = self.function_stack.last() { + *current_fn == sym::main + } else { + false + } + } + + fn empty_tuple(&mut self, span: Span) -> P { + P(Expr { kind: ExprKind::Tup(vec![]), span, attrs: AttrVec::new(), id: self.next_ast_node_id() }) + } + + fn wrap_and_count_expr(&mut self, coverage_span: &Span, wrapped_expr: P, print_coverage_report: bool) -> P { + let mut injector = CoverageInjector::at(&mut self.resolver, wrapped_expr.span.clone()); + let counter_hash = CoverageRegion::generate_hash(coverage_span); + let coverage_count_fn = injector.coverage_count_fn_path(print_coverage_report); + let args = vec![injector.coverage_counter_hash_lit(counter_hash), wrapped_expr]; + injector.call(coverage_count_fn, args) + } + + fn wrap_and_count_stmt(&mut self, coverage_span: &Span, wrapped_expr: P, print_coverage_report: bool) -> Stmt { + Stmt { id: self.next_ast_node_id(), span: wrapped_expr.span.clone(), kind: StmtKind::Semi(self.wrap_and_count_expr(coverage_span, wrapped_expr, print_coverage_report)) } + } + + fn count_stmt(&mut self, coverage_span: &Span, inject_site: Span, print_coverage_report: bool) -> Stmt { + let empty_tuple = self.empty_tuple(inject_site); + self.wrap_and_count_stmt(coverage_span, empty_tuple, print_coverage_report) + } fn instrument_block(&mut self, block: &mut Block) { trace!("instrument_block: {:?}", block); - let _ = self.resolver; - if let Some(last) = block.stmts.last() { - let inject_site = if last.is_expr() { - last.span.shrink_to_lo() - } else { - last.span.shrink_to_hi() - }; + if let Some(mut last) = block.stmts.pop() { - let mut coverage_injector = CoverageInjector::at(&mut self.resolver, inject_site); - let counter_stmt = coverage_injector.counter_stmt(&block.span); - if last.is_expr() { - block.stmts.insert(block.stmts.len()-1, counter_stmt); - } else { - block.stmts.push(counter_stmt); + let mut report = false; + if let Some(main) = self.main_block_id { + report = block.id == main } - // TODO(richkadel): The span should stop at the first occurrence of an early return - // (if any), and if there is one, a new counter should be inserted just after the branch - // that ended in the early return (or panic?), and a new span should start from just after that - // injected counter, and ending at the end of the block, or at another early return if - // there is another. - - // FIRST DEMO VERSION SHOULD WORK FOR BASIC BLOCKS - // THAT DON'T `break`, `continue`, `return`, or panic in any nested - // branches. For those, we need to add additional counters beyond the code that might be - // skipped. - // - // IMPORTANT! BREAK UP BLOCKS THAT MAY PANIC, SIMILAR TO EARLY RETURN, OR INJECT COUNTER - // AFTER THE LAST `Semi` Stmt, AND NOTE PANICS IN SOME OTHER WAY? - - // There may be existing checks of similar types. See for instance "targeted_by_break" - // in librustc_ast_lowering/src/lib.rs + match &mut last.kind { + StmtKind::Expr(result_expr) => { + let wrapped_expr = result_expr.clone(); + *result_expr = self.wrap_and_count_expr(&block.span, wrapped_expr, report); + block.stmts.push(last); + } + StmtKind::Semi(expr) => { + if let ExprKind::Ret(..) = expr.kind { + report = self.is_visiting_main(); + } + + match &mut expr.deref_mut().kind { + ExprKind::Break(_, result_expr) | ExprKind::Ret(result_expr) | ExprKind::Yield(result_expr) => { + match result_expr.take() { + Some(wrapped_expr) => { + *result_expr = Some(self.wrap_and_count_expr(&block.span, wrapped_expr, report)); + } + None => { + block.stmts.push(self.count_stmt(&block.span, last.span.shrink_to_lo(), report)); + } + } + block.stmts.push(last); + } + ExprKind::Continue(..) => { + block.stmts.push(self.count_stmt(&block.span, last.span.shrink_to_lo(), report)); + block.stmts.push(last); + } + _ => { + let insert_after_last = last.span.shrink_to_hi(); + block.stmts.push(last); + block.stmts.push(self.count_stmt(&block.span, insert_after_last, report)); + } + } + } + _ => () + } } } } -impl MutVisitor for CoverageVisitor<'_,'_> { - +impl MutVisitor for CoverageVisitor<'_, '_> { fn visit_block(&mut self, block: &mut P) { self.instrument_block(block.deref_mut()); mut_visit::noop_visit_block(block, self); } - // TODO(richkadel): visit match arm expr (if not block, wrap expr in block and inject counter) - // ALSO! There are language constructs that have statement spans that don't require + fn flat_map_item(&mut self, item: P) -> SmallVec<[P; 1]> { + if let ItemKind::Fn(_defaultness, _signature, _generics, block) = &item.kind { + if item.ident.name == sym::main { + if let Some(block) = block { + self.main_block_id = Some(block.id); + } + } + self.function_stack.push(item.ident.name); + let result = mut_visit::noop_flat_map_item(item, self); + self.function_stack.pop(); + result + } else { + mut_visit::noop_flat_map_item(item, self) + } + } + + // FIXME(richkadel): + // add more visit_???() functions for language constructs that are branched statements without + // blocks, such as: + // visit match arm expr + // if not block, wrap expr in block and inject counter + // + // There are language constructs that have statement spans that don't require // braces if only one statement, in which case, they PROBABLY don't hit "Block", and // therefore, I need to insert the counters in other parts of the AST as well, while // also virtually inserting the curly braces: // * closures: |...| stmt -> |...| { coverage::counter(n); stmt } // * match arms: match variant { pat => stmt, ...} -> match variant { pat => { coverage::counter(n); stmt } ... } - // any others? + // Lazy boolean operators: logical expressions that may not be executed if prior expressions obviate the need + // "the right-hand operand is only evaluated when the left-hand operand does not already + // determine the result of the expression. That is, || only evaluates its right-hand + // operand when the left-hand operand evaluates to false, and && only when it evaluates to + // true." + // * if true || stmt -> if true || coverage::wrap_and_count(n, { stmt } ) + // make sure operator precedence is handled with boolean expressions + // (?? IS THAT if false && coverage::count(condition2 && coverage::count(condition3))) + // I THINK it doesn't matter if the operator is && or || + // Unless there are parentheses, each operator requires a coverage wrap_and_count around + // ALL remaining && or || conditions on the right side, and then recursively nested. + // Rust calls parentheticals "Grouped expressions" + // `?` operator which can invoke the equivalent of an early return (of `Err(...)`). + // * {expr_possibly_in_block}? -> coverage::counter(n, {expr_possibly_in_block})? + // Any others? + // * NOT let var: type = expr (since there's no branching to worry about) + // * NOT for or while loop expressions because they aren't optionally executed + + // FIXME(richkadel): if multi-threaded, are we assured that exiting main means all threads have completed? } -pub fn instrument<'res>(krate: &mut Crate, resolver: &'res mut Resolver<'_>) { //, resolver: &'a mut Resolver<'a>) { - debug!("Calling instrument for {:?}", krate); - mut_visit::noop_visit_crate(krate, &mut CoverageVisitor { resolver } ); // , &mut CoverageVisitor::new(resolver)); +pub fn instrument<'res>(mut krate: Crate, resolver: &'res mut Resolver<'_>) -> Result { + trace!("Calling coverage::instrument() for {:?}", &krate); + mut_visit::noop_visit_crate(&mut krate, &mut CoverageVisitor::new(resolver)); CoverageRegion::write_coverage_regions(); -} \ No newline at end of file + Ok(krate) +} diff --git a/src/librustc_coverage/lib.rs b/src/librustc_coverage/lib.rs index 8d0c6d2a05fc1..0474207a238fa 100644 --- a/src/librustc_coverage/lib.rs +++ b/src/librustc_coverage/lib.rs @@ -2,4 +2,4 @@ #![feature(nll)] #![recursion_limit = "256"] -pub mod coverage; \ No newline at end of file +pub mod coverage; diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index c3780a5be4b47..9097a72f36f49 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -46,7 +46,6 @@ use rustc_session::{early_error, early_warn}; use rustc_span::source_map::{FileLoader, FileName}; use rustc_span::symbol::sym; -use log::debug; use std::borrow::Cow; use std::cmp::max; use std::default::Default; @@ -338,9 +337,7 @@ pub fn run_compiler( } } - debug!("before death"); queries.expansion()?; - debug!("AFTER DEATH (we don't get here?)"); if callbacks.after_expansion(compiler, queries) == Compilation::Stop { return early_exit(); } diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 60bb4a661fd01..7cfb608f11c89 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -15,6 +15,7 @@ use rustc_ast::mut_visit::MutVisitor; use rustc_ast::{self, ast, visit}; use rustc_codegen_ssa::back::link::emit_metadata; use rustc_codegen_ssa::traits::CodegenBackend; +use rustc_coverage::coverage; use rustc_data_structures::sync::{par_iter, Lrc, Once, ParallelIterator, WorkerLocal}; use rustc_data_structures::{box_region_allow_access, declare_box_region_type, parallel}; use rustc_errors::PResult; @@ -93,8 +94,8 @@ declare_box_region_type!( /// Runs the "early phases" of the compiler: initial `cfg` processing, loading compiler plugins, /// syntax expansion, secondary `cfg` expansion, synthesis of a test -/// harness if one is to be provided, injection of a dependency on the -/// standard library and prelude, and name resolution. +/// harness if one is to be provided, and injection of a dependency on the +/// standard library and prelude. /// /// Returns `None` if we're aborting after handling -W help. pub fn configure_and_expand( @@ -137,6 +138,43 @@ pub fn configure_and_expand( result.map(|k| (k, resolver)) } +/// If enabled by command line option, this pass injects coverage statements into the AST to +/// increment region counters and generate a coverage report at program exit. +/// The AST Crate cannot be resolved until after the AST is instrumented. +pub fn instrument(krate: ast::Crate, resolver: &mut Resolver<'_>) -> Result { + coverage::instrument(krate, resolver) +} + +/// Performs name resolution on the expanded and optionally instrumented crate. +pub fn resolve_crate( + sess: Lrc, + krate: ast::Crate, + resolver: &mut Resolver<'_>, +) -> Result { + let sess = &*sess; + + resolver.resolve_crate(&krate); + + // FIXME(richkadel): Run tests and confirm that check_crate must go after resolve_crate(), + // and if not, can/should I move it into configure_and_expand_inner() (which would mean + // running this before instrumentation, if that's OK). + + // FIXME(richkadel): original comment here with small modification: + + // Needs to go *after* expansion and name resolution, to be able to check the results of macro + // expansion. + sess.time("complete_gated_feature_checking", || { + rustc_ast_passes::feature_gate::check_crate( + &krate, + &sess.parse_sess, + &sess.features_untracked(), + sess.opts.unstable_features, + ); + }); + + Ok(krate) +} + impl BoxedResolver { pub fn to_resolver_outputs(resolver: Rc>) -> ResolverOutputs { match Rc::try_unwrap(resolver) { @@ -405,21 +443,10 @@ fn configure_and_expand_inner<'a>( } if sess.opts.debugging_opts.ast_json { + // Note this version of the AST will not include injected code, such as coverage counters. println!("{}", json::as_json(&krate)); } - resolver.resolve_crate(&krate); - - // Needs to go *after* expansion to be able to check the results of macro expansion. - sess.time("complete_gated_feature_checking", || { - rustc_ast_passes::feature_gate::check_crate( - &krate, - &sess.parse_sess, - &sess.features_untracked(), - sess.opts.unstable_features, - ); - }); - // Add all buffered lints from the `ParseSess` to the `Session`. sess.parse_sess.buffered_lints.with_lock(|buffered_lints| { info!("{} parse sess buffered_lints", buffered_lints.len()); @@ -428,6 +455,16 @@ fn configure_and_expand_inner<'a>( } }); + // // Needs to go *after* expansion, to be able to check the results of macro expansion. + // sess.time("complete_gated_feature_checking", || { + // rustc_ast_passes::feature_gate::check_crate( + // &krate, + // &sess.parse_sess, + // &sess.features_untracked(), + // sess.opts.unstable_features, + // ); + // }); + Ok((krate, resolver)) } diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index 600ce202d19cd..92f3490772929 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -8,7 +8,6 @@ use rustc::ty::{GlobalCtxt, ResolverOutputs, TyCtxt}; use rustc::util::common::ErrorReported; use rustc_ast::{self, ast}; use rustc_codegen_ssa::traits::CodegenBackend; -use rustc_coverage::coverage; use rustc_data_structures::sync::{Lrc, Once, WorkerLocal}; use rustc_hir::def_id::LOCAL_CRATE; use rustc_hir::Crate; @@ -18,7 +17,6 @@ use rustc_session::config::{OutputFilenames, OutputType}; use rustc_session::{output::find_crate_name, Session}; use rustc_span::symbol::sym; -use log::debug; use std::any::Any; use std::cell::{Ref, RefCell, RefMut}; use std::mem; @@ -77,8 +75,9 @@ pub struct Queries<'tcx> { parse: Query, crate_name: Query, register_plugins: Query<(ast::Crate, Lrc)>, - expansion: Query<(ast::Crate, Steal>>, Lrc)>, + unresolved_expansion: Query<(ast::Crate, Steal>>, Lrc)>, instrument: Query<(ast::Crate, Steal>>, Lrc)>, + expansion: Query<(ast::Crate, Steal>>, Lrc)>, dep_graph: Query, lower_to_hir: Query<(&'tcx Crate<'tcx>, Steal)>, prepare_outputs: Query, @@ -97,6 +96,7 @@ impl<'tcx> Queries<'tcx> { parse: Default::default(), crate_name: Default::default(), register_plugins: Default::default(), + unresolved_expansion: Default::default(), expansion: Default::default(), instrument: Default::default(), dep_graph: Default::default(), @@ -115,9 +115,7 @@ impl<'tcx> Queries<'tcx> { } pub fn dep_graph_future(&self) -> Result<&Query>> { - debug!("query dep_graph_future"); self.dep_graph_future.compute(|| { - debug!("compute query dep_graph_future"); Ok(self .session() .opts @@ -127,9 +125,7 @@ impl<'tcx> Queries<'tcx> { } pub fn parse(&self) -> Result<&Query> { - debug!("query parse"); self.parse.compute(|| { - debug!("compute query parse"); passes::parse(self.session(), &self.compiler.input).map_err(|mut parse_error| { parse_error.emit(); ErrorReported @@ -138,9 +134,7 @@ impl<'tcx> Queries<'tcx> { } pub fn register_plugins(&self) -> Result<&Query<(ast::Crate, Lrc)>> { - debug!("query register_plugins"); self.register_plugins.compute(|| { - debug!("compute query register_plugins"); let crate_name = self.crate_name()?.peek().clone(); let krate = self.parse()?.take(); @@ -165,9 +159,7 @@ impl<'tcx> Queries<'tcx> { } pub fn crate_name(&self) -> Result<&Query> { - debug!("query crate_name"); self.crate_name.compute(|| { - debug!("compute query crate_name"); Ok(match self.compiler.crate_name { Some(ref crate_name) => crate_name.clone(), None => { @@ -179,16 +171,15 @@ impl<'tcx> Queries<'tcx> { }) } - pub fn expansion( + /// Expands built-ins and other macros, but does not perform name resolution, pending potential + /// instrumentation. + pub fn unresolved_expansion( &self, ) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { - debug!("query expansion"); - self.expansion.compute(|| { - debug!("compute query expansion"); + self.unresolved_expansion.compute(|| { let crate_name = self.crate_name()?.peek().clone(); let (krate, lint_store) = self.register_plugins()?.take(); let _timer = self.session().timer("configure_and_expand"); -debug!("just before death"); passes::configure_and_expand( self.session().clone(), lint_store.clone(), @@ -197,16 +188,53 @@ debug!("just before death"); &crate_name, ) .map(|(krate, resolver)| { -debug!("NEVER GET HERE"); (krate, Steal::new(Rc::new(RefCell::new(resolver))), lint_store) }) }) } + /// Returns a modified ast::Crate with injected instrumentation code, if requested by command + /// line option. + /// + /// The `instrument` pass/query depends on (and implies) `unresolved_expansion`. + pub fn instrument( + &self, + ) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { + self.instrument.compute(|| { + let (krate, boxed_resolver, lint_store) = self.unresolved_expansion()?.take(); + let instrument_coverage = match self.session().opts.debugging_opts.instrument_coverage { + Some(opt) => opt, + None => false, + }; + if instrument_coverage { + boxed_resolver.borrow().borrow_mut().access(|resolver| { + let _timer = self.session().timer("instrument_coverage"); + passes::instrument(krate, resolver) + }) + } else { + Ok(krate) + } + .map(|krate| (krate, boxed_resolver, lint_store)) + }) + } + + /// Updates the ast::Crate, first querying `instrument`, which queries `unresolved_expansion`. + /// After all expansions and instrumentation, performs name resolution on the final AST. + pub fn expansion( + &self, + ) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { + self.expansion.compute(|| { + let (krate, boxed_resolver, lint_store) = self.instrument()?.take(); + let result = boxed_resolver.borrow().borrow_mut().access(|resolver| { + let _timer = self.session().timer("resolve_expansion"); + passes::resolve_crate(self.session().clone(), krate, resolver) + }); + result.map(|krate| (krate, boxed_resolver, lint_store)) + }) + } + pub fn dep_graph(&self) -> Result<&Query> { - debug!("query dep_graph"); self.dep_graph.compute(|| { - debug!("compute query dep_graph"); Ok(match self.dep_graph_future()?.take() { None => DepGraph::new_disabled(), Some(future) => { @@ -225,42 +253,10 @@ debug!("NEVER GET HERE"); }) } - /// Instrumentation is optional (typically included based on `rustc` option). The - /// `instrument` pass/query depends on (and implies) `expansion`. To ensure the instrumentation - /// pass is executed (if requested), replace queries to `expansion` with queries to - /// `instrument`. The query results are the same. If instrumentation is not requested, - /// `expansion` will be queried and returned. If instrumentation is requested, `instrument` - /// will query `expansion`, inject instrumentation code (modifying the crate and updating the - /// `next_node_id()` in the resolver), and then return the query result from `expansion`, with - /// the changes to crate and resolver. - pub fn instrument(&'tcx self) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { - debug!("query instrument (depends on expansion), will instrument coverage if requested"); - let expansion = self.expansion(); - let instrument_coverage = match self.session().opts.debugging_opts.instrument_coverage { - Some(opt) => opt, - None => false, - }; - if instrument_coverage { - self.instrument.compute(|| { - debug!("compute query instrument (depends on expansion), will instrument coverage if requested"); - let (mut krate, boxed_resolver, lint_store) = expansion?.take(); - let resolver = boxed_resolver.steal(); - resolver.borrow_mut().access(|resolver| { - coverage::instrument(&mut krate, resolver) - }); - Ok((krate, Steal::new(resolver), lint_store)) - }) - } else { - expansion - } - } - pub fn lower_to_hir(&'tcx self) -> Result<&Query<(&'tcx Crate<'tcx>, Steal)>> { - debug!("query lower_to_hir"); self.lower_to_hir.compute(|| { - debug!("compute query lower_to_hir"); - let instrument_result = self.instrument()?; - let peeked = instrument_result.peek(); + let expansion_result = self.expansion()?; + let peeked = expansion_result.peek(); let krate = &peeked.0; let resolver = peeked.1.steal(); let lint_store = &peeked.2; @@ -280,9 +276,7 @@ debug!("NEVER GET HERE"); } pub fn prepare_outputs(&self) -> Result<&Query> { - debug!("query prepare_outputs"); self.prepare_outputs.compute(|| { - debug!("compute query prepare_outputs"); let expansion_result = self.expansion()?; let (krate, boxed_resolver, _) = &*expansion_result.peek(); let crate_name = self.crate_name()?; @@ -298,9 +292,7 @@ debug!("NEVER GET HERE"); } pub fn global_ctxt(&'tcx self) -> Result<&Query>> { - debug!("query global_ctxt"); self.global_ctxt.compute(|| { - debug!("compute query global_ctxt"); let crate_name = self.crate_name()?.peek().clone(); let outputs = self.prepare_outputs()?.peek().clone(); let lint_store = self.expansion()?.peek().2.clone(); @@ -323,9 +315,7 @@ debug!("NEVER GET HERE"); } pub fn ongoing_codegen(&'tcx self) -> Result<&Query>> { - debug!("query ongoing_codegen"); self.ongoing_codegen.compute(|| { - debug!("compute query ongoing_codegen"); let outputs = self.prepare_outputs()?; self.global_ctxt()?.peek_mut().enter(|tcx| { tcx.analysis(LOCAL_CRATE).ok(); @@ -413,7 +403,6 @@ pub struct Linker { impl Linker { pub fn link(self) -> Result<()> { - debug!("Linker::link"); let codegen_results = self.codegen_backend.join_codegen(self.ongoing_codegen, &self.sess, &self.dep_graph)?; let prof = self.sess.prof.clone(); diff --git a/src/librustc_session/session.rs b/src/librustc_session/session.rs index adc6532028237..80f59aff69137 100644 --- a/src/librustc_session/session.rs +++ b/src/librustc_session/session.rs @@ -440,7 +440,6 @@ impl Session { } pub fn init_features(&self, features: rustc_feature::Features) { - debug!("ABOUT TO PANIC?") self.features.set(features); } diff --git a/src/libstd/coverage.rs b/src/libstd/coverage.rs new file mode 100644 index 0000000000000..b5ffb2ef38cea --- /dev/null +++ b/src/libstd/coverage.rs @@ -0,0 +1,81 @@ +//! Code coverage counters and report +//! +//! The code coverage library is typically not included in Rust source files. +//! +//! Instead, the `rustc` compiler optionally injects coverage calls into the internal representation +//! of the source if requested by command line option to the compiler. The end result is an +//! executable that includes the coverage counters, and writes a coverage report upon exit. +//! +//! The injected calls behave as if code like the following examples was actually part of the +//! original source. +//! +//! Example: +//! +//! ``` +//! main() { +//! let value = if (true) { +//! std::coverage::count(1, { +//! // any expression +//! 1000 +//! }) +//! } else { +//! std::coverage::count(2, 500) +//! } +//! std::coverage::count_and_report(3, ()) +//! } +//! ``` + +#![stable(feature = "coverage", since = "1.44.0")] + +use crate::collections::HashMap; +use crate::sync::LockResult; +use crate::sync::Mutex; +use crate::sync::MutexGuard; +use crate::sync::Once; + +static mut COUNTERS: Option>> = None; + +static INIT: Once = Once::new(); + +#[stable(feature = "coverage", since = "1.44.0")] +#[inline] +fn increment_counter(counter: u128) -> LockResult>> { + let mut lock = unsafe { + INIT.call_once(|| { + COUNTERS = Some(Mutex::new(HashMap::with_capacity(1024))); + }); + COUNTERS.as_mut().unwrap().lock() + }; + let counters = lock.as_mut().unwrap(); + match counters.get_mut(&counter) { + Some(count) => *count += 1, + None => { counters.insert(counter,1); } + } + lock +} + +/// The statement may be the last statement of a block, the value of a `return` or `break`, +/// a match pattern arm statement that might not be in a block (but if it is, don't wrap both the +/// block and the last statement of the block), or a closure statement without braces. +/// +/// coverage::count(234234, {some_statement_with_or_without_semicolon()}) +#[stable(feature = "coverage", since = "1.44.0")] +#[inline] +pub fn count(counter: u128, result: T) -> T { + let _ = increment_counter(counter); + result +} + +/// Increment the specified counter and then write the coverage report. This function normally wraps +/// the final expression in a `main()` function. There can be more than one statement, for example +/// if the `main()` has one or more `return` statements. In this case, all returns and the last +/// statement of `main()` (unless not reachable) should use this function. +#[stable(feature = "coverage", since = "1.44.0")] +pub fn count_and_report(counter: u128, result: T) -> T { + println!("Print the coverage counters:"); + let mut counters = increment_counter(counter).unwrap(); + for (counter, count) in counters.drain() { + println!("Counter '{}' has count: {}", counter, count); + } + result +} diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index e5dad307a209a..df35ed4a533bb 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -460,6 +460,7 @@ pub mod path; pub mod process; pub mod sync; pub mod time; +pub mod coverage; #[stable(feature = "futures_api", since = "1.36.0")] pub mod task { From 3c728ad646af1f493519de340014ae264f74bfd0 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Tue, 24 Mar 2020 11:13:20 -0700 Subject: [PATCH 4/6] WIP - Add LLVM coverage instrumentation https://github.com/rust-lang/rust/issues/34701 Prototype coverage implementation with partial support for the various branch types to be covered. cargo -vv rustc -- -Z instrument-coverage With the `-Z instrument-coverage` flag, the compiler injects calls to a new std::coverage module, adding counters at various points in the code. At the end of the compile, a list of coverage regions (`Span`s) and corresponding coverage retion IDs are printed to stdout. When executing the program, the counters increment statically, and upon normal exit, the counters are printed to stdout with the same corresponding coverage region IDs. The stdout results will be replaced by output files in a format compatible with LLVM coverage reports. Currently counts the following coverage regions: * All blocks ending in a semicolon or expression * break, return, and yield statements, with or without a value expression * continue statment Not yet supported are: * match pattern arms without a block * closures without a block * expressions ending in '?' (that return on Result::Err) * lazy boolean right-hand-side expressions that may not be invoked --- Cargo.lock | 1 + src/librustc_ast_lowering/lib.rs | 184 ------------------- src/librustc_coverage/Cargo.toml | 3 +- src/librustc_coverage/coverage.rs | 291 +++++++++++++++++++++--------- src/librustc_coverage/lib.rs | 2 +- src/librustc_driver/lib.rs | 3 - src/librustc_interface/passes.rs | 65 +++++-- src/librustc_interface/queries.rs | 109 +++++------ src/librustc_session/session.rs | 1 - src/libstd/coverage.rs | 83 +++++++++ src/libstd/lib.rs | 1 + 11 files changed, 389 insertions(+), 354 deletions(-) create mode 100644 src/libstd/coverage.rs diff --git a/Cargo.lock b/Cargo.lock index a8f39d5b8ab22..8e511999d56e9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3622,6 +3622,7 @@ dependencies = [ "rustc_ast", "rustc_resolve", "rustc_span", + "smallvec 1.0.0", ] [[package]] diff --git a/src/librustc_ast_lowering/lib.rs b/src/librustc_ast_lowering/lib.rs index 4d529ce77ce47..68cb1f8585fc7 100644 --- a/src/librustc_ast_lowering/lib.rs +++ b/src/librustc_ast_lowering/lib.rs @@ -40,7 +40,6 @@ use rustc_ast::ast; use rustc_ast::ast::*; use rustc_ast::attr; use rustc_ast::node_id::NodeMap; -use rustc_ast::ptr::P; use rustc_ast::token::{self, Nonterminal, Token}; use rustc_ast::tokenstream::{TokenStream, TokenTree}; use rustc_ast::visit::{self, AssocCtxt, Visitor}; @@ -70,7 +69,6 @@ use log::{debug, trace}; use smallvec::{smallvec, SmallVec}; use std::collections::BTreeMap; use std::mem; -use std::sync::Mutex; macro_rules! arena_vec { ($this:expr; $($x:expr),*) => ({ @@ -173,9 +171,6 @@ struct LoweringContext<'a, 'hir: 'a> { allow_try_trait: Option>, allow_gen_future: Option>, - - /// `true` if `rustc` was invoked with an option to inject code coverage instrumentation. - is_instrumenting_for_coverage: bool, } pub trait Resolver { @@ -305,7 +300,6 @@ pub fn lower_crate<'a, 'hir>( in_scope_lifetimes: Vec::new(), allow_try_trait: Some([sym::try_trait][..].into()), allow_gen_future: Some([sym::gen_future][..].into()), - is_instrumenting_for_coverage: false, } .lower_crate(krate) } @@ -400,148 +394,6 @@ impl Visitor<'_> for ImplTraitTypeIdVisitor<'_> { } } -struct CoverageRegion { - region: Span, - counter_hash: u128, -} - -static mut COVERAGE_REGIONS: Option>> = None; - -impl CoverageRegion { - - /// Generates a unique coverage region identifier to associate with - /// a counter. The counter increment statement is injected into the - /// code at the start of a coverage region. - // TODO(richkadel): This function will need additional arguments and/or context - // data from which to generate the hash values. - fn generate_hash(region: &Span) -> u128 { - // THIS IS NOT THREAD SAFE, BUT WILL BE REPLACED WITH HASH FUNCTION ANYWAY. - // Seems like lazy_static is not used in the compiler at all. - static mut NEXT_COUNTER_ID: Option> = None; - let counter_hash = { - let counter_id = unsafe { - &match NEXT_COUNTER_ID.as_ref() { - Some(counter_id) => counter_id, - None => { - NEXT_COUNTER_ID = Some(Mutex::new(0)); - NEXT_COUNTER_ID.as_ref().unwrap() - } - } - }; - let mut locked_counter_id = counter_id.lock().unwrap(); - *locked_counter_id += 1; - *locked_counter_id - }; - - let coverage_regions = unsafe { - &match COVERAGE_REGIONS.as_ref() { - Some(coverage_regions) => coverage_regions, - None => { - COVERAGE_REGIONS = Some(Mutex::new(vec![])); - COVERAGE_REGIONS.as_ref().unwrap() - } - } - }; - let mut locked_coverage_regions = coverage_regions.lock().unwrap(); - locked_coverage_regions.push(CoverageRegion { - region: region.clone(), - counter_hash, - }); - - // return the counter hash value - counter_hash - } - - pub fn write_coverage_regions(/* filename param? */) { - unsafe { - if let Some(coverage_regions) = COVERAGE_REGIONS.as_ref() { - let locked_coverage_regions = coverage_regions.lock().unwrap(); - for coverage in locked_coverage_regions.iter() { - println!("{}: {:?}", coverage.counter_hash, coverage.region); - } - } - } - } -} - -struct CoverageInjector<'tcx, 'lowering, 'hir> { - lctx: &'tcx mut LoweringContext<'lowering, 'hir>, - span: Span, -} - -impl<'tcx, 'lowering, 'hir> CoverageInjector<'tcx, 'lowering, 'hir> { - fn at(lctx: &'tcx mut LoweringContext<'lowering, 'hir>, before: &Span) -> CoverageInjector<'tcx, 'lowering, 'hir> { - CoverageInjector { - lctx, - span: before.shrink_to_lo(), - } - } - - fn next_ast_node_id(&mut self) -> NodeId { - self.lctx.resolver.next_node_id() - } - - fn span(&self) -> Span { - self.span.clone() - } - - fn expr(&mut self, kind: ExprKind) -> P { - P(Expr { - kind, - span: self.span(), - attrs: AttrVec::new(), - id: self.next_ast_node_id(), - }) - } - - fn path_segment(&mut self, string: &str) -> PathSegment { - PathSegment { - ident: Ident::from_str_and_span(string, self.span()), - id: self.next_ast_node_id(), - args: None, - } - } - - fn coverage_count_fn_path(&mut self) -> P { - let path = Path { - span: self.span(), - segments: vec![ - self.path_segment("coverage"), - self.path_segment("count"), - ], - }; - self.expr(ExprKind::Path(None, path)) - } - - fn coverage_counter_hash_lit(&mut self, counter_hash: u128) -> P { - let token = token::Lit::new(token::Integer, sym::integer(counter_hash), /*suffix=*/None); - let kind = LitKind::Int( - counter_hash, - LitIntType::Unsigned(UintTy::U128), // TODO: this should not be necessary (should be "Unsuffixed" per JSON) - ); - let lit = Lit { token, kind, span: self.span() }; - self.expr(ExprKind::Lit(lit)) - } - - fn call(&mut self, fn_path: P, args: Vec>) -> P { - self.expr(ExprKind::Call(fn_path, args)) - } - - fn counter_stmt(&mut self, coverage_span: &Span) -> Stmt { - let counter_hash = CoverageRegion::generate_hash(coverage_span); - let coverage_count_fn = self.coverage_count_fn_path(); - let args = vec![ self.coverage_counter_hash_lit(counter_hash) ]; - let call = self.call(coverage_count_fn, args); - - Stmt { - id: self.next_ast_node_id(), - span: self.span(), - kind: StmtKind::Semi(call) - } - } -} - - impl<'a, 'hir> LoweringContext<'a, 'hir> { fn lower_crate(mut self, c: &Crate) -> hir::Crate<'hir> { /// Full-crate AST visitor that inserts into a fresh @@ -669,21 +521,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } } - self.is_instrumenting_for_coverage = match self.sess.opts.debugging_opts.instrument_coverage { - Some(opt) => opt, - None => false, - }; - self.lower_node_id(CRATE_NODE_ID); debug_assert!(self.node_id_to_hir_id[CRATE_NODE_ID] == hir::CRATE_HIR_ID); visit::walk_crate(&mut MiscCollector { lctx: &mut self, hir_id_owner: None }, c); visit::walk_crate(&mut item::ItemLowerer { lctx: &mut self }, c); - if self.is_instrumenting_for_coverage { - CoverageRegion::write_coverage_regions(); - } - let module = self.lower_mod(&c.module); let attrs = self.lower_attrs(&c.attrs); let body_ids = body_ids(&self.bodies); @@ -2365,33 +2208,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let mut stmts = vec![]; let mut expr: Option<&'hir _> = None; - // THIS COVERAGE CODE MAY NEED TO BE IN A DIFFERENT LOCATION? BUT SEEMS LIKE IT SHOULD - // WORK FOR BASIC BLOCKS THAT DON'T `break`, `continue`, or `return` in any nested - // branches. For those, we need to add additional counters beyond the code that might be - // skipped. - if self.is_instrumenting_for_coverage && ! b.stmts.is_empty() { - let inject_site = &b.stmts[0].span; - let mut coverage_injector = CoverageInjector::at(self, inject_site); - let counter_stmt = coverage_injector.counter_stmt(&b.span); - stmts.extend(self.lower_stmt(&counter_stmt)); - // TODO(richkadel): The span should stop at the first occurrence of an early return - // (if any), and if there is one, a new counter should be inserted just after the branch - // that ended in the early return, and a new span should start from just after that - // injected counter, and ending at the end of the block, or at another early return if - // there is another. - - // ALSO! There are language constructs that have statement spans that don't require - // braces if only one statement, in which case, they PROBABLY don't hit "Block", and - // therefore, I need to insert the counters in other parts of the AST as well, while - // also virtually inserting the curly braces: - // * closures: |...| stmt -> |...| { coverage::counter(n); stmt } - // * match arms: match variant { pat => stmt, -> match variant { pat => coverage::counter(n); stmt } - // any others? - - // There may be existing checks of similar types. See for instance "targeted_by_break" - // in this file. - } - for (index, stmt) in b.stmts.iter().enumerate() { if index == b.stmts.len() - 1 { if let StmtKind::Expr(ref e) = stmt.kind { diff --git a/src/librustc_coverage/Cargo.toml b/src/librustc_coverage/Cargo.toml index 6a1b06a2b96ff..9f5989a23e00d 100644 --- a/src/librustc_coverage/Cargo.toml +++ b/src/librustc_coverage/Cargo.toml @@ -13,4 +13,5 @@ log = "0.4" rustc = { path = "../librustc" } rustc_resolve = { path = "../librustc_resolve" } rustc_ast = { path = "../librustc_ast" } -rustc_span = { path = "../librustc_span" } \ No newline at end of file +rustc_span = { path = "../librustc_span" } +smallvec = { version = "1.0", features = ["union", "may_dangle"] } \ No newline at end of file diff --git a/src/librustc_coverage/coverage.rs b/src/librustc_coverage/coverage.rs index fca64905308b5..2e5ea859823cb 100644 --- a/src/librustc_coverage/coverage.rs +++ b/src/librustc_coverage/coverage.rs @@ -2,14 +2,16 @@ use rustc::util::common::ErrorReported; use rustc_ast::ast::*; +use rustc_ast::mut_visit::{self, MutVisitor}; use rustc_ast::ptr::P; use rustc_ast::token; -use rustc_ast::mut_visit::{self, MutVisitor}; use rustc_resolve::Resolver; use rustc_span::symbol::sym; +use rustc_span::symbol::Symbol; use rustc_span::Span; +use smallvec::SmallVec; -use log::{debug, trace}; +use log::trace; use std::ops::DerefMut; use std::sync::Mutex; @@ -23,11 +25,10 @@ struct CoverageRegion { static mut COVERAGE_REGIONS: Option>> = None; impl CoverageRegion { - /// Generates a unique coverage region identifier to associate with /// a counter. The counter increment statement is injected into the /// code at the start of a coverage region. - // TODO(richkadel): This function will need additional arguments and/or context + // FIXME(richkadel): This function will need additional arguments and/or context // data from which to generate the hash values. fn generate_hash(region: &Span) -> u128 { // THIS IS NOT THREAD SAFE, BUT WILL BE REPLACED WITH HASH FUNCTION ANYWAY. @@ -58,10 +59,7 @@ impl CoverageRegion { } }; let mut locked_coverage_regions = coverage_regions.lock().unwrap(); - locked_coverage_regions.push(CoverageRegion { - region: region.clone(), - counter_hash, - }); + locked_coverage_regions.push(CoverageRegion { region: region.clone(), counter_hash }); // return the counter hash value counter_hash @@ -79,146 +77,259 @@ impl CoverageRegion { } } -struct CoverageInjector<'res,'internal> { +struct CoverageInjector<'res, 'internal> { resolver: &'res mut Resolver<'internal>, span: Span, } -impl CoverageInjector<'_,'_> { - fn at<'res,'internal>(resolver: &'res mut Resolver<'internal>, span: Span) -> CoverageInjector<'res,'internal> { - CoverageInjector { - resolver, - span, - } +impl CoverageInjector<'_, '_> { + fn at<'res, 'internal>( + resolver: &'res mut Resolver<'internal>, + span: Span, + ) -> CoverageInjector<'res, 'internal> { + CoverageInjector { resolver, span } } fn next_ast_node_id(&mut self) -> NodeId { self.resolver.next_node_id() } - fn span(&self) -> Span { - self.span.clone() - } - - fn expr(&mut self, kind: ExprKind) -> P { - P(Expr { - kind, - span: self.span(), - attrs: AttrVec::new(), - id: self.next_ast_node_id(), - }) + fn expr(&mut self, kind: ExprKind, span: Span) -> P { + P(Expr { kind, span, attrs: AttrVec::new(), id: self.next_ast_node_id() }) } fn path_segment(&mut self, string: &str) -> PathSegment { PathSegment { - ident: Ident::from_str_and_span(string, self.span()), + ident: Ident::from_str_and_span(string, self.span.shrink_to_lo()), id: self.next_ast_node_id(), args: None, } } - fn coverage_count_fn_path(&mut self) -> P { + fn coverage_count_fn_path(&mut self, print_coverage_report: bool) -> P { + let fn_name = if print_coverage_report { "count_and_report" } else { "count" }; let path = Path { - span: self.span(), + span: self.span.shrink_to_lo(), segments: vec![ + self.path_segment("std"), self.path_segment("coverage"), - self.path_segment("count"), + self.path_segment(fn_name), ], }; - self.expr(ExprKind::Path(None, path)) + self.expr(ExprKind::Path(None, path), self.span.shrink_to_lo()) } fn coverage_counter_hash_lit(&mut self, counter_hash: u128) -> P { - let token = token::Lit::new(token::Integer, sym::integer(counter_hash), /*suffix=*/None); - let kind = LitKind::Int( - counter_hash, - LitIntType::Unsigned(UintTy::U128), // TODO: this should not be necessary (should be "Unsuffixed" per JSON) - ); - let lit = Lit { token, kind, span: self.span() }; - self.expr(ExprKind::Lit(lit)) + let token = + token::Lit::new(token::Integer, sym::integer(counter_hash), /*suffix=*/ None); + let kind = LitKind::Int(counter_hash, LitIntType::Unsuffixed); + let lit = Lit { token, kind, span: self.span.shrink_to_lo() }; + self.expr(ExprKind::Lit(lit), self.span.shrink_to_lo()) } fn call(&mut self, fn_path: P, args: Vec>) -> P { - self.expr(ExprKind::Call(fn_path, args)) + self.expr(ExprKind::Call(fn_path, args), self.span.clone()) + } +} + +struct CoverageVisitor<'res, 'internal> { + resolver: &'res mut Resolver<'internal>, + function_stack: Vec, + main_block_id: Option, +} + +impl CoverageVisitor<'_, '_> { + fn new<'res, 'internal>( + resolver: &'res mut Resolver<'internal>, + ) -> CoverageVisitor<'res, 'internal> { + CoverageVisitor { resolver, function_stack: vec![], main_block_id: None } + } + + fn next_ast_node_id(&mut self) -> NodeId { + self.resolver.next_node_id() + } + + fn is_visiting_main(&self) -> bool { + if let Some(current_fn) = self.function_stack.last() { + *current_fn == sym::main + } else { + false + } + } + + fn empty_tuple(&mut self, span: Span) -> P { + P(Expr { + kind: ExprKind::Tup(vec![]), + span, + attrs: AttrVec::new(), + id: self.next_ast_node_id(), + }) } - fn counter_stmt(&mut self, coverage_span: &Span) -> Stmt { + fn wrap_and_count_expr( + &mut self, + coverage_span: &Span, + wrapped_expr: P, + print_coverage_report: bool, + ) -> P { + let mut injector = CoverageInjector::at(&mut self.resolver, wrapped_expr.span.clone()); let counter_hash = CoverageRegion::generate_hash(coverage_span); - let coverage_count_fn = self.coverage_count_fn_path(); - let args = vec![ self.coverage_counter_hash_lit(counter_hash) ]; - let call = self.call(coverage_count_fn, args); + let coverage_count_fn = injector.coverage_count_fn_path(print_coverage_report); + let args = vec![injector.coverage_counter_hash_lit(counter_hash), wrapped_expr]; + injector.call(coverage_count_fn, args) + } + fn wrap_and_count_stmt( + &mut self, + coverage_span: &Span, + wrapped_expr: P, + print_coverage_report: bool, + ) -> Stmt { Stmt { id: self.next_ast_node_id(), - span: self.span(), - kind: StmtKind::Semi(call) + span: wrapped_expr.span.clone(), + kind: StmtKind::Semi(self.wrap_and_count_expr( + coverage_span, + wrapped_expr, + print_coverage_report, + )), } } -} -struct CoverageVisitor<'res,'internal> { - resolver: &'res mut Resolver<'internal>, -} - -impl CoverageVisitor<'_,'_> { + fn count_stmt( + &mut self, + coverage_span: &Span, + inject_site: Span, + print_coverage_report: bool, + ) -> Stmt { + let empty_tuple = self.empty_tuple(inject_site); + self.wrap_and_count_stmt(coverage_span, empty_tuple, print_coverage_report) + } fn instrument_block(&mut self, block: &mut Block) { trace!("instrument_block: {:?}", block); - let _ = self.resolver; - if let Some(last) = block.stmts.last() { - let inject_site = if last.is_expr() { - last.span.shrink_to_lo() - } else { - last.span.shrink_to_hi() - }; - - let mut coverage_injector = CoverageInjector::at(&mut self.resolver, inject_site); - let counter_stmt = coverage_injector.counter_stmt(&block.span); - if last.is_expr() { - block.stmts.insert(block.stmts.len()-1, counter_stmt); - } else { - block.stmts.push(counter_stmt); + if let Some(mut last) = block.stmts.pop() { + let mut report = false; + if let Some(main) = self.main_block_id { + report = block.id == main } - // TODO(richkadel): The span should stop at the first occurrence of an early return - // (if any), and if there is one, a new counter should be inserted just after the branch - // that ended in the early return (or panic?), and a new span should start from just after that - // injected counter, and ending at the end of the block, or at another early return if - // there is another. - - // FIRST DEMO VERSION SHOULD WORK FOR BASIC BLOCKS - // THAT DON'T `break`, `continue`, `return`, or panic in any nested - // branches. For those, we need to add additional counters beyond the code that might be - // skipped. - // - // IMPORTANT! BREAK UP BLOCKS THAT MAY PANIC, SIMILAR TO EARLY RETURN, OR INJECT COUNTER - // AFTER THE LAST `Semi` Stmt, AND NOTE PANICS IN SOME OTHER WAY? - - // There may be existing checks of similar types. See for instance "targeted_by_break" - // in librustc_ast_lowering/src/lib.rs + match &mut last.kind { + StmtKind::Expr(result_expr) => { + let wrapped_expr = result_expr.clone(); + *result_expr = self.wrap_and_count_expr(&block.span, wrapped_expr, report); + block.stmts.push(last); + } + StmtKind::Semi(expr) => { + if let ExprKind::Ret(..) = expr.kind { + report = self.is_visiting_main(); + } + + match &mut expr.deref_mut().kind { + ExprKind::Break(_, result_expr) + | ExprKind::Ret(result_expr) + | ExprKind::Yield(result_expr) => { + match result_expr.take() { + Some(wrapped_expr) => { + *result_expr = Some(self.wrap_and_count_expr( + &block.span, + wrapped_expr, + report, + )); + } + None => { + block.stmts.push(self.count_stmt( + &block.span, + last.span.shrink_to_lo(), + report, + )); + } + } + block.stmts.push(last); + } + ExprKind::Continue(..) => { + block.stmts.push(self.count_stmt( + &block.span, + last.span.shrink_to_lo(), + report, + )); + block.stmts.push(last); + } + _ => { + let insert_after_last = last.span.shrink_to_hi(); + block.stmts.push(last); + block.stmts.push(self.count_stmt( + &block.span, + insert_after_last, + report, + )); + } + } + } + _ => (), + } } } } -impl MutVisitor for CoverageVisitor<'_,'_> { - +impl MutVisitor for CoverageVisitor<'_, '_> { fn visit_block(&mut self, block: &mut P) { self.instrument_block(block.deref_mut()); mut_visit::noop_visit_block(block, self); } - // TODO(richkadel): visit match arm expr (if not block, wrap expr in block and inject counter) - // ALSO! There are language constructs that have statement spans that don't require + fn flat_map_item(&mut self, item: P) -> SmallVec<[P; 1]> { + if let ItemKind::Fn(_defaultness, _signature, _generics, block) = &item.kind { + if item.ident.name == sym::main { + if let Some(block) = block { + self.main_block_id = Some(block.id); + } + } + self.function_stack.push(item.ident.name); + let result = mut_visit::noop_flat_map_item(item, self); + self.function_stack.pop(); + result + } else { + mut_visit::noop_flat_map_item(item, self) + } + } + + // FIXME(richkadel): + // add more visit_???() functions for language constructs that are branched statements without + // blocks, such as: + // visit match arm expr + // if not block, wrap expr in block and inject counter + // + // There are language constructs that have statement spans that don't require // braces if only one statement, in which case, they PROBABLY don't hit "Block", and // therefore, I need to insert the counters in other parts of the AST as well, while // also virtually inserting the curly braces: // * closures: |...| stmt -> |...| { coverage::counter(n); stmt } // * match arms: match variant { pat => stmt, ...} -> match variant { pat => { coverage::counter(n); stmt } ... } - // any others? + // Lazy boolean operators: logical expressions that may not be executed if prior expressions obviate the need + // "the right-hand operand is only evaluated when the left-hand operand does not already + // determine the result of the expression. That is, || only evaluates its right-hand + // operand when the left-hand operand evaluates to false, and && only when it evaluates to + // true." + // * if true || stmt -> if true || coverage::wrap_and_count(n, { stmt } ) + // make sure operator precedence is handled with boolean expressions + // (?? IS THAT if false && coverage::count(condition2 && coverage::count(condition3))) + // I THINK it doesn't matter if the operator is && or || + // Unless there are parentheses, each operator requires a coverage wrap_and_count around + // ALL remaining && or || conditions on the right side, and then recursively nested. + // Rust calls parentheticals "Grouped expressions" + // `?` operator which can invoke the equivalent of an early return (of `Err(...)`). + // * {expr_possibly_in_block}? -> coverage::counter(n, {expr_possibly_in_block})? + // Any others? + // * NOT let var: type = expr (since there's no branching to worry about) + // * NOT for or while loop expressions because they aren't optionally executed + + // FIXME(richkadel): if multi-threaded, are we assured that exiting main means all threads have completed? } -pub fn instrument<'res>(krate: &mut Crate, resolver: &'res mut Resolver<'_>) { //, resolver: &'a mut Resolver<'a>) { - debug!("Calling instrument for {:?}", krate); - mut_visit::noop_visit_crate(krate, &mut CoverageVisitor { resolver } ); // , &mut CoverageVisitor::new(resolver)); +pub fn instrument<'res>(mut krate: Crate, resolver: &'res mut Resolver<'_>) -> Result { + trace!("Calling coverage::instrument() for {:?}", &krate); + mut_visit::noop_visit_crate(&mut krate, &mut CoverageVisitor::new(resolver)); CoverageRegion::write_coverage_regions(); -} \ No newline at end of file + Ok(krate) +} diff --git a/src/librustc_coverage/lib.rs b/src/librustc_coverage/lib.rs index 8d0c6d2a05fc1..0474207a238fa 100644 --- a/src/librustc_coverage/lib.rs +++ b/src/librustc_coverage/lib.rs @@ -2,4 +2,4 @@ #![feature(nll)] #![recursion_limit = "256"] -pub mod coverage; \ No newline at end of file +pub mod coverage; diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index c3780a5be4b47..9097a72f36f49 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -46,7 +46,6 @@ use rustc_session::{early_error, early_warn}; use rustc_span::source_map::{FileLoader, FileName}; use rustc_span::symbol::sym; -use log::debug; use std::borrow::Cow; use std::cmp::max; use std::default::Default; @@ -338,9 +337,7 @@ pub fn run_compiler( } } - debug!("before death"); queries.expansion()?; - debug!("AFTER DEATH (we don't get here?)"); if callbacks.after_expansion(compiler, queries) == Compilation::Stop { return early_exit(); } diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 60bb4a661fd01..7cfb608f11c89 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -15,6 +15,7 @@ use rustc_ast::mut_visit::MutVisitor; use rustc_ast::{self, ast, visit}; use rustc_codegen_ssa::back::link::emit_metadata; use rustc_codegen_ssa::traits::CodegenBackend; +use rustc_coverage::coverage; use rustc_data_structures::sync::{par_iter, Lrc, Once, ParallelIterator, WorkerLocal}; use rustc_data_structures::{box_region_allow_access, declare_box_region_type, parallel}; use rustc_errors::PResult; @@ -93,8 +94,8 @@ declare_box_region_type!( /// Runs the "early phases" of the compiler: initial `cfg` processing, loading compiler plugins, /// syntax expansion, secondary `cfg` expansion, synthesis of a test -/// harness if one is to be provided, injection of a dependency on the -/// standard library and prelude, and name resolution. +/// harness if one is to be provided, and injection of a dependency on the +/// standard library and prelude. /// /// Returns `None` if we're aborting after handling -W help. pub fn configure_and_expand( @@ -137,6 +138,43 @@ pub fn configure_and_expand( result.map(|k| (k, resolver)) } +/// If enabled by command line option, this pass injects coverage statements into the AST to +/// increment region counters and generate a coverage report at program exit. +/// The AST Crate cannot be resolved until after the AST is instrumented. +pub fn instrument(krate: ast::Crate, resolver: &mut Resolver<'_>) -> Result { + coverage::instrument(krate, resolver) +} + +/// Performs name resolution on the expanded and optionally instrumented crate. +pub fn resolve_crate( + sess: Lrc, + krate: ast::Crate, + resolver: &mut Resolver<'_>, +) -> Result { + let sess = &*sess; + + resolver.resolve_crate(&krate); + + // FIXME(richkadel): Run tests and confirm that check_crate must go after resolve_crate(), + // and if not, can/should I move it into configure_and_expand_inner() (which would mean + // running this before instrumentation, if that's OK). + + // FIXME(richkadel): original comment here with small modification: + + // Needs to go *after* expansion and name resolution, to be able to check the results of macro + // expansion. + sess.time("complete_gated_feature_checking", || { + rustc_ast_passes::feature_gate::check_crate( + &krate, + &sess.parse_sess, + &sess.features_untracked(), + sess.opts.unstable_features, + ); + }); + + Ok(krate) +} + impl BoxedResolver { pub fn to_resolver_outputs(resolver: Rc>) -> ResolverOutputs { match Rc::try_unwrap(resolver) { @@ -405,21 +443,10 @@ fn configure_and_expand_inner<'a>( } if sess.opts.debugging_opts.ast_json { + // Note this version of the AST will not include injected code, such as coverage counters. println!("{}", json::as_json(&krate)); } - resolver.resolve_crate(&krate); - - // Needs to go *after* expansion to be able to check the results of macro expansion. - sess.time("complete_gated_feature_checking", || { - rustc_ast_passes::feature_gate::check_crate( - &krate, - &sess.parse_sess, - &sess.features_untracked(), - sess.opts.unstable_features, - ); - }); - // Add all buffered lints from the `ParseSess` to the `Session`. sess.parse_sess.buffered_lints.with_lock(|buffered_lints| { info!("{} parse sess buffered_lints", buffered_lints.len()); @@ -428,6 +455,16 @@ fn configure_and_expand_inner<'a>( } }); + // // Needs to go *after* expansion, to be able to check the results of macro expansion. + // sess.time("complete_gated_feature_checking", || { + // rustc_ast_passes::feature_gate::check_crate( + // &krate, + // &sess.parse_sess, + // &sess.features_untracked(), + // sess.opts.unstable_features, + // ); + // }); + Ok((krate, resolver)) } diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index 600ce202d19cd..92f3490772929 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -8,7 +8,6 @@ use rustc::ty::{GlobalCtxt, ResolverOutputs, TyCtxt}; use rustc::util::common::ErrorReported; use rustc_ast::{self, ast}; use rustc_codegen_ssa::traits::CodegenBackend; -use rustc_coverage::coverage; use rustc_data_structures::sync::{Lrc, Once, WorkerLocal}; use rustc_hir::def_id::LOCAL_CRATE; use rustc_hir::Crate; @@ -18,7 +17,6 @@ use rustc_session::config::{OutputFilenames, OutputType}; use rustc_session::{output::find_crate_name, Session}; use rustc_span::symbol::sym; -use log::debug; use std::any::Any; use std::cell::{Ref, RefCell, RefMut}; use std::mem; @@ -77,8 +75,9 @@ pub struct Queries<'tcx> { parse: Query, crate_name: Query, register_plugins: Query<(ast::Crate, Lrc)>, - expansion: Query<(ast::Crate, Steal>>, Lrc)>, + unresolved_expansion: Query<(ast::Crate, Steal>>, Lrc)>, instrument: Query<(ast::Crate, Steal>>, Lrc)>, + expansion: Query<(ast::Crate, Steal>>, Lrc)>, dep_graph: Query, lower_to_hir: Query<(&'tcx Crate<'tcx>, Steal)>, prepare_outputs: Query, @@ -97,6 +96,7 @@ impl<'tcx> Queries<'tcx> { parse: Default::default(), crate_name: Default::default(), register_plugins: Default::default(), + unresolved_expansion: Default::default(), expansion: Default::default(), instrument: Default::default(), dep_graph: Default::default(), @@ -115,9 +115,7 @@ impl<'tcx> Queries<'tcx> { } pub fn dep_graph_future(&self) -> Result<&Query>> { - debug!("query dep_graph_future"); self.dep_graph_future.compute(|| { - debug!("compute query dep_graph_future"); Ok(self .session() .opts @@ -127,9 +125,7 @@ impl<'tcx> Queries<'tcx> { } pub fn parse(&self) -> Result<&Query> { - debug!("query parse"); self.parse.compute(|| { - debug!("compute query parse"); passes::parse(self.session(), &self.compiler.input).map_err(|mut parse_error| { parse_error.emit(); ErrorReported @@ -138,9 +134,7 @@ impl<'tcx> Queries<'tcx> { } pub fn register_plugins(&self) -> Result<&Query<(ast::Crate, Lrc)>> { - debug!("query register_plugins"); self.register_plugins.compute(|| { - debug!("compute query register_plugins"); let crate_name = self.crate_name()?.peek().clone(); let krate = self.parse()?.take(); @@ -165,9 +159,7 @@ impl<'tcx> Queries<'tcx> { } pub fn crate_name(&self) -> Result<&Query> { - debug!("query crate_name"); self.crate_name.compute(|| { - debug!("compute query crate_name"); Ok(match self.compiler.crate_name { Some(ref crate_name) => crate_name.clone(), None => { @@ -179,16 +171,15 @@ impl<'tcx> Queries<'tcx> { }) } - pub fn expansion( + /// Expands built-ins and other macros, but does not perform name resolution, pending potential + /// instrumentation. + pub fn unresolved_expansion( &self, ) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { - debug!("query expansion"); - self.expansion.compute(|| { - debug!("compute query expansion"); + self.unresolved_expansion.compute(|| { let crate_name = self.crate_name()?.peek().clone(); let (krate, lint_store) = self.register_plugins()?.take(); let _timer = self.session().timer("configure_and_expand"); -debug!("just before death"); passes::configure_and_expand( self.session().clone(), lint_store.clone(), @@ -197,16 +188,53 @@ debug!("just before death"); &crate_name, ) .map(|(krate, resolver)| { -debug!("NEVER GET HERE"); (krate, Steal::new(Rc::new(RefCell::new(resolver))), lint_store) }) }) } + /// Returns a modified ast::Crate with injected instrumentation code, if requested by command + /// line option. + /// + /// The `instrument` pass/query depends on (and implies) `unresolved_expansion`. + pub fn instrument( + &self, + ) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { + self.instrument.compute(|| { + let (krate, boxed_resolver, lint_store) = self.unresolved_expansion()?.take(); + let instrument_coverage = match self.session().opts.debugging_opts.instrument_coverage { + Some(opt) => opt, + None => false, + }; + if instrument_coverage { + boxed_resolver.borrow().borrow_mut().access(|resolver| { + let _timer = self.session().timer("instrument_coverage"); + passes::instrument(krate, resolver) + }) + } else { + Ok(krate) + } + .map(|krate| (krate, boxed_resolver, lint_store)) + }) + } + + /// Updates the ast::Crate, first querying `instrument`, which queries `unresolved_expansion`. + /// After all expansions and instrumentation, performs name resolution on the final AST. + pub fn expansion( + &self, + ) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { + self.expansion.compute(|| { + let (krate, boxed_resolver, lint_store) = self.instrument()?.take(); + let result = boxed_resolver.borrow().borrow_mut().access(|resolver| { + let _timer = self.session().timer("resolve_expansion"); + passes::resolve_crate(self.session().clone(), krate, resolver) + }); + result.map(|krate| (krate, boxed_resolver, lint_store)) + }) + } + pub fn dep_graph(&self) -> Result<&Query> { - debug!("query dep_graph"); self.dep_graph.compute(|| { - debug!("compute query dep_graph"); Ok(match self.dep_graph_future()?.take() { None => DepGraph::new_disabled(), Some(future) => { @@ -225,42 +253,10 @@ debug!("NEVER GET HERE"); }) } - /// Instrumentation is optional (typically included based on `rustc` option). The - /// `instrument` pass/query depends on (and implies) `expansion`. To ensure the instrumentation - /// pass is executed (if requested), replace queries to `expansion` with queries to - /// `instrument`. The query results are the same. If instrumentation is not requested, - /// `expansion` will be queried and returned. If instrumentation is requested, `instrument` - /// will query `expansion`, inject instrumentation code (modifying the crate and updating the - /// `next_node_id()` in the resolver), and then return the query result from `expansion`, with - /// the changes to crate and resolver. - pub fn instrument(&'tcx self) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { - debug!("query instrument (depends on expansion), will instrument coverage if requested"); - let expansion = self.expansion(); - let instrument_coverage = match self.session().opts.debugging_opts.instrument_coverage { - Some(opt) => opt, - None => false, - }; - if instrument_coverage { - self.instrument.compute(|| { - debug!("compute query instrument (depends on expansion), will instrument coverage if requested"); - let (mut krate, boxed_resolver, lint_store) = expansion?.take(); - let resolver = boxed_resolver.steal(); - resolver.borrow_mut().access(|resolver| { - coverage::instrument(&mut krate, resolver) - }); - Ok((krate, Steal::new(resolver), lint_store)) - }) - } else { - expansion - } - } - pub fn lower_to_hir(&'tcx self) -> Result<&Query<(&'tcx Crate<'tcx>, Steal)>> { - debug!("query lower_to_hir"); self.lower_to_hir.compute(|| { - debug!("compute query lower_to_hir"); - let instrument_result = self.instrument()?; - let peeked = instrument_result.peek(); + let expansion_result = self.expansion()?; + let peeked = expansion_result.peek(); let krate = &peeked.0; let resolver = peeked.1.steal(); let lint_store = &peeked.2; @@ -280,9 +276,7 @@ debug!("NEVER GET HERE"); } pub fn prepare_outputs(&self) -> Result<&Query> { - debug!("query prepare_outputs"); self.prepare_outputs.compute(|| { - debug!("compute query prepare_outputs"); let expansion_result = self.expansion()?; let (krate, boxed_resolver, _) = &*expansion_result.peek(); let crate_name = self.crate_name()?; @@ -298,9 +292,7 @@ debug!("NEVER GET HERE"); } pub fn global_ctxt(&'tcx self) -> Result<&Query>> { - debug!("query global_ctxt"); self.global_ctxt.compute(|| { - debug!("compute query global_ctxt"); let crate_name = self.crate_name()?.peek().clone(); let outputs = self.prepare_outputs()?.peek().clone(); let lint_store = self.expansion()?.peek().2.clone(); @@ -323,9 +315,7 @@ debug!("NEVER GET HERE"); } pub fn ongoing_codegen(&'tcx self) -> Result<&Query>> { - debug!("query ongoing_codegen"); self.ongoing_codegen.compute(|| { - debug!("compute query ongoing_codegen"); let outputs = self.prepare_outputs()?; self.global_ctxt()?.peek_mut().enter(|tcx| { tcx.analysis(LOCAL_CRATE).ok(); @@ -413,7 +403,6 @@ pub struct Linker { impl Linker { pub fn link(self) -> Result<()> { - debug!("Linker::link"); let codegen_results = self.codegen_backend.join_codegen(self.ongoing_codegen, &self.sess, &self.dep_graph)?; let prof = self.sess.prof.clone(); diff --git a/src/librustc_session/session.rs b/src/librustc_session/session.rs index adc6532028237..80f59aff69137 100644 --- a/src/librustc_session/session.rs +++ b/src/librustc_session/session.rs @@ -440,7 +440,6 @@ impl Session { } pub fn init_features(&self, features: rustc_feature::Features) { - debug!("ABOUT TO PANIC?") self.features.set(features); } diff --git a/src/libstd/coverage.rs b/src/libstd/coverage.rs new file mode 100644 index 0000000000000..668cdf5ef0edd --- /dev/null +++ b/src/libstd/coverage.rs @@ -0,0 +1,83 @@ +//! Code coverage counters and report +//! +//! The code coverage library is typically not included in Rust source files. +//! +//! Instead, the `rustc` compiler optionally injects coverage calls into the internal representation +//! of the source if requested by command line option to the compiler. The end result is an +//! executable that includes the coverage counters, and writes a coverage report upon exit. +//! +//! The injected calls behave as if code like the following examples was actually part of the +//! original source. +//! +//! Example: +//! +//! ``` +//! main() { +//! let value = if (true) { +//! std::coverage::count(1, { +//! // any expression +//! 1000 +//! }) +//! } else { +//! std::coverage::count(2, 500) +//! } +//! std::coverage::count_and_report(3, ()) +//! } +//! ``` + +#![stable(feature = "coverage", since = "1.44.0")] + +use crate::collections::HashMap; +use crate::sync::LockResult; +use crate::sync::Mutex; +use crate::sync::MutexGuard; +use crate::sync::Once; + +static mut COUNTERS: Option>> = None; + +static INIT: Once = Once::new(); + +#[stable(feature = "coverage", since = "1.44.0")] +#[inline] +fn increment_counter(counter: u128) -> LockResult>> { + let mut lock = unsafe { + INIT.call_once(|| { + COUNTERS = Some(Mutex::new(HashMap::with_capacity(1024))); + }); + COUNTERS.as_mut().unwrap().lock() + }; + let counters = lock.as_mut().unwrap(); + match counters.get_mut(&counter) { + Some(count) => *count += 1, + None => { + counters.insert(counter, 1); + } + } + lock +} + +/// The statement may be the last statement of a block, the value of a `return` or `break`, +/// a match pattern arm statement that might not be in a block (but if it is, don't wrap both the +/// block and the last statement of the block), or a closure statement without braces. +/// +/// coverage::count(234234, {some_statement_with_or_without_semicolon()}) +#[stable(feature = "coverage", since = "1.44.0")] +#[inline] +pub fn count(counter: u128, result: T) -> T { + let _ = increment_counter(counter); + result +} + +/// Increment the specified counter and then write the coverage report. This function normally wraps +/// the final expression in a `main()` function. There can be more than one statement, for example +/// if the `main()` has one or more `return` statements. In this case, all returns and the last +/// statement of `main()` (unless not reachable) should use this function. +#[stable(feature = "coverage", since = "1.44.0")] +pub fn count_and_report(counter: u128, result: T) -> T { + println!("Print the coverage counters:"); + let mut counters = increment_counter(counter).unwrap(); + for (counter, count) in counters.drain() { + println!("Counter '{}' has count: {}", counter, count); + } + result +} diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index e5dad307a209a..0f75f3fc797d1 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -447,6 +447,7 @@ pub mod thread; pub mod ascii; pub mod backtrace; pub mod collections; +pub mod coverage; pub mod env; pub mod error; pub mod ffi; From d2811641460e657badc739c73b9e22fcf54faa0a Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Thu, 9 Apr 2020 08:22:37 -0700 Subject: [PATCH 5/6] moved instrument pass from query to expansion pass --- Cargo.lock | 3 +- src/librustc_coverage/Cargo.toml | 4 +- src/librustc_coverage/coverage.rs | 13 +++--- src/librustc_interface/passes.rs | 75 ++++++++++--------------------- src/librustc_interface/queries.rs | 51 +-------------------- src/libstd/coverage.rs | 40 ++++++++++++++--- src/libstd/lib.rs | 1 - 7 files changed, 70 insertions(+), 117 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c306bfbd02be7..f9feed8d1cd4f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -483,6 +483,7 @@ dependencies = [ "if_chain", "itertools 0.9.0", "lazy_static 1.4.0", + "matches", "pulldown-cmark 0.7.0", "quine-mc_cluskey", "regex-syntax", @@ -3625,8 +3626,8 @@ name = "rustc_coverage" version = "0.0.0" dependencies = [ "log", - "rustc", "rustc_ast", + "rustc_errors", "rustc_resolve", "rustc_span", "smallvec 1.0.0", diff --git a/src/librustc_coverage/Cargo.toml b/src/librustc_coverage/Cargo.toml index 9f5989a23e00d..7f7db2314ef78 100644 --- a/src/librustc_coverage/Cargo.toml +++ b/src/librustc_coverage/Cargo.toml @@ -10,8 +10,8 @@ path = "lib.rs" [dependencies] log = "0.4" -rustc = { path = "../librustc" } +rustc_errors = { path = "../librustc_errors" } rustc_resolve = { path = "../librustc_resolve" } rustc_ast = { path = "../librustc_ast" } rustc_span = { path = "../librustc_span" } -smallvec = { version = "1.0", features = ["union", "may_dangle"] } \ No newline at end of file +smallvec = { version = "1.0", features = ["union", "may_dangle"] } diff --git a/src/librustc_coverage/coverage.rs b/src/librustc_coverage/coverage.rs index 25557601d4c35..25af9e891313f 100644 --- a/src/librustc_coverage/coverage.rs +++ b/src/librustc_coverage/coverage.rs @@ -1,10 +1,10 @@ //! Injects code coverage instrumentation into the AST. -use rustc::util::common::ErrorReported; use rustc_ast::ast::*; use rustc_ast::mut_visit::{self, MutVisitor}; use rustc_ast::ptr::P; use rustc_ast::token; +use rustc_errors::ErrorReported; use rustc_resolve::Resolver; use rustc_span::symbol::sym; use rustc_span::symbol::Symbol; @@ -150,11 +150,8 @@ impl CoverageVisitor<'_, '_> { } fn is_visiting_main(&self) -> bool { - if let Some(current_fn) = self.function_stack.last() { - *current_fn == sym::main - } else { - false - } + // len == 1 ensures a function is being visited and the function is not a nested function + self.function_stack.len() == 1 && *self.function_stack.last().unwrap() == sym::main } fn empty_tuple(&mut self, span: Span) -> P { @@ -165,7 +162,6 @@ impl CoverageVisitor<'_, '_> { id: self.next_ast_node_id(), }) } -} fn wrap_and_count_expr( &mut self, @@ -321,6 +317,9 @@ impl MutVisitor for CoverageVisitor<'_, '_> { // Rust calls parentheticals "Grouped expressions" // `?` operator which can invoke the equivalent of an early return (of `Err(...)`). // * {expr_possibly_in_block}? -> coverage::counter(n, {expr_possibly_in_block})? + // * Expression initializers for const/static values (where legal to invoke a function?) or + // assign a coverage region to the entire file, and increment a counter if/when it's known + // that the consts and statics were initialized for the file/mod. // Any others? // * NOT let var: type = expr (since there's no branching to worry about) // * NOT for or while loop expressions because they aren't optionally executed diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index e487bc05371b3..a23a4a37219b2 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -94,8 +94,8 @@ declare_box_region_type!( /// Runs the "early phases" of the compiler: initial `cfg` processing, loading compiler plugins, /// syntax expansion, secondary `cfg` expansion, synthesis of a test -/// harness if one is to be provided, and injection of a dependency on the -/// standard library and prelude. +/// harness if one is to be provided, injection of a dependency on the +/// standard library and prelude, and name resolution. /// /// Returns `None` if we're aborting after handling -W help. pub fn configure_and_expand( @@ -138,43 +138,6 @@ pub fn configure_and_expand( result.map(|k| (k, resolver)) } -/// If enabled by command line option, this pass injects coverage statements into the AST to -/// increment region counters and generate a coverage report at program exit. -/// The AST Crate cannot be resolved until after the AST is instrumented. -pub fn instrument(krate: ast::Crate, resolver: &mut Resolver<'_>) -> Result { - coverage::instrument(krate, resolver) -} - -/// Performs name resolution on the expanded and optionally instrumented crate. -pub fn resolve_crate( - sess: Lrc, - krate: ast::Crate, - resolver: &mut Resolver<'_>, -) -> Result { - let sess = &*sess; - - resolver.resolve_crate(&krate); - - // FIXME(richkadel): Run tests and confirm that check_crate must go after resolve_crate(), - // and if not, can/should I move it into configure_and_expand_inner() (which would mean - // running this before instrumentation, if that's OK). - - // FIXME(richkadel): original comment here with small modification: - - // Needs to go *after* expansion and name resolution, to be able to check the results of macro - // expansion. - sess.time("complete_gated_feature_checking", || { - rustc_ast_passes::feature_gate::check_crate( - &krate, - &sess.parse_sess, - &sess.features_untracked(), - sess.opts.unstable_features, - ); - }); - - Ok(krate) -} - impl BoxedResolver { pub fn to_resolver_outputs(resolver: Rc>) -> ResolverOutputs { match Rc::try_unwrap(resolver) { @@ -443,10 +406,30 @@ fn configure_and_expand_inner<'a>( } if sess.opts.debugging_opts.ast_json { - // Note this version of the AST will not include injected code, such as coverage counters. println!("{}", json::as_json(&krate)); } + let instrument_coverage = match sess.opts.debugging_opts.instrument_coverage { + Some(opt) => opt, + None => false, + }; + + if instrument_coverage { + krate = coverage::instrument(krate, &mut resolver)?; + } + + resolver.resolve_crate(&krate); + + // Needs to go *after* expansion to be able to check the results of macro expansion. + sess.time("complete_gated_feature_checking", || { + rustc_ast_passes::feature_gate::check_crate( + &krate, + &sess.parse_sess, + &sess.features_untracked(), + sess.opts.unstable_features, + ); + }); + // Add all buffered lints from the `ParseSess` to the `Session`. sess.parse_sess.buffered_lints.with_lock(|buffered_lints| { info!("{} parse sess buffered_lints", buffered_lints.len()); @@ -455,16 +438,6 @@ fn configure_and_expand_inner<'a>( } }); - // // Needs to go *after* expansion, to be able to check the results of macro expansion. - // sess.time("complete_gated_feature_checking", || { - // rustc_ast_passes::feature_gate::check_crate( - // &krate, - // &sess.parse_sess, - // &sess.features_untracked(), - // sess.opts.unstable_features, - // ); - // }); - Ok((krate, resolver)) } @@ -748,7 +721,7 @@ impl<'tcx> QueryContext<'tcx> { } pub fn print_stats(&mut self) { - self.enter(ty::query::print_stats) + self.enter(|tcx| ty::query::print_stats(tcx)) } } diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index 013e96f12f820..995776552dd18 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -16,7 +16,6 @@ use rustc_middle::util::common::ErrorReported; use rustc_session::config::{OutputFilenames, OutputType}; use rustc_session::{output::find_crate_name, Session}; use rustc_span::symbol::sym; - use std::any::Any; use std::cell::{Ref, RefCell, RefMut}; use std::mem; @@ -75,8 +74,6 @@ pub struct Queries<'tcx> { parse: Query, crate_name: Query, register_plugins: Query<(ast::Crate, Lrc)>, - unresolved_expansion: Query<(ast::Crate, Steal>>, Lrc)>, - instrument: Query<(ast::Crate, Steal>>, Lrc)>, expansion: Query<(ast::Crate, Steal>>, Lrc)>, dep_graph: Query, lower_to_hir: Query<(&'tcx Crate<'tcx>, Steal)>, @@ -96,9 +93,7 @@ impl<'tcx> Queries<'tcx> { parse: Default::default(), crate_name: Default::default(), register_plugins: Default::default(), - unresolved_expansion: Default::default(), expansion: Default::default(), - instrument: Default::default(), dep_graph: Default::default(), lower_to_hir: Default::default(), prepare_outputs: Default::default(), @@ -171,12 +166,10 @@ impl<'tcx> Queries<'tcx> { }) } - /// Expands built-ins and other macros, but does not perform name resolution, pending potential - /// instrumentation. - pub fn unresolved_expansion( + pub fn expansion( &self, ) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { - self.unresolved_expansion.compute(|| { + self.expansion.compute(|| { let crate_name = self.crate_name()?.peek().clone(); let (krate, lint_store) = self.register_plugins()?.take(); let _timer = self.session().timer("configure_and_expand"); @@ -193,46 +186,6 @@ impl<'tcx> Queries<'tcx> { }) } - /// Returns a modified ast::Crate with injected instrumentation code, if requested by command - /// line option. - /// - /// The `instrument` pass/query depends on (and implies) `unresolved_expansion`. - pub fn instrument( - &self, - ) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { - self.instrument.compute(|| { - let (krate, boxed_resolver, lint_store) = self.unresolved_expansion()?.take(); - let instrument_coverage = match self.session().opts.debugging_opts.instrument_coverage { - Some(opt) => opt, - None => false, - }; - if instrument_coverage { - boxed_resolver.borrow().borrow_mut().access(|resolver| { - let _timer = self.session().timer("instrument_coverage"); - passes::instrument(krate, resolver) - }) - } else { - Ok(krate) - } - .map(|krate| (krate, boxed_resolver, lint_store)) - }) - } - - /// Updates the ast::Crate, first querying `instrument`, which queries `unresolved_expansion`. - /// After all expansions and instrumentation, performs name resolution on the final AST. - pub fn expansion( - &self, - ) -> Result<&Query<(ast::Crate, Steal>>, Lrc)>> { - self.expansion.compute(|| { - let (krate, boxed_resolver, lint_store) = self.instrument()?.take(); - let result = boxed_resolver.borrow().borrow_mut().access(|resolver| { - let _timer = self.session().timer("resolve_expansion"); - passes::resolve_crate(self.session().clone(), krate, resolver) - }); - result.map(|krate| (krate, boxed_resolver, lint_store)) - }) - } - pub fn dep_graph(&self) -> Result<&Query> { self.dep_graph.compute(|| { Ok(match self.dep_graph_future()?.take() { diff --git a/src/libstd/coverage.rs b/src/libstd/coverage.rs index 668cdf5ef0edd..77aded607436c 100644 --- a/src/libstd/coverage.rs +++ b/src/libstd/coverage.rs @@ -12,15 +12,15 @@ //! Example: //! //! ``` -//! main() { -//! let value = if (true) { +//! fn main() { +//! let value = if true { //! std::coverage::count(1, { //! // any expression //! 1000 //! }) //! } else { //! std::coverage::count(2, 500) -//! } +//! }; //! std::coverage::count_and_report(3, ()) //! } //! ``` @@ -37,8 +37,18 @@ static mut COUNTERS: Option>> = None; static INIT: Once = Once::new(); +// FIXME(richkadel): Thinking about ways of optimizing executing coverage-instrumented code, I may +// be able to replace the hashmap lookup with a simple vector index, for most invocations, by +// assigning an index to each hash, at execution time, and storing that index in an injected static +// at the site of each counter. If the static is initialized to an unused index (say, u64::MAX), I +// can pass it into the std::coverage::count() function as mutable, and update it to the next index +// available. The vector would contain the hashed region IDs and corresponding counts. +// I just need to be sure the change can be done atomically, and if the same counter is called from +// multiple threads before the index is assigned, that the behavior is reasonable. (I suppose it +// *could* even tolerate the possibility that two indexes were assigned. Printing the report could +// sum the counts for any identical coverage region IDs, if we need to allow for that.) #[stable(feature = "coverage", since = "1.44.0")] -#[inline] +#[inline(always)] fn increment_counter(counter: u128) -> LockResult>> { let mut lock = unsafe { INIT.call_once(|| { @@ -60,10 +70,27 @@ fn increment_counter(counter: u128) -> LockResult(counter: u128, result: T) -> T { + // FIXME(richkadel): replace increment_counter with a call to the LLVM intrinsic: + // ``` + // declare void @llvm.instrprof.increment(i8* , i64 , + // i32 , i32 ) + // ``` + // See: http://llvm.org/docs/LangRef.html#llvm-instrprof-increment-intrinsic let _ = increment_counter(counter); result } @@ -73,6 +100,7 @@ pub fn count(counter: u128, result: T) -> T { /// if the `main()` has one or more `return` statements. In this case, all returns and the last /// statement of `main()` (unless not reachable) should use this function. #[stable(feature = "coverage", since = "1.44.0")] +#[inline(always)] pub fn count_and_report(counter: u128, result: T) -> T { println!("Print the coverage counters:"); let mut counters = increment_counter(counter).unwrap(); diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 88c991aa97506..b0f9ea5dfcfdc 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -465,7 +465,6 @@ pub mod path; pub mod process; pub mod sync; pub mod time; -pub mod coverage; #[stable(feature = "futures_api", since = "1.36.0")] pub mod task { From e843ff58c93b752a9cd4a9712b5ca67d808dc909 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Thu, 9 Apr 2020 13:19:38 -0700 Subject: [PATCH 6/6] =fixed merge issue and outdated submodule dependency (clippy) --- Cargo.lock | 1 - src/librustc_interface/passes.rs | 2 +- src/tools/clippy | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f9feed8d1cd4f..ce1a0b0ff4bbd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -483,7 +483,6 @@ dependencies = [ "if_chain", "itertools 0.9.0", "lazy_static 1.4.0", - "matches", "pulldown-cmark 0.7.0", "quine-mc_cluskey", "regex-syntax", diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index a23a4a37219b2..11f926821459c 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -721,7 +721,7 @@ impl<'tcx> QueryContext<'tcx> { } pub fn print_stats(&mut self) { - self.enter(|tcx| ty::query::print_stats(tcx)) + self.enter(ty::query::print_stats) } } diff --git a/src/tools/clippy b/src/tools/clippy index 1ff81c1b6d7ab..34763a5f3632b 160000 --- a/src/tools/clippy +++ b/src/tools/clippy @@ -1 +1 @@ -Subproject commit 1ff81c1b6d7abdcc9ee47f4a8ab175082cad4421 +Subproject commit 34763a5f3632b1d9081ba0245a729174996f0b38