diff --git a/crates/aptos/src/move_tool/bytecode.rs b/crates/aptos/src/move_tool/bytecode.rs index 6a662300b20db..6efe59a70fe7e 100644 --- a/crates/aptos/src/move_tool/bytecode.rs +++ b/crates/aptos/src/move_tool/bytecode.rs @@ -27,7 +27,7 @@ use move_disassembler::disassembler::{Disassembler, DisassemblerOptions}; use move_ir_types::location::Spanned; use std::{ fs, - path::{Path, PathBuf}, + path::{Component, Path, PathBuf}, process::Command, }; @@ -63,6 +63,9 @@ pub struct BytecodeCommand { #[clap(long)] pub is_script: bool, + #[clap(long)] + pub show_locs: bool, + #[clap(flatten)] input: BytecodeCommandInput, @@ -203,11 +206,43 @@ impl BytecodeCommand { fn disassemble(&self, bytecode_path: &Path) -> Result { let bytecode_bytes = read_from_file(bytecode_path)?; - let move_path = bytecode_path.with_extension(MOVE_EXTENSION); - let source_map_path = bytecode_path.with_extension(SOURCE_MAP_EXTENSION); - let source = fs::read_to_string(move_path).ok(); - let source_map = source_map_from_file(&source_map_path).ok(); + let source = { + let move_path = bytecode_path.with_extension(MOVE_EXTENSION); + if let Ok(source) = fs::read_to_string(move_path.clone()) { + Some(source) + } else { + let move_path = move_path + .components() + .map(|elt| { + if elt.as_os_str() == "bytecode_modules" { + Component::Normal("sources".as_ref()) + } else { + elt.clone() + } + }) + .collect::(); + fs::read_to_string(move_path).ok() + } + }; + let source_map = { + let source_map_path = bytecode_path.with_extension(SOURCE_MAP_EXTENSION); + if let Ok(source_map) = source_map_from_file(&source_map_path) { + Some(source_map) + } else { + let source_map_path = source_map_path + .components() + .map(|elt| { + if elt.as_os_str() == "bytecode_modules" { + Component::Normal("source_maps".as_ref()) + } else { + elt.clone() + } + }) + .collect::(); + source_map_from_file(&source_map_path).ok() + } + }; let disassembler_options = DisassemblerOptions { print_code: true, @@ -215,6 +250,7 @@ impl BytecodeCommand { print_basic_blocks: true, print_locals: true, print_bytecode_stats: false, + show_locs: self.show_locs, }; let no_loc = Spanned::unsafe_no_loc(()).loc; let module: CompiledModule; @@ -232,6 +268,11 @@ impl BytecodeCommand { let mut source_mapping = if let Some(s) = source_map { SourceMapping::new(s, bytecode) } else { + if self.show_locs { + return Err(CliError::UnexpectedError( + "Missing source_map with option show-locs".to_string(), + )); + } SourceMapping::new_from_view(bytecode, no_loc) .context("Unable to build dummy source mapping")? }; diff --git a/crates/aptos/src/move_tool/coverage.rs b/crates/aptos/src/move_tool/coverage.rs index 53add94da75b6..b85aad965a277 100644 --- a/crates/aptos/src/move_tool/coverage.rs +++ b/crates/aptos/src/move_tool/coverage.rs @@ -124,7 +124,17 @@ impl CliCommand<()> for SourceCoverage { }) => (module, source_map), _ => panic!("Should all be modules"), }; - let source_coverage = SourceCoverageBuilder::new(module, &coverage_map, source_map); + let packages: Vec<_> = package + .root_modules() + .map(|unit| match &unit.unit { + CompiledUnit::Module(NamedCompiledModule { + module, source_map, .. + }) => (module, source_map), + _ => panic!("Should all be modules"), + }) + .collect(); + let source_coverage = + SourceCoverageBuilder::new(module, &coverage_map, source_map, packages); let source_coverage = source_coverage.compute_source_coverage(source_path); let output_result = source_coverage.output_source_coverage(&mut std::io::stdout(), self.color, self.tag); diff --git a/third_party/move/move-compiler-v2/src/lib.rs b/third_party/move/move-compiler-v2/src/lib.rs index 84eda127b9f03..81a63ad02e406 100644 --- a/third_party/move/move-compiler-v2/src/lib.rs +++ b/third_party/move/move-compiler-v2/src/lib.rs @@ -504,14 +504,7 @@ pub fn bytecode_pipeline(env: &GlobalEnv) -> FunctionTargetPipeline { pub fn disassemble_compiled_units(units: &[CompiledUnit]) -> anyhow::Result { let disassembled_units: anyhow::Result> = units .iter() - .map(|unit| { - let view = match unit { - CompiledUnit::Module(module) => BinaryIndexedView::Module(&module.module), - CompiledUnit::Script(script) => BinaryIndexedView::Script(&script.script), - }; - Disassembler::from_view(view, location::Loc::new(FileHash::empty(), 0, 0)) - .and_then(|d| d.disassemble()) - }) + .map(|unit| Disassembler::from_unit(unit).disassemble()) .collect(); Ok(disassembled_units?.concat()) } diff --git a/third_party/move/move-ir/types/src/location.rs b/third_party/move/move-ir/types/src/location.rs index f97cd2e752606..cc47f30e82093 100644 --- a/third_party/move/move-ir/types/src/location.rs +++ b/third_party/move/move-ir/types/src/location.rs @@ -36,6 +36,7 @@ pub struct Loc { impl Loc { pub fn new(file_hash: FileHash, start: ByteIndex, end: ByteIndex) -> Loc { + assert!(start <= end); Loc { file_hash, start, @@ -61,6 +62,94 @@ impl Loc { end: self.end as usize, } } + + pub fn contains(&self, other: &Self) -> bool { + if self.file_hash != other.file_hash { + false + } else { + self.start <= other.start && other.end <= self.end + } + } + + pub fn overlaps(&self, other: &Self) -> bool { + if self.file_hash != other.file_hash { + false + } else { + // [a, b] overlaps? [c, d] + // a <= b + // c <= d + other.start <= self.end && self.start <= other.end + // c <= b + // a <= d + // One of these: + // [a <= (c <= b] <= d) + // (c <= [a <= b] <= d) + // (c <= [a <= d) <= b] + } + } + + pub fn overlaps_or_abuts(&self, other: &Self) -> bool { + if self.file_hash != other.file_hash { + false + } else { + // [a, b] overlaps? [c, d] + // a <= b + // c <= d + other.start <= self.end + 1 && self.start <= other.end + 1 + // c <= b + 1 + // a <= d + 1 + // One of these: + // a <= c <= b+1 <= d+1 + // c <= a <= b+1 <= d+1 + // c <= a <= d+1 <= b+1 + } + } + + pub fn try_merge(&mut self, other: &Self) -> bool { + if self.overlaps_or_abuts(other) { + self.start = std::cmp::min(self.start, other.start); + self.end = std::cmp::max(self.end, other.end); + true + } else { + false + } + } + + // if other overlaps with this, then subtract it out. + pub fn subtract(self, other: &Self) -> Vec { + if !self.overlaps(other) { + vec![self] + } else { + if other.start <= self.start { + if self.end <= other.end { + vec![] + } else { + vec![Loc { + start: other.end + 1, + ..self + }] + } + } else { + if self.end <= other.end { + vec![Loc { + end: other.start - 1, + ..self + }] + } else { + vec![ + Loc { + end: other.start - 1, + ..self + }, + Loc { + start: other.end + 1, + ..self + }, + ] + } + } + } + } } impl PartialOrd for Loc { diff --git a/third_party/move/move-model/src/model.rs b/third_party/move/move-model/src/model.rs index 9428d08ec9617..e63c4a9a98963 100644 --- a/third_party/move/move-model/src/model.rs +++ b/third_party/move/move-model/src/model.rs @@ -3337,6 +3337,7 @@ impl<'env> ModuleEnv<'env> { print_basic_blocks: true, print_locals: true, print_bytecode_stats: false, + show_locs: false, }); Some( disas diff --git a/third_party/move/tools/move-cli/src/base/coverage.rs b/third_party/move/tools/move-cli/src/base/coverage.rs index 51a514f31cba1..f5049555be014 100644 --- a/third_party/move/tools/move-cli/src/base/coverage.rs +++ b/third_party/move/tools/move-cli/src/base/coverage.rs @@ -79,8 +79,17 @@ impl Coverage { }) => (module, source_map), _ => panic!("Should all be modules"), }; + let packages: Vec<_> = package + .root_modules() + .map(|unit| match &unit.unit { + CompiledUnit::Module(NamedCompiledModule { + module, source_map, .. + }) => (module, source_map), + _ => panic!("Should all be modules"), + }) + .collect(); let source_coverage_builder = - SourceCoverageBuilder::new(module, &coverage_map, source_map); + SourceCoverageBuilder::new(module, &coverage_map, source_map, packages); let source_coverage = source_coverage_builder.compute_source_coverage(source_path); source_coverage .output_source_coverage(&mut std::io::stdout(), self.color, self.tag) diff --git a/third_party/move/tools/move-coverage/src/source_coverage.rs b/third_party/move/tools/move-coverage/src/source_coverage.rs index 59aa38eb534de..b2ab82212e1a6 100644 --- a/third_party/move/tools/move-coverage/src/source_coverage.rs +++ b/third_party/move/tools/move-coverage/src/source_coverage.rs @@ -6,7 +6,7 @@ use crate::coverage_map::CoverageMap; use clap::ValueEnum; -use codespan::{Files, Span}; +use codespan::{FileId, Files, Span}; use colored::{self, Colorize}; use move_binary_format::{ access::ModuleAccess, @@ -15,11 +15,11 @@ use move_binary_format::{ }; use move_bytecode_source_map::source_map::SourceMap; use move_command_line_common::files::FileHash; -use move_core_types::identifier::Identifier; use move_ir_types::location::Loc; use serde::Serialize; use std::{ - collections::BTreeMap, + cmp::{Ordering, PartialOrd}, + collections::{BTreeMap, BTreeSet}, fmt::{Display, Formatter}, fs, io::{self, Write}, @@ -38,20 +38,172 @@ pub struct FunctionSourceCoverage { pub uncovered_locations: Vec, } +/// Source-level positive coverage information for a function. +#[derive(Clone, Debug, Serialize)] +pub struct FunctionSourceCoverage2 { + /// Is this a native function? + /// If so, then `uncovered_locations` is empty. + pub fn_is_native: bool, + + /// List of source locations in the function that were covered. + pub covered_locations: Vec, + + /// List of all source locations in the function. + pub uncovered_locations: Vec, +} + +fn minimize_locations(mut locs: Vec) -> Vec { + locs.sort(); + let mut result = vec![]; + let mut locs_iter = locs.into_iter(); + if let Some(mut current_loc) = locs_iter.next() { + for next_loc in locs_iter { + let loc_tmp = current_loc; + if !current_loc.try_merge(&next_loc) { + eprintln!("Not merging {:?} with {:?}", loc_tmp, next_loc); + result.push(current_loc); + current_loc = next_loc; + } + } + result.push(current_loc); + } + result +} + +// locs1 and locs2 should each be sorted or this won't work. +fn subtract_locations(locs1: BTreeSet, locs2: &BTreeSet) -> Vec { + let mut result = vec![]; + let mut locs1_iter = locs1.into_iter(); + let mut locs2_iter = locs2.iter(); + if let Some(mut current_loc1) = locs1_iter.next() { + if let Some(mut current_loc2) = locs2_iter.next() { + loop { + if current_loc1.overlaps(current_loc2) { + let mut diff = current_loc1.subtract(current_loc2); + if let Some(new_loc1) = diff.pop() { + result.append(&mut diff); + current_loc1 = new_loc1; + // continue + } else { + // diff was empty, get a new loc1 + if let Some(new_loc1) = locs1_iter.next() { + current_loc1 = new_loc1; + // retry loc2 + // continue + } else { + // no more loc1, return + return result; + } + } + } else { + // no overlap + if current_loc1 <= *current_loc2 { + // loc1 is done. save it and get a new one. + result.push(current_loc1); + if let Some(new_loc1) = locs1_iter.next() { + current_loc1 = new_loc1; + // continue + } else { + // No more loc1, return + return result; + } + } else { + // loc1 might have more overlaps, try another loc2 + if let Some(new_loc2) = locs2_iter.next() { + current_loc2 = new_loc2; + // continue + } else { + // loc2 is finished but loc1 is not, + // finish adding all loc1 + result.push(current_loc1); + for loc1 in locs1_iter { + result.push(loc1); + } + return result; + } + } + } + } + } + result.push(current_loc1); + } + result +} + /// Builder for the source code coverage. #[derive(Debug, Serialize)] pub struct SourceCoverageBuilder<'a> { /// Mapping from function name to the source-level uncovered locations for that function. - pub uncovered_locations: BTreeMap, + pub uncovered_locations: Vec, + pub covered_locations: Vec, source_map: &'a SourceMap, } -#[derive(Debug, Serialize, Eq, PartialEq, Ord, PartialOrd)] +#[derive(Debug, Serialize, Eq, PartialEq)] pub enum AbstractSegment { - Bounded { start: u32, end: u32 }, - BoundedRight { end: u32 }, - BoundedLeft { start: u32 }, + Bounded { + is_covered: bool, + start: u32, + end: u32, + }, + BoundedRight { + is_covered: bool, + end: u32, + }, + BoundedLeft { + is_covered: bool, + start: u32, + }, + // Unbounded {}, +} + +impl AbstractSegment { + fn is_covered(&self) -> bool { + use AbstractSegment::*; + match self { + Bounded { is_covered, .. } + | BoundedRight { is_covered, .. } + | BoundedLeft { is_covered, .. } => *is_covered, + } + } + + fn get_start(&self) -> u32 { + use AbstractSegment::*; + match self { + Bounded { start, .. } => *start, + BoundedRight { .. } => 0u32, + BoundedLeft { start, .. } => *start, + // Unbounded {} => 0u32, + } + } + + fn get_number(&self) -> u8 { + use AbstractSegment::*; + match self { + Bounded { .. } => 0, + BoundedRight { .. } => 1, + BoundedLeft { .. } => 2, + // Unbounded {} => 3, + } + } +} + +impl PartialOrd for AbstractSegment { + fn partial_cmp(&self, other: &AbstractSegment) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for AbstractSegment { + fn cmp(&self, other: &AbstractSegment) -> Ordering { + use Ordering::*; + match self.get_start().cmp(&other.get_start()) { + Less => Less, + Greater => Greater, + Equal => self.get_number().cmp(&other.get_number()), + } + } } /// Option to control use of color escape codes in coverage output @@ -168,6 +320,7 @@ impl ColorChoice { pub enum StringSegment { Covered(String), Uncovered(String), + NotCounted(String), } pub type AnnotatedLine = Vec; @@ -182,83 +335,270 @@ impl<'a> SourceCoverageBuilder<'a> { module: &CompiledModule, coverage_map: &CoverageMap, source_map: &'a SourceMap, + packages: Vec<(&CompiledModule, &'a SourceMap)>, ) -> Self { eprintln!("coverage_map is {:#?}", coverage_map); eprintln!("source_map is {:#?}", source_map); eprintln!("module is {:#?}", module); + let module_loc = source_map.definition_location; + let module_name = module.self_id(); let unified_exec_map = coverage_map.to_unified_exec_map(); let module_map = unified_exec_map .module_maps .get(&(*module_name.address(), module_name.name().to_owned())); - let uncovered_locations: BTreeMap = module - .function_defs() - .iter() - .enumerate() - .flat_map(|(function_def_idx, function_def)| { - let fn_handle = module.function_handle_at(function_def.function); - let fn_name = module.identifier_at(fn_handle.name).to_owned(); - let function_def_idx = FunctionDefinitionIndex(function_def_idx as u16); - - // If the function summary doesn't exist then that function hasn't been called yet. - let coverage = match &function_def.code { - None => Some(FunctionSourceCoverage { - fn_is_native: true, - uncovered_locations: Vec::new(), - }), - Some(code_unit) => { - module_map.map(|fn_map| match fn_map.function_maps.get(&fn_name) { + eprintln!("unified_exec_map is {:#?}", &unified_exec_map); + eprintln!("module_map is {:#?}", &module_map); + + eprintln!("Computing covered_locations"); + + let mut fun_coverage: Vec = Vec::new(); + for (module, source_map) in packages.iter() { + let module_name = module.self_id(); + let module_map = unified_exec_map + .module_maps + .get(&(*module_name.address(), module_name.name().to_owned())); + if let Some(module_map) = module_map { + for (function_def_idx, function_def) in module.function_defs().iter().enumerate() { + let fn_handle = module.function_handle_at(function_def.function); + let fn_name = module.identifier_at(fn_handle.name).to_owned(); + let function_def_idx = FunctionDefinitionIndex(function_def_idx as u16); + let function_map = source_map + .get_function_source_map(function_def_idx) + .unwrap(); + let _function_loc = function_map.definition_location; + let function_covered_locations: FunctionSourceCoverage2 = match &function_def + .code + { + None => FunctionSourceCoverage2 { + fn_is_native: true, + covered_locations: vec![], + uncovered_locations: vec![], + }, + Some(code_unit) => match module_map.function_maps.get(&fn_name) { None => { - let function_map = source_map - .get_function_source_map(function_def_idx) - .unwrap(); - let mut uncovered_locations = - vec![function_map.definition_location]; - uncovered_locations.extend(function_map.code_map.values()); - - FunctionSourceCoverage { + let locations: Vec<_> = (0..code_unit.code.len()) + .filter_map(|code_offset| { + if let Ok(loc) = source_map.get_code_location( + function_def_idx, + code_offset as CodeOffset, + ) { + Some(loc) + } else { + None + } + }) + .collect(); + FunctionSourceCoverage2 { fn_is_native: false, - uncovered_locations, + covered_locations: vec![], + uncovered_locations: locations, } }, Some(function_coverage) => { - let uncovered_locations: Vec<_> = (0..code_unit.code.len()) - .flat_map(|code_offset| { - if !function_coverage.contains_key(&(code_offset as u64)) { - Some( - source_map - .get_code_location( - function_def_idx, - code_offset as CodeOffset, - ) - .unwrap(), - ) + let (fun_cov, fun_uncov): (Vec<_>, Vec<_>) = (0..code_unit + .code + .len()) + .filter_map(|code_offset| { + if let Ok(loc) = source_map.get_code_location( + function_def_idx, + code_offset as CodeOffset, + ) { + if function_coverage.contains_key(&(code_offset as u64)) + { + Some((loc, true)) + } else { + Some((loc, false)) + } } else { None } }) + .partition(|(_loc, test)| *test); + + let covered_locations: Vec<_> = fun_cov + .iter() + .filter_map( + |(loc, covered)| if *covered { Some(*loc) } else { None }, + ) + .collect(); + let uncovered_locations: Vec<_> = fun_uncov + .iter() + .filter_map( + |(loc, covered)| if !*covered { Some(*loc) } else { None }, + ) .collect(); - FunctionSourceCoverage { + + FunctionSourceCoverage2 { fn_is_native: false, + covered_locations, uncovered_locations, } }, - }) - }, - }; - coverage.map(|x| (fn_name, x)) + }, + }; + fun_coverage.push(function_covered_locations); + } + } + } + eprintln!("fun_coverage is {:#?}", fun_coverage); + + // Filter locations for this module and build 2 sets: covered and uncovered locations + // Note that Move 1 compiler sets module_loc = location of symbol in definition, so if + // there are multiple modules in one file we may leave others in the picture. + eprintln!("module_loc is {:?}", module_loc); + let module_file_hash = module_loc.file_hash(); + eprintln!("module_file_hash is {:?}", module_file_hash); + + let (covered, uncovered): (BTreeSet, BTreeSet) = fun_coverage + .iter() + .map( + |FunctionSourceCoverage2 { + covered_locations, + uncovered_locations, + .. + }| { + let cov: BTreeSet<_> = covered_locations + .iter() + .filter(|loc| loc.file_hash() == module_file_hash) + .cloned() + .collect(); + let uncov: BTreeSet<_> = uncovered_locations + .iter() + .filter(|loc| loc.file_hash() == module_file_hash) + .cloned() + .collect(); + (cov, uncov) + }, + ) + .reduce(|(c1, u1), (c2, u2)| { + ( + c1.union(&c2).cloned().collect(), + u1.union(&u2).cloned().collect(), + ) }) - .collect(); - eprintln!("uncovered_locations is {:#?}", uncovered_locations); + .unwrap_or_else(|| (BTreeSet::new(), BTreeSet::new())); + + eprintln!("covered 0 is {:#?}", covered); + eprintln!("uncovered 0 is {:#?}", uncovered); + + // Some locations contain others, but should not subsume them, e.g.: + // 31: if (p) { + // 32: ... + // 33: } else { + // 34: ... + // 35 } + // May have 3 corresponding bytecodes, with locations: + // L1: 31-35 + // L2: 32 + // L3: 34 + // If L1 is covered but L3 is not, then we want to subtract + // L3 from the covered region. + // + // OTOH, we may see L3 in multiple runs, once covered and once + // not. So we need to + // (1) subtract identical covered locations from uncovered locations + // (2) subtract uncovered locations from any covered locations they overlap + // (3) subtract covered locations from any uncovered lcoations they overlap + + // (1) + let uncovered: BTreeSet<_> = uncovered.difference(&covered).cloned().collect(); + + eprintln!("uncovered 1 is {:#?}", uncovered); + + // (2) + let covered: Vec<_> = subtract_locations(covered, &uncovered); + + eprintln!("covered 2a is {:#?}", covered); + + let covered: Vec<_> = minimize_locations(covered); + + eprintln!("covered 2b is {:#?}", covered); + + let uncovered: Vec<_> = uncovered.into_iter().collect(); + eprintln!("uncovered 2a is {:#?}", uncovered); + + let uncovered: Vec<_> = minimize_locations(uncovered); + + eprintln!("uncovered 2b is {:#?}", uncovered); + + let uncovered: BTreeSet<_> = uncovered.into_iter().collect(); + let covered: BTreeSet<_> = covered.into_iter().collect(); + + // (3) + let uncovered: Vec<_> = subtract_locations(uncovered, &covered); + let covered: Vec<_> = covered.into_iter().collect(); + + eprintln!("uncovered 3 is {:#?}", uncovered); + eprintln!("uncovered is {:#?}", uncovered); Self { - uncovered_locations, + uncovered_locations: uncovered, + covered_locations: covered, source_map, } } + // Converts a (sorted) list of non-overlapping `Loc` into a + // map from line number to set of `AsbtractSegment` for each line + // of the file `file_id` in fileset `files`. + fn locs_to_segments( + &self, + files: &mut Files, + file_id: FileId, + spans: Vec, + is_covered: bool, + ) -> BTreeMap> { + let mut segments = BTreeMap::new(); + + for span in spans.into_iter() { + let start_loc = files.location(file_id, span.start()).unwrap(); + let end_loc = files.location(file_id, span.end()).unwrap(); + let start_line = start_loc.line.0; + let end_line = end_loc.line.0; + eprintln!( + "Looking at span = ({}, {}), line = ({}, {})", + span.start(), + span.end(), + start_line, + end_line + ); + + let line_segments = segments.entry(start_line).or_insert_with(Vec::new); + if start_line == end_line { + let segment = AbstractSegment::Bounded { + start: start_loc.column.0, + end: end_loc.column.0, + is_covered, + }; + if !line_segments.contains(&segment) { + line_segments.push(segment); + } + } else { + line_segments.push(AbstractSegment::BoundedLeft { + start: start_loc.column.0, + is_covered, + }); + for i in start_line + 1..end_line { + let line_segments = segments.entry(i).or_insert_with(Vec::new); + line_segments.push(AbstractSegment::BoundedLeft { + start: 0, + is_covered, + }); + } + let last_line_segments = segments.entry(end_line).or_insert_with(Vec::new); + last_line_segments.push(AbstractSegment::BoundedRight { + end: end_loc.column.0, + is_covered, + }); + } + } + segments.values_mut().for_each(|v| v.sort()); + segments + } + pub fn compute_source_coverage(&self, file_path: &Path) -> SourceCoverage { eprintln!("Reading file {}", file_path.display()); let file_contents = fs::read_to_string(file_path).unwrap(); @@ -267,64 +607,41 @@ impl<'a> SourceCoverageBuilder<'a> { "File contents {} out of sync with source map", file_path.display() ); - let file_hash = self.source_map.definition_location.file_hash(); let mut files = Files::new(); let file_id = files.add(file_path.as_os_str().to_os_string(), file_contents.clone()); - let mut uncovered_segments = BTreeMap::new(); + let covs: Vec<_> = self + .covered_locations + .iter() + .map(|loc| Span::new(loc.start(), loc.end())) + .collect(); + let uncovs: Vec<_> = self + .uncovered_locations + .iter() + .map(|loc| Span::new(loc.start(), loc.end())) + .collect(); - for (_key, fn_cov) in self.uncovered_locations.iter() { - for span in merge_spans(file_hash, fn_cov.clone()).into_iter() { - let start_loc = files.location(file_id, span.start()).unwrap(); - let end_loc = files.location(file_id, span.end()).unwrap(); - let start_line = start_loc.line.0; - let end_line = end_loc.line.0; - eprintln!( - "Looking at key = {}, span = ({}, {}), line = ({}, {})", - _key.clone().into_string(), - span.start(), - span.end(), - start_line, - end_line - ); - let segments = uncovered_segments - .entry(start_line) - .or_insert_with(Vec::new); - if start_line == end_line { - let segment = AbstractSegment::Bounded { - start: start_loc.column.0, - end: end_loc.column.0, - }; - // TODO: There is some issue with the source map where we have multiple spans - // from different functions. This can be seen in the source map for `Roles.move` - if !segments.contains(&segment) { - segments.push(segment); - } - } else { - segments.push(AbstractSegment::BoundedLeft { - start: start_loc.column.0, - }); - for i in start_line + 1..end_line { - let segment = uncovered_segments.entry(i).or_insert_with(Vec::new); - segment.push(AbstractSegment::BoundedLeft { start: 0 }); - } - let last_segment = uncovered_segments.entry(end_line).or_insert_with(Vec::new); - last_segment.push(AbstractSegment::BoundedRight { - end: end_loc.column.0, - }); - } + let mut uncovered_segments = self.locs_to_segments(&mut files, file_id, uncovs, false); + let mut covered_segments = self.locs_to_segments(&mut files, file_id, covs, true); + + covered_segments.values_mut().for_each(|v| v.sort()); + for (key, value) in covered_segments.iter_mut() { + match uncovered_segments.get_mut(key) { + None => {}, + Some(value2) => { + value.append(value2); + value.sort(); + }, } } - uncovered_segments.values_mut().for_each(|v| v.sort()); let mut annotated_lines = Vec::new(); for (line_number, mut line) in file_contents.lines().map(|x| x.to_owned()).enumerate() { eprintln!("looking at {}, {}", line_number, line); - match uncovered_segments.get(&(line_number as u32)) { - None => annotated_lines.push(vec![StringSegment::Covered(line)]), + let line_no_u32 = line_number as u32; + match covered_segments.get(&line_no_u32) { + None => annotated_lines.push(vec![StringSegment::NotCounted(line)]), Some(segments) => { - // Note: segments are already pre-sorted by construction so don't need to be - // resorted. eprintln!( "Segments for line {} are: {}", line_number, @@ -336,43 +653,69 @@ impl<'a> SourceCoverageBuilder<'a> { ); let mut line_acc = Vec::new(); let mut cursor = 0; + // Note: segments are already pre-sorted by construction so don't need to be + // resorted. + // let mut segment_start: Option = None; + // let mut segment_end: Option = None; for segment in segments { match segment { - AbstractSegment::Bounded { start, end } => { + AbstractSegment::Bounded { + start, + end, + is_covered, + } => { eprintln!("Bounded {}, {}, cursor = {}", start, end, cursor); + let length = end - start; let (before, after) = line.split_at((start - cursor) as usize); - let (uncovered, rest) = after.split_at(length as usize); - line_acc.push(StringSegment::Covered(before.to_string())); - line_acc.push(StringSegment::Uncovered(uncovered.to_string())); + let (covered, rest) = after.split_at(length as usize); + line_acc.push(StringSegment::NotCounted(before.to_string())); + line_acc.push( + if *is_covered { + StringSegment::Covered(covered.to_string()) + } else { + StringSegment::Uncovered(covered.to_string()) + }, + ); line = rest.to_string(); cursor = *end; }, - AbstractSegment::BoundedRight { end } => { + AbstractSegment::BoundedRight { end, is_covered } => { eprintln!("BoundedRight {}, cursor = {}", end, cursor); let (uncovered, rest) = line.split_at((end - cursor) as usize); - line_acc.push(StringSegment::Uncovered(uncovered.to_string())); + line_acc.push( + if *is_covered { + StringSegment::Covered(uncovered.to_string()) + } else { + StringSegment::Uncovered(uncovered.to_string()) + }, + ); line = rest.to_string(); cursor = *end; }, - AbstractSegment::BoundedLeft { start } => { + AbstractSegment::BoundedLeft { start, is_covered } => { eprintln!("BoundedLeft {}, cursor = {}", start, cursor); let (before, after) = line.split_at((start - cursor) as usize); - line_acc.push(StringSegment::Covered(before.to_string())); - line_acc.push(StringSegment::Uncovered(after.to_string())); + line_acc.push(StringSegment::NotCounted(before.to_string())); + line_acc.push( + if *is_covered { + StringSegment::Covered(after.to_string()) + } else { + StringSegment::Uncovered(after.to_string()) + }, + ); line = "".to_string(); cursor = 0; }, } } if !line.is_empty() { - line_acc.push(StringSegment::Covered(line)) + line_acc.push(StringSegment::NotCounted(line)) } annotated_lines.push(line_acc) }, - } + }; } - SourceCoverage { annotated_lines } } } @@ -402,13 +745,18 @@ impl SourceCoverage { let has_uncovered = line .iter() .any(|string_segment| matches!(string_segment, StringSegment::Uncovered(_))); + let has_covered = line + .iter() + .any(|string_segment| matches!(string_segment, StringSegment::Covered(_))); write!( output_writer, "{} ", if has_uncovered { "-".to_string().red() - } else { + } else if has_covered { "+".to_string().green() + } else { + " ".to_string().normal() } )?; } @@ -416,6 +764,7 @@ impl SourceCoverage { match string_segment { StringSegment::Covered(s) => write!(output_writer, "{}", s.green())?, StringSegment::Uncovered(s) => write!(output_writer, "{}", s.bold().red())?, + StringSegment::NotCounted(s) => write!(output_writer, "{}", s.normal())?, } } writeln!(output_writer)?; diff --git a/third_party/move/tools/move-disassembler/src/disassembler.rs b/third_party/move/tools/move-disassembler/src/disassembler.rs index 3dedaa3f942b1..3348852ba2023 100644 --- a/third_party/move/tools/move-disassembler/src/disassembler.rs +++ b/third_party/move/tools/move-disassembler/src/disassembler.rs @@ -48,6 +48,10 @@ pub struct DisassemblerOptions { /// Print bytecode statistics for the module. #[clap(long = "print-bytecode-stats")] pub print_bytecode_stats: bool, + + /// Show source locations for bytecodes. + #[clap(long = "show-locs")] + pub show_locs: bool, } impl DisassemblerOptions { @@ -58,6 +62,7 @@ impl DisassemblerOptions { print_basic_blocks: true, print_locals: true, print_bytecode_stats: false, + show_locs: false, } } } @@ -297,12 +302,12 @@ impl<'a> Disassembler<'a> { instruction: String, ) -> String { if self.coverage_map.is_none() { - return format!("\t{}: {}", pc, instruction); + return format!("\t{:5}: {}", pc, instruction); } let coverage = function_coverage_map.and_then(|map| map.get(&(pc as u64))); match coverage { - Some(coverage) => format!("[{}]\t{}: {}", coverage, pc, instruction).green(), - None => format!("\t{}: {}", pc, instruction).red(), + Some(coverage) => format!("[{}]\t{:5}: {}", coverage, pc, instruction).green(), + None => format!("\t{}: {:5}", pc, instruction).red(), } .to_string() } @@ -1158,17 +1163,39 @@ impl<'a> Disassembler<'a> { let instrs: Vec = code .code .iter() - .map(|instruction| { - self.disassemble_instruction( + .enumerate() + .map(|(idx, instruction)| { + match self.disassemble_instruction( parameters, instruction, locals_sigs, function_source_map, decl_location, - ) + ) { + Ok(instr_str) => { + if self.options.show_locs { + if let Some(loc) = function_source_map.get_code_location(idx as u16) { + let hash = + (format!("{}", loc.file_hash()).to_string())[..8].to_string(); + Ok(format!( + "{:60} {}:{}-{}", + instr_str, + hash, + loc.start(), + loc.end() + ) + .to_string()) + } else { + Ok(instr_str) + } + } else { + Ok(instr_str) + } + }, + Err(err) => Err(err), + } }) .collect::>>()?; - let mut instrs: Vec = instrs .into_iter() .enumerate()