diff --git a/Cargo.lock b/Cargo.lock index 0628799c64..e086d558b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8375,6 +8375,7 @@ dependencies = [ name = "move-mutator" version = "0.1.0" dependencies = [ + "ahash 0.8.11", "anyhow", "clap 4.5.20", "codespan", diff --git a/Cargo.toml b/Cargo.toml index c51fa9ce27..a91b498d95 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,7 @@ repository = "https://github.com/eigerco/move-spec-testing" rust-version = "1.78.0" [workspace.dependencies] +ahash = "0.8" anyhow = "1.0" aptos = { git = "https://github.com/aptos-labs/aptos-core.git", branch = "main" } aptos-framework = { git = "https://github.com/aptos-labs/aptos-core.git", branch = "main" } diff --git a/move-mutation-test/src/mutation_test.rs b/move-mutation-test/src/mutation_test.rs index 7c419febd7..f7a0a0d591 100644 --- a/move-mutation-test/src/mutation_test.rs +++ b/move-mutation-test/src/mutation_test.rs @@ -160,19 +160,28 @@ fn run_tests( aptos_test_feature_flags_genesis(), gas_limit, cost_table, - cfg.apply_coverage, + // We cannot use `true` here since that would set a static variable TRACING_ENABLED deep + // within MoveVM to true, and that could cause a huge slowdown, even test failures in the + // later "mutation-test" phase. + // Until we can somehow reconfigure: + // https://github.com/aptos-labs/aptos-core/blob/2bb2d43037a93d883729869d65c7c6c75b028fa1/third_party/move/move-vm/runtime/src/tracing.rs#L40 + // we are forced to avoid computing coverage before running the tests. + // How it works: compute_coverage sets `MOVE_VM_TRACE` env variable that configures this + // once_cell value above and then we can't change it back anymore. + false, &mut error_writer, ) .map_err(|err| Error::msg(format!("failed to run unit tests: {err:#}")))?; - // Disk space optimization: if cfg.apply_coverage { + // Disk space optimization: let trace_path = package_path.join(".trace"); - info!("removing {}", trace_path.display()); // Our tool doesn't use the .trace file at all, only the .coverage_map.mvcov file, and // since the tool copy package directory to temp directories for when running tests, // so let's keep copied directory as small as possible. - let _ = fs::remove_file(trace_path); + if fs::remove_file(&trace_path).is_ok() { + info!("removing {}", trace_path.display()); + } } match result { diff --git a/move-mutation-test/tests/integration_tests.rs b/move-mutation-test/tests/integration_tests.rs index ef22a417e4..42b2b75102 100644 --- a/move-mutation-test/tests/integration_tests.rs +++ b/move-mutation-test/tests/integration_tests.rs @@ -33,8 +33,10 @@ fn test_run_mutation_test(path: &Path, expected_report: String) -> datatest_stab dump_state: false, filter: None, ignore_compile_warnings: false, - apply_coverage: true, - gas_limit: 1_000, + // TODO(rqnsom): maybe we could do set it to true, but it would require `aptos` command in + // the `build.rs` - using `process::Command` slowed down the execution a lot + apply_coverage: false, + gas_limit: 2000, }; let report_file = PathBuf::from("report.txt"); diff --git a/move-mutator/Cargo.toml b/move-mutator/Cargo.toml index a6dc6d2217..7a23bad03b 100644 --- a/move-mutator/Cargo.toml +++ b/move-mutator/Cargo.toml @@ -15,6 +15,7 @@ name = "move-mutator" path = "src/main.rs" [dependencies] +ahash = { workspace = true } anyhow = { workspace = true } clap = { workspace = true } codespan = { workspace = true } diff --git a/move-mutator/src/coverage.rs b/move-mutator/src/coverage.rs index 90ae439ebe..ae34c0cbc7 100644 --- a/move-mutator/src/coverage.rs +++ b/move-mutator/src/coverage.rs @@ -1,5 +1,5 @@ use crate::compiler::compile_package; -use anyhow::Error; +use anyhow::{bail, Error}; use codespan::Span; use move_command_line_common::files::FileHash; use move_compiler::compiled_unit::{CompiledUnit, NamedCompiledModule}; @@ -15,6 +15,8 @@ use std::{ path::{Path, PathBuf}, }; +const COVERAGE_MAP_NAME: &str = ".coverage_map.mvcov"; + /// Contains all uncovered spans in the project. #[derive(Debug, Default)] pub(crate) struct Coverage { @@ -32,7 +34,14 @@ impl Coverage { ) -> anyhow::Result<()> { info!("computing coverage"); - let coverage_map = CoverageMap::from_binary_file(package_path.join(".coverage_map.mvcov")) + let coverage_file = package_path.join(COVERAGE_MAP_NAME); + if !coverage_file.exists() { + bail!( + "Coverage map not found, please run `aptos move test --coverage` for the package" + ); + } + + let coverage_map = CoverageMap::from_binary_file(coverage_file) .map_err(|e| Error::msg(format!("failed to retrieve the coverage map: {e}")))?; let mut coverage_config = build_config.clone(); diff --git a/move-mutator/src/lib.rs b/move-mutator/src/lib.rs index b3b6170b2e..5c00dd40ca 100644 --- a/move-mutator/src/lib.rs +++ b/move-mutator/src/lib.rs @@ -185,7 +185,8 @@ pub fn run_move_mutator( } } - let Ok(mutant_path) = output::setup_mutant_path(&output_dir, &path) else { + let mutant_id = mutated_info.unique_id(); + let Ok(mutant_path) = output::setup_mutant_path(&output_dir, &path, mutant_id) else { // If we cannot set up the mutant path, we skip the mutant. trace!("Cannot set up mutant path for {path:?}"); return None; diff --git a/move-mutator/src/operator.rs b/move-mutator/src/operator.rs index 6693141fcf..718433d399 100644 --- a/move-mutator/src/operator.rs +++ b/move-mutator/src/operator.rs @@ -3,14 +3,16 @@ // SPDX-License-Identifier: Apache-2.0 use crate::report::Mutation; +use ahash::RandomState; use codespan::FileId; use std::{ fmt, fmt::{Debug, Display}, + hash::Hash, }; /// Mutation result that contains the mutated source code and the modification that was applied. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Hash)] pub struct MutantInfo { /// The mutated source code. pub mutated_source: String, @@ -26,6 +28,12 @@ impl MutantInfo { mutation, } } + + /// Calculates the unique hash of the mutant. + pub fn unique_id(&self) -> u64 { + let fixed_randomness = RandomState::with_seeds(1, 2, 3, 4); + fixed_randomness.hash_one(self) + } } /// Trait for mutation operators. diff --git a/move-mutator/src/output.rs b/move-mutator/src/output.rs index 2f795524d4..f7069c875e 100644 --- a/move-mutator/src/output.rs +++ b/move-mutator/src/output.rs @@ -17,13 +17,13 @@ use std::{ /// according to its relative path inside the package. If the file is not inside any package, /// it creates the directory structure in the output directory. /// Example: -/// The file to be mutated is located in "/a/b/c/sources/X/Y/file.move" (`file_path`). +/// The file to be mutated is located in "/a/b/c/sources/X/Y/file.move" (`original_file`). /// This function constructs the following output path for file.move: /// "`output_dir/X/Y/file_index.move`" /// It finds the package root for the file, which is "/a/b/c", then it append the relative path to the output directory. /// /// If the file is not inside any package, it creates the directory structure in the output directory like: -/// The file to be mutated is located in "/a/b/c/file.move" (`file_path`). +/// The file to be mutated is located in "/a/b/c/file.move" (`original_file`). /// This function constructs the following output path for file.move: /// "`output_dir/file_index.move`" /// @@ -39,10 +39,14 @@ use std::{ /// # Returns /// /// * `PathBuf` - The path to the mutant. -pub(crate) fn setup_mutant_path(output_dir: &Path, file_path: &Path) -> anyhow::Result { - trace!("Trying to set up mutant path for {file_path:?}"); +pub(crate) fn setup_mutant_path( + output_dir: &Path, + original_file: &Path, + unique_mutant_id: u64, +) -> anyhow::Result { + trace!("Trying to set up mutant path for {original_file:?}"); - let file_path_canonicalized = file_path.canonicalize()?; + let file_path_canonicalized = original_file.canonicalize()?; // Try to find package root for the file. If the file is not inside any package, assume that it is a single file. let root = SourcePackageLayout::try_find_root(&file_path_canonicalized); @@ -66,31 +70,21 @@ pub(crate) fn setup_mutant_path(output_dir: &Path, file_path: &Path) -> anyhow:: if let Err(e) = fs::create_dir_all(&output_struct) { if e.kind() != std::io::ErrorKind::AlreadyExists { return Err(anyhow::anyhow!( - "Cannot create directory structure for {file_path:?} in {output_dir:?}" + "Cannot create directory structure for {original_file:?} in {output_dir:?}" )); } } - let filename = file_path + let filename = original_file .file_stem() - .ok_or(anyhow::anyhow!("Cannot get file stem of {file_path:?}"))?; + .ok_or(anyhow::anyhow!("Cannot get file stem of {original_file:?}"))?; // Deal with the file as OsString to avoid problems with non-UTF8 characters. - let filename = filename.to_os_string(); - for i in 0u32..u32::MAX { - let mut mutant_path = filename.clone(); - mutant_path.push(OsString::from(format!("_mut{i}.move"))); - - let mutant_path = output_struct.join(mutant_path); - if !mutant_path.exists() { - return Ok(mutant_path); - } - } + let mut mutant_path = filename.to_os_string(); + mutant_path.push(OsString::from(format!("_mutant_{unique_mutant_id:x}.move"))); - Err(anyhow::anyhow!( - "There is more than {} mutants in {output_dir:?}", - u32::MAX - )) + let mutant_path = output_struct.join(mutant_path); + Ok(mutant_path) } /// Sets up the output directory for the mutants. @@ -140,14 +134,16 @@ mod tests { fn setup_mutant_path_handles_non_utf8_characters() { let output_dir = Path::new("mutants_output"); let filename = Path::new("💖"); + let mutant_id = 0; + fs::File::create(filename).unwrap(); - let result = setup_mutant_path(output_dir, filename); + let result = setup_mutant_path(output_dir, filename, mutant_id); fs::remove_file(filename).unwrap(); fs::remove_dir_all(output_dir).unwrap(); assert!(result.is_ok()); assert_eq!( result.unwrap(), - PathBuf::from("mutants_output/💖_mut0.move") + PathBuf::from(format!("mutants_output/💖_mutant_{mutant_id}.move")) ); } @@ -155,14 +151,18 @@ mod tests { fn setup_mutant_path_handles_file_without_extension() { let output_dir = Path::new("mutants_output_no_extension"); let filename = Path::new("file1"); + let mutant_id = 0; + fs::File::create(filename).unwrap(); - let result = setup_mutant_path(output_dir, filename); + let result = setup_mutant_path(output_dir, filename, mutant_id); fs::remove_file(filename).unwrap(); fs::remove_dir_all(output_dir).unwrap(); assert!(result.is_ok()); assert_eq!( result.unwrap(), - PathBuf::from("mutants_output_no_extension/file1_mut0.move") + PathBuf::from(format!( + "mutants_output_no_extension/file1_mutant_{mutant_id}.move" + )) ); } @@ -170,14 +170,18 @@ mod tests { fn setup_mutant_path_creates_correct_path() { let output_dir = Path::new("mutants_output_correct"); let filename = Path::new("test_correct"); + let mutant_id = 0; + fs::File::create(filename).unwrap(); - let result = setup_mutant_path(output_dir, filename); + let result = setup_mutant_path(output_dir, filename, mutant_id); fs::remove_file(filename).unwrap(); fs::remove_dir_all(output_dir).unwrap(); assert!(result.is_ok()); assert_eq!( result.unwrap(), - PathBuf::from("mutants_output_correct/test_correct_mut0.move") + PathBuf::from(format!( + "mutants_output_correct/test_correct_mutant_{mutant_id}.move" + )) ); } @@ -185,18 +189,25 @@ mod tests { fn setup_mutant_path_handles_empty_output_dir() { let output_dir = Path::new(""); let filename = Path::new("test_empty"); + let mutant_id = 0; + fs::File::create(filename).unwrap(); - let result = setup_mutant_path(output_dir, filename); + let result = setup_mutant_path(output_dir, filename, mutant_id); fs::remove_file(filename).unwrap(); assert!(result.is_ok()); - assert_eq!(result.unwrap(), PathBuf::from("test_empty_mut0.move")); + assert_eq!( + result.unwrap(), + PathBuf::from(format!("test_empty_mutant_{mutant_id}.move")) + ); } #[test] fn setup_mutant_path_handles_empty_filename() { let output_dir = Path::new("mutants_output_empty_filename"); let filename = Path::new(""); - let result = setup_mutant_path(output_dir, filename); + let mutant_id = 0; + + let result = setup_mutant_path(output_dir, filename, mutant_id); assert!(result.is_err()); } diff --git a/move-mutator/src/report.rs b/move-mutator/src/report.rs index 3fc2148c5e..354d79f0bd 100644 --- a/move-mutator/src/report.rs +++ b/move-mutator/src/report.rs @@ -115,7 +115,7 @@ impl Default for Report { /// The `Range` struct represents a range with a start and end. /// It is used to represent the location of a mutation inside the source file. -#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Hash)] pub struct Range { /// The start of the range. start: usize, @@ -139,7 +139,7 @@ impl Range { /// /// It contains the location of the modification, the name of the mutation operator, the old value and the new value. /// It is used to represent a single modification inside a `ReportEntry`. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Hash)] pub struct Mutation { /// The location of the modification. changed_place: Range, diff --git a/move-mutator/tests/move-assets/simple/report.txt.mutation-exp b/move-mutator/tests/move-assets/simple/report.txt.mutation-exp index eb7c21d86f..73607f13a4 100644 --- a/move-mutator/tests/move-assets/simple/report.txt.mutation-exp +++ b/move-mutator/tests/move-assets/simple/report.txt.mutation-exp @@ -99,12 +99,16 @@ }, { "module_func": "BinaryReplacement::is_zero_silly_code", - "tested": 11, - "killed": 8, + "tested": 21, + "killed": 14, "mutants_alive_diffs": [ "--- original\n+++ modified\n@@ -28,7 +28,7 @@\n // and that would be a clear indication of silly code that has\n // one identical reduntant check.\n fun is_zero_silly_code(x: u64): bool {\n- if (0 == x)\n+ if (false)\n return true;\n\n // Another check which does the same is silly:\n", "--- original\n+++ modified\n@@ -28,7 +28,7 @@\n // and that would be a clear indication of silly code that has\n // one identical reduntant check.\n fun is_zero_silly_code(x: u64): bool {\n- if (0 == x)\n+ if (0 > x)\n return true;\n\n // Another check which does the same is silly:\n", - "--- original\n+++ modified\n@@ -28,7 +28,7 @@\n // and that would be a clear indication of silly code that has\n // one identical reduntant check.\n fun is_zero_silly_code(x: u64): bool {\n- if (0 == x)\n+ if (18446744073709551615 == x)\n return true;\n\n // Another check which does the same is silly:\n" + "--- original\n+++ modified\n@@ -28,7 +28,7 @@\n // and that would be a clear indication of silly code that has\n // one identical reduntant check.\n fun is_zero_silly_code(x: u64): bool {\n- if (0 == x)\n+ if (18446744073709551615 == x)\n return true;\n\n // Another check which does the same is silly:\n", + "--- original\n+++ modified\n@@ -32,7 +32,7 @@\n return true;\n\n // Another check which does the same is silly:\n- if (x == 0)\n+ if (false)\n return true;\n\n false\n", + "--- original\n+++ modified\n@@ -32,7 +32,7 @@\n return true;\n\n // Another check which does the same is silly:\n- if (x == 0)\n+ if (x < 0)\n return true;\n\n false\n", + "--- original\n+++ modified\n@@ -32,7 +32,7 @@\n return true;\n\n // Another check which does the same is silly:\n- if (x == 0)\n+ if (x == 18446744073709551615)\n return true;\n\n false\n", + "--- original\n+++ modified\n@@ -33,7 +33,7 @@\n\n // Another check which does the same is silly:\n if (x == 0)\n- return true;\n+ return false;\n\n false\n }\n" ], "mutants_killed_diff": [ "--- original\n+++ modified\n@@ -28,7 +28,7 @@\n // and that would be a clear indication of silly code that has\n // one identical reduntant check.\n fun is_zero_silly_code(x: u64): bool {\n- if (0 == x)\n+ if (true)\n return true;\n\n // Another check which does the same is silly:\n", @@ -114,6 +118,12 @@ "--- original\n+++ modified\n@@ -28,7 +28,7 @@\n // and that would be a clear indication of silly code that has\n // one identical reduntant check.\n fun is_zero_silly_code(x: u64): bool {\n- if (0 == x)\n+ if (0 <= x)\n return true;\n\n // Another check which does the same is silly:\n", "--- original\n+++ modified\n@@ -28,7 +28,7 @@\n // and that would be a clear indication of silly code that has\n // one identical reduntant check.\n fun is_zero_silly_code(x: u64): bool {\n- if (0 == x)\n+ if (1 == x)\n return true;\n\n // Another check which does the same is silly:\n", "--- original\n+++ modified\n@@ -29,7 +29,7 @@\n // one identical reduntant check.\n fun is_zero_silly_code(x: u64): bool {\n if (0 == x)\n- return true;\n+ return false;\n\n // Another check which does the same is silly:\n if (x == 0)\n", + "--- original\n+++ modified\n@@ -32,7 +32,7 @@\n return true;\n\n // Another check which does the same is silly:\n- if (x == 0)\n+ if (true)\n return true;\n\n false\n", + "--- original\n+++ modified\n@@ -32,7 +32,7 @@\n return true;\n\n // Another check which does the same is silly:\n- if (x == 0)\n+ if (!(x == 0))\n return true;\n\n false\n", + "--- original\n+++ modified\n@@ -32,7 +32,7 @@\n return true;\n\n // Another check which does the same is silly:\n- if (x == 0)\n+ if (x != 0)\n return true;\n\n false\n", + "--- original\n+++ modified\n@@ -32,7 +32,7 @@\n return true;\n\n // Another check which does the same is silly:\n- if (x == 0)\n+ if (x > 0)\n return true;\n\n false\n", + "--- original\n+++ modified\n@@ -32,7 +32,7 @@\n return true;\n\n // Another check which does the same is silly:\n- if (x == 0)\n+ if (x >= 0)\n return true;\n\n false\n", + "--- original\n+++ modified\n@@ -32,7 +32,7 @@\n return true;\n\n // Another check which does the same is silly:\n- if (x == 0)\n+ if (x == 1)\n return true;\n\n false\n", "--- original\n+++ modified\n@@ -35,7 +35,7 @@\n if (x == 0)\n return true;\n\n- false\n+ true\n }\n\n #[test]\n" ] } @@ -327,12 +337,17 @@ "sources/StillSimple.move": [ { "module_func": "StillSimple::sample1", - "tested": 18, + "tested": 23, "killed": 15, "mutants_alive_diffs": [ "--- original\n+++ modified\n@@ -1,6 +1,6 @@\n module TestAccount::StillSimple {\n fun sample1(x: u128, y: u128) {\n- let _sum_r = x + y;\n+ let _sum_r = x * y;\n\n // Impossible condition here:\n if ((x + y) < 0) abort 1;\n", "--- original\n+++ modified\n@@ -3,7 +3,7 @@\n let _sum_r = x + y;\n\n // Impossible condition here:\n- if ((x + y) < 0) abort 1;\n+ if (false) abort 1;\n }\n\n // This test will generate mutants that will survive and indicate the impossible condition in the code.\n", - "--- original\n+++ modified\n@@ -3,7 +3,7 @@\n let _sum_r = x + y;\n\n // Impossible condition here:\n- if ((x + y) < 0) abort 1;\n+ if ((x * y) < 0) abort 1;\n }\n\n // This test will generate mutants that will survive and indicate the impossible condition in the code.\n" + "--- original\n+++ modified\n@@ -3,7 +3,7 @@\n let _sum_r = x + y;\n\n // Impossible condition here:\n- if ((x + y) < 0) abort 1;\n+ if ((x * y) < 0) abort 1;\n }\n\n // This test will generate mutants that will survive and indicate the impossible condition in the code.\n", + "--- original\n+++ modified\n@@ -3,7 +3,7 @@\n let _sum_r = x + y;\n\n // Impossible condition here:\n- if ((x + y) < 0) abort 1;\n+ if ((x + y) < 0) {};\n }\n\n // This test will generate mutants that will survive and indicate the impossible condition in the code.\n", + "--- original\n+++ modified\n@@ -3,7 +3,7 @@\n let _sum_r = x + y;\n\n // Impossible condition here:\n- if ((x + y) < 0) abort 1;\n+ if ((x + y) < 0) abort 0;\n }\n\n // This test will generate mutants that will survive and indicate the impossible condition in the code.\n", + "--- original\n+++ modified\n@@ -3,7 +3,7 @@\n let _sum_r = x + y;\n\n // Impossible condition here:\n- if ((x + y) < 0) abort 1;\n+ if ((x + y) < 0) abort 18446744073709551615;\n }\n\n // This test will generate mutants that will survive and indicate the impossible condition in the code.\n", + "--- original\n+++ modified\n@@ -3,7 +3,7 @@\n let _sum_r = x + y;\n\n // Impossible condition here:\n- if ((x + y) < 0) abort 1;\n+ if ((x + y) < 0) abort 2;\n }\n\n // This test will generate mutants that will survive and indicate the impossible condition in the code.\n", + "--- original\n+++ modified\n@@ -3,7 +3,7 @@\n let _sum_r = x + y;\n\n // Impossible condition here:\n- if ((x + y) < 0) abort 1;\n+ if ((x + y) < 0) abort 0;\n }\n\n // This test will generate mutants that will survive and indicate the impossible condition in the code.\n" ], "mutants_killed_diff": [ "--- original\n+++ modified\n@@ -1,6 +1,6 @@\n module TestAccount::StillSimple {\n fun sample1(x: u128, y: u128) {\n- let _sum_r = x + y;\n+ let _sum_r = x - y;\n\n // Impossible condition here:\n if ((x + y) < 0) abort 1;\n",