From 1a050d4436b53e5a423a60281ea8cd05494d9c86 Mon Sep 17 00:00:00 2001 From: Todd Nowacki Date: Mon, 22 Apr 2024 10:57:45 -0700 Subject: [PATCH] [move] Remove proptests from execution version cut (#17240) ## Description - Removed proptests from execution version cut - As a result, we no longer need invalid-mutations in the cut ## Test plan - CI --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- Cargo.toml | 1 + external-crates/move/Cargo.lock | 53 +- external-crates/move/Cargo.toml | 4 - .../bytecode-verifier-prop-tests/Cargo.toml | 28 + .../bytecode-verifier-prop-tests/src/lib.rs | 5 + .../src/unit_tests/mod.rs | 4 + .../src/unit_tests/prop_tests.rs | 161 ++++++ .../crates/bytecode-verifier-tests/Cargo.toml | 8 +- .../ability_field_requirements_tests.rs | 14 - .../src/unit_tests/bounds_tests.rs | 95 +--- .../src/unit_tests/constants_tests.rs | 10 +- .../src/unit_tests/duplication_tests.rs | 8 - .../src/unit_tests/mod.rs | 3 - .../src/unit_tests/multi_pass_tests.rs | 23 - .../src/unit_tests/signature_tests.rs | 35 -- .../src/unit_tests/struct_defs_tests.rs | 14 - .../crates/move-bytecode-verifier/Cargo.toml | 2 - .../crates/bytecode-verifier-tests/Cargo.toml | 5 +- .../ability_field_requirements_tests.rs | 14 - .../src/unit_tests/bounds_tests.rs | 95 +--- .../src/unit_tests/constants_tests.rs | 16 +- .../src/unit_tests/duplication_tests.rs | 8 - .../src/unit_tests/mod.rs | 3 - .../src/unit_tests/multi_pass_tests.rs | 23 - .../src/unit_tests/signature_tests.rs | 35 -- .../src/unit_tests/struct_defs_tests.rs | 14 - .../v0/crates/invalid-mutations/Cargo.toml | 18 - .../v0/crates/invalid-mutations/src/bounds.rs | 364 ------------- .../invalid-mutations/src/bounds/code_unit.rs | 476 ----------------- .../crates/invalid-mutations/src/helpers.rs | 46 -- .../v0/crates/invalid-mutations/src/lib.rs | 9 - .../crates/invalid-mutations/src/signature.rs | 100 ---- .../crates/move-bytecode-verifier/Cargo.toml | 1 - .../crates/bytecode-verifier-tests/Cargo.toml | 5 +- .../ability_field_requirements_tests.rs | 14 - .../src/unit_tests/bounds_tests.rs | 95 +--- .../src/unit_tests/constants_tests.rs | 10 +- .../src/unit_tests/duplication_tests.rs | 8 - .../src/unit_tests/mod.rs | 3 - .../src/unit_tests/multi_pass_tests.rs | 23 - .../src/unit_tests/signature_tests.rs | 35 -- .../src/unit_tests/struct_defs_tests.rs | 14 - .../v1/crates/invalid-mutations/Cargo.toml | 18 - .../v1/crates/invalid-mutations/src/bounds.rs | 364 ------------- .../invalid-mutations/src/bounds/code_unit.rs | 479 ------------------ .../crates/invalid-mutations/src/helpers.rs | 46 -- .../v1/crates/invalid-mutations/src/lib.rs | 9 - .../crates/invalid-mutations/src/signature.rs | 100 ---- .../crates/move-bytecode-verifier/Cargo.toml | 1 - .../crates/bytecode-verifier-tests/Cargo.toml | 5 +- .../ability_field_requirements_tests.rs | 14 - .../src/unit_tests/bounds_tests.rs | 95 +--- .../src/unit_tests/constants_tests.rs | 10 +- .../src/unit_tests/duplication_tests.rs | 8 - .../src/unit_tests/mod.rs | 3 - .../src/unit_tests/multi_pass_tests.rs | 23 - .../src/unit_tests/signature_tests.rs | 35 -- .../src/unit_tests/struct_defs_tests.rs | 14 - .../v2/crates/invalid-mutations/Cargo.toml | 18 - .../v2/crates/invalid-mutations/src/bounds.rs | 364 ------------- .../invalid-mutations/src/bounds/code_unit.rs | 479 ------------------ .../crates/invalid-mutations/src/helpers.rs | 46 -- .../v2/crates/invalid-mutations/src/lib.rs | 9 - .../crates/invalid-mutations/src/signature.rs | 100 ---- .../crates/move-bytecode-verifier/Cargo.toml | 1 - scripts/execution_layer.py | 3 - 66 files changed, 236 insertions(+), 3910 deletions(-) create mode 100644 external-crates/move/crates/bytecode-verifier-prop-tests/Cargo.toml create mode 100644 external-crates/move/crates/bytecode-verifier-prop-tests/src/lib.rs create mode 100644 external-crates/move/crates/bytecode-verifier-prop-tests/src/unit_tests/mod.rs create mode 100644 external-crates/move/crates/bytecode-verifier-prop-tests/src/unit_tests/prop_tests.rs delete mode 100644 external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs delete mode 100644 external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs delete mode 100644 external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs delete mode 100644 external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs delete mode 100644 external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs delete mode 100644 external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs delete mode 100644 external-crates/move/move-execution/v0/crates/invalid-mutations/Cargo.toml delete mode 100644 external-crates/move/move-execution/v0/crates/invalid-mutations/src/bounds.rs delete mode 100644 external-crates/move/move-execution/v0/crates/invalid-mutations/src/bounds/code_unit.rs delete mode 100644 external-crates/move/move-execution/v0/crates/invalid-mutations/src/helpers.rs delete mode 100644 external-crates/move/move-execution/v0/crates/invalid-mutations/src/lib.rs delete mode 100644 external-crates/move/move-execution/v0/crates/invalid-mutations/src/signature.rs delete mode 100644 external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs delete mode 100644 external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs delete mode 100644 external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs delete mode 100644 external-crates/move/move-execution/v1/crates/invalid-mutations/Cargo.toml delete mode 100644 external-crates/move/move-execution/v1/crates/invalid-mutations/src/bounds.rs delete mode 100644 external-crates/move/move-execution/v1/crates/invalid-mutations/src/bounds/code_unit.rs delete mode 100644 external-crates/move/move-execution/v1/crates/invalid-mutations/src/helpers.rs delete mode 100644 external-crates/move/move-execution/v1/crates/invalid-mutations/src/lib.rs delete mode 100644 external-crates/move/move-execution/v1/crates/invalid-mutations/src/signature.rs delete mode 100644 external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs delete mode 100644 external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs delete mode 100644 external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs delete mode 100644 external-crates/move/move-execution/v2/crates/invalid-mutations/Cargo.toml delete mode 100644 external-crates/move/move-execution/v2/crates/invalid-mutations/src/bounds.rs delete mode 100644 external-crates/move/move-execution/v2/crates/invalid-mutations/src/bounds/code_unit.rs delete mode 100644 external-crates/move/move-execution/v2/crates/invalid-mutations/src/helpers.rs delete mode 100644 external-crates/move/move-execution/v2/crates/invalid-mutations/src/lib.rs delete mode 100644 external-crates/move/move-execution/v2/crates/invalid-mutations/src/signature.rs diff --git a/Cargo.toml b/Cargo.toml index 6256c8f97f886..9bda85bed6790 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,6 +6,7 @@ exclude = [ "external-crates/move/crates/bytecode-interpreter-testsuite", "external-crates/move/crates/bytecode-verifier-libfuzzer", "external-crates/move/crates/bytecode-verifier-tests", + "external-crates/move/crates/bytecode-verifier-prop-tests", "external-crates/move/crates/bytecode-verifier-transactional-tests", "external-crates/move/crates/enum-compat-util", "external-crates/move/crates/invalid-mutations", diff --git a/external-crates/move/Cargo.lock b/external-crates/move/Cargo.lock index 38dc9f6e39e1f..7ab3ffcc7ff03 100644 --- a/external-crates/move/Cargo.lock +++ b/external-crates/move/Cargo.lock @@ -314,7 +314,7 @@ dependencies = [ ] [[package]] -name = "bytecode-verifier-tests" +name = "bytecode-verifier-prop-tests" version = "0.1.0" dependencies = [ "fail", @@ -329,20 +329,32 @@ dependencies = [ "proptest", ] +[[package]] +name = "bytecode-verifier-tests" +version = "0.1.0" +dependencies = [ + "fail", + "hex", + "move-binary-format", + "move-bytecode-verifier", + "move-bytecode-verifier-meter", + "move-core-types", + "move-vm-config", + "petgraph", +] + [[package]] name = "bytecode-verifier-tests-v0" version = "0.1.0" dependencies = [ "fail", "hex", - "invalid-mutations-v0", "move-binary-format", "move-bytecode-verifier-meter", "move-bytecode-verifier-v0", "move-core-types", "move-vm-config", "petgraph", - "proptest", ] [[package]] @@ -351,14 +363,12 @@ version = "0.1.0" dependencies = [ "fail", "hex", - "invalid-mutations-v1", "move-binary-format", "move-bytecode-verifier-meter", "move-bytecode-verifier-v1", "move-core-types", "move-vm-config", "petgraph", - "proptest", ] [[package]] @@ -367,14 +377,12 @@ version = "0.1.0" dependencies = [ "fail", "hex", - "invalid-mutations-v2", "move-binary-format", "move-bytecode-verifier-meter", "move-bytecode-verifier-v2", "move-core-types", "move-vm-config", "petgraph", - "proptest", ] [[package]] @@ -1303,33 +1311,6 @@ dependencies = [ "proptest", ] -[[package]] -name = "invalid-mutations-v0" -version = "0.1.0" -dependencies = [ - "move-binary-format", - "move-core-types", - "proptest", -] - -[[package]] -name = "invalid-mutations-v1" -version = "0.1.0" -dependencies = [ - "move-binary-format", - "move-core-types", - "proptest", -] - -[[package]] -name = "invalid-mutations-v2" -version = "0.1.0" -dependencies = [ - "move-binary-format", - "move-core-types", - "proptest", -] - [[package]] name = "io-lifetimes" version = "1.0.10" @@ -1652,7 +1633,6 @@ name = "move-bytecode-verifier" version = "0.1.0" dependencies = [ "hex-literal", - "invalid-mutations", "move-abstract-stack", "move-binary-format", "move-borrow-graph", @@ -1676,7 +1656,6 @@ name = "move-bytecode-verifier-v0" version = "0.1.0" dependencies = [ "hex-literal", - "invalid-mutations-v0", "move-abstract-stack", "move-binary-format", "move-borrow-graph", @@ -1691,7 +1670,6 @@ name = "move-bytecode-verifier-v1" version = "0.1.0" dependencies = [ "hex-literal", - "invalid-mutations-v1", "move-abstract-stack", "move-binary-format", "move-borrow-graph", @@ -1706,7 +1684,6 @@ name = "move-bytecode-verifier-v2" version = "0.1.0" dependencies = [ "hex-literal", - "invalid-mutations-v2", "move-abstract-stack", "move-binary-format", "move-borrow-graph", diff --git a/external-crates/move/Cargo.toml b/external-crates/move/Cargo.toml index db044e964b55d..83712d3b614b0 100644 --- a/external-crates/move/Cargo.toml +++ b/external-crates/move/Cargo.toml @@ -7,10 +7,6 @@ members = [ "move-execution/v1/crates/bytecode-verifier-tests", "move-execution/v2/crates/bytecode-verifier-tests", # "move-execution/$CUT/crates/bytecode-verifier-tests", - "move-execution/v0/crates/invalid-mutations", - "move-execution/v1/crates/invalid-mutations", - "move-execution/v2/crates/invalid-mutations", - # "move-execution/$CUT/crates/invalid-mutations", "move-execution/v0/crates/move-bytecode-verifier", "move-execution/v1/crates/move-bytecode-verifier", "move-execution/v2/crates/move-bytecode-verifier", diff --git a/external-crates/move/crates/bytecode-verifier-prop-tests/Cargo.toml b/external-crates/move/crates/bytecode-verifier-prop-tests/Cargo.toml new file mode 100644 index 0000000000000..b312f21142984 --- /dev/null +++ b/external-crates/move/crates/bytecode-verifier-prop-tests/Cargo.toml @@ -0,0 +1,28 @@ +[package] +name = "bytecode-verifier-prop-tests" +version = "0.1.0" +authors = ["Diem Association "] +description = "Diem bytecode verifier tests" +repository = "https://github.com/diem/diem" +homepage = "https://diem.com" +license = "Apache-2.0" +publish = false +edition = "2021" + +[dev-dependencies] +petgraph.workspace = true +proptest.workspace = true +fail = { workspace = true, features = ["failpoints"] } +hex.workspace = true + +invalid-mutations.workspace = true +move-binary-format = { workspace = true, features = ["fuzzing"] } +move-bytecode-verifier.workspace = true +move-bytecode-verifier-meter.workspace = true +move-core-types.workspace = true +move-vm-config.workspace = true + +[features] +fuzzing = ["move-binary-format/fuzzing"] + +[dependencies] diff --git a/external-crates/move/crates/bytecode-verifier-prop-tests/src/lib.rs b/external-crates/move/crates/bytecode-verifier-prop-tests/src/lib.rs new file mode 100644 index 0000000000000..9126fbe3f9e77 --- /dev/null +++ b/external-crates/move/crates/bytecode-verifier-prop-tests/src/lib.rs @@ -0,0 +1,5 @@ +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + +#[cfg(test)] +pub mod unit_tests; diff --git a/external-crates/move/crates/bytecode-verifier-prop-tests/src/unit_tests/mod.rs b/external-crates/move/crates/bytecode-verifier-prop-tests/src/unit_tests/mod.rs new file mode 100644 index 0000000000000..46691d07e4993 --- /dev/null +++ b/external-crates/move/crates/bytecode-verifier-prop-tests/src/unit_tests/mod.rs @@ -0,0 +1,4 @@ +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + +pub mod prop_tests; diff --git a/external-crates/move/crates/bytecode-verifier-prop-tests/src/unit_tests/prop_tests.rs b/external-crates/move/crates/bytecode-verifier-prop-tests/src/unit_tests/prop_tests.rs new file mode 100644 index 0000000000000..790c2b5654b5f --- /dev/null +++ b/external-crates/move/crates/bytecode-verifier-prop-tests/src/unit_tests/prop_tests.rs @@ -0,0 +1,161 @@ +// Copyright (c) The Move Contributors +// SPDX-License-Identifier: Apache-2.0 + +use invalid_mutations::{ + bounds::{ + ApplyCodeUnitBoundsContext, ApplyOutOfBoundsContext, CodeUnitBoundsMutation, + OutOfBoundsMutation, + }, + signature::{FieldRefMutation, SignatureRefMutation}, +}; +use move_binary_format::{ + check_bounds::BoundsChecker, file_format::CompiledModule, + proptest_types::CompiledModuleStrategyGen, +}; +use move_bytecode_verifier::{ + ability_field_requirements, constants, instantiation_loops::InstantiationLoopChecker, + DuplicationChecker, InstructionConsistency, RecursiveStructDefChecker, SignatureChecker, +}; +use move_core_types::{ + account_address::AccountAddress, identifier::Identifier, vm_status::StatusCode, +}; +use proptest::{collection::vec, prelude::*, sample::Index as PropIndex}; + +proptest! { + // Generating arbitrary compiled modules is really slow, possibly because of + // https://github.com/AltSysrq/proptest/issues/143. + #![proptest_config(ProptestConfig::with_cases(16))] + + #[test] + fn valid_ability_transitivity(module in CompiledModule::valid_strategy(20)) { + prop_assert!(ability_field_requirements::verify_module(&module).is_ok()); + } + + #[test] + fn valid_bounds(_module in CompiledModule::valid_strategy(20)) { + // valid_strategy will panic if there are any bounds check issues. + } + + #[test] + fn invalid_out_of_bounds( + module in CompiledModule::valid_strategy(20), + oob_mutations in vec(OutOfBoundsMutation::strategy(), 0..40), + ) { + let (module, expected_violations) = { + let oob_context = ApplyOutOfBoundsContext::new(module, oob_mutations); + oob_context.apply() + }; + + let actual_violations = BoundsChecker::verify_module(&module); + prop_assert_eq!(expected_violations.is_empty(), actual_violations.is_ok()); + } + + #[test] + fn code_unit_out_of_bounds( + mut module in CompiledModule::valid_strategy(20), + mutations in vec(CodeUnitBoundsMutation::strategy(), 0..40), + ) { + let expected_violations = { + let context = ApplyCodeUnitBoundsContext::new(&mut module, mutations); + context.apply() + }; + + let actual_violations = BoundsChecker::verify_module(&module); + prop_assert_eq!(expected_violations.is_empty(), actual_violations.is_ok()); + } + + #[test] + fn no_module_handles( + identifiers in vec(any::(), 0..20), + address_identifiers in vec(any::(), 0..20), + ) { + // If there are no module handles, the only other things that can be stored are intrinsic + // data. + let module = CompiledModule { + identifiers, + address_identifiers, + ..Default::default() + }; + + prop_assert_eq!( + BoundsChecker::verify_module(&module).map_err(|e| e.major_status()), + Err(StatusCode::NO_MODULE_HANDLES) + ); + } + + /// Make sure that garbage inputs don't crash the bounds checker. + #[test] + fn garbage_inputs(module in any_with::(16)) { + let _ = BoundsChecker::verify_module(&module); + } + + #[test] + fn valid_generated_constants(module in CompiledModule::valid_strategy(20)) { + prop_assert!(constants::verify_module(&module).is_ok()); + } + + #[test] + fn valid_duplication(module in CompiledModule::valid_strategy(20)) { + prop_assert!(DuplicationChecker::verify_module(&module).is_ok()); + } + + #[test] + fn check_verifier_passes(module in CompiledModule::valid_strategy(20)) { + DuplicationChecker::verify_module(&module).expect("DuplicationChecker failure"); + SignatureChecker::verify_module(&module).expect("SignatureChecker failure"); + InstructionConsistency::verify_module(&module).expect("InstructionConsistency failure"); + constants::verify_module(&module).expect("constants failure"); + ability_field_requirements::verify_module(&module).expect("ability_field_requirements failure"); + RecursiveStructDefChecker::verify_module(&module).expect("RecursiveStructDefChecker failure"); + InstantiationLoopChecker::verify_module(&module).expect("InstantiationLoopChecker failure"); + } + + #[test] + fn valid_signatures(module in CompiledModule::valid_strategy(20)) { + prop_assert!(SignatureChecker::verify_module(&module).is_ok()) + } + + #[test] + fn double_refs( + mut module in CompiledModule::valid_strategy(20), + mutations in vec((any::(), any::()), 0..20), + ) { + let context = SignatureRefMutation::new(&mut module, mutations); + let expected_violations = context.apply(); + + let result = SignatureChecker::verify_module(&module); + + prop_assert_eq!(expected_violations, result.is_err()); + } + + #[test] + fn field_def_references( + mut module in CompiledModule::valid_strategy(20), + mutations in vec((any::(), any::()), 0..40), + ) { + let context = FieldRefMutation::new(&mut module, mutations); + let expected_violations = context.apply(); + + let result = SignatureChecker::verify_module(&module); + + prop_assert_eq!(expected_violations, result.is_err()); + } + + #[test] + fn valid_recursive_struct_defs(module in CompiledModule::valid_strategy(20)) { + prop_assert!(RecursiveStructDefChecker::verify_module(&module).is_ok()); + } +} + +/// Ensure that valid modules that don't have any members (e.g. function args, struct fields) pass +/// bounds checks. +/// +/// There are some potentially tricky edge cases around ranges that are captured here. +#[test] +fn valid_bounds_no_members() { + let mut gen = CompiledModuleStrategyGen::new(20); + gen.zeros_all(); + proptest!(|(_module in gen.generate())| { + // gen.generate() will panic if there are any bounds check issues. + }); +} diff --git a/external-crates/move/crates/bytecode-verifier-tests/Cargo.toml b/external-crates/move/crates/bytecode-verifier-tests/Cargo.toml index 6840260382058..54b3f12682ade 100644 --- a/external-crates/move/crates/bytecode-verifier-tests/Cargo.toml +++ b/external-crates/move/crates/bytecode-verifier-tests/Cargo.toml @@ -11,19 +11,15 @@ edition = "2021" [dev-dependencies] petgraph.workspace = true -proptest.workspace = true fail = { workspace = true, features = ["failpoints"] } hex.workspace = true -# referred to via path for execution versioning -invalid-mutations = { path = "../invalid-mutations" } move-binary-format = { workspace = true, features = ["fuzzing"] } -move-bytecode-verifier.workspace = true +# referred to via path for execution versioning +move-bytecode-verifier = { path = "../move-bytecode-verifier" } move-bytecode-verifier-meter.workspace = true move-core-types.workspace = true move-vm-config.workspace = true [features] fuzzing = ["move-binary-format/fuzzing"] - -[dependencies] diff --git a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs b/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs deleted file mode 100644 index 08bbdca20c0dc..0000000000000 --- a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::file_format::CompiledModule; -use move_bytecode_verifier::ability_field_requirements; -use proptest::prelude::*; - -proptest! { - #[test] - fn valid_ability_transitivity(module in CompiledModule::valid_strategy(20)) { - prop_assert!(ability_field_requirements::verify_module(&module).is_ok()); - } -} diff --git a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs b/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs index 4ca73841dcd61..905bc6540ba44 100644 --- a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs +++ b/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs @@ -2,18 +2,8 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use invalid_mutations::bounds::{ - ApplyCodeUnitBoundsContext, ApplyOutOfBoundsContext, CodeUnitBoundsMutation, - OutOfBoundsMutation, -}; -use move_binary_format::{ - check_bounds::BoundsChecker, file_format::*, file_format_common, - proptest_types::CompiledModuleStrategyGen, -}; -use move_core_types::{ - account_address::AccountAddress, identifier::Identifier, vm_status::StatusCode, -}; -use proptest::{collection::vec, prelude::*}; +use move_binary_format::{check_bounds::BoundsChecker, file_format::*, file_format_common}; +use move_core_types::vm_status::StatusCode; #[test] fn empty_module_no_errors() { @@ -296,84 +286,3 @@ fn invalid_type_param_for_vector_operation() { ); } } - -proptest! { - #[test] - fn valid_bounds(_module in CompiledModule::valid_strategy(20)) { - // valid_strategy will panic if there are any bounds check issues. - } -} - -/// Ensure that valid modules that don't have any members (e.g. function args, struct fields) pass -/// bounds checks. -/// -/// There are some potentially tricky edge cases around ranges that are captured here. -#[test] -fn valid_bounds_no_members() { - let mut gen = CompiledModuleStrategyGen::new(20); - gen.zeros_all(); - proptest!(|(_module in gen.generate())| { - // gen.generate() will panic if there are any bounds check issues. - }); -} - -proptest! { - #[test] - fn invalid_out_of_bounds( - module in CompiledModule::valid_strategy(20), - oob_mutations in vec(OutOfBoundsMutation::strategy(), 0..40), - ) { - let (module, expected_violations) = { - let oob_context = ApplyOutOfBoundsContext::new(module, oob_mutations); - oob_context.apply() - }; - - let actual_violations = BoundsChecker::verify_module(&module); - prop_assert_eq!(expected_violations.is_empty(), actual_violations.is_ok()); - } - - #[test] - fn code_unit_out_of_bounds( - mut module in CompiledModule::valid_strategy(20), - mutations in vec(CodeUnitBoundsMutation::strategy(), 0..40), - ) { - let expected_violations = { - let context = ApplyCodeUnitBoundsContext::new(&mut module, mutations); - context.apply() - }; - - let actual_violations = BoundsChecker::verify_module(&module); - prop_assert_eq!(expected_violations.is_empty(), actual_violations.is_ok()); - } - - #[test] - fn no_module_handles( - identifiers in vec(any::(), 0..20), - address_identifiers in vec(any::(), 0..20), - ) { - // If there are no module handles, the only other things that can be stored are intrinsic - // data. - let module = CompiledModule { - identifiers, - address_identifiers, - ..Default::default() - }; - - prop_assert_eq!( - BoundsChecker::verify_module(&module).map_err(|e| e.major_status()), - Err(StatusCode::NO_MODULE_HANDLES) - ); - } -} - -proptest! { - // Generating arbitrary compiled modules is really slow, possibly because of - // https://github.com/AltSysrq/proptest/issues/143. - #![proptest_config(ProptestConfig::with_cases(16))] - - /// Make sure that garbage inputs don't crash the bounds checker. - #[test] - fn garbage_inputs(module in any_with::(16)) { - let _ = BoundsChecker::verify_module(&module); - } -} diff --git a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs b/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs index b446d72e413f7..653654119714d 100644 --- a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs +++ b/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs @@ -1,17 +1,9 @@ // Copyright (c) The Diem Core Contributors // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use move_binary_format::file_format::{empty_module, CompiledModule, Constant, SignatureToken}; +use move_binary_format::file_format::{empty_module, Constant, SignatureToken}; use move_bytecode_verifier::constants; use move_core_types::vm_status::StatusCode; -use proptest::prelude::*; - -proptest! { - #[test] - fn valid_generated(module in CompiledModule::valid_strategy(20)) { - prop_assert!(constants::verify_module(&module).is_ok()); - } -} #[test] fn valid_primitives() { diff --git a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs b/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs index 640af70c6638e..da0d1fbeb6bb2 100644 --- a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs +++ b/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs @@ -4,7 +4,6 @@ use move_binary_format::file_format::*; use move_bytecode_verifier::DuplicationChecker; -use proptest::prelude::*; #[test] fn duplicated_friend_decls() { @@ -17,10 +16,3 @@ fn duplicated_friend_decls() { m.friend_decls.push(handle); DuplicationChecker::verify_module(&m).unwrap_err(); } - -proptest! { - #[test] - fn valid_duplication(module in CompiledModule::valid_strategy(20)) { - prop_assert!(DuplicationChecker::verify_module(&module).is_ok()); - } -} diff --git a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/mod.rs b/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/mod.rs index 65aa86c275656..3238641a6e76e 100644 --- a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/mod.rs +++ b/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/mod.rs @@ -6,7 +6,6 @@ use move_vm_config::verifier::{ MeterConfig, VerifierConfig, DEFAULT_MAX_CONSTANT_VECTOR_LEN, DEFAULT_MAX_IDENTIFIER_LENGTH, }; -pub mod ability_field_requirements_tests; pub mod binary_samples; pub mod bounds_tests; pub mod code_unit_tests; @@ -19,11 +18,9 @@ pub mod limit_tests; pub mod locals; pub mod loop_summary_tests; pub mod many_back_edges; -pub mod multi_pass_tests; pub mod negative_stack_size_tests; pub mod reference_safety_tests; pub mod signature_tests; -pub mod struct_defs_tests; pub mod vec_pack_tests; /// Configuration used in production. diff --git a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs b/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs deleted file mode 100644 index d244f6dd37e5a..0000000000000 --- a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::CompiledModule; -use move_bytecode_verifier::{ - ability_field_requirements, constants, instantiation_loops::InstantiationLoopChecker, - DuplicationChecker, InstructionConsistency, RecursiveStructDefChecker, SignatureChecker, -}; -use proptest::prelude::*; - -proptest! { - #[test] - fn check_verifier_passes(module in CompiledModule::valid_strategy(20)) { - DuplicationChecker::verify_module(&module).expect("DuplicationChecker failure"); - SignatureChecker::verify_module(&module).expect("SignatureChecker failure"); - InstructionConsistency::verify_module(&module).expect("InstructionConsistency failure"); - constants::verify_module(&module).expect("constants failure"); - ability_field_requirements::verify_module(&module).expect("ability_field_requirements failure"); - RecursiveStructDefChecker::verify_module(&module).expect("RecursiveStructDefChecker failure"); - InstantiationLoopChecker::verify_module(&module).expect("InstantiationLoopChecker failure"); - } -} diff --git a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs b/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs index a57c18530bb1c..41d2d4e541081 100644 --- a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs +++ b/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs @@ -3,7 +3,6 @@ // SPDX-License-Identifier: Apache-2.0 use crate::unit_tests::production_config; -use invalid_mutations::signature::{FieldRefMutation, SignatureRefMutation}; use move_binary_format::file_format::{ Bytecode::*, CompiledModule, SignatureToken::*, Visibility::Public, *, }; @@ -14,7 +13,6 @@ use move_bytecode_verifier_meter::dummy::DummyMeter; use move_core_types::{ account_address::AccountAddress, identifier::Identifier, vm_status::StatusCode, }; -use proptest::{collection::vec, prelude::*, sample::Index as PropIndex}; #[test] fn test_reference_of_reference() { @@ -26,39 +24,6 @@ fn test_reference_of_reference() { assert!(errors.is_err()); } -proptest! { - #[test] - fn valid_signatures(module in CompiledModule::valid_strategy(20)) { - prop_assert!(SignatureChecker::verify_module(&module).is_ok()) - } - - #[test] - fn double_refs( - mut module in CompiledModule::valid_strategy(20), - mutations in vec((any::(), any::()), 0..20), - ) { - let context = SignatureRefMutation::new(&mut module, mutations); - let expected_violations = context.apply(); - - let result = SignatureChecker::verify_module(&module); - - prop_assert_eq!(expected_violations, result.is_err()); - } - - #[test] - fn field_def_references( - mut module in CompiledModule::valid_strategy(20), - mutations in vec((any::(), any::()), 0..40), - ) { - let context = FieldRefMutation::new(&mut module, mutations); - let expected_violations = context.apply(); - - let result = SignatureChecker::verify_module(&module); - - prop_assert_eq!(expected_violations, result.is_err()); - } -} - #[test] fn no_verify_locals_good() { let compiled_module_good = CompiledModule { diff --git a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs b/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs deleted file mode 100644 index 71fdc466003f8..0000000000000 --- a/external-crates/move/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::file_format::CompiledModule; -use move_bytecode_verifier::RecursiveStructDefChecker; -use proptest::prelude::*; - -proptest! { - #[test] - fn valid_recursive_struct_defs(module in CompiledModule::valid_strategy(20)) { - prop_assert!(RecursiveStructDefChecker::verify_module(&module).is_ok()); - } -} diff --git a/external-crates/move/crates/move-bytecode-verifier/Cargo.toml b/external-crates/move/crates/move-bytecode-verifier/Cargo.toml index dc77a5ea34f80..a957bebee3074 100644 --- a/external-crates/move/crates/move-bytecode-verifier/Cargo.toml +++ b/external-crates/move/crates/move-bytecode-verifier/Cargo.toml @@ -21,8 +21,6 @@ move-abstract-stack.workspace = true [dev-dependencies] hex-literal.workspace = true -# referred to via path for execution versioning -invalid-mutations = { path = "../invalid-mutations" } [features] default = [] diff --git a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/Cargo.toml b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/Cargo.toml index 4ee7336a90cc3..89a0090cf86af 100644 --- a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/Cargo.toml +++ b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/Cargo.toml @@ -11,12 +11,11 @@ edition = "2021" [dev-dependencies] petgraph.workspace = true -proptest.workspace = true -fail = { workspace = true, features = ['failpoints']} +fail = { workspace = true, features = ["failpoints"] } hex.workspace = true -invalid-mutations = { path = "../invalid-mutations", package = "invalid-mutations-v0" } move-binary-format = { workspace = true, features = ["fuzzing"] } +# referred to via path for execution versioning move-bytecode-verifier = { path = "../move-bytecode-verifier", package = "move-bytecode-verifier-v0" } move-bytecode-verifier-meter.workspace = true move-core-types.workspace = true diff --git a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs deleted file mode 100644 index 08bbdca20c0dc..0000000000000 --- a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::file_format::CompiledModule; -use move_bytecode_verifier::ability_field_requirements; -use proptest::prelude::*; - -proptest! { - #[test] - fn valid_ability_transitivity(module in CompiledModule::valid_strategy(20)) { - prop_assert!(ability_field_requirements::verify_module(&module).is_ok()); - } -} diff --git a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs index 4ca73841dcd61..905bc6540ba44 100644 --- a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs +++ b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs @@ -2,18 +2,8 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use invalid_mutations::bounds::{ - ApplyCodeUnitBoundsContext, ApplyOutOfBoundsContext, CodeUnitBoundsMutation, - OutOfBoundsMutation, -}; -use move_binary_format::{ - check_bounds::BoundsChecker, file_format::*, file_format_common, - proptest_types::CompiledModuleStrategyGen, -}; -use move_core_types::{ - account_address::AccountAddress, identifier::Identifier, vm_status::StatusCode, -}; -use proptest::{collection::vec, prelude::*}; +use move_binary_format::{check_bounds::BoundsChecker, file_format::*, file_format_common}; +use move_core_types::vm_status::StatusCode; #[test] fn empty_module_no_errors() { @@ -296,84 +286,3 @@ fn invalid_type_param_for_vector_operation() { ); } } - -proptest! { - #[test] - fn valid_bounds(_module in CompiledModule::valid_strategy(20)) { - // valid_strategy will panic if there are any bounds check issues. - } -} - -/// Ensure that valid modules that don't have any members (e.g. function args, struct fields) pass -/// bounds checks. -/// -/// There are some potentially tricky edge cases around ranges that are captured here. -#[test] -fn valid_bounds_no_members() { - let mut gen = CompiledModuleStrategyGen::new(20); - gen.zeros_all(); - proptest!(|(_module in gen.generate())| { - // gen.generate() will panic if there are any bounds check issues. - }); -} - -proptest! { - #[test] - fn invalid_out_of_bounds( - module in CompiledModule::valid_strategy(20), - oob_mutations in vec(OutOfBoundsMutation::strategy(), 0..40), - ) { - let (module, expected_violations) = { - let oob_context = ApplyOutOfBoundsContext::new(module, oob_mutations); - oob_context.apply() - }; - - let actual_violations = BoundsChecker::verify_module(&module); - prop_assert_eq!(expected_violations.is_empty(), actual_violations.is_ok()); - } - - #[test] - fn code_unit_out_of_bounds( - mut module in CompiledModule::valid_strategy(20), - mutations in vec(CodeUnitBoundsMutation::strategy(), 0..40), - ) { - let expected_violations = { - let context = ApplyCodeUnitBoundsContext::new(&mut module, mutations); - context.apply() - }; - - let actual_violations = BoundsChecker::verify_module(&module); - prop_assert_eq!(expected_violations.is_empty(), actual_violations.is_ok()); - } - - #[test] - fn no_module_handles( - identifiers in vec(any::(), 0..20), - address_identifiers in vec(any::(), 0..20), - ) { - // If there are no module handles, the only other things that can be stored are intrinsic - // data. - let module = CompiledModule { - identifiers, - address_identifiers, - ..Default::default() - }; - - prop_assert_eq!( - BoundsChecker::verify_module(&module).map_err(|e| e.major_status()), - Err(StatusCode::NO_MODULE_HANDLES) - ); - } -} - -proptest! { - // Generating arbitrary compiled modules is really slow, possibly because of - // https://github.com/AltSysrq/proptest/issues/143. - #![proptest_config(ProptestConfig::with_cases(16))] - - /// Make sure that garbage inputs don't crash the bounds checker. - #[test] - fn garbage_inputs(module in any_with::(16)) { - let _ = BoundsChecker::verify_module(&module); - } -} diff --git a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs index 2589a8d353627..653654119714d 100644 --- a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs +++ b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs @@ -1,17 +1,9 @@ // Copyright (c) The Diem Core Contributors // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use move_binary_format::file_format::{empty_module, CompiledModule, Constant, SignatureToken}; +use move_binary_format::file_format::{empty_module, Constant, SignatureToken}; use move_bytecode_verifier::constants; use move_core_types::vm_status::StatusCode; -use proptest::prelude::*; - -proptest! { - #[test] - fn valid_generated(module in CompiledModule::valid_strategy(20)) { - prop_assert!(constants::verify_module(&module).is_ok()); - } -} #[test] fn valid_primitives() { @@ -69,7 +61,8 @@ fn invalid_primitives() { malformed(SignatureToken::U256, vec![0, 0]); let data = vec![ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, ]; malformed(SignatureToken::Address, data); } @@ -220,7 +213,8 @@ fn invalid_vectors() { tvec(SignatureToken::Address), vec![ 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, ], ); // wrong lens diff --git a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs index 640af70c6638e..da0d1fbeb6bb2 100644 --- a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs +++ b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs @@ -4,7 +4,6 @@ use move_binary_format::file_format::*; use move_bytecode_verifier::DuplicationChecker; -use proptest::prelude::*; #[test] fn duplicated_friend_decls() { @@ -17,10 +16,3 @@ fn duplicated_friend_decls() { m.friend_decls.push(handle); DuplicationChecker::verify_module(&m).unwrap_err(); } - -proptest! { - #[test] - fn valid_duplication(module in CompiledModule::valid_strategy(20)) { - prop_assert!(DuplicationChecker::verify_module(&module).is_ok()); - } -} diff --git a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/mod.rs b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/mod.rs index 65aa86c275656..3238641a6e76e 100644 --- a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/mod.rs +++ b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/mod.rs @@ -6,7 +6,6 @@ use move_vm_config::verifier::{ MeterConfig, VerifierConfig, DEFAULT_MAX_CONSTANT_VECTOR_LEN, DEFAULT_MAX_IDENTIFIER_LENGTH, }; -pub mod ability_field_requirements_tests; pub mod binary_samples; pub mod bounds_tests; pub mod code_unit_tests; @@ -19,11 +18,9 @@ pub mod limit_tests; pub mod locals; pub mod loop_summary_tests; pub mod many_back_edges; -pub mod multi_pass_tests; pub mod negative_stack_size_tests; pub mod reference_safety_tests; pub mod signature_tests; -pub mod struct_defs_tests; pub mod vec_pack_tests; /// Configuration used in production. diff --git a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs deleted file mode 100644 index d244f6dd37e5a..0000000000000 --- a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::CompiledModule; -use move_bytecode_verifier::{ - ability_field_requirements, constants, instantiation_loops::InstantiationLoopChecker, - DuplicationChecker, InstructionConsistency, RecursiveStructDefChecker, SignatureChecker, -}; -use proptest::prelude::*; - -proptest! { - #[test] - fn check_verifier_passes(module in CompiledModule::valid_strategy(20)) { - DuplicationChecker::verify_module(&module).expect("DuplicationChecker failure"); - SignatureChecker::verify_module(&module).expect("SignatureChecker failure"); - InstructionConsistency::verify_module(&module).expect("InstructionConsistency failure"); - constants::verify_module(&module).expect("constants failure"); - ability_field_requirements::verify_module(&module).expect("ability_field_requirements failure"); - RecursiveStructDefChecker::verify_module(&module).expect("RecursiveStructDefChecker failure"); - InstantiationLoopChecker::verify_module(&module).expect("InstantiationLoopChecker failure"); - } -} diff --git a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs index a57c18530bb1c..41d2d4e541081 100644 --- a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs +++ b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs @@ -3,7 +3,6 @@ // SPDX-License-Identifier: Apache-2.0 use crate::unit_tests::production_config; -use invalid_mutations::signature::{FieldRefMutation, SignatureRefMutation}; use move_binary_format::file_format::{ Bytecode::*, CompiledModule, SignatureToken::*, Visibility::Public, *, }; @@ -14,7 +13,6 @@ use move_bytecode_verifier_meter::dummy::DummyMeter; use move_core_types::{ account_address::AccountAddress, identifier::Identifier, vm_status::StatusCode, }; -use proptest::{collection::vec, prelude::*, sample::Index as PropIndex}; #[test] fn test_reference_of_reference() { @@ -26,39 +24,6 @@ fn test_reference_of_reference() { assert!(errors.is_err()); } -proptest! { - #[test] - fn valid_signatures(module in CompiledModule::valid_strategy(20)) { - prop_assert!(SignatureChecker::verify_module(&module).is_ok()) - } - - #[test] - fn double_refs( - mut module in CompiledModule::valid_strategy(20), - mutations in vec((any::(), any::()), 0..20), - ) { - let context = SignatureRefMutation::new(&mut module, mutations); - let expected_violations = context.apply(); - - let result = SignatureChecker::verify_module(&module); - - prop_assert_eq!(expected_violations, result.is_err()); - } - - #[test] - fn field_def_references( - mut module in CompiledModule::valid_strategy(20), - mutations in vec((any::(), any::()), 0..40), - ) { - let context = FieldRefMutation::new(&mut module, mutations); - let expected_violations = context.apply(); - - let result = SignatureChecker::verify_module(&module); - - prop_assert_eq!(expected_violations, result.is_err()); - } -} - #[test] fn no_verify_locals_good() { let compiled_module_good = CompiledModule { diff --git a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs b/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs deleted file mode 100644 index 71fdc466003f8..0000000000000 --- a/external-crates/move/move-execution/v0/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::file_format::CompiledModule; -use move_bytecode_verifier::RecursiveStructDefChecker; -use proptest::prelude::*; - -proptest! { - #[test] - fn valid_recursive_struct_defs(module in CompiledModule::valid_strategy(20)) { - prop_assert!(RecursiveStructDefChecker::verify_module(&module).is_ok()); - } -} diff --git a/external-crates/move/move-execution/v0/crates/invalid-mutations/Cargo.toml b/external-crates/move/move-execution/v0/crates/invalid-mutations/Cargo.toml deleted file mode 100644 index 16e8f326c86de..0000000000000 --- a/external-crates/move/move-execution/v0/crates/invalid-mutations/Cargo.toml +++ /dev/null @@ -1,18 +0,0 @@ -[package] -name = "invalid-mutations-v0" -version = "0.1.0" -edition = "2021" -authors = ["Diem Association "] -description = "Diem invalid mutations" -repository = "https://github.com/diem/diem" -homepage = "https://diem.com" -license = "Apache-2.0" -publish = false - -[dependencies] -move-core-types.workspace = true -move-binary-format.workspace = true -proptest.workspace = true - -[features] -default = [] diff --git a/external-crates/move/move-execution/v0/crates/invalid-mutations/src/bounds.rs b/external-crates/move/move-execution/v0/crates/invalid-mutations/src/bounds.rs deleted file mode 100644 index c6784e3f47a78..0000000000000 --- a/external-crates/move/move-execution/v0/crates/invalid-mutations/src/bounds.rs +++ /dev/null @@ -1,364 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::{ - errors::{bounds_error, PartialVMError}, - file_format::{ - AddressIdentifierIndex, CompiledModule, FunctionHandleIndex, IdentifierIndex, - ModuleHandleIndex, SignatureIndex, StructDefinitionIndex, StructHandleIndex, TableIndex, - }, - internals::ModuleIndex, - IndexKind, -}; -use move_core_types::vm_status::StatusCode; -use proptest::{ - prelude::*, - sample::{self, Index as PropIndex}, -}; -use std::collections::BTreeMap; - -mod code_unit; -pub use code_unit::{ApplyCodeUnitBoundsContext, CodeUnitBoundsMutation}; -use move_binary_format::file_format::SignatureToken; - -/// Represents the number of pointers that exist out from a node of a particular kind. -#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] -pub enum PointerKind { - /// Exactly one pointer out with this index kind as its destination. - One(IndexKind), - /// Zero or one pointer out with this index kind as its destination. Like the `?` operator in - /// regular expressions. - Optional(IndexKind), - /// Zero or more pointers out with this index kind as its destination. Like the `*` operator - /// in regular expressions. - Star(IndexKind), -} - -impl PointerKind { - /// A list of what pointers (indexes) exist out from a particular kind of node within the - /// module. - /// - /// The only special case is `FunctionDefinition`, which contains a `CodeUnit` that can contain - /// one of several kinds of pointers out. That is not represented in this table. - #[inline] - pub fn pointers_from(src_kind: IndexKind) -> &'static [PointerKind] { - use IndexKind::*; - use PointerKind::*; - - match src_kind { - ModuleHandle => &[One(AddressIdentifier), One(Identifier)], - StructHandle => &[One(ModuleHandle), One(Identifier)], - FunctionHandle => &[ - One(ModuleHandle), - One(Identifier), - One(Signature), - One(Signature), - ], - StructDefinition => &[One(StructHandle), Star(StructHandle)], - FunctionDefinition => &[One(FunctionHandle), One(Signature)], - FriendDeclaration => &[One(AddressIdentifier), One(Identifier)], - Signature => &[Star(StructHandle)], - FieldHandle => &[One(StructDefinition)], - _ => &[], - } - } - - #[inline] - pub fn to_index_kind(self) -> IndexKind { - match self { - PointerKind::One(idx) | PointerKind::Optional(idx) | PointerKind::Star(idx) => idx, - } - } -} - -pub static VALID_POINTER_SRCS: &[IndexKind] = &[ - IndexKind::ModuleHandle, - IndexKind::StructHandle, - IndexKind::FunctionHandle, - IndexKind::FieldHandle, - IndexKind::StructDefinition, - IndexKind::FunctionDefinition, - IndexKind::FriendDeclaration, - IndexKind::Signature, -]; - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn pointer_kind_sanity() { - for variant in IndexKind::variants() { - if VALID_POINTER_SRCS.iter().any(|x| x == variant) { - assert!( - !PointerKind::pointers_from(*variant).is_empty(), - "expected variant {:?} to be a valid pointer source", - variant, - ); - } else { - assert!( - PointerKind::pointers_from(*variant).is_empty(), - "expected variant {:?} to not be a valid pointer source", - variant, - ); - } - } - } -} - -/// Represents a single mutation to a `CompiledModule` to produce an out-of-bounds situation. -/// -/// Use `OutOfBoundsMutation::strategy()` to generate them, preferably using `Vec` to generate -/// many at a time. Then use `ApplyOutOfBoundsContext` to apply those mutations. -#[derive(Debug)] -pub struct OutOfBoundsMutation { - src_kind: IndexKind, - src_idx: PropIndex, - dst_kind: IndexKind, - offset: usize, -} - -impl OutOfBoundsMutation { - pub fn strategy() -> impl Strategy { - ( - Self::src_kind_strategy(), - any::(), - any::(), - 0..16_usize, - ) - .prop_map(|(src_kind, src_idx, dst_kind_idx, offset)| { - let dst_kind = Self::dst_kind(src_kind, dst_kind_idx); - Self { - src_kind, - src_idx, - dst_kind, - offset, - } - }) - } - - // Not all source kinds can be made to be out of bounds (e.g. inherent types can't.) - fn src_kind_strategy() -> impl Strategy { - sample::select(VALID_POINTER_SRCS) - } - - fn dst_kind(src_kind: IndexKind, dst_kind_idx: PropIndex) -> IndexKind { - dst_kind_idx - .get(PointerKind::pointers_from(src_kind)) - .to_index_kind() - } -} - -/// This is used for source indexing, to work with pick_slice_idxs. -impl AsRef for OutOfBoundsMutation { - #[inline] - fn as_ref(&self) -> &PropIndex { - &self.src_idx - } -} - -pub struct ApplyOutOfBoundsContext { - module: CompiledModule, - // This is an Option because it gets moved out in apply before apply_one is called. Rust - // doesn't let you call another con-consuming method after a partial move out. - mutations: Option>, - - // Some precomputations done for signatures. - sig_structs: Vec<(SignatureIndex, usize)>, -} - -impl ApplyOutOfBoundsContext { - pub fn new(module: CompiledModule, mutations: Vec) -> Self { - let sig_structs: Vec<_> = Self::sig_structs(&module).collect(); - - Self { - module, - mutations: Some(mutations), - sig_structs, - } - } - - pub fn apply(mut self) -> (CompiledModule, Vec) { - // This is a map from (source kind, dest kind) to the actual mutations -- this is done to - // figure out how many mutations to do for a particular pair, which is required for - // pick_slice_idxs below. - let mut mutation_map = BTreeMap::new(); - for mutation in self - .mutations - .take() - .expect("mutations should always be present") - { - mutation_map - .entry((mutation.src_kind, mutation.dst_kind)) - .or_insert_with(Vec::new) - .push(mutation); - } - - let mut results = vec![]; - - for ((src_kind, dst_kind), mutations) in mutation_map { - // It would be cool to use an iterator here, if someone could figure out exactly how - // to get the lifetimes right :) - results.extend(self.apply_one(src_kind, dst_kind, mutations)); - } - (self.module, results) - } - - fn apply_one( - &mut self, - src_kind: IndexKind, - dst_kind: IndexKind, - mutations: Vec, - ) -> Vec { - let src_count = match src_kind { - IndexKind::Signature => self.sig_structs.len(), - // For the other sorts it's always possible to change an index. - src_kind => self.module.kind_count(src_kind), - }; - // Any signature can be a destination, not just the ones that have structs in them. - let dst_count = self.module.kind_count(dst_kind); - let to_mutate = crate::helpers::pick_slice_idxs(src_count, &mutations); - - mutations - .iter() - .zip(to_mutate) - .map(move |(mutation, src_idx)| { - self.set_index( - src_kind, - src_idx, - dst_kind, - dst_count, - (dst_count + mutation.offset) as TableIndex, - ) - }) - .collect() - } - - /// Sets the particular index in the table - /// - /// For example, with `src_kind` set to `ModuleHandle` and `dst_kind` set to `AddressPool`, - /// this will set self.module_handles[src_idx].address to new_idx. - /// - /// This is mainly used for test generation. - fn set_index( - &mut self, - src_kind: IndexKind, - src_idx: usize, - dst_kind: IndexKind, - dst_count: usize, - new_idx: TableIndex, - ) -> PartialVMError { - use IndexKind::*; - - // These are default values, but some of the match arms below mutate them. - let mut src_idx = src_idx; - let err = bounds_error( - StatusCode::INDEX_OUT_OF_BOUNDS, - dst_kind, - new_idx, - dst_count, - ); - - // A dynamic type system would be able to express this next block of code far more - // concisely. A static type system would require some sort of complicated dependent type - // structure that Rust doesn't have. As things stand today, every possible case needs to - // be listed out. - - match (src_kind, dst_kind) { - (ModuleHandle, AddressIdentifier) => { - self.module.module_handles[src_idx].address = AddressIdentifierIndex(new_idx) - } - (ModuleHandle, Identifier) => { - self.module.module_handles[src_idx].name = IdentifierIndex(new_idx) - } - (StructHandle, ModuleHandle) => { - self.module.struct_handles[src_idx].module = ModuleHandleIndex(new_idx) - } - (StructHandle, Identifier) => { - self.module.struct_handles[src_idx].name = IdentifierIndex(new_idx) - } - (FunctionHandle, ModuleHandle) => { - self.module.function_handles[src_idx].module = ModuleHandleIndex(new_idx) - } - (FunctionHandle, Identifier) => { - self.module.function_handles[src_idx].name = IdentifierIndex(new_idx) - } - (FunctionHandle, Signature) => { - self.module.function_handles[src_idx].parameters = SignatureIndex(new_idx) - } - (StructDefinition, StructHandle) => { - self.module.struct_defs[src_idx].struct_handle = StructHandleIndex(new_idx) - } - (FunctionDefinition, FunctionHandle) => { - self.module.function_defs[src_idx].function = FunctionHandleIndex(new_idx) - } - (FunctionDefinition, Signature) => { - self.module.function_defs[src_idx] - .code - .as_mut() - .unwrap() - .locals = SignatureIndex(new_idx) - } - (Signature, StructHandle) => { - let (actual_src_idx, arg_idx) = self.sig_structs[src_idx]; - src_idx = actual_src_idx.into_index(); - self.module.signatures[src_idx].0[arg_idx] - .debug_set_sh_idx(StructHandleIndex(new_idx)); - } - (FieldHandle, StructDefinition) => { - self.module.field_handles[src_idx].owner = StructDefinitionIndex(new_idx) - } - (FriendDeclaration, AddressIdentifier) => { - self.module.friend_decls[src_idx].address = AddressIdentifierIndex(new_idx) - } - (FriendDeclaration, Identifier) => { - self.module.friend_decls[src_idx].name = IdentifierIndex(new_idx) - } - _ => panic!("Invalid pointer kind: {:?} -> {:?}", src_kind, dst_kind), - } - - err.at_index(src_kind, src_idx as TableIndex) - } - - /// Returns the indexes of locals signatures that contain struct handles inside them. - fn sig_structs(module: &CompiledModule) -> impl Iterator + '_ { - module - .signatures() - .iter() - .enumerate() - .flat_map(|(idx, signature)| { - let idx = SignatureIndex(idx as u16); - Self::find_struct_tokens(&signature.0, move |arg_idx| (idx, arg_idx)) - }) - } - - #[inline] - fn find_struct_tokens<'b, F, T>( - tokens: impl IntoIterator + 'b, - map_fn: F, - ) -> impl Iterator + 'b - where - F: Fn(usize) -> T + 'b, - { - tokens - .into_iter() - .enumerate() - .filter_map(move |(arg_idx, token)| struct_handle(token).map(|_| map_fn(arg_idx))) - } -} - -fn struct_handle(token: &SignatureToken) -> Option { - use SignatureToken::*; - - match token { - Struct(sh_idx) => Some(*sh_idx), - StructInstantiation(struct_inst) => { - let (sh_idx, _) = &**struct_inst; - Some(*sh_idx) - } - Reference(token) | MutableReference(token) => struct_handle(token), - Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address | Signer | Vector(_) - | TypeParameter(_) => None, - } -} diff --git a/external-crates/move/move-execution/v0/crates/invalid-mutations/src/bounds/code_unit.rs b/external-crates/move/move-execution/v0/crates/invalid-mutations/src/bounds/code_unit.rs deleted file mode 100644 index 0011719128f7b..0000000000000 --- a/external-crates/move/move-execution/v0/crates/invalid-mutations/src/bounds/code_unit.rs +++ /dev/null @@ -1,476 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::{ - errors::{offset_out_of_bounds, PartialVMError}, - file_format::{ - Bytecode, CodeOffset, CompiledModule, ConstantPoolIndex, FieldHandleIndex, - FieldInstantiationIndex, FunctionDefinitionIndex, FunctionHandleIndex, - FunctionInstantiationIndex, LocalIndex, SignatureIndex, StructDefInstantiationIndex, - StructDefinitionIndex, TableIndex, - }, - internals::ModuleIndex, - IndexKind, -}; -use move_core_types::vm_status::StatusCode; -use proptest::{prelude::*, sample::Index as PropIndex}; -use std::collections::BTreeMap; - -/// Represents a single mutation onto a code unit to make it out of bounds. -#[derive(Debug)] -pub struct CodeUnitBoundsMutation { - function_def: PropIndex, - bytecode: PropIndex, - offset: usize, -} - -impl CodeUnitBoundsMutation { - pub fn strategy() -> impl Strategy { - (any::(), any::(), 0..16_usize).prop_map( - |(function_def, bytecode, offset)| Self { - function_def, - bytecode, - offset, - }, - ) - } -} - -impl AsRef for CodeUnitBoundsMutation { - #[inline] - fn as_ref(&self) -> &PropIndex { - &self.bytecode - } -} - -pub struct ApplyCodeUnitBoundsContext<'a> { - module: &'a mut CompiledModule, - // This is so apply_one can be called after mutations has been iterated on. - mutations: Option>, -} - -macro_rules! new_bytecode { - ($dst_len:expr, $fidx:expr, $bcidx:expr, $offset:expr, $kind:ident, $bytecode_ident:tt) => {{ - let dst_len: usize = $dst_len; - let new_idx: usize = dst_len + $offset; - ( - $bytecode_ident($kind::new(new_idx as TableIndex)), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - $kind::KIND, - new_idx, - dst_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; - - ($dst_len:expr, $fidx:expr, $bcidx:expr, $offset:expr, $kind:ident, $bytecode_ident:tt, $($others:expr),+) => {{ - let dst_len: usize = $dst_len; - let new_idx: usize = dst_len + $offset; - ( - $bytecode_ident($kind::new(new_idx as TableIndex), $($others),+), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - $kind::KIND, - new_idx, - dst_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; -} - -macro_rules! struct_bytecode { - ($dst_len: expr, $fidx:expr, $bcidx: expr, $offset: expr, $idx_type: ident, $bytecode_ident: tt) => {{ - let dst_len = $dst_len; - let new_idx = dst_len + $offset; - ( - $bytecode_ident($idx_type::new(new_idx as TableIndex)), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - $idx_type::KIND, - new_idx, - dst_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; -} - -macro_rules! code_bytecode { - ($code_len: expr, $fidx:expr, $bcidx: expr, $offset: expr, $bytecode_ident: tt) => {{ - let code_len = $code_len; - let new_idx = code_len + $offset; - ( - $bytecode_ident(new_idx as CodeOffset), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - IndexKind::CodeDefinition, - new_idx, - code_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; -} - -macro_rules! locals_bytecode { - ($locals_len: expr, $fidx:expr, $bcidx: expr, $offset: expr, $bytecode_ident: tt) => {{ - let locals_len = $locals_len; - let new_idx = locals_len + $offset; - ( - $bytecode_ident(new_idx as LocalIndex), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - IndexKind::LocalPool, - new_idx, - locals_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; -} - -impl<'a> ApplyCodeUnitBoundsContext<'a> { - pub fn new(module: &'a mut CompiledModule, mutations: Vec) -> Self { - Self { - module, - mutations: Some(mutations), - } - } - - pub fn apply(mut self) -> Vec { - let function_def_len = self.module.function_defs.len(); - - let mut mutation_map: BTreeMap<_, Vec<_>> = BTreeMap::new(); - for mutation in self - .mutations - .take() - .expect("mutations should always be present") - { - let picked_idx = mutation.function_def.index(function_def_len); - mutation_map.entry(picked_idx).or_default().push(mutation); - } - - let mut results = vec![]; - - for (idx, mutations) in mutation_map { - results.extend(self.apply_one(idx, mutations)); - } - results - } - - fn apply_one( - &mut self, - fidx: usize, - mutations: Vec, - ) -> Vec { - // For this function def, find all the places where a bounds mutation can be applied. - let func_def = &mut self.module.function_defs[fidx]; - let current_fdef = FunctionDefinitionIndex(fidx as TableIndex); - let func_handle = &self.module.function_handles[func_def.function.into_index()]; - let code = func_def.code.as_mut().unwrap(); - let locals_len = self.module.signatures[func_handle.parameters.into_index()].len() - + self.module.signatures[code.locals.into_index()].len(); - let code = &mut code.code; - let code_len = code.len(); - - let interesting_offsets: Vec = (0..code.len()) - .filter(|bytecode_idx| is_interesting(&code[*bytecode_idx])) - .collect(); - let to_mutate = crate::helpers::pick_slice_idxs(interesting_offsets.len(), &mutations); - - // These have to be computed upfront because self.module is being mutated below. - let constant_pool_len = self.module.constant_pool.len(); - let function_handles_len = self.module.function_handles.len(); - let field_handle_len = self.module.field_handles.len(); - let struct_defs_len = self.module.struct_defs.len(); - let struct_inst_len = self.module.struct_def_instantiations.len(); - let function_inst_len = self.module.function_instantiations.len(); - let field_inst_len = self.module.field_instantiations.len(); - let signature_pool_len = self.module.signatures.len(); - - mutations - .iter() - .zip(to_mutate) - .map(|(mutation, interesting_offsets_idx)| { - let bytecode_idx = interesting_offsets[interesting_offsets_idx]; - let offset = mutation.offset; - use Bytecode::*; - - let (new_bytecode, err) = match code[bytecode_idx] { - LdConst(_) => new_bytecode!( - constant_pool_len, - current_fdef, - bytecode_idx, - offset, - ConstantPoolIndex, - LdConst - ), - ImmBorrowField(_) => new_bytecode!( - field_handle_len, - current_fdef, - bytecode_idx, - offset, - FieldHandleIndex, - ImmBorrowField - ), - ImmBorrowFieldGeneric(_) => new_bytecode!( - field_inst_len, - current_fdef, - bytecode_idx, - offset, - FieldInstantiationIndex, - ImmBorrowFieldGeneric - ), - MutBorrowField(_) => new_bytecode!( - field_handle_len, - current_fdef, - bytecode_idx, - offset, - FieldHandleIndex, - MutBorrowField - ), - MutBorrowFieldGeneric(_) => new_bytecode!( - field_inst_len, - current_fdef, - bytecode_idx, - offset, - FieldInstantiationIndex, - MutBorrowFieldGeneric - ), - Call(_) => struct_bytecode!( - function_handles_len, - current_fdef, - bytecode_idx, - offset, - FunctionHandleIndex, - Call - ), - CallGeneric(_) => struct_bytecode!( - function_inst_len, - current_fdef, - bytecode_idx, - offset, - FunctionInstantiationIndex, - CallGeneric - ), - Pack(_) => struct_bytecode!( - struct_defs_len, - current_fdef, - bytecode_idx, - offset, - StructDefinitionIndex, - Pack - ), - PackGeneric(_) => struct_bytecode!( - struct_inst_len, - current_fdef, - bytecode_idx, - offset, - StructDefInstantiationIndex, - PackGeneric - ), - Unpack(_) => struct_bytecode!( - struct_defs_len, - current_fdef, - bytecode_idx, - offset, - StructDefinitionIndex, - Unpack - ), - UnpackGeneric(_) => struct_bytecode!( - struct_inst_len, - current_fdef, - bytecode_idx, - offset, - StructDefInstantiationIndex, - UnpackGeneric - ), - BrTrue(_) => { - code_bytecode!(code_len, current_fdef, bytecode_idx, offset, BrTrue) - } - BrFalse(_) => { - code_bytecode!(code_len, current_fdef, bytecode_idx, offset, BrFalse) - } - Branch(_) => { - code_bytecode!(code_len, current_fdef, bytecode_idx, offset, Branch) - } - CopyLoc(_) => { - locals_bytecode!(locals_len, current_fdef, bytecode_idx, offset, CopyLoc) - } - MoveLoc(_) => { - locals_bytecode!(locals_len, current_fdef, bytecode_idx, offset, MoveLoc) - } - StLoc(_) => { - locals_bytecode!(locals_len, current_fdef, bytecode_idx, offset, StLoc) - } - MutBorrowLoc(_) => locals_bytecode!( - locals_len, - current_fdef, - bytecode_idx, - offset, - MutBorrowLoc - ), - ImmBorrowLoc(_) => locals_bytecode!( - locals_len, - current_fdef, - bytecode_idx, - offset, - ImmBorrowLoc - ), - VecPack(_, num) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecPack, - num - ), - VecLen(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecLen - ), - VecImmBorrow(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecImmBorrow - ), - VecMutBorrow(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecMutBorrow - ), - VecPushBack(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecPushBack - ), - VecPopBack(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecPopBack - ), - VecUnpack(_, num) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecUnpack, - num - ), - VecSwap(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecSwap - ), - - // Deprecated bytecodes - ExistsDeprecated(_) - | ExistsGenericDeprecated(_) - | MutBorrowGlobalDeprecated(_) - | MutBorrowGlobalGenericDeprecated(_) - | ImmBorrowGlobalDeprecated(_) - | ImmBorrowGlobalGenericDeprecated(_) - | MoveFromDeprecated(_) - | MoveFromGenericDeprecated(_) - | MoveToDeprecated(_) - | MoveToGenericDeprecated(_) => { - panic!("Bytecode deprecated: {:?}", code[bytecode_idx]) - } - FreezeRef | Pop | Ret | LdU8(_) | LdU16(_) | LdU32(_) | LdU64(_) - | LdU128(_) | LdU256(_) | CastU8 | CastU16 | CastU32 | CastU64 | CastU128 - | CastU256 | LdTrue | LdFalse | ReadRef | WriteRef | Add | Sub | Mul | Mod - | Div | BitOr | BitAnd | Xor | Shl | Shr | Or | And | Not | Eq | Neq | Lt - | Gt | Le | Ge | Abort | Nop => { - panic!("Bytecode has no internal index: {:?}", code[bytecode_idx]) - } - }; - - code[bytecode_idx] = new_bytecode; - - err.at_index(IndexKind::FunctionDefinition, fidx as TableIndex) - }) - .collect() - } -} - -fn is_interesting(bytecode: &Bytecode) -> bool { - use Bytecode::*; - - match bytecode { - LdConst(_) - | ImmBorrowField(_) - | ImmBorrowFieldGeneric(_) - | MutBorrowField(_) - | MutBorrowFieldGeneric(_) - | Call(_) - | CallGeneric(_) - | Pack(_) - | PackGeneric(_) - | Unpack(_) - | UnpackGeneric(_) - | BrTrue(_) - | BrFalse(_) - | Branch(_) - | CopyLoc(_) - | MoveLoc(_) - | StLoc(_) - | MutBorrowLoc(_) - | ImmBorrowLoc(_) - | VecPack(..) - | VecLen(_) - | VecImmBorrow(_) - | VecMutBorrow(_) - | VecPushBack(_) - | VecPopBack(_) - | VecUnpack(..) - | VecSwap(_) => true, - - // Deprecated bytecodes - ExistsDeprecated(_) - | ExistsGenericDeprecated(_) - | MutBorrowGlobalDeprecated(_) - | MutBorrowGlobalGenericDeprecated(_) - | ImmBorrowGlobalDeprecated(_) - | ImmBorrowGlobalGenericDeprecated(_) - | MoveFromDeprecated(_) - | MoveFromGenericDeprecated(_) - | MoveToDeprecated(_) - | MoveToGenericDeprecated(_) => false, - // List out the other options explicitly so there's a compile error if a new - // bytecode gets added. - FreezeRef | Pop | Ret | LdU8(_) | LdU16(_) | LdU32(_) | LdU64(_) | LdU128(_) - | LdU256(_) | CastU8 | CastU16 | CastU32 | CastU64 | CastU128 | CastU256 | LdTrue - | LdFalse | ReadRef | WriteRef | Add | Sub | Mul | Mod | Div | BitOr | BitAnd | Xor - | Shl | Shr | Or | And | Not | Eq | Neq | Lt | Gt | Le | Ge | Abort | Nop => false, - } -} diff --git a/external-crates/move/move-execution/v0/crates/invalid-mutations/src/helpers.rs b/external-crates/move/move-execution/v0/crates/invalid-mutations/src/helpers.rs deleted file mode 100644 index c122d58c87fea..0000000000000 --- a/external-crates/move/move-execution/v0/crates/invalid-mutations/src/helpers.rs +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use proptest::sample::Index as PropIndex; -use std::{collections::BTreeSet, ops::Index as OpsIndex}; - -/// Given a maximum value `max` and a list of [`Index`](proptest::sample::Index) instances, picks -/// integers in the range `[0, max)` uniformly randomly and without duplication. -/// -/// If `indexes_len` is greater than `max`, all indexes will be returned. -/// -/// This function implements Robert Floyd's F2 algorithm for sampling without replacement. -pub(crate) fn pick_idxs(max: usize, indexes: &T, indexes_len: usize) -> Vec -where - T: OpsIndex + ?Sized, - P: AsRef, -{ - // See https://dl.acm.org/doi/10.1145/30401.315746 (the F2 algorithm) - // for a longer explanation. This is a variant that works with zero-indexing. - let mut selected = BTreeSet::new(); - let to_select = indexes_len.min(max); - for (iter_idx, choice) in ((max - to_select)..max).enumerate() { - // "RandInt(1, J)" in the original algorithm means a number between 1 - // and choice, both inclusive. `PropIndex::index` picks a number between 0 and - // whatever's passed in, with the latter exclusive. Pass in "+1" to ensure the same - // range of values is picked from. (This also ensures that if choice is 0 then `index` - // doesn't panic. - let idx = indexes[iter_idx].as_ref().index(choice + 1); - if !selected.insert(idx) { - selected.insert(choice); - } - } - selected.into_iter().collect() -} - -/// Given a maximum value `max` and a slice of [`Index`](proptest::sample::Index) instances, picks -/// integers in the range `[0, max)` uniformly randomly and without duplication. -/// -/// If the number of `Index` instances is greater than `max`, all indexes will be returned. -/// -/// This function implements Robert Floyd's F2 algorithm for sampling without replacement. -#[inline] -pub(crate) fn pick_slice_idxs(max: usize, indexes: &[impl AsRef]) -> Vec { - pick_idxs(max, indexes, indexes.len()) -} diff --git a/external-crates/move/move-execution/v0/crates/invalid-mutations/src/lib.rs b/external-crates/move/move-execution/v0/crates/invalid-mutations/src/lib.rs deleted file mode 100644 index d6574245153dd..0000000000000 --- a/external-crates/move/move-execution/v0/crates/invalid-mutations/src/lib.rs +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -#![forbid(unsafe_code)] - -pub mod bounds; -mod helpers; -pub mod signature; diff --git a/external-crates/move/move-execution/v0/crates/invalid-mutations/src/signature.rs b/external-crates/move/move-execution/v0/crates/invalid-mutations/src/signature.rs deleted file mode 100644 index 6181bf66d3ce6..0000000000000 --- a/external-crates/move/move-execution/v0/crates/invalid-mutations/src/signature.rs +++ /dev/null @@ -1,100 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::file_format::{ - CompiledModule, Signature, SignatureToken, StructFieldInformation, TypeSignature, -}; -use proptest::sample::Index as PropIndex; - -pub struct SignatureRefMutation<'a> { - module: &'a mut CompiledModule, - mutations: Vec<(PropIndex, PropIndex)>, -} - -impl<'a> SignatureRefMutation<'a> { - pub fn new(module: &'a mut CompiledModule, mutations: Vec<(PropIndex, PropIndex)>) -> Self { - Self { module, mutations } - } - - pub fn apply(self) -> bool { - if self.module.signatures.is_empty() { - return false; - } - let module = self.module; - let mut mutations = false; - for (s_idx, t_idx) in self.mutations { - let sig_idx = s_idx.index(module.signatures.len()); - let sig = &module.signatures[sig_idx]; - if sig.is_empty() { - continue; - } - let token_idx = t_idx.index(sig.len()); - let new_sig = mutate_sig(sig, token_idx); - module.signatures[sig_idx] = new_sig; - mutations = true; - } - mutations - } -} - -/// Context for applying a list of `FieldRefMutation` instances. -pub struct FieldRefMutation<'a> { - module: &'a mut CompiledModule, - mutations: Vec<(PropIndex, PropIndex)>, -} - -impl<'a> FieldRefMutation<'a> { - pub fn new(module: &'a mut CompiledModule, mutations: Vec<(PropIndex, PropIndex)>) -> Self { - Self { module, mutations } - } - - #[inline] - pub fn apply(self) -> bool { - if self.module.struct_defs.is_empty() { - return false; - } - let module = self.module; - let mut mutations = false; - for (s_idx, f_idx) in self.mutations { - let struct_idx = s_idx.index(module.struct_defs.len()); - let struct_def = &mut module.struct_defs[struct_idx]; - if let StructFieldInformation::Declared(fields) = &mut struct_def.field_information { - if fields.is_empty() { - continue; - } - let field_idx = f_idx.index(fields.len()); - let field_def = &mut fields[field_idx]; - let new_ty = mutate_field(&field_def.signature.0); - fields[field_idx].signature = TypeSignature(new_ty); - mutations = true; - } - } - mutations - } -} - -fn mutate_sig(sig: &Signature, token_idx: usize) -> Signature { - use SignatureToken::*; - - Signature( - sig.0 - .iter() - .enumerate() - .map(|(idx, token)| { - if idx == token_idx { - match &token { - Reference(_) | MutableReference(_) => Reference(Box::new(token.clone())), - _ => Reference(Box::new(Reference(Box::new(token.clone())))), - } - } else { - token.clone() - } - }) - .collect(), - ) -} - -fn mutate_field(token: &SignatureToken) -> SignatureToken { - SignatureToken::Reference(Box::new(token.clone())) -} diff --git a/external-crates/move/move-execution/v0/crates/move-bytecode-verifier/Cargo.toml b/external-crates/move/move-execution/v0/crates/move-bytecode-verifier/Cargo.toml index 7211f683912d8..9bd798c0946d6 100644 --- a/external-crates/move/move-execution/v0/crates/move-bytecode-verifier/Cargo.toml +++ b/external-crates/move/move-execution/v0/crates/move-bytecode-verifier/Cargo.toml @@ -21,7 +21,6 @@ move-abstract-stack.workspace = true [dev-dependencies] hex-literal.workspace = true -invalid-mutations = { path = "../invalid-mutations", package = "invalid-mutations-v0" } [features] default = [] diff --git a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/Cargo.toml b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/Cargo.toml index 25711201e2156..98630015a5aff 100644 --- a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/Cargo.toml +++ b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/Cargo.toml @@ -11,12 +11,11 @@ edition = "2021" [dev-dependencies] petgraph.workspace = true -proptest.workspace = true fail = { workspace = true, features = ["failpoints"] } hex.workspace = true -invalid-mutations = { path = "../invalid-mutations", package = "invalid-mutations-v1" } move-binary-format = { workspace = true, features = ["fuzzing"] } +# referred to via path for execution versioning move-bytecode-verifier = { path = "../move-bytecode-verifier", package = "move-bytecode-verifier-v1" } move-bytecode-verifier-meter.workspace = true move-core-types.workspace = true @@ -24,5 +23,3 @@ move-vm-config.workspace = true [features] fuzzing = ["move-binary-format/fuzzing"] - -[dependencies] diff --git a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs deleted file mode 100644 index 08bbdca20c0dc..0000000000000 --- a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::file_format::CompiledModule; -use move_bytecode_verifier::ability_field_requirements; -use proptest::prelude::*; - -proptest! { - #[test] - fn valid_ability_transitivity(module in CompiledModule::valid_strategy(20)) { - prop_assert!(ability_field_requirements::verify_module(&module).is_ok()); - } -} diff --git a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs index 4ca73841dcd61..905bc6540ba44 100644 --- a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs +++ b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs @@ -2,18 +2,8 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use invalid_mutations::bounds::{ - ApplyCodeUnitBoundsContext, ApplyOutOfBoundsContext, CodeUnitBoundsMutation, - OutOfBoundsMutation, -}; -use move_binary_format::{ - check_bounds::BoundsChecker, file_format::*, file_format_common, - proptest_types::CompiledModuleStrategyGen, -}; -use move_core_types::{ - account_address::AccountAddress, identifier::Identifier, vm_status::StatusCode, -}; -use proptest::{collection::vec, prelude::*}; +use move_binary_format::{check_bounds::BoundsChecker, file_format::*, file_format_common}; +use move_core_types::vm_status::StatusCode; #[test] fn empty_module_no_errors() { @@ -296,84 +286,3 @@ fn invalid_type_param_for_vector_operation() { ); } } - -proptest! { - #[test] - fn valid_bounds(_module in CompiledModule::valid_strategy(20)) { - // valid_strategy will panic if there are any bounds check issues. - } -} - -/// Ensure that valid modules that don't have any members (e.g. function args, struct fields) pass -/// bounds checks. -/// -/// There are some potentially tricky edge cases around ranges that are captured here. -#[test] -fn valid_bounds_no_members() { - let mut gen = CompiledModuleStrategyGen::new(20); - gen.zeros_all(); - proptest!(|(_module in gen.generate())| { - // gen.generate() will panic if there are any bounds check issues. - }); -} - -proptest! { - #[test] - fn invalid_out_of_bounds( - module in CompiledModule::valid_strategy(20), - oob_mutations in vec(OutOfBoundsMutation::strategy(), 0..40), - ) { - let (module, expected_violations) = { - let oob_context = ApplyOutOfBoundsContext::new(module, oob_mutations); - oob_context.apply() - }; - - let actual_violations = BoundsChecker::verify_module(&module); - prop_assert_eq!(expected_violations.is_empty(), actual_violations.is_ok()); - } - - #[test] - fn code_unit_out_of_bounds( - mut module in CompiledModule::valid_strategy(20), - mutations in vec(CodeUnitBoundsMutation::strategy(), 0..40), - ) { - let expected_violations = { - let context = ApplyCodeUnitBoundsContext::new(&mut module, mutations); - context.apply() - }; - - let actual_violations = BoundsChecker::verify_module(&module); - prop_assert_eq!(expected_violations.is_empty(), actual_violations.is_ok()); - } - - #[test] - fn no_module_handles( - identifiers in vec(any::(), 0..20), - address_identifiers in vec(any::(), 0..20), - ) { - // If there are no module handles, the only other things that can be stored are intrinsic - // data. - let module = CompiledModule { - identifiers, - address_identifiers, - ..Default::default() - }; - - prop_assert_eq!( - BoundsChecker::verify_module(&module).map_err(|e| e.major_status()), - Err(StatusCode::NO_MODULE_HANDLES) - ); - } -} - -proptest! { - // Generating arbitrary compiled modules is really slow, possibly because of - // https://github.com/AltSysrq/proptest/issues/143. - #![proptest_config(ProptestConfig::with_cases(16))] - - /// Make sure that garbage inputs don't crash the bounds checker. - #[test] - fn garbage_inputs(module in any_with::(16)) { - let _ = BoundsChecker::verify_module(&module); - } -} diff --git a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs index b446d72e413f7..653654119714d 100644 --- a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs +++ b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs @@ -1,17 +1,9 @@ // Copyright (c) The Diem Core Contributors // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use move_binary_format::file_format::{empty_module, CompiledModule, Constant, SignatureToken}; +use move_binary_format::file_format::{empty_module, Constant, SignatureToken}; use move_bytecode_verifier::constants; use move_core_types::vm_status::StatusCode; -use proptest::prelude::*; - -proptest! { - #[test] - fn valid_generated(module in CompiledModule::valid_strategy(20)) { - prop_assert!(constants::verify_module(&module).is_ok()); - } -} #[test] fn valid_primitives() { diff --git a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs index 640af70c6638e..da0d1fbeb6bb2 100644 --- a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs +++ b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs @@ -4,7 +4,6 @@ use move_binary_format::file_format::*; use move_bytecode_verifier::DuplicationChecker; -use proptest::prelude::*; #[test] fn duplicated_friend_decls() { @@ -17,10 +16,3 @@ fn duplicated_friend_decls() { m.friend_decls.push(handle); DuplicationChecker::verify_module(&m).unwrap_err(); } - -proptest! { - #[test] - fn valid_duplication(module in CompiledModule::valid_strategy(20)) { - prop_assert!(DuplicationChecker::verify_module(&module).is_ok()); - } -} diff --git a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/mod.rs b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/mod.rs index 65aa86c275656..3238641a6e76e 100644 --- a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/mod.rs +++ b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/mod.rs @@ -6,7 +6,6 @@ use move_vm_config::verifier::{ MeterConfig, VerifierConfig, DEFAULT_MAX_CONSTANT_VECTOR_LEN, DEFAULT_MAX_IDENTIFIER_LENGTH, }; -pub mod ability_field_requirements_tests; pub mod binary_samples; pub mod bounds_tests; pub mod code_unit_tests; @@ -19,11 +18,9 @@ pub mod limit_tests; pub mod locals; pub mod loop_summary_tests; pub mod many_back_edges; -pub mod multi_pass_tests; pub mod negative_stack_size_tests; pub mod reference_safety_tests; pub mod signature_tests; -pub mod struct_defs_tests; pub mod vec_pack_tests; /// Configuration used in production. diff --git a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs deleted file mode 100644 index d244f6dd37e5a..0000000000000 --- a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::CompiledModule; -use move_bytecode_verifier::{ - ability_field_requirements, constants, instantiation_loops::InstantiationLoopChecker, - DuplicationChecker, InstructionConsistency, RecursiveStructDefChecker, SignatureChecker, -}; -use proptest::prelude::*; - -proptest! { - #[test] - fn check_verifier_passes(module in CompiledModule::valid_strategy(20)) { - DuplicationChecker::verify_module(&module).expect("DuplicationChecker failure"); - SignatureChecker::verify_module(&module).expect("SignatureChecker failure"); - InstructionConsistency::verify_module(&module).expect("InstructionConsistency failure"); - constants::verify_module(&module).expect("constants failure"); - ability_field_requirements::verify_module(&module).expect("ability_field_requirements failure"); - RecursiveStructDefChecker::verify_module(&module).expect("RecursiveStructDefChecker failure"); - InstantiationLoopChecker::verify_module(&module).expect("InstantiationLoopChecker failure"); - } -} diff --git a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs index a57c18530bb1c..41d2d4e541081 100644 --- a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs +++ b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs @@ -3,7 +3,6 @@ // SPDX-License-Identifier: Apache-2.0 use crate::unit_tests::production_config; -use invalid_mutations::signature::{FieldRefMutation, SignatureRefMutation}; use move_binary_format::file_format::{ Bytecode::*, CompiledModule, SignatureToken::*, Visibility::Public, *, }; @@ -14,7 +13,6 @@ use move_bytecode_verifier_meter::dummy::DummyMeter; use move_core_types::{ account_address::AccountAddress, identifier::Identifier, vm_status::StatusCode, }; -use proptest::{collection::vec, prelude::*, sample::Index as PropIndex}; #[test] fn test_reference_of_reference() { @@ -26,39 +24,6 @@ fn test_reference_of_reference() { assert!(errors.is_err()); } -proptest! { - #[test] - fn valid_signatures(module in CompiledModule::valid_strategy(20)) { - prop_assert!(SignatureChecker::verify_module(&module).is_ok()) - } - - #[test] - fn double_refs( - mut module in CompiledModule::valid_strategy(20), - mutations in vec((any::(), any::()), 0..20), - ) { - let context = SignatureRefMutation::new(&mut module, mutations); - let expected_violations = context.apply(); - - let result = SignatureChecker::verify_module(&module); - - prop_assert_eq!(expected_violations, result.is_err()); - } - - #[test] - fn field_def_references( - mut module in CompiledModule::valid_strategy(20), - mutations in vec((any::(), any::()), 0..40), - ) { - let context = FieldRefMutation::new(&mut module, mutations); - let expected_violations = context.apply(); - - let result = SignatureChecker::verify_module(&module); - - prop_assert_eq!(expected_violations, result.is_err()); - } -} - #[test] fn no_verify_locals_good() { let compiled_module_good = CompiledModule { diff --git a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs b/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs deleted file mode 100644 index 71fdc466003f8..0000000000000 --- a/external-crates/move/move-execution/v1/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::file_format::CompiledModule; -use move_bytecode_verifier::RecursiveStructDefChecker; -use proptest::prelude::*; - -proptest! { - #[test] - fn valid_recursive_struct_defs(module in CompiledModule::valid_strategy(20)) { - prop_assert!(RecursiveStructDefChecker::verify_module(&module).is_ok()); - } -} diff --git a/external-crates/move/move-execution/v1/crates/invalid-mutations/Cargo.toml b/external-crates/move/move-execution/v1/crates/invalid-mutations/Cargo.toml deleted file mode 100644 index ef3893ad6d1a1..0000000000000 --- a/external-crates/move/move-execution/v1/crates/invalid-mutations/Cargo.toml +++ /dev/null @@ -1,18 +0,0 @@ -[package] -name = "invalid-mutations-v1" -version = "0.1.0" -edition = "2021" -authors = ["Diem Association "] -description = "Diem invalid mutations" -repository = "https://github.com/diem/diem" -homepage = "https://diem.com" -license = "Apache-2.0" -publish = false - -[dependencies] -move-core-types.workspace = true -move-binary-format.workspace = true -proptest.workspace = true - -[features] -default = [] diff --git a/external-crates/move/move-execution/v1/crates/invalid-mutations/src/bounds.rs b/external-crates/move/move-execution/v1/crates/invalid-mutations/src/bounds.rs deleted file mode 100644 index c6784e3f47a78..0000000000000 --- a/external-crates/move/move-execution/v1/crates/invalid-mutations/src/bounds.rs +++ /dev/null @@ -1,364 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::{ - errors::{bounds_error, PartialVMError}, - file_format::{ - AddressIdentifierIndex, CompiledModule, FunctionHandleIndex, IdentifierIndex, - ModuleHandleIndex, SignatureIndex, StructDefinitionIndex, StructHandleIndex, TableIndex, - }, - internals::ModuleIndex, - IndexKind, -}; -use move_core_types::vm_status::StatusCode; -use proptest::{ - prelude::*, - sample::{self, Index as PropIndex}, -}; -use std::collections::BTreeMap; - -mod code_unit; -pub use code_unit::{ApplyCodeUnitBoundsContext, CodeUnitBoundsMutation}; -use move_binary_format::file_format::SignatureToken; - -/// Represents the number of pointers that exist out from a node of a particular kind. -#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] -pub enum PointerKind { - /// Exactly one pointer out with this index kind as its destination. - One(IndexKind), - /// Zero or one pointer out with this index kind as its destination. Like the `?` operator in - /// regular expressions. - Optional(IndexKind), - /// Zero or more pointers out with this index kind as its destination. Like the `*` operator - /// in regular expressions. - Star(IndexKind), -} - -impl PointerKind { - /// A list of what pointers (indexes) exist out from a particular kind of node within the - /// module. - /// - /// The only special case is `FunctionDefinition`, which contains a `CodeUnit` that can contain - /// one of several kinds of pointers out. That is not represented in this table. - #[inline] - pub fn pointers_from(src_kind: IndexKind) -> &'static [PointerKind] { - use IndexKind::*; - use PointerKind::*; - - match src_kind { - ModuleHandle => &[One(AddressIdentifier), One(Identifier)], - StructHandle => &[One(ModuleHandle), One(Identifier)], - FunctionHandle => &[ - One(ModuleHandle), - One(Identifier), - One(Signature), - One(Signature), - ], - StructDefinition => &[One(StructHandle), Star(StructHandle)], - FunctionDefinition => &[One(FunctionHandle), One(Signature)], - FriendDeclaration => &[One(AddressIdentifier), One(Identifier)], - Signature => &[Star(StructHandle)], - FieldHandle => &[One(StructDefinition)], - _ => &[], - } - } - - #[inline] - pub fn to_index_kind(self) -> IndexKind { - match self { - PointerKind::One(idx) | PointerKind::Optional(idx) | PointerKind::Star(idx) => idx, - } - } -} - -pub static VALID_POINTER_SRCS: &[IndexKind] = &[ - IndexKind::ModuleHandle, - IndexKind::StructHandle, - IndexKind::FunctionHandle, - IndexKind::FieldHandle, - IndexKind::StructDefinition, - IndexKind::FunctionDefinition, - IndexKind::FriendDeclaration, - IndexKind::Signature, -]; - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn pointer_kind_sanity() { - for variant in IndexKind::variants() { - if VALID_POINTER_SRCS.iter().any(|x| x == variant) { - assert!( - !PointerKind::pointers_from(*variant).is_empty(), - "expected variant {:?} to be a valid pointer source", - variant, - ); - } else { - assert!( - PointerKind::pointers_from(*variant).is_empty(), - "expected variant {:?} to not be a valid pointer source", - variant, - ); - } - } - } -} - -/// Represents a single mutation to a `CompiledModule` to produce an out-of-bounds situation. -/// -/// Use `OutOfBoundsMutation::strategy()` to generate them, preferably using `Vec` to generate -/// many at a time. Then use `ApplyOutOfBoundsContext` to apply those mutations. -#[derive(Debug)] -pub struct OutOfBoundsMutation { - src_kind: IndexKind, - src_idx: PropIndex, - dst_kind: IndexKind, - offset: usize, -} - -impl OutOfBoundsMutation { - pub fn strategy() -> impl Strategy { - ( - Self::src_kind_strategy(), - any::(), - any::(), - 0..16_usize, - ) - .prop_map(|(src_kind, src_idx, dst_kind_idx, offset)| { - let dst_kind = Self::dst_kind(src_kind, dst_kind_idx); - Self { - src_kind, - src_idx, - dst_kind, - offset, - } - }) - } - - // Not all source kinds can be made to be out of bounds (e.g. inherent types can't.) - fn src_kind_strategy() -> impl Strategy { - sample::select(VALID_POINTER_SRCS) - } - - fn dst_kind(src_kind: IndexKind, dst_kind_idx: PropIndex) -> IndexKind { - dst_kind_idx - .get(PointerKind::pointers_from(src_kind)) - .to_index_kind() - } -} - -/// This is used for source indexing, to work with pick_slice_idxs. -impl AsRef for OutOfBoundsMutation { - #[inline] - fn as_ref(&self) -> &PropIndex { - &self.src_idx - } -} - -pub struct ApplyOutOfBoundsContext { - module: CompiledModule, - // This is an Option because it gets moved out in apply before apply_one is called. Rust - // doesn't let you call another con-consuming method after a partial move out. - mutations: Option>, - - // Some precomputations done for signatures. - sig_structs: Vec<(SignatureIndex, usize)>, -} - -impl ApplyOutOfBoundsContext { - pub fn new(module: CompiledModule, mutations: Vec) -> Self { - let sig_structs: Vec<_> = Self::sig_structs(&module).collect(); - - Self { - module, - mutations: Some(mutations), - sig_structs, - } - } - - pub fn apply(mut self) -> (CompiledModule, Vec) { - // This is a map from (source kind, dest kind) to the actual mutations -- this is done to - // figure out how many mutations to do for a particular pair, which is required for - // pick_slice_idxs below. - let mut mutation_map = BTreeMap::new(); - for mutation in self - .mutations - .take() - .expect("mutations should always be present") - { - mutation_map - .entry((mutation.src_kind, mutation.dst_kind)) - .or_insert_with(Vec::new) - .push(mutation); - } - - let mut results = vec![]; - - for ((src_kind, dst_kind), mutations) in mutation_map { - // It would be cool to use an iterator here, if someone could figure out exactly how - // to get the lifetimes right :) - results.extend(self.apply_one(src_kind, dst_kind, mutations)); - } - (self.module, results) - } - - fn apply_one( - &mut self, - src_kind: IndexKind, - dst_kind: IndexKind, - mutations: Vec, - ) -> Vec { - let src_count = match src_kind { - IndexKind::Signature => self.sig_structs.len(), - // For the other sorts it's always possible to change an index. - src_kind => self.module.kind_count(src_kind), - }; - // Any signature can be a destination, not just the ones that have structs in them. - let dst_count = self.module.kind_count(dst_kind); - let to_mutate = crate::helpers::pick_slice_idxs(src_count, &mutations); - - mutations - .iter() - .zip(to_mutate) - .map(move |(mutation, src_idx)| { - self.set_index( - src_kind, - src_idx, - dst_kind, - dst_count, - (dst_count + mutation.offset) as TableIndex, - ) - }) - .collect() - } - - /// Sets the particular index in the table - /// - /// For example, with `src_kind` set to `ModuleHandle` and `dst_kind` set to `AddressPool`, - /// this will set self.module_handles[src_idx].address to new_idx. - /// - /// This is mainly used for test generation. - fn set_index( - &mut self, - src_kind: IndexKind, - src_idx: usize, - dst_kind: IndexKind, - dst_count: usize, - new_idx: TableIndex, - ) -> PartialVMError { - use IndexKind::*; - - // These are default values, but some of the match arms below mutate them. - let mut src_idx = src_idx; - let err = bounds_error( - StatusCode::INDEX_OUT_OF_BOUNDS, - dst_kind, - new_idx, - dst_count, - ); - - // A dynamic type system would be able to express this next block of code far more - // concisely. A static type system would require some sort of complicated dependent type - // structure that Rust doesn't have. As things stand today, every possible case needs to - // be listed out. - - match (src_kind, dst_kind) { - (ModuleHandle, AddressIdentifier) => { - self.module.module_handles[src_idx].address = AddressIdentifierIndex(new_idx) - } - (ModuleHandle, Identifier) => { - self.module.module_handles[src_idx].name = IdentifierIndex(new_idx) - } - (StructHandle, ModuleHandle) => { - self.module.struct_handles[src_idx].module = ModuleHandleIndex(new_idx) - } - (StructHandle, Identifier) => { - self.module.struct_handles[src_idx].name = IdentifierIndex(new_idx) - } - (FunctionHandle, ModuleHandle) => { - self.module.function_handles[src_idx].module = ModuleHandleIndex(new_idx) - } - (FunctionHandle, Identifier) => { - self.module.function_handles[src_idx].name = IdentifierIndex(new_idx) - } - (FunctionHandle, Signature) => { - self.module.function_handles[src_idx].parameters = SignatureIndex(new_idx) - } - (StructDefinition, StructHandle) => { - self.module.struct_defs[src_idx].struct_handle = StructHandleIndex(new_idx) - } - (FunctionDefinition, FunctionHandle) => { - self.module.function_defs[src_idx].function = FunctionHandleIndex(new_idx) - } - (FunctionDefinition, Signature) => { - self.module.function_defs[src_idx] - .code - .as_mut() - .unwrap() - .locals = SignatureIndex(new_idx) - } - (Signature, StructHandle) => { - let (actual_src_idx, arg_idx) = self.sig_structs[src_idx]; - src_idx = actual_src_idx.into_index(); - self.module.signatures[src_idx].0[arg_idx] - .debug_set_sh_idx(StructHandleIndex(new_idx)); - } - (FieldHandle, StructDefinition) => { - self.module.field_handles[src_idx].owner = StructDefinitionIndex(new_idx) - } - (FriendDeclaration, AddressIdentifier) => { - self.module.friend_decls[src_idx].address = AddressIdentifierIndex(new_idx) - } - (FriendDeclaration, Identifier) => { - self.module.friend_decls[src_idx].name = IdentifierIndex(new_idx) - } - _ => panic!("Invalid pointer kind: {:?} -> {:?}", src_kind, dst_kind), - } - - err.at_index(src_kind, src_idx as TableIndex) - } - - /// Returns the indexes of locals signatures that contain struct handles inside them. - fn sig_structs(module: &CompiledModule) -> impl Iterator + '_ { - module - .signatures() - .iter() - .enumerate() - .flat_map(|(idx, signature)| { - let idx = SignatureIndex(idx as u16); - Self::find_struct_tokens(&signature.0, move |arg_idx| (idx, arg_idx)) - }) - } - - #[inline] - fn find_struct_tokens<'b, F, T>( - tokens: impl IntoIterator + 'b, - map_fn: F, - ) -> impl Iterator + 'b - where - F: Fn(usize) -> T + 'b, - { - tokens - .into_iter() - .enumerate() - .filter_map(move |(arg_idx, token)| struct_handle(token).map(|_| map_fn(arg_idx))) - } -} - -fn struct_handle(token: &SignatureToken) -> Option { - use SignatureToken::*; - - match token { - Struct(sh_idx) => Some(*sh_idx), - StructInstantiation(struct_inst) => { - let (sh_idx, _) = &**struct_inst; - Some(*sh_idx) - } - Reference(token) | MutableReference(token) => struct_handle(token), - Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address | Signer | Vector(_) - | TypeParameter(_) => None, - } -} diff --git a/external-crates/move/move-execution/v1/crates/invalid-mutations/src/bounds/code_unit.rs b/external-crates/move/move-execution/v1/crates/invalid-mutations/src/bounds/code_unit.rs deleted file mode 100644 index bbbc500c0ebd0..0000000000000 --- a/external-crates/move/move-execution/v1/crates/invalid-mutations/src/bounds/code_unit.rs +++ /dev/null @@ -1,479 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::{ - errors::{offset_out_of_bounds, PartialVMError}, - file_format::{ - Bytecode, CodeOffset, CompiledModule, ConstantPoolIndex, FieldHandleIndex, - FieldInstantiationIndex, FunctionDefinitionIndex, FunctionHandleIndex, - FunctionInstantiationIndex, LocalIndex, SignatureIndex, StructDefInstantiationIndex, - StructDefinitionIndex, TableIndex, - }, - internals::ModuleIndex, - IndexKind, -}; -use move_core_types::vm_status::StatusCode; -use proptest::{prelude::*, sample::Index as PropIndex}; -use std::collections::BTreeMap; - -/// Represents a single mutation onto a code unit to make it out of bounds. -#[derive(Debug)] -pub struct CodeUnitBoundsMutation { - function_def: PropIndex, - bytecode: PropIndex, - offset: usize, -} - -impl CodeUnitBoundsMutation { - pub fn strategy() -> impl Strategy { - (any::(), any::(), 0..16_usize).prop_map( - |(function_def, bytecode, offset)| Self { - function_def, - bytecode, - offset, - }, - ) - } -} - -impl AsRef for CodeUnitBoundsMutation { - #[inline] - fn as_ref(&self) -> &PropIndex { - &self.bytecode - } -} - -pub struct ApplyCodeUnitBoundsContext<'a> { - module: &'a mut CompiledModule, - // This is so apply_one can be called after mutations has been iterated on. - mutations: Option>, -} - -macro_rules! new_bytecode { - ($dst_len:expr, $fidx:expr, $bcidx:expr, $offset:expr, $kind:ident, $bytecode_ident:tt) => {{ - let dst_len: usize = $dst_len; - let new_idx: usize = dst_len + $offset; - ( - $bytecode_ident($kind::new(new_idx as TableIndex)), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - $kind::KIND, - new_idx, - dst_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; - - ($dst_len:expr, $fidx:expr, $bcidx:expr, $offset:expr, $kind:ident, $bytecode_ident:tt, $($others:expr),+) => {{ - let dst_len: usize = $dst_len; - let new_idx: usize = dst_len + $offset; - ( - $bytecode_ident($kind::new(new_idx as TableIndex), $($others),+), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - $kind::KIND, - new_idx, - dst_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; -} - -macro_rules! struct_bytecode { - ($dst_len: expr, $fidx:expr, $bcidx: expr, $offset: expr, $idx_type: ident, $bytecode_ident: tt) => {{ - let dst_len = $dst_len; - let new_idx = dst_len + $offset; - ( - $bytecode_ident($idx_type::new(new_idx as TableIndex)), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - $idx_type::KIND, - new_idx, - dst_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; -} - -macro_rules! code_bytecode { - ($code_len: expr, $fidx:expr, $bcidx: expr, $offset: expr, $bytecode_ident: tt) => {{ - let code_len = $code_len; - let new_idx = code_len + $offset; - ( - $bytecode_ident(new_idx as CodeOffset), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - IndexKind::CodeDefinition, - new_idx, - code_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; -} - -macro_rules! locals_bytecode { - ($locals_len: expr, $fidx:expr, $bcidx: expr, $offset: expr, $bytecode_ident: tt) => {{ - let locals_len = $locals_len; - let new_idx = locals_len + $offset; - ( - $bytecode_ident(new_idx as LocalIndex), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - IndexKind::LocalPool, - new_idx, - locals_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; -} - -impl<'a> ApplyCodeUnitBoundsContext<'a> { - pub fn new(module: &'a mut CompiledModule, mutations: Vec) -> Self { - Self { - module, - mutations: Some(mutations), - } - } - - pub fn apply(mut self) -> Vec { - let function_def_len = self.module.function_defs.len(); - - let mut mutation_map = BTreeMap::new(); - for mutation in self - .mutations - .take() - .expect("mutations should always be present") - { - let picked_idx = mutation.function_def.index(function_def_len); - mutation_map - .entry(picked_idx) - .or_insert_with(Vec::new) - .push(mutation); - } - - let mut results = vec![]; - - for (idx, mutations) in mutation_map { - results.extend(self.apply_one(idx, mutations)); - } - results - } - - fn apply_one( - &mut self, - fidx: usize, - mutations: Vec, - ) -> Vec { - // For this function def, find all the places where a bounds mutation can be applied. - let func_def = &mut self.module.function_defs[fidx]; - let current_fdef = FunctionDefinitionIndex(fidx as TableIndex); - let func_handle = &self.module.function_handles[func_def.function.into_index()]; - let code = func_def.code.as_mut().unwrap(); - let locals_len = self.module.signatures[func_handle.parameters.into_index()].len() - + self.module.signatures[code.locals.into_index()].len(); - let code = &mut code.code; - let code_len = code.len(); - - let interesting_offsets: Vec = (0..code.len()) - .filter(|bytecode_idx| is_interesting(&code[*bytecode_idx])) - .collect(); - let to_mutate = crate::helpers::pick_slice_idxs(interesting_offsets.len(), &mutations); - - // These have to be computed upfront because self.module is being mutated below. - let constant_pool_len = self.module.constant_pool.len(); - let function_handles_len = self.module.function_handles.len(); - let field_handle_len = self.module.field_handles.len(); - let struct_defs_len = self.module.struct_defs.len(); - let struct_inst_len = self.module.struct_def_instantiations.len(); - let function_inst_len = self.module.function_instantiations.len(); - let field_inst_len = self.module.field_instantiations.len(); - let signature_pool_len = self.module.signatures.len(); - - mutations - .iter() - .zip(to_mutate) - .map(|(mutation, interesting_offsets_idx)| { - let bytecode_idx = interesting_offsets[interesting_offsets_idx]; - let offset = mutation.offset; - use Bytecode::*; - - let (new_bytecode, err) = match code[bytecode_idx] { - LdConst(_) => new_bytecode!( - constant_pool_len, - current_fdef, - bytecode_idx, - offset, - ConstantPoolIndex, - LdConst - ), - ImmBorrowField(_) => new_bytecode!( - field_handle_len, - current_fdef, - bytecode_idx, - offset, - FieldHandleIndex, - ImmBorrowField - ), - ImmBorrowFieldGeneric(_) => new_bytecode!( - field_inst_len, - current_fdef, - bytecode_idx, - offset, - FieldInstantiationIndex, - ImmBorrowFieldGeneric - ), - MutBorrowField(_) => new_bytecode!( - field_handle_len, - current_fdef, - bytecode_idx, - offset, - FieldHandleIndex, - MutBorrowField - ), - MutBorrowFieldGeneric(_) => new_bytecode!( - field_inst_len, - current_fdef, - bytecode_idx, - offset, - FieldInstantiationIndex, - MutBorrowFieldGeneric - ), - Call(_) => struct_bytecode!( - function_handles_len, - current_fdef, - bytecode_idx, - offset, - FunctionHandleIndex, - Call - ), - CallGeneric(_) => struct_bytecode!( - function_inst_len, - current_fdef, - bytecode_idx, - offset, - FunctionInstantiationIndex, - CallGeneric - ), - Pack(_) => struct_bytecode!( - struct_defs_len, - current_fdef, - bytecode_idx, - offset, - StructDefinitionIndex, - Pack - ), - PackGeneric(_) => struct_bytecode!( - struct_inst_len, - current_fdef, - bytecode_idx, - offset, - StructDefInstantiationIndex, - PackGeneric - ), - Unpack(_) => struct_bytecode!( - struct_defs_len, - current_fdef, - bytecode_idx, - offset, - StructDefinitionIndex, - Unpack - ), - UnpackGeneric(_) => struct_bytecode!( - struct_inst_len, - current_fdef, - bytecode_idx, - offset, - StructDefInstantiationIndex, - UnpackGeneric - ), - BrTrue(_) => { - code_bytecode!(code_len, current_fdef, bytecode_idx, offset, BrTrue) - } - BrFalse(_) => { - code_bytecode!(code_len, current_fdef, bytecode_idx, offset, BrFalse) - } - Branch(_) => { - code_bytecode!(code_len, current_fdef, bytecode_idx, offset, Branch) - } - CopyLoc(_) => { - locals_bytecode!(locals_len, current_fdef, bytecode_idx, offset, CopyLoc) - } - MoveLoc(_) => { - locals_bytecode!(locals_len, current_fdef, bytecode_idx, offset, MoveLoc) - } - StLoc(_) => { - locals_bytecode!(locals_len, current_fdef, bytecode_idx, offset, StLoc) - } - MutBorrowLoc(_) => locals_bytecode!( - locals_len, - current_fdef, - bytecode_idx, - offset, - MutBorrowLoc - ), - ImmBorrowLoc(_) => locals_bytecode!( - locals_len, - current_fdef, - bytecode_idx, - offset, - ImmBorrowLoc - ), - VecPack(_, num) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecPack, - num - ), - VecLen(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecLen - ), - VecImmBorrow(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecImmBorrow - ), - VecMutBorrow(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecMutBorrow - ), - VecPushBack(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecPushBack - ), - VecPopBack(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecPopBack - ), - VecUnpack(_, num) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecUnpack, - num - ), - VecSwap(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecSwap - ), - - // List out the other options explicitly so there's a compile error if a new - // bytecode gets added. - ExistsDeprecated(_) - | ExistsGenericDeprecated(_) - | MutBorrowGlobalDeprecated(_) - | MutBorrowGlobalGenericDeprecated(_) - | ImmBorrowGlobalDeprecated(_) - | ImmBorrowGlobalGenericDeprecated(_) - | MoveFromDeprecated(_) - | MoveFromGenericDeprecated(_) - | MoveToDeprecated(_) - | MoveToGenericDeprecated(_) => { - panic!("Bytecode deprecated: {:?}", code[bytecode_idx]) - } - FreezeRef | Pop | Ret | LdU8(_) | LdU16(_) | LdU32(_) | LdU64(_) - | LdU128(_) | LdU256(_) | CastU8 | CastU16 | CastU32 | CastU64 | CastU128 - | CastU256 | LdTrue | LdFalse | ReadRef | WriteRef | Add | Sub | Mul | Mod - | Div | BitOr | BitAnd | Xor | Shl | Shr | Or | And | Not | Eq | Neq | Lt - | Gt | Le | Ge | Abort | Nop => { - panic!("Bytecode has no internal index: {:?}", code[bytecode_idx]) - } - }; - - code[bytecode_idx] = new_bytecode; - - err.at_index(IndexKind::FunctionDefinition, fidx as TableIndex) - }) - .collect() - } -} - -fn is_interesting(bytecode: &Bytecode) -> bool { - use Bytecode::*; - - match bytecode { - LdConst(_) - | ImmBorrowField(_) - | ImmBorrowFieldGeneric(_) - | MutBorrowField(_) - | MutBorrowFieldGeneric(_) - | Call(_) - | CallGeneric(_) - | Pack(_) - | PackGeneric(_) - | Unpack(_) - | UnpackGeneric(_) - | BrTrue(_) - | BrFalse(_) - | Branch(_) - | CopyLoc(_) - | MoveLoc(_) - | StLoc(_) - | MutBorrowLoc(_) - | ImmBorrowLoc(_) - | VecPack(..) - | VecLen(_) - | VecImmBorrow(_) - | VecMutBorrow(_) - | VecPushBack(_) - | VecPopBack(_) - | VecUnpack(..) - | VecSwap(_) => true, - // Deprecated bytecodes - ExistsDeprecated(_) - | ExistsGenericDeprecated(_) - | MutBorrowGlobalDeprecated(_) - | MutBorrowGlobalGenericDeprecated(_) - | ImmBorrowGlobalDeprecated(_) - | ImmBorrowGlobalGenericDeprecated(_) - | MoveFromDeprecated(_) - | MoveFromGenericDeprecated(_) - | MoveToDeprecated(_) - | MoveToGenericDeprecated(_) => false, - // List out the other options explicitly so there's a compile error if a new - // bytecode gets added. - FreezeRef | Pop | Ret | LdU8(_) | LdU16(_) | LdU32(_) | LdU64(_) | LdU128(_) - | LdU256(_) | CastU8 | CastU16 | CastU32 | CastU64 | CastU128 | CastU256 | LdTrue - | LdFalse | ReadRef | WriteRef | Add | Sub | Mul | Mod | Div | BitOr | BitAnd | Xor - | Shl | Shr | Or | And | Not | Eq | Neq | Lt | Gt | Le | Ge | Abort | Nop => false, - } -} diff --git a/external-crates/move/move-execution/v1/crates/invalid-mutations/src/helpers.rs b/external-crates/move/move-execution/v1/crates/invalid-mutations/src/helpers.rs deleted file mode 100644 index c122d58c87fea..0000000000000 --- a/external-crates/move/move-execution/v1/crates/invalid-mutations/src/helpers.rs +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use proptest::sample::Index as PropIndex; -use std::{collections::BTreeSet, ops::Index as OpsIndex}; - -/// Given a maximum value `max` and a list of [`Index`](proptest::sample::Index) instances, picks -/// integers in the range `[0, max)` uniformly randomly and without duplication. -/// -/// If `indexes_len` is greater than `max`, all indexes will be returned. -/// -/// This function implements Robert Floyd's F2 algorithm for sampling without replacement. -pub(crate) fn pick_idxs(max: usize, indexes: &T, indexes_len: usize) -> Vec -where - T: OpsIndex + ?Sized, - P: AsRef, -{ - // See https://dl.acm.org/doi/10.1145/30401.315746 (the F2 algorithm) - // for a longer explanation. This is a variant that works with zero-indexing. - let mut selected = BTreeSet::new(); - let to_select = indexes_len.min(max); - for (iter_idx, choice) in ((max - to_select)..max).enumerate() { - // "RandInt(1, J)" in the original algorithm means a number between 1 - // and choice, both inclusive. `PropIndex::index` picks a number between 0 and - // whatever's passed in, with the latter exclusive. Pass in "+1" to ensure the same - // range of values is picked from. (This also ensures that if choice is 0 then `index` - // doesn't panic. - let idx = indexes[iter_idx].as_ref().index(choice + 1); - if !selected.insert(idx) { - selected.insert(choice); - } - } - selected.into_iter().collect() -} - -/// Given a maximum value `max` and a slice of [`Index`](proptest::sample::Index) instances, picks -/// integers in the range `[0, max)` uniformly randomly and without duplication. -/// -/// If the number of `Index` instances is greater than `max`, all indexes will be returned. -/// -/// This function implements Robert Floyd's F2 algorithm for sampling without replacement. -#[inline] -pub(crate) fn pick_slice_idxs(max: usize, indexes: &[impl AsRef]) -> Vec { - pick_idxs(max, indexes, indexes.len()) -} diff --git a/external-crates/move/move-execution/v1/crates/invalid-mutations/src/lib.rs b/external-crates/move/move-execution/v1/crates/invalid-mutations/src/lib.rs deleted file mode 100644 index d6574245153dd..0000000000000 --- a/external-crates/move/move-execution/v1/crates/invalid-mutations/src/lib.rs +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -#![forbid(unsafe_code)] - -pub mod bounds; -mod helpers; -pub mod signature; diff --git a/external-crates/move/move-execution/v1/crates/invalid-mutations/src/signature.rs b/external-crates/move/move-execution/v1/crates/invalid-mutations/src/signature.rs deleted file mode 100644 index 6181bf66d3ce6..0000000000000 --- a/external-crates/move/move-execution/v1/crates/invalid-mutations/src/signature.rs +++ /dev/null @@ -1,100 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::file_format::{ - CompiledModule, Signature, SignatureToken, StructFieldInformation, TypeSignature, -}; -use proptest::sample::Index as PropIndex; - -pub struct SignatureRefMutation<'a> { - module: &'a mut CompiledModule, - mutations: Vec<(PropIndex, PropIndex)>, -} - -impl<'a> SignatureRefMutation<'a> { - pub fn new(module: &'a mut CompiledModule, mutations: Vec<(PropIndex, PropIndex)>) -> Self { - Self { module, mutations } - } - - pub fn apply(self) -> bool { - if self.module.signatures.is_empty() { - return false; - } - let module = self.module; - let mut mutations = false; - for (s_idx, t_idx) in self.mutations { - let sig_idx = s_idx.index(module.signatures.len()); - let sig = &module.signatures[sig_idx]; - if sig.is_empty() { - continue; - } - let token_idx = t_idx.index(sig.len()); - let new_sig = mutate_sig(sig, token_idx); - module.signatures[sig_idx] = new_sig; - mutations = true; - } - mutations - } -} - -/// Context for applying a list of `FieldRefMutation` instances. -pub struct FieldRefMutation<'a> { - module: &'a mut CompiledModule, - mutations: Vec<(PropIndex, PropIndex)>, -} - -impl<'a> FieldRefMutation<'a> { - pub fn new(module: &'a mut CompiledModule, mutations: Vec<(PropIndex, PropIndex)>) -> Self { - Self { module, mutations } - } - - #[inline] - pub fn apply(self) -> bool { - if self.module.struct_defs.is_empty() { - return false; - } - let module = self.module; - let mut mutations = false; - for (s_idx, f_idx) in self.mutations { - let struct_idx = s_idx.index(module.struct_defs.len()); - let struct_def = &mut module.struct_defs[struct_idx]; - if let StructFieldInformation::Declared(fields) = &mut struct_def.field_information { - if fields.is_empty() { - continue; - } - let field_idx = f_idx.index(fields.len()); - let field_def = &mut fields[field_idx]; - let new_ty = mutate_field(&field_def.signature.0); - fields[field_idx].signature = TypeSignature(new_ty); - mutations = true; - } - } - mutations - } -} - -fn mutate_sig(sig: &Signature, token_idx: usize) -> Signature { - use SignatureToken::*; - - Signature( - sig.0 - .iter() - .enumerate() - .map(|(idx, token)| { - if idx == token_idx { - match &token { - Reference(_) | MutableReference(_) => Reference(Box::new(token.clone())), - _ => Reference(Box::new(Reference(Box::new(token.clone())))), - } - } else { - token.clone() - } - }) - .collect(), - ) -} - -fn mutate_field(token: &SignatureToken) -> SignatureToken { - SignatureToken::Reference(Box::new(token.clone())) -} diff --git a/external-crates/move/move-execution/v1/crates/move-bytecode-verifier/Cargo.toml b/external-crates/move/move-execution/v1/crates/move-bytecode-verifier/Cargo.toml index 9c1c7c13a2de9..ab15208c133a4 100644 --- a/external-crates/move/move-execution/v1/crates/move-bytecode-verifier/Cargo.toml +++ b/external-crates/move/move-execution/v1/crates/move-bytecode-verifier/Cargo.toml @@ -21,7 +21,6 @@ move-abstract-stack.workspace = true [dev-dependencies] hex-literal.workspace = true -invalid-mutations = { path = "../invalid-mutations", package = "invalid-mutations-v1" } [features] default = [] diff --git a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/Cargo.toml b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/Cargo.toml index 7eddaa6e7fd4c..cd0e294320e14 100644 --- a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/Cargo.toml +++ b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/Cargo.toml @@ -11,12 +11,11 @@ edition = "2021" [dev-dependencies] petgraph.workspace = true -proptest.workspace = true fail = { workspace = true, features = ["failpoints"] } hex.workspace = true -invalid-mutations = { path = "../invalid-mutations", package = "invalid-mutations-v2" } move-binary-format = { workspace = true, features = ["fuzzing"] } +# referred to via path for execution versioning move-bytecode-verifier = { path = "../move-bytecode-verifier", package = "move-bytecode-verifier-v2" } move-bytecode-verifier-meter.workspace = true move-core-types.workspace = true @@ -24,5 +23,3 @@ move-vm-config.workspace = true [features] fuzzing = ["move-binary-format/fuzzing"] - -[dependencies] diff --git a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs deleted file mode 100644 index 08bbdca20c0dc..0000000000000 --- a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/ability_field_requirements_tests.rs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::file_format::CompiledModule; -use move_bytecode_verifier::ability_field_requirements; -use proptest::prelude::*; - -proptest! { - #[test] - fn valid_ability_transitivity(module in CompiledModule::valid_strategy(20)) { - prop_assert!(ability_field_requirements::verify_module(&module).is_ok()); - } -} diff --git a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs index 4ca73841dcd61..905bc6540ba44 100644 --- a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs +++ b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/bounds_tests.rs @@ -2,18 +2,8 @@ // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use invalid_mutations::bounds::{ - ApplyCodeUnitBoundsContext, ApplyOutOfBoundsContext, CodeUnitBoundsMutation, - OutOfBoundsMutation, -}; -use move_binary_format::{ - check_bounds::BoundsChecker, file_format::*, file_format_common, - proptest_types::CompiledModuleStrategyGen, -}; -use move_core_types::{ - account_address::AccountAddress, identifier::Identifier, vm_status::StatusCode, -}; -use proptest::{collection::vec, prelude::*}; +use move_binary_format::{check_bounds::BoundsChecker, file_format::*, file_format_common}; +use move_core_types::vm_status::StatusCode; #[test] fn empty_module_no_errors() { @@ -296,84 +286,3 @@ fn invalid_type_param_for_vector_operation() { ); } } - -proptest! { - #[test] - fn valid_bounds(_module in CompiledModule::valid_strategy(20)) { - // valid_strategy will panic if there are any bounds check issues. - } -} - -/// Ensure that valid modules that don't have any members (e.g. function args, struct fields) pass -/// bounds checks. -/// -/// There are some potentially tricky edge cases around ranges that are captured here. -#[test] -fn valid_bounds_no_members() { - let mut gen = CompiledModuleStrategyGen::new(20); - gen.zeros_all(); - proptest!(|(_module in gen.generate())| { - // gen.generate() will panic if there are any bounds check issues. - }); -} - -proptest! { - #[test] - fn invalid_out_of_bounds( - module in CompiledModule::valid_strategy(20), - oob_mutations in vec(OutOfBoundsMutation::strategy(), 0..40), - ) { - let (module, expected_violations) = { - let oob_context = ApplyOutOfBoundsContext::new(module, oob_mutations); - oob_context.apply() - }; - - let actual_violations = BoundsChecker::verify_module(&module); - prop_assert_eq!(expected_violations.is_empty(), actual_violations.is_ok()); - } - - #[test] - fn code_unit_out_of_bounds( - mut module in CompiledModule::valid_strategy(20), - mutations in vec(CodeUnitBoundsMutation::strategy(), 0..40), - ) { - let expected_violations = { - let context = ApplyCodeUnitBoundsContext::new(&mut module, mutations); - context.apply() - }; - - let actual_violations = BoundsChecker::verify_module(&module); - prop_assert_eq!(expected_violations.is_empty(), actual_violations.is_ok()); - } - - #[test] - fn no_module_handles( - identifiers in vec(any::(), 0..20), - address_identifiers in vec(any::(), 0..20), - ) { - // If there are no module handles, the only other things that can be stored are intrinsic - // data. - let module = CompiledModule { - identifiers, - address_identifiers, - ..Default::default() - }; - - prop_assert_eq!( - BoundsChecker::verify_module(&module).map_err(|e| e.major_status()), - Err(StatusCode::NO_MODULE_HANDLES) - ); - } -} - -proptest! { - // Generating arbitrary compiled modules is really slow, possibly because of - // https://github.com/AltSysrq/proptest/issues/143. - #![proptest_config(ProptestConfig::with_cases(16))] - - /// Make sure that garbage inputs don't crash the bounds checker. - #[test] - fn garbage_inputs(module in any_with::(16)) { - let _ = BoundsChecker::verify_module(&module); - } -} diff --git a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs index b446d72e413f7..653654119714d 100644 --- a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs +++ b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/constants_tests.rs @@ -1,17 +1,9 @@ // Copyright (c) The Diem Core Contributors // Copyright (c) The Move Contributors // SPDX-License-Identifier: Apache-2.0 -use move_binary_format::file_format::{empty_module, CompiledModule, Constant, SignatureToken}; +use move_binary_format::file_format::{empty_module, Constant, SignatureToken}; use move_bytecode_verifier::constants; use move_core_types::vm_status::StatusCode; -use proptest::prelude::*; - -proptest! { - #[test] - fn valid_generated(module in CompiledModule::valid_strategy(20)) { - prop_assert!(constants::verify_module(&module).is_ok()); - } -} #[test] fn valid_primitives() { diff --git a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs index 640af70c6638e..da0d1fbeb6bb2 100644 --- a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs +++ b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/duplication_tests.rs @@ -4,7 +4,6 @@ use move_binary_format::file_format::*; use move_bytecode_verifier::DuplicationChecker; -use proptest::prelude::*; #[test] fn duplicated_friend_decls() { @@ -17,10 +16,3 @@ fn duplicated_friend_decls() { m.friend_decls.push(handle); DuplicationChecker::verify_module(&m).unwrap_err(); } - -proptest! { - #[test] - fn valid_duplication(module in CompiledModule::valid_strategy(20)) { - prop_assert!(DuplicationChecker::verify_module(&module).is_ok()); - } -} diff --git a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/mod.rs b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/mod.rs index 65aa86c275656..3238641a6e76e 100644 --- a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/mod.rs +++ b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/mod.rs @@ -6,7 +6,6 @@ use move_vm_config::verifier::{ MeterConfig, VerifierConfig, DEFAULT_MAX_CONSTANT_VECTOR_LEN, DEFAULT_MAX_IDENTIFIER_LENGTH, }; -pub mod ability_field_requirements_tests; pub mod binary_samples; pub mod bounds_tests; pub mod code_unit_tests; @@ -19,11 +18,9 @@ pub mod limit_tests; pub mod locals; pub mod loop_summary_tests; pub mod many_back_edges; -pub mod multi_pass_tests; pub mod negative_stack_size_tests; pub mod reference_safety_tests; pub mod signature_tests; -pub mod struct_defs_tests; pub mod vec_pack_tests; /// Configuration used in production. diff --git a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs deleted file mode 100644 index d244f6dd37e5a..0000000000000 --- a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/multi_pass_tests.rs +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::CompiledModule; -use move_bytecode_verifier::{ - ability_field_requirements, constants, instantiation_loops::InstantiationLoopChecker, - DuplicationChecker, InstructionConsistency, RecursiveStructDefChecker, SignatureChecker, -}; -use proptest::prelude::*; - -proptest! { - #[test] - fn check_verifier_passes(module in CompiledModule::valid_strategy(20)) { - DuplicationChecker::verify_module(&module).expect("DuplicationChecker failure"); - SignatureChecker::verify_module(&module).expect("SignatureChecker failure"); - InstructionConsistency::verify_module(&module).expect("InstructionConsistency failure"); - constants::verify_module(&module).expect("constants failure"); - ability_field_requirements::verify_module(&module).expect("ability_field_requirements failure"); - RecursiveStructDefChecker::verify_module(&module).expect("RecursiveStructDefChecker failure"); - InstantiationLoopChecker::verify_module(&module).expect("InstantiationLoopChecker failure"); - } -} diff --git a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs index a57c18530bb1c..41d2d4e541081 100644 --- a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs +++ b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/signature_tests.rs @@ -3,7 +3,6 @@ // SPDX-License-Identifier: Apache-2.0 use crate::unit_tests::production_config; -use invalid_mutations::signature::{FieldRefMutation, SignatureRefMutation}; use move_binary_format::file_format::{ Bytecode::*, CompiledModule, SignatureToken::*, Visibility::Public, *, }; @@ -14,7 +13,6 @@ use move_bytecode_verifier_meter::dummy::DummyMeter; use move_core_types::{ account_address::AccountAddress, identifier::Identifier, vm_status::StatusCode, }; -use proptest::{collection::vec, prelude::*, sample::Index as PropIndex}; #[test] fn test_reference_of_reference() { @@ -26,39 +24,6 @@ fn test_reference_of_reference() { assert!(errors.is_err()); } -proptest! { - #[test] - fn valid_signatures(module in CompiledModule::valid_strategy(20)) { - prop_assert!(SignatureChecker::verify_module(&module).is_ok()) - } - - #[test] - fn double_refs( - mut module in CompiledModule::valid_strategy(20), - mutations in vec((any::(), any::()), 0..20), - ) { - let context = SignatureRefMutation::new(&mut module, mutations); - let expected_violations = context.apply(); - - let result = SignatureChecker::verify_module(&module); - - prop_assert_eq!(expected_violations, result.is_err()); - } - - #[test] - fn field_def_references( - mut module in CompiledModule::valid_strategy(20), - mutations in vec((any::(), any::()), 0..40), - ) { - let context = FieldRefMutation::new(&mut module, mutations); - let expected_violations = context.apply(); - - let result = SignatureChecker::verify_module(&module); - - prop_assert_eq!(expected_violations, result.is_err()); - } -} - #[test] fn no_verify_locals_good() { let compiled_module_good = CompiledModule { diff --git a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs b/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs deleted file mode 100644 index 71fdc466003f8..0000000000000 --- a/external-crates/move/move-execution/v2/crates/bytecode-verifier-tests/src/unit_tests/struct_defs_tests.rs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::file_format::CompiledModule; -use move_bytecode_verifier::RecursiveStructDefChecker; -use proptest::prelude::*; - -proptest! { - #[test] - fn valid_recursive_struct_defs(module in CompiledModule::valid_strategy(20)) { - prop_assert!(RecursiveStructDefChecker::verify_module(&module).is_ok()); - } -} diff --git a/external-crates/move/move-execution/v2/crates/invalid-mutations/Cargo.toml b/external-crates/move/move-execution/v2/crates/invalid-mutations/Cargo.toml deleted file mode 100644 index 15eceaee7579b..0000000000000 --- a/external-crates/move/move-execution/v2/crates/invalid-mutations/Cargo.toml +++ /dev/null @@ -1,18 +0,0 @@ -[package] -name = "invalid-mutations-v2" -version = "0.1.0" -edition = "2021" -authors = ["Diem Association "] -description = "Diem invalid mutations" -repository = "https://github.com/diem/diem" -homepage = "https://diem.com" -license = "Apache-2.0" -publish = false - -[dependencies] -move-core-types.workspace = true -move-binary-format.workspace = true -proptest.workspace = true - -[features] -default = [] diff --git a/external-crates/move/move-execution/v2/crates/invalid-mutations/src/bounds.rs b/external-crates/move/move-execution/v2/crates/invalid-mutations/src/bounds.rs deleted file mode 100644 index c6784e3f47a78..0000000000000 --- a/external-crates/move/move-execution/v2/crates/invalid-mutations/src/bounds.rs +++ /dev/null @@ -1,364 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::{ - errors::{bounds_error, PartialVMError}, - file_format::{ - AddressIdentifierIndex, CompiledModule, FunctionHandleIndex, IdentifierIndex, - ModuleHandleIndex, SignatureIndex, StructDefinitionIndex, StructHandleIndex, TableIndex, - }, - internals::ModuleIndex, - IndexKind, -}; -use move_core_types::vm_status::StatusCode; -use proptest::{ - prelude::*, - sample::{self, Index as PropIndex}, -}; -use std::collections::BTreeMap; - -mod code_unit; -pub use code_unit::{ApplyCodeUnitBoundsContext, CodeUnitBoundsMutation}; -use move_binary_format::file_format::SignatureToken; - -/// Represents the number of pointers that exist out from a node of a particular kind. -#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] -pub enum PointerKind { - /// Exactly one pointer out with this index kind as its destination. - One(IndexKind), - /// Zero or one pointer out with this index kind as its destination. Like the `?` operator in - /// regular expressions. - Optional(IndexKind), - /// Zero or more pointers out with this index kind as its destination. Like the `*` operator - /// in regular expressions. - Star(IndexKind), -} - -impl PointerKind { - /// A list of what pointers (indexes) exist out from a particular kind of node within the - /// module. - /// - /// The only special case is `FunctionDefinition`, which contains a `CodeUnit` that can contain - /// one of several kinds of pointers out. That is not represented in this table. - #[inline] - pub fn pointers_from(src_kind: IndexKind) -> &'static [PointerKind] { - use IndexKind::*; - use PointerKind::*; - - match src_kind { - ModuleHandle => &[One(AddressIdentifier), One(Identifier)], - StructHandle => &[One(ModuleHandle), One(Identifier)], - FunctionHandle => &[ - One(ModuleHandle), - One(Identifier), - One(Signature), - One(Signature), - ], - StructDefinition => &[One(StructHandle), Star(StructHandle)], - FunctionDefinition => &[One(FunctionHandle), One(Signature)], - FriendDeclaration => &[One(AddressIdentifier), One(Identifier)], - Signature => &[Star(StructHandle)], - FieldHandle => &[One(StructDefinition)], - _ => &[], - } - } - - #[inline] - pub fn to_index_kind(self) -> IndexKind { - match self { - PointerKind::One(idx) | PointerKind::Optional(idx) | PointerKind::Star(idx) => idx, - } - } -} - -pub static VALID_POINTER_SRCS: &[IndexKind] = &[ - IndexKind::ModuleHandle, - IndexKind::StructHandle, - IndexKind::FunctionHandle, - IndexKind::FieldHandle, - IndexKind::StructDefinition, - IndexKind::FunctionDefinition, - IndexKind::FriendDeclaration, - IndexKind::Signature, -]; - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn pointer_kind_sanity() { - for variant in IndexKind::variants() { - if VALID_POINTER_SRCS.iter().any(|x| x == variant) { - assert!( - !PointerKind::pointers_from(*variant).is_empty(), - "expected variant {:?} to be a valid pointer source", - variant, - ); - } else { - assert!( - PointerKind::pointers_from(*variant).is_empty(), - "expected variant {:?} to not be a valid pointer source", - variant, - ); - } - } - } -} - -/// Represents a single mutation to a `CompiledModule` to produce an out-of-bounds situation. -/// -/// Use `OutOfBoundsMutation::strategy()` to generate them, preferably using `Vec` to generate -/// many at a time. Then use `ApplyOutOfBoundsContext` to apply those mutations. -#[derive(Debug)] -pub struct OutOfBoundsMutation { - src_kind: IndexKind, - src_idx: PropIndex, - dst_kind: IndexKind, - offset: usize, -} - -impl OutOfBoundsMutation { - pub fn strategy() -> impl Strategy { - ( - Self::src_kind_strategy(), - any::(), - any::(), - 0..16_usize, - ) - .prop_map(|(src_kind, src_idx, dst_kind_idx, offset)| { - let dst_kind = Self::dst_kind(src_kind, dst_kind_idx); - Self { - src_kind, - src_idx, - dst_kind, - offset, - } - }) - } - - // Not all source kinds can be made to be out of bounds (e.g. inherent types can't.) - fn src_kind_strategy() -> impl Strategy { - sample::select(VALID_POINTER_SRCS) - } - - fn dst_kind(src_kind: IndexKind, dst_kind_idx: PropIndex) -> IndexKind { - dst_kind_idx - .get(PointerKind::pointers_from(src_kind)) - .to_index_kind() - } -} - -/// This is used for source indexing, to work with pick_slice_idxs. -impl AsRef for OutOfBoundsMutation { - #[inline] - fn as_ref(&self) -> &PropIndex { - &self.src_idx - } -} - -pub struct ApplyOutOfBoundsContext { - module: CompiledModule, - // This is an Option because it gets moved out in apply before apply_one is called. Rust - // doesn't let you call another con-consuming method after a partial move out. - mutations: Option>, - - // Some precomputations done for signatures. - sig_structs: Vec<(SignatureIndex, usize)>, -} - -impl ApplyOutOfBoundsContext { - pub fn new(module: CompiledModule, mutations: Vec) -> Self { - let sig_structs: Vec<_> = Self::sig_structs(&module).collect(); - - Self { - module, - mutations: Some(mutations), - sig_structs, - } - } - - pub fn apply(mut self) -> (CompiledModule, Vec) { - // This is a map from (source kind, dest kind) to the actual mutations -- this is done to - // figure out how many mutations to do for a particular pair, which is required for - // pick_slice_idxs below. - let mut mutation_map = BTreeMap::new(); - for mutation in self - .mutations - .take() - .expect("mutations should always be present") - { - mutation_map - .entry((mutation.src_kind, mutation.dst_kind)) - .or_insert_with(Vec::new) - .push(mutation); - } - - let mut results = vec![]; - - for ((src_kind, dst_kind), mutations) in mutation_map { - // It would be cool to use an iterator here, if someone could figure out exactly how - // to get the lifetimes right :) - results.extend(self.apply_one(src_kind, dst_kind, mutations)); - } - (self.module, results) - } - - fn apply_one( - &mut self, - src_kind: IndexKind, - dst_kind: IndexKind, - mutations: Vec, - ) -> Vec { - let src_count = match src_kind { - IndexKind::Signature => self.sig_structs.len(), - // For the other sorts it's always possible to change an index. - src_kind => self.module.kind_count(src_kind), - }; - // Any signature can be a destination, not just the ones that have structs in them. - let dst_count = self.module.kind_count(dst_kind); - let to_mutate = crate::helpers::pick_slice_idxs(src_count, &mutations); - - mutations - .iter() - .zip(to_mutate) - .map(move |(mutation, src_idx)| { - self.set_index( - src_kind, - src_idx, - dst_kind, - dst_count, - (dst_count + mutation.offset) as TableIndex, - ) - }) - .collect() - } - - /// Sets the particular index in the table - /// - /// For example, with `src_kind` set to `ModuleHandle` and `dst_kind` set to `AddressPool`, - /// this will set self.module_handles[src_idx].address to new_idx. - /// - /// This is mainly used for test generation. - fn set_index( - &mut self, - src_kind: IndexKind, - src_idx: usize, - dst_kind: IndexKind, - dst_count: usize, - new_idx: TableIndex, - ) -> PartialVMError { - use IndexKind::*; - - // These are default values, but some of the match arms below mutate them. - let mut src_idx = src_idx; - let err = bounds_error( - StatusCode::INDEX_OUT_OF_BOUNDS, - dst_kind, - new_idx, - dst_count, - ); - - // A dynamic type system would be able to express this next block of code far more - // concisely. A static type system would require some sort of complicated dependent type - // structure that Rust doesn't have. As things stand today, every possible case needs to - // be listed out. - - match (src_kind, dst_kind) { - (ModuleHandle, AddressIdentifier) => { - self.module.module_handles[src_idx].address = AddressIdentifierIndex(new_idx) - } - (ModuleHandle, Identifier) => { - self.module.module_handles[src_idx].name = IdentifierIndex(new_idx) - } - (StructHandle, ModuleHandle) => { - self.module.struct_handles[src_idx].module = ModuleHandleIndex(new_idx) - } - (StructHandle, Identifier) => { - self.module.struct_handles[src_idx].name = IdentifierIndex(new_idx) - } - (FunctionHandle, ModuleHandle) => { - self.module.function_handles[src_idx].module = ModuleHandleIndex(new_idx) - } - (FunctionHandle, Identifier) => { - self.module.function_handles[src_idx].name = IdentifierIndex(new_idx) - } - (FunctionHandle, Signature) => { - self.module.function_handles[src_idx].parameters = SignatureIndex(new_idx) - } - (StructDefinition, StructHandle) => { - self.module.struct_defs[src_idx].struct_handle = StructHandleIndex(new_idx) - } - (FunctionDefinition, FunctionHandle) => { - self.module.function_defs[src_idx].function = FunctionHandleIndex(new_idx) - } - (FunctionDefinition, Signature) => { - self.module.function_defs[src_idx] - .code - .as_mut() - .unwrap() - .locals = SignatureIndex(new_idx) - } - (Signature, StructHandle) => { - let (actual_src_idx, arg_idx) = self.sig_structs[src_idx]; - src_idx = actual_src_idx.into_index(); - self.module.signatures[src_idx].0[arg_idx] - .debug_set_sh_idx(StructHandleIndex(new_idx)); - } - (FieldHandle, StructDefinition) => { - self.module.field_handles[src_idx].owner = StructDefinitionIndex(new_idx) - } - (FriendDeclaration, AddressIdentifier) => { - self.module.friend_decls[src_idx].address = AddressIdentifierIndex(new_idx) - } - (FriendDeclaration, Identifier) => { - self.module.friend_decls[src_idx].name = IdentifierIndex(new_idx) - } - _ => panic!("Invalid pointer kind: {:?} -> {:?}", src_kind, dst_kind), - } - - err.at_index(src_kind, src_idx as TableIndex) - } - - /// Returns the indexes of locals signatures that contain struct handles inside them. - fn sig_structs(module: &CompiledModule) -> impl Iterator + '_ { - module - .signatures() - .iter() - .enumerate() - .flat_map(|(idx, signature)| { - let idx = SignatureIndex(idx as u16); - Self::find_struct_tokens(&signature.0, move |arg_idx| (idx, arg_idx)) - }) - } - - #[inline] - fn find_struct_tokens<'b, F, T>( - tokens: impl IntoIterator + 'b, - map_fn: F, - ) -> impl Iterator + 'b - where - F: Fn(usize) -> T + 'b, - { - tokens - .into_iter() - .enumerate() - .filter_map(move |(arg_idx, token)| struct_handle(token).map(|_| map_fn(arg_idx))) - } -} - -fn struct_handle(token: &SignatureToken) -> Option { - use SignatureToken::*; - - match token { - Struct(sh_idx) => Some(*sh_idx), - StructInstantiation(struct_inst) => { - let (sh_idx, _) = &**struct_inst; - Some(*sh_idx) - } - Reference(token) | MutableReference(token) => struct_handle(token), - Bool | U8 | U16 | U32 | U64 | U128 | U256 | Address | Signer | Vector(_) - | TypeParameter(_) => None, - } -} diff --git a/external-crates/move/move-execution/v2/crates/invalid-mutations/src/bounds/code_unit.rs b/external-crates/move/move-execution/v2/crates/invalid-mutations/src/bounds/code_unit.rs deleted file mode 100644 index bbbc500c0ebd0..0000000000000 --- a/external-crates/move/move-execution/v2/crates/invalid-mutations/src/bounds/code_unit.rs +++ /dev/null @@ -1,479 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::{ - errors::{offset_out_of_bounds, PartialVMError}, - file_format::{ - Bytecode, CodeOffset, CompiledModule, ConstantPoolIndex, FieldHandleIndex, - FieldInstantiationIndex, FunctionDefinitionIndex, FunctionHandleIndex, - FunctionInstantiationIndex, LocalIndex, SignatureIndex, StructDefInstantiationIndex, - StructDefinitionIndex, TableIndex, - }, - internals::ModuleIndex, - IndexKind, -}; -use move_core_types::vm_status::StatusCode; -use proptest::{prelude::*, sample::Index as PropIndex}; -use std::collections::BTreeMap; - -/// Represents a single mutation onto a code unit to make it out of bounds. -#[derive(Debug)] -pub struct CodeUnitBoundsMutation { - function_def: PropIndex, - bytecode: PropIndex, - offset: usize, -} - -impl CodeUnitBoundsMutation { - pub fn strategy() -> impl Strategy { - (any::(), any::(), 0..16_usize).prop_map( - |(function_def, bytecode, offset)| Self { - function_def, - bytecode, - offset, - }, - ) - } -} - -impl AsRef for CodeUnitBoundsMutation { - #[inline] - fn as_ref(&self) -> &PropIndex { - &self.bytecode - } -} - -pub struct ApplyCodeUnitBoundsContext<'a> { - module: &'a mut CompiledModule, - // This is so apply_one can be called after mutations has been iterated on. - mutations: Option>, -} - -macro_rules! new_bytecode { - ($dst_len:expr, $fidx:expr, $bcidx:expr, $offset:expr, $kind:ident, $bytecode_ident:tt) => {{ - let dst_len: usize = $dst_len; - let new_idx: usize = dst_len + $offset; - ( - $bytecode_ident($kind::new(new_idx as TableIndex)), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - $kind::KIND, - new_idx, - dst_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; - - ($dst_len:expr, $fidx:expr, $bcidx:expr, $offset:expr, $kind:ident, $bytecode_ident:tt, $($others:expr),+) => {{ - let dst_len: usize = $dst_len; - let new_idx: usize = dst_len + $offset; - ( - $bytecode_ident($kind::new(new_idx as TableIndex), $($others),+), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - $kind::KIND, - new_idx, - dst_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; -} - -macro_rules! struct_bytecode { - ($dst_len: expr, $fidx:expr, $bcidx: expr, $offset: expr, $idx_type: ident, $bytecode_ident: tt) => {{ - let dst_len = $dst_len; - let new_idx = dst_len + $offset; - ( - $bytecode_ident($idx_type::new(new_idx as TableIndex)), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - $idx_type::KIND, - new_idx, - dst_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; -} - -macro_rules! code_bytecode { - ($code_len: expr, $fidx:expr, $bcidx: expr, $offset: expr, $bytecode_ident: tt) => {{ - let code_len = $code_len; - let new_idx = code_len + $offset; - ( - $bytecode_ident(new_idx as CodeOffset), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - IndexKind::CodeDefinition, - new_idx, - code_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; -} - -macro_rules! locals_bytecode { - ($locals_len: expr, $fidx:expr, $bcidx: expr, $offset: expr, $bytecode_ident: tt) => {{ - let locals_len = $locals_len; - let new_idx = locals_len + $offset; - ( - $bytecode_ident(new_idx as LocalIndex), - offset_out_of_bounds( - StatusCode::INDEX_OUT_OF_BOUNDS, - IndexKind::LocalPool, - new_idx, - locals_len, - $fidx, - $bcidx as CodeOffset, - ), - ) - }}; -} - -impl<'a> ApplyCodeUnitBoundsContext<'a> { - pub fn new(module: &'a mut CompiledModule, mutations: Vec) -> Self { - Self { - module, - mutations: Some(mutations), - } - } - - pub fn apply(mut self) -> Vec { - let function_def_len = self.module.function_defs.len(); - - let mut mutation_map = BTreeMap::new(); - for mutation in self - .mutations - .take() - .expect("mutations should always be present") - { - let picked_idx = mutation.function_def.index(function_def_len); - mutation_map - .entry(picked_idx) - .or_insert_with(Vec::new) - .push(mutation); - } - - let mut results = vec![]; - - for (idx, mutations) in mutation_map { - results.extend(self.apply_one(idx, mutations)); - } - results - } - - fn apply_one( - &mut self, - fidx: usize, - mutations: Vec, - ) -> Vec { - // For this function def, find all the places where a bounds mutation can be applied. - let func_def = &mut self.module.function_defs[fidx]; - let current_fdef = FunctionDefinitionIndex(fidx as TableIndex); - let func_handle = &self.module.function_handles[func_def.function.into_index()]; - let code = func_def.code.as_mut().unwrap(); - let locals_len = self.module.signatures[func_handle.parameters.into_index()].len() - + self.module.signatures[code.locals.into_index()].len(); - let code = &mut code.code; - let code_len = code.len(); - - let interesting_offsets: Vec = (0..code.len()) - .filter(|bytecode_idx| is_interesting(&code[*bytecode_idx])) - .collect(); - let to_mutate = crate::helpers::pick_slice_idxs(interesting_offsets.len(), &mutations); - - // These have to be computed upfront because self.module is being mutated below. - let constant_pool_len = self.module.constant_pool.len(); - let function_handles_len = self.module.function_handles.len(); - let field_handle_len = self.module.field_handles.len(); - let struct_defs_len = self.module.struct_defs.len(); - let struct_inst_len = self.module.struct_def_instantiations.len(); - let function_inst_len = self.module.function_instantiations.len(); - let field_inst_len = self.module.field_instantiations.len(); - let signature_pool_len = self.module.signatures.len(); - - mutations - .iter() - .zip(to_mutate) - .map(|(mutation, interesting_offsets_idx)| { - let bytecode_idx = interesting_offsets[interesting_offsets_idx]; - let offset = mutation.offset; - use Bytecode::*; - - let (new_bytecode, err) = match code[bytecode_idx] { - LdConst(_) => new_bytecode!( - constant_pool_len, - current_fdef, - bytecode_idx, - offset, - ConstantPoolIndex, - LdConst - ), - ImmBorrowField(_) => new_bytecode!( - field_handle_len, - current_fdef, - bytecode_idx, - offset, - FieldHandleIndex, - ImmBorrowField - ), - ImmBorrowFieldGeneric(_) => new_bytecode!( - field_inst_len, - current_fdef, - bytecode_idx, - offset, - FieldInstantiationIndex, - ImmBorrowFieldGeneric - ), - MutBorrowField(_) => new_bytecode!( - field_handle_len, - current_fdef, - bytecode_idx, - offset, - FieldHandleIndex, - MutBorrowField - ), - MutBorrowFieldGeneric(_) => new_bytecode!( - field_inst_len, - current_fdef, - bytecode_idx, - offset, - FieldInstantiationIndex, - MutBorrowFieldGeneric - ), - Call(_) => struct_bytecode!( - function_handles_len, - current_fdef, - bytecode_idx, - offset, - FunctionHandleIndex, - Call - ), - CallGeneric(_) => struct_bytecode!( - function_inst_len, - current_fdef, - bytecode_idx, - offset, - FunctionInstantiationIndex, - CallGeneric - ), - Pack(_) => struct_bytecode!( - struct_defs_len, - current_fdef, - bytecode_idx, - offset, - StructDefinitionIndex, - Pack - ), - PackGeneric(_) => struct_bytecode!( - struct_inst_len, - current_fdef, - bytecode_idx, - offset, - StructDefInstantiationIndex, - PackGeneric - ), - Unpack(_) => struct_bytecode!( - struct_defs_len, - current_fdef, - bytecode_idx, - offset, - StructDefinitionIndex, - Unpack - ), - UnpackGeneric(_) => struct_bytecode!( - struct_inst_len, - current_fdef, - bytecode_idx, - offset, - StructDefInstantiationIndex, - UnpackGeneric - ), - BrTrue(_) => { - code_bytecode!(code_len, current_fdef, bytecode_idx, offset, BrTrue) - } - BrFalse(_) => { - code_bytecode!(code_len, current_fdef, bytecode_idx, offset, BrFalse) - } - Branch(_) => { - code_bytecode!(code_len, current_fdef, bytecode_idx, offset, Branch) - } - CopyLoc(_) => { - locals_bytecode!(locals_len, current_fdef, bytecode_idx, offset, CopyLoc) - } - MoveLoc(_) => { - locals_bytecode!(locals_len, current_fdef, bytecode_idx, offset, MoveLoc) - } - StLoc(_) => { - locals_bytecode!(locals_len, current_fdef, bytecode_idx, offset, StLoc) - } - MutBorrowLoc(_) => locals_bytecode!( - locals_len, - current_fdef, - bytecode_idx, - offset, - MutBorrowLoc - ), - ImmBorrowLoc(_) => locals_bytecode!( - locals_len, - current_fdef, - bytecode_idx, - offset, - ImmBorrowLoc - ), - VecPack(_, num) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecPack, - num - ), - VecLen(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecLen - ), - VecImmBorrow(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecImmBorrow - ), - VecMutBorrow(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecMutBorrow - ), - VecPushBack(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecPushBack - ), - VecPopBack(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecPopBack - ), - VecUnpack(_, num) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecUnpack, - num - ), - VecSwap(_) => new_bytecode!( - signature_pool_len, - current_fdef, - bytecode_idx, - offset, - SignatureIndex, - VecSwap - ), - - // List out the other options explicitly so there's a compile error if a new - // bytecode gets added. - ExistsDeprecated(_) - | ExistsGenericDeprecated(_) - | MutBorrowGlobalDeprecated(_) - | MutBorrowGlobalGenericDeprecated(_) - | ImmBorrowGlobalDeprecated(_) - | ImmBorrowGlobalGenericDeprecated(_) - | MoveFromDeprecated(_) - | MoveFromGenericDeprecated(_) - | MoveToDeprecated(_) - | MoveToGenericDeprecated(_) => { - panic!("Bytecode deprecated: {:?}", code[bytecode_idx]) - } - FreezeRef | Pop | Ret | LdU8(_) | LdU16(_) | LdU32(_) | LdU64(_) - | LdU128(_) | LdU256(_) | CastU8 | CastU16 | CastU32 | CastU64 | CastU128 - | CastU256 | LdTrue | LdFalse | ReadRef | WriteRef | Add | Sub | Mul | Mod - | Div | BitOr | BitAnd | Xor | Shl | Shr | Or | And | Not | Eq | Neq | Lt - | Gt | Le | Ge | Abort | Nop => { - panic!("Bytecode has no internal index: {:?}", code[bytecode_idx]) - } - }; - - code[bytecode_idx] = new_bytecode; - - err.at_index(IndexKind::FunctionDefinition, fidx as TableIndex) - }) - .collect() - } -} - -fn is_interesting(bytecode: &Bytecode) -> bool { - use Bytecode::*; - - match bytecode { - LdConst(_) - | ImmBorrowField(_) - | ImmBorrowFieldGeneric(_) - | MutBorrowField(_) - | MutBorrowFieldGeneric(_) - | Call(_) - | CallGeneric(_) - | Pack(_) - | PackGeneric(_) - | Unpack(_) - | UnpackGeneric(_) - | BrTrue(_) - | BrFalse(_) - | Branch(_) - | CopyLoc(_) - | MoveLoc(_) - | StLoc(_) - | MutBorrowLoc(_) - | ImmBorrowLoc(_) - | VecPack(..) - | VecLen(_) - | VecImmBorrow(_) - | VecMutBorrow(_) - | VecPushBack(_) - | VecPopBack(_) - | VecUnpack(..) - | VecSwap(_) => true, - // Deprecated bytecodes - ExistsDeprecated(_) - | ExistsGenericDeprecated(_) - | MutBorrowGlobalDeprecated(_) - | MutBorrowGlobalGenericDeprecated(_) - | ImmBorrowGlobalDeprecated(_) - | ImmBorrowGlobalGenericDeprecated(_) - | MoveFromDeprecated(_) - | MoveFromGenericDeprecated(_) - | MoveToDeprecated(_) - | MoveToGenericDeprecated(_) => false, - // List out the other options explicitly so there's a compile error if a new - // bytecode gets added. - FreezeRef | Pop | Ret | LdU8(_) | LdU16(_) | LdU32(_) | LdU64(_) | LdU128(_) - | LdU256(_) | CastU8 | CastU16 | CastU32 | CastU64 | CastU128 | CastU256 | LdTrue - | LdFalse | ReadRef | WriteRef | Add | Sub | Mul | Mod | Div | BitOr | BitAnd | Xor - | Shl | Shr | Or | And | Not | Eq | Neq | Lt | Gt | Le | Ge | Abort | Nop => false, - } -} diff --git a/external-crates/move/move-execution/v2/crates/invalid-mutations/src/helpers.rs b/external-crates/move/move-execution/v2/crates/invalid-mutations/src/helpers.rs deleted file mode 100644 index c122d58c87fea..0000000000000 --- a/external-crates/move/move-execution/v2/crates/invalid-mutations/src/helpers.rs +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use proptest::sample::Index as PropIndex; -use std::{collections::BTreeSet, ops::Index as OpsIndex}; - -/// Given a maximum value `max` and a list of [`Index`](proptest::sample::Index) instances, picks -/// integers in the range `[0, max)` uniformly randomly and without duplication. -/// -/// If `indexes_len` is greater than `max`, all indexes will be returned. -/// -/// This function implements Robert Floyd's F2 algorithm for sampling without replacement. -pub(crate) fn pick_idxs(max: usize, indexes: &T, indexes_len: usize) -> Vec -where - T: OpsIndex + ?Sized, - P: AsRef, -{ - // See https://dl.acm.org/doi/10.1145/30401.315746 (the F2 algorithm) - // for a longer explanation. This is a variant that works with zero-indexing. - let mut selected = BTreeSet::new(); - let to_select = indexes_len.min(max); - for (iter_idx, choice) in ((max - to_select)..max).enumerate() { - // "RandInt(1, J)" in the original algorithm means a number between 1 - // and choice, both inclusive. `PropIndex::index` picks a number between 0 and - // whatever's passed in, with the latter exclusive. Pass in "+1" to ensure the same - // range of values is picked from. (This also ensures that if choice is 0 then `index` - // doesn't panic. - let idx = indexes[iter_idx].as_ref().index(choice + 1); - if !selected.insert(idx) { - selected.insert(choice); - } - } - selected.into_iter().collect() -} - -/// Given a maximum value `max` and a slice of [`Index`](proptest::sample::Index) instances, picks -/// integers in the range `[0, max)` uniformly randomly and without duplication. -/// -/// If the number of `Index` instances is greater than `max`, all indexes will be returned. -/// -/// This function implements Robert Floyd's F2 algorithm for sampling without replacement. -#[inline] -pub(crate) fn pick_slice_idxs(max: usize, indexes: &[impl AsRef]) -> Vec { - pick_idxs(max, indexes, indexes.len()) -} diff --git a/external-crates/move/move-execution/v2/crates/invalid-mutations/src/lib.rs b/external-crates/move/move-execution/v2/crates/invalid-mutations/src/lib.rs deleted file mode 100644 index d6574245153dd..0000000000000 --- a/external-crates/move/move-execution/v2/crates/invalid-mutations/src/lib.rs +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -#![forbid(unsafe_code)] - -pub mod bounds; -mod helpers; -pub mod signature; diff --git a/external-crates/move/move-execution/v2/crates/invalid-mutations/src/signature.rs b/external-crates/move/move-execution/v2/crates/invalid-mutations/src/signature.rs deleted file mode 100644 index 6181bf66d3ce6..0000000000000 --- a/external-crates/move/move-execution/v2/crates/invalid-mutations/src/signature.rs +++ /dev/null @@ -1,100 +0,0 @@ -// Copyright (c) The Diem Core Contributors -// Copyright (c) The Move Contributors -// SPDX-License-Identifier: Apache-2.0 - -use move_binary_format::file_format::{ - CompiledModule, Signature, SignatureToken, StructFieldInformation, TypeSignature, -}; -use proptest::sample::Index as PropIndex; - -pub struct SignatureRefMutation<'a> { - module: &'a mut CompiledModule, - mutations: Vec<(PropIndex, PropIndex)>, -} - -impl<'a> SignatureRefMutation<'a> { - pub fn new(module: &'a mut CompiledModule, mutations: Vec<(PropIndex, PropIndex)>) -> Self { - Self { module, mutations } - } - - pub fn apply(self) -> bool { - if self.module.signatures.is_empty() { - return false; - } - let module = self.module; - let mut mutations = false; - for (s_idx, t_idx) in self.mutations { - let sig_idx = s_idx.index(module.signatures.len()); - let sig = &module.signatures[sig_idx]; - if sig.is_empty() { - continue; - } - let token_idx = t_idx.index(sig.len()); - let new_sig = mutate_sig(sig, token_idx); - module.signatures[sig_idx] = new_sig; - mutations = true; - } - mutations - } -} - -/// Context for applying a list of `FieldRefMutation` instances. -pub struct FieldRefMutation<'a> { - module: &'a mut CompiledModule, - mutations: Vec<(PropIndex, PropIndex)>, -} - -impl<'a> FieldRefMutation<'a> { - pub fn new(module: &'a mut CompiledModule, mutations: Vec<(PropIndex, PropIndex)>) -> Self { - Self { module, mutations } - } - - #[inline] - pub fn apply(self) -> bool { - if self.module.struct_defs.is_empty() { - return false; - } - let module = self.module; - let mut mutations = false; - for (s_idx, f_idx) in self.mutations { - let struct_idx = s_idx.index(module.struct_defs.len()); - let struct_def = &mut module.struct_defs[struct_idx]; - if let StructFieldInformation::Declared(fields) = &mut struct_def.field_information { - if fields.is_empty() { - continue; - } - let field_idx = f_idx.index(fields.len()); - let field_def = &mut fields[field_idx]; - let new_ty = mutate_field(&field_def.signature.0); - fields[field_idx].signature = TypeSignature(new_ty); - mutations = true; - } - } - mutations - } -} - -fn mutate_sig(sig: &Signature, token_idx: usize) -> Signature { - use SignatureToken::*; - - Signature( - sig.0 - .iter() - .enumerate() - .map(|(idx, token)| { - if idx == token_idx { - match &token { - Reference(_) | MutableReference(_) => Reference(Box::new(token.clone())), - _ => Reference(Box::new(Reference(Box::new(token.clone())))), - } - } else { - token.clone() - } - }) - .collect(), - ) -} - -fn mutate_field(token: &SignatureToken) -> SignatureToken { - SignatureToken::Reference(Box::new(token.clone())) -} diff --git a/external-crates/move/move-execution/v2/crates/move-bytecode-verifier/Cargo.toml b/external-crates/move/move-execution/v2/crates/move-bytecode-verifier/Cargo.toml index 19146c4489c1a..9fcd628c8ea97 100644 --- a/external-crates/move/move-execution/v2/crates/move-bytecode-verifier/Cargo.toml +++ b/external-crates/move/move-execution/v2/crates/move-bytecode-verifier/Cargo.toml @@ -21,7 +21,6 @@ move-abstract-stack.workspace = true [dev-dependencies] hex-literal.workspace = true -invalid-mutations = { path = "../invalid-mutations", package = "invalid-mutations-v2" } [features] default = [] diff --git a/scripts/execution_layer.py b/scripts/execution_layer.py index a47aef580f35e..25b13d7ba516a 100755 --- a/scripts/execution_layer.py +++ b/scripts/execution_layer.py @@ -362,7 +362,6 @@ def cut_command(f): *["-p", "move-stdlib-natives"], *["-p", "move-vm-runtime"], *["-p", "bytecode-verifier-tests"], - *["-p", "invalid-mutations"], ] @@ -384,7 +383,6 @@ def cut_directories(f): external / "move" / "crates" / "move-stdlib-natives", external / "move" / "crates" / "move-vm-runtime", external / "move" / "crates" / "bytecode-verifier-tests", - external / "move" / "crates" / "invalid-mutations", ] ) else: @@ -394,7 +392,6 @@ def cut_directories(f): external / "move" / "move-execution" / f / "crates" / "move-stdlib-natives", external / "move" / "move-execution" / f / "crates" / "move-vm-runtime", external / "move" / "move-execution" / f / "crates" / "bytecode-verifier-tests", - external / "move" / "move-execution" / f / "crates" / "invalid-mutations", ] )