Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coverage work in progress #15250

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/aptos/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ repository = { workspace = true }
rust-version = { workspace = true }

[dependencies]
anyhow = { workspace = true }
anyhow = { workspace = true, features = [ "backtrace" ] }
aptos-api-types = { workspace = true }
aptos-backup-cli = { workspace = true }
aptos-bitvec = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion crates/aptos/src/common/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::{
genesis::git::from_yaml,
move_tool::{ArgWithType, FunctionArgType, MemberId},
};
use anyhow::{bail, Context};
use anyhow::{bail, Context, Result};
use aptos_api_types::ViewFunction;
use aptos_crypto::{
ed25519::{Ed25519PrivateKey, Ed25519PublicKey, Ed25519Signature},
Expand Down
51 changes: 46 additions & 5 deletions crates/aptos/src/move_tool/bytecode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -63,6 +63,9 @@ pub struct BytecodeCommand {
#[clap(long)]
pub is_script: bool,

#[clap(long)]
pub show_locs: bool,

#[clap(flatten)]
input: BytecodeCommandInput,

Expand Down Expand Up @@ -203,18 +206,51 @@ impl BytecodeCommand {

fn disassemble(&self, bytecode_path: &Path) -> Result<String, CliError> {
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::<PathBuf>();
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::<PathBuf>();
source_map_from_file(&source_map_path).ok()
}
};

let disassembler_options = DisassemblerOptions {
print_code: true,
only_externally_visible: false,
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;
Expand All @@ -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")?
};
Expand Down
14 changes: 12 additions & 2 deletions crates/aptos/src/move_tool/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -184,7 +194,7 @@ fn compile_coverage(

let path = move_options.get_package_path()?;
let coverage_map =
CoverageMap::from_binary_file(path.join(".coverage_map.mvcov")).map_err(|err| {
CoverageMap::from_binary_file(&path.join(".coverage_map.mvcov")).map_err(|err| {
CliError::UnexpectedError(format!("Failed to retrieve coverage map {}", err))
})?;
let package = config
Expand Down
10 changes: 5 additions & 5 deletions third_party/move/move-compiler-v2/src/env_pipeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
//! This module contains a set of transformers and analyzers of global environment.
//! Those can be arranged in a pipeline and executed in sequence.

use log::trace;
use log::debug;
use move_model::model::GlobalEnv;
use std::io::Write;

Expand Down Expand Up @@ -43,10 +43,10 @@ impl<'a> EnvProcessorPipeline<'a> {
/// Runs the pipeline. Running will be ended if any of the steps produces an error.
/// The function returns true if all steps succeeded without errors.
pub fn run(&self, env: &mut GlobalEnv) -> bool {
trace!("before env processor pipeline: {}", env.dump_env());
debug!("before env processor pipeline: {}", env.dump_env());
for (name, proc) in &self.processors {
proc(env);
trace!("after env processor {}", name);
debug!("after env processor {}: {}", name, env.dump_env());
if env.has_errors() {
return false;
}
Expand All @@ -58,13 +58,13 @@ impl<'a> EnvProcessorPipeline<'a> {
/// only.
pub fn run_and_record(&self, env: &mut GlobalEnv, w: &mut impl Write) -> anyhow::Result<bool> {
let msg = format!("before env processor pipeline:\n{}\n", env.dump_env());
trace!("{}", msg);
debug!("{}", msg);
writeln!(w, "// -- Model dump {}", msg)?;
for (name, proc) in &self.processors {
proc(env);
if !env.has_errors() {
let msg = format!("after env processor {}:\n{}\n", name, env.dump_env());
trace!("{}", msg);
debug!("{}", msg);
writeln!(w, "// -- Model dump {}", msg)?;
} else {
return Ok(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::{
livevar_analysis_processor::LiveVarAnnotation,
},
};
use log::debug;
use move_binary_format::{
file_format as FF,
file_format::{CodeOffset, FunctionDefinitionIndex},
Expand Down Expand Up @@ -108,12 +109,13 @@ impl<'a> FunctionGenerator<'a> {
fun_env: FunctionEnv<'b>,
acquires_list: &BTreeSet<StructId>,
) {
let id_loc = fun_env.get_id_loc();
let loc = fun_env.get_id_loc();
let function = gen.function_index(ctx, &loc, &fun_env);
let visibility = fun_env.visibility();
let fun_count = gen.module.function_defs.len();
let def_idx = FunctionDefinitionIndex::new(ctx.checked_bound(
&loc,
&id_loc,
fun_count,
MAX_FUNCTION_DEF_COUNT,
"defined function",
Expand Down Expand Up @@ -208,6 +210,13 @@ impl<'a> FunctionGenerator<'a> {
for i in 0..bytecode.len() {
let code_offset = i as FF::CodeOffset;
let bc = &bytecode[i];
debug!(
"Generating code for bytecode {:?} ({}) at offset {} with attr_id {:?}",
bc,
bc.display(&ctx.fun, &BTreeMap::new()),
i,
bc.get_attr_id(),
);
let bytecode_ctx = BytecodeContext {
fun_ctx: ctx,
code_offset,
Expand Down Expand Up @@ -244,6 +253,14 @@ impl<'a> FunctionGenerator<'a> {
}
}

let func_name = ctx.fun.func_env.get_full_name_str();
if func_name == "red_black_map::add" {
debug!(
"After Function {} source_map is {:#?}",
func_name, self.gen.source_map
);
}

// Deliver result
let locals = self.gen.signature(
&ctx.module,
Expand All @@ -259,16 +276,17 @@ impl<'a> FunctionGenerator<'a> {
/// Generate file-format bytecode from a stackless bytecode and an optional next bytecode
/// for peephole optimizations.
fn gen_bytecode(&mut self, ctx: &BytecodeContext, bc: &Bytecode, next_bc: Option<&Bytecode>) {
let loc = ctx.fun_ctx.fun.get_bytecode_loc(ctx.attr_id);
let ir_loc = ctx.fun_ctx.module.env.to_ir_loc(&loc);
let offset = self.code.len();
let func_name = ctx.fun_ctx.fun.func_env.get_full_name_str();
debug!(
"Setting location for {}:{} with attr_id {:?} to {:?} (from {:?})",
func_name, offset, ctx.attr_id, ir_loc, loc
);
self.gen
.source_map
.add_code_mapping(
ctx.fun_ctx.def_idx,
self.code.len() as FF::CodeOffset,
ctx.fun_ctx
.module
.env
.to_ir_loc(&ctx.fun_ctx.fun.get_bytecode_loc(ctx.attr_id)),
)
.add_code_mapping(ctx.fun_ctx.def_idx, offset as FF::CodeOffset, ir_loc)
.expect(SOURCE_MAP_OK);
match bc {
Bytecode::Assign(_, dest, source, mode) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::{
};
use codespan_reporting::diagnostic::Severity;
use itertools::Itertools;
use log::debug;
use move_binary_format::{
file_format as FF,
file_format::{AccessKind, FunctionHandle, ModuleHandle, StructDefinitionIndex, TableIndex},
Expand Down Expand Up @@ -211,6 +212,14 @@ impl ModuleGenerator {
FunctionGenerator::run(self, ctx, fun_env, acquires_list);
}

let module_name = module_env.get_full_name_str();
if module_name == "0xabc::red_black_map" {
debug!(
"After generating module {}, source_map is {:#?}",
module_name, self.source_map
);
}

// At handles of friend modules
for mid in module_env.get_friend_modules() {
let handle = self.module_handle(ctx, &module_env.get_loc(), &ctx.env.get_module(mid));
Expand Down
11 changes: 2 additions & 9 deletions third_party/move/move-compiler-v2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ where
&env,
&mut targets,
&dump_base_name,
false,
command_line::get_move_compiler_dump_from_env(),
&pipeline::register_formatters,
|| !env.has_errors(),
)
Expand Down Expand Up @@ -504,14 +504,7 @@ pub fn bytecode_pipeline(env: &GlobalEnv) -> FunctionTargetPipeline {
pub fn disassemble_compiled_units(units: &[CompiledUnit]) -> anyhow::Result<String> {
let disassembled_units: anyhow::Result<Vec<_>> = 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())
}
Expand Down
5 changes: 5 additions & 0 deletions third_party/move/move-compiler/src/command_line/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,8 @@ pub const MVC_BACKTRACE_ENV_VAR: &str = "MVC_BACKTRACE";
pub fn get_move_compiler_backtrace_from_env() -> bool {
read_bool_env_var(MOVE_COMPILER_BACKTRACE_ENV_VAR) || read_bool_env_var(MVC_BACKTRACE_ENV_VAR)
}

// Flag to dump bytecode files
pub fn get_move_compiler_dump_from_env() -> bool {
read_bool_env_var(MOVE_COMPILER_DUMP_ENV_VAR) || read_bool_env_var(MVC_DUMP_ENV_VAR)
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// SPDX-License-Identifier: Apache-2.0

use crate::source_map::SourceMap;
use anyhow::{format_err, Result};
use anyhow::{Context, Result};
use move_ir_types::location::Loc;
use std::{fs::File, io::Read, path::Path};

Expand All @@ -13,9 +13,17 @@ pub type Errors = Vec<Error>;
pub fn source_map_from_file(file_path: &Path) -> Result<SourceMap> {
let mut bytes = Vec::new();
File::open(file_path)
.ok()
.and_then(|mut file| file.read_to_end(&mut bytes).ok())
.ok_or_else(|| format_err!("Error while reading in source map information"))?;
bcs::from_bytes::<SourceMap>(&bytes)
.map_err(|_| format_err!("Error deserializing into source map"))
.and_then(|mut file| file.read_to_end(&mut bytes))
.with_context(|| {
format!(
"Reading in source map information for file {}",
file_path.to_string_lossy(),
)
})?;
bcs::from_bytes::<SourceMap>(&bytes).with_context(|| {
format!(
"Deserializing source map information for file {}",
file_path.to_string_lossy()
)
})
}
Loading
Loading