diff --git a/third_party/move/move-compiler-v2/src/file_format_generator/function_generator.rs b/third_party/move/move-compiler-v2/src/file_format_generator/function_generator.rs index e35c6257716cb..3a61f63bd19e6 100644 --- a/third_party/move/move-compiler-v2/src/file_format_generator/function_generator.rs +++ b/third_party/move/move-compiler-v2/src/file_format_generator/function_generator.rs @@ -159,8 +159,15 @@ impl<'a> FunctionGenerator<'a> { .get_extension::() .expect("Options is available"); if options.experiment_on(Experiment::PEEPHOLE_OPTIMIZATION) { - // TODO: fix source mapping (#14167) - peephole_optimizer::run(&mut code); + let transformed_code_chunk = peephole_optimizer::optimize(&code.code); + // Fix the source map for the optimized code. + fun_gen + .gen + .source_map + .remap_code_map(def_idx, transformed_code_chunk.original_offsets) + .expect(SOURCE_MAP_OK); + // Replace the code with the optimized one. + code.code = transformed_code_chunk.code; } } else { // Write the spec block table back to the environment. diff --git a/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer.rs b/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer.rs index 70919d6dafa7a..812e662f70b3e 100644 --- a/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer.rs +++ b/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer.rs @@ -12,17 +12,18 @@ pub mod reducible_pairs; use inefficient_loads::InefficientLoads; use move_binary_format::{ control_flow_graph::{ControlFlowGraph, VMControlFlowGraph}, - file_format::{Bytecode, CodeOffset, CodeUnit}, + file_format::{Bytecode, CodeOffset}, }; -use optimizers::{BasicBlockOptimizer, WindowProcessor}; +use optimizers::{BasicBlockOptimizer, TransformedCodeChunk, WindowProcessor}; use reducible_pairs::ReduciblePairs; -use std::{collections::BTreeMap, mem}; +use std::collections::BTreeMap; /// Pre-requisite: `code` should not have spec block associations. /// Run peephole optimizers on the given `code`, possibly modifying it. -pub fn run(code: &mut CodeUnit) { - let original_code = mem::take(&mut code.code); - code.code = BasicBlockOptimizerPipeline::default().optimize(original_code); +/// Returns the optimized code, along with mapping to original offsets +/// in `code`. +pub fn optimize(code: &[Bytecode]) -> TransformedCodeChunk { + BasicBlockOptimizerPipeline::default().optimize(code) } /// A pipeline of basic block optimizers. @@ -44,23 +45,24 @@ impl BasicBlockOptimizerPipeline { /// Run the basic block optimization pipeline on the given `code`, /// returning new (possibly optimized) code. - pub fn optimize(&self, mut code: Vec) -> Vec { - let mut cfg = VMControlFlowGraph::new(&code); + pub fn optimize(&self, code: &[Bytecode]) -> TransformedCodeChunk { + let mut code_chunk = TransformedCodeChunk::make_from(code); + let mut cfg = VMControlFlowGraph::new(&code_chunk.code); loop { - let optimized_blocks = self.get_optimized_blocks(&code, &cfg); - let optimized_code = Self::flatten_blocks(optimized_blocks); - let optimized_cfg = VMControlFlowGraph::new(&optimized_code); + let optimized_blocks = self.get_optimized_blocks(&code_chunk.code, &cfg); + let optimized_code_chunk = Self::flatten_blocks(optimized_blocks); + let optimized_cfg = VMControlFlowGraph::new(&optimized_code_chunk.code); if optimized_cfg.num_blocks() == cfg.num_blocks() { // Proxy for convergence of basic block optimizations. // This is okay for peephole optimizations that merge basic blocks. // But may need to revisit if we have peephole optimizations that can // split a basic block. - return optimized_code; + return optimized_code_chunk; } else { // Number of basic blocks changed, re-run the basic-block // optimization pipeline again on the new basic blocks. cfg = optimized_cfg; - code = optimized_code; + code_chunk = optimized_code_chunk.remap(code_chunk.original_offsets); } } } @@ -71,14 +73,16 @@ impl BasicBlockOptimizerPipeline { &self, code: &[Bytecode], cfg: &VMControlFlowGraph, - ) -> BTreeMap> { + ) -> BTreeMap { let mut optimized_blocks = BTreeMap::new(); for block_id in cfg.blocks() { let start = cfg.block_start(block_id); let end = cfg.block_end(block_id); // `end` is inclusive - let mut block = code[start as usize..=end as usize].to_vec(); + let mut block = TransformedCodeChunk::make_from(&code[start as usize..=end as usize]); for bb_optimizer in self.optimizers.iter() { - block = bb_optimizer.optimize(&block); + block = bb_optimizer + .optimize(&block.code) + .remap(block.original_offsets); } optimized_blocks.insert(start, block); } @@ -86,14 +90,16 @@ impl BasicBlockOptimizerPipeline { } /// Flatten the individually optimized basic blocks into a single code vector. - fn flatten_blocks(optimized_blocks: BTreeMap>) -> Vec { - let mut optimized_code = vec![]; + fn flatten_blocks( + optimized_blocks: BTreeMap, + ) -> TransformedCodeChunk { + let mut optimized_code = TransformedCodeChunk::empty(); let mut block_mapping = BTreeMap::new(); - for (offset, mut block) in optimized_blocks { - block_mapping.insert(offset, optimized_code.len() as CodeOffset); - optimized_code.append(&mut block); + for (offset, block) in optimized_blocks { + block_mapping.insert(offset, optimized_code.code.len() as CodeOffset); + optimized_code.extend(block, offset); } - Self::remap_branch_targets(&mut optimized_code, &block_mapping); + Self::remap_branch_targets(&mut optimized_code.code, &block_mapping); optimized_code } diff --git a/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer/inefficient_loads.rs b/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer/inefficient_loads.rs index 6aa84b2812c29..16719913cc029 100644 --- a/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer/inefficient_loads.rs +++ b/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer/inefficient_loads.rs @@ -24,8 +24,11 @@ //! cannot use it without a subsequent store. //! So, skipping the store to `u` is safe. -use crate::file_format_generator::peephole_optimizer::optimizers::WindowOptimizer; -use move_binary_format::file_format::Bytecode; +use crate::file_format_generator::peephole_optimizer::optimizers::{ + TransformedCodeChunk, WindowOptimizer, +}; +use move_binary_format::file_format::{Bytecode, CodeOffset}; +use std::iter; /// An optimizer for inefficient loads. pub struct InefficientLoads; @@ -37,7 +40,7 @@ impl InefficientLoads { } impl WindowOptimizer for InefficientLoads { - fn optimize_window(&self, window: &[Bytecode]) -> Option<(Vec, usize)> { + fn optimize_window(&self, window: &[Bytecode]) -> Option<(TransformedCodeChunk, usize)> { use Bytecode::*; if window.len() < Self::MIN_WINDOW_SIZE { return None; @@ -61,8 +64,14 @@ impl WindowOptimizer for InefficientLoads { // We have reached the end of the pattern (point 4 in the module documentation). let sequence = &window[2..index + 2]; let load_constant = &window[0..1]; + let transformed_code = [sequence, load_constant].concat(); + // original_offsets are 2..index+2 (representing `sequence`), + // followed by 0 (representing `load_constant`). + let original_offsets = (2..(index + 2) as CodeOffset) + .chain(iter::once(0)) + .collect::>(); return Some(( - [sequence, load_constant].concat(), + TransformedCodeChunk::new(transformed_code, original_offsets), index + Self::MIN_WINDOW_SIZE, )); }, diff --git a/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer/optimizers.rs b/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer/optimizers.rs index 0bcabb5675092..d29335b56b3a5 100644 --- a/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer/optimizers.rs +++ b/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer/optimizers.rs @@ -3,33 +3,91 @@ //! This module contains setup for basic block peephole optimizers. -use move_binary_format::file_format::Bytecode; +use move_binary_format::file_format::{Bytecode, CodeOffset}; + +/// A contiguous chunk of bytecode that may have been transformed from some +/// other "original" contiguous chunk of bytecode. +pub struct TransformedCodeChunk { + /// The transformed bytecode. + pub code: Vec, + /// Mapping to the original offsets. + /// The instruction in `code[i]` corresponds to the instruction at + /// `original_offsets[i]` in the original bytecode. + pub original_offsets: Vec, +} + +impl TransformedCodeChunk { + /// Create an instance of `TransformedCodeChunk` from the given `code` + /// and `original_offsets`. + pub fn new(code: Vec, original_offsets: Vec) -> Self { + debug_assert_eq!(code.len(), original_offsets.len()); + Self { + code, + original_offsets, + } + } + + /// Create an empty chunk. + pub fn empty() -> Self { + Self::new(vec![], vec![]) + } + + /// Extend this chunk with another `other` chunk. + /// The `original_offsets` for the `other` chunk are incremented by `adjust`. + pub fn extend(&mut self, other: TransformedCodeChunk, adjust: CodeOffset) { + self.code.extend(other.code); + self.original_offsets + .extend(other.original_offsets.into_iter().map(|off| off + adjust)); + } + + /// Make a new chunk from the given `code`. + pub fn make_from(code: &[Bytecode]) -> Self { + Self::new(code.to_vec(), Vec::from_iter(0..code.len() as CodeOffset)) + } + + /// Remap the original offsets using the given `previous_offsets`. + pub fn remap(self, previous_offsets: Vec) -> Self { + Self::new( + self.code, + self.original_offsets + .into_iter() + .map(|off| previous_offsets[off as usize]) + .collect(), + ) + } +} /// A basic block optimizer that optimizes a basic block of bytecode. pub trait BasicBlockOptimizer { - /// Given a basic `block`, return its optimized version. - fn optimize(&self, block: &[Bytecode]) -> Vec; + /// Given a basic `block`, return its optimized version [*]. + /// + /// [*] The optimized version returned via `TransformedCodeChunk` maintains a + /// mapping to the original offsets in `block`. + fn optimize(&self, block: &[Bytecode]) -> TransformedCodeChunk; } /// An optimizer for a window of bytecode within a basic block. /// The window is always a suffix of a basic block. pub trait WindowOptimizer { /// Given a `window` of bytecode, return a tuple containing: - /// 1. an optimized version of a non-empty prefix of the `window`. + /// 1. an optimized version of a non-empty prefix of the `window`. [*] /// 2. size of this prefix (should be non-zero). /// If `None` is returned, the `window` is not optimized. - fn optimize_window(&self, window: &[Bytecode]) -> Option<(Vec, usize)>; + /// + /// [*] When an optimized version is returned, the corresponding `TransformedCodeChunk` + /// maintains a mapping to the original offsets of `window`. + fn optimize_window(&self, window: &[Bytecode]) -> Option<(TransformedCodeChunk, usize)>; } /// A processor to perform window optimizations of a particular style on a basic block. pub struct WindowProcessor(T); impl BasicBlockOptimizer for WindowProcessor { - fn optimize(&self, block: &[Bytecode]) -> Vec { - let mut old_block = block.to_vec(); + fn optimize(&self, block: &[Bytecode]) -> TransformedCodeChunk { + let mut old_block = TransformedCodeChunk::make_from(block); // Run single passes until code stops changing. - while let Some(new_block) = self.optimize_single_pass(&old_block) { - old_block = new_block; + while let Some(new_chunk) = self.optimize_single_pass(&old_block.code) { + old_block = new_chunk.remap(old_block.original_offsets); } old_block } @@ -43,19 +101,22 @@ impl WindowProcessor { /// Run a single pass of the window peephole optimization on the given basic `block`. /// If the block cannot be optimized further, return `None`. - fn optimize_single_pass(&self, block: &[Bytecode]) -> Option> { + fn optimize_single_pass(&self, block: &[Bytecode]) -> Option { let mut changed = false; - let mut new_block: Vec = vec![]; + let mut new_block = TransformedCodeChunk::empty(); let mut left = 0; while left < block.len() { let window = &block[left..]; if let Some((optimized_window, consumed)) = self.0.optimize_window(window) { debug_assert!(consumed != 0); - new_block.extend(optimized_window); + new_block.extend(optimized_window, left as CodeOffset); left += consumed; changed = true; } else { - new_block.push(block[left].clone()); + new_block.extend( + TransformedCodeChunk::make_from(&block[left..left + 1]), + left as CodeOffset, + ); left += 1; } } diff --git a/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer/reducible_pairs.rs b/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer/reducible_pairs.rs index ed6813e4196fa..55319dbba1ef6 100644 --- a/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer/reducible_pairs.rs +++ b/third_party/move/move-compiler-v2/src/file_format_generator/peephole_optimizer/reducible_pairs.rs @@ -39,7 +39,9 @@ //! Finally, note that fixed window optimizations are performed on windows within a basic //! block, not spanning across multiple basic blocks. -use crate::file_format_generator::peephole_optimizer::optimizers::WindowOptimizer; +use crate::file_format_generator::peephole_optimizer::optimizers::{ + TransformedCodeChunk, WindowOptimizer, +}; use move_binary_format::file_format::Bytecode; pub struct ReduciblePairs; @@ -49,7 +51,7 @@ impl ReduciblePairs { } impl WindowOptimizer for ReduciblePairs { - fn optimize_window(&self, window: &[Bytecode]) -> Option<(Vec, usize)> { + fn optimize_window(&self, window: &[Bytecode]) -> Option<(TransformedCodeChunk, usize)> { use Bytecode::*; if window.len() < Self::WINDOW_SIZE { return None; @@ -59,13 +61,17 @@ impl WindowOptimizer for ReduciblePairs { (StLoc(u), MoveLoc(v)) | (CopyLoc(u), StLoc(v)) | (MoveLoc(u), StLoc(v)) if *u == *v => { - vec![] + TransformedCodeChunk::new(vec![], vec![]) }, - (CopyLoc(_), Pop) => vec![], - (LdTrue, BrTrue(target)) | (LdFalse, BrFalse(target)) => vec![Branch(*target)], - (LdTrue, BrFalse(_)) | (LdFalse, BrTrue(_)) => vec![], - (Not, BrFalse(target)) => vec![BrTrue(*target)], - (Not, BrTrue(target)) => vec![BrFalse(*target)], + (CopyLoc(_), Pop) => TransformedCodeChunk::new(vec![], vec![]), + (LdTrue, BrTrue(target)) | (LdFalse, BrFalse(target)) => { + TransformedCodeChunk::new(vec![Branch(*target)], vec![0]) + }, + (LdTrue, BrFalse(_)) | (LdFalse, BrTrue(_)) => { + TransformedCodeChunk::new(vec![], vec![]) + }, + (Not, BrFalse(target)) => TransformedCodeChunk::new(vec![BrTrue(*target)], vec![0]), + (Not, BrTrue(target)) => TransformedCodeChunk::new(vec![BrFalse(*target)], vec![0]), _ => return None, }; Some((optimized, Self::WINDOW_SIZE)) diff --git a/third_party/move/move-compiler-v2/tests/reference-safety/v1-borrow-tests/writeref_borrow_invalid.old.exp b/third_party/move/move-compiler-v2/tests/reference-safety/v1-borrow-tests/writeref_borrow_invalid.old.exp index 781744ed45a72..8b5dcbf17e00a 100644 --- a/third_party/move/move-compiler-v2/tests/reference-safety/v1-borrow-tests/writeref_borrow_invalid.old.exp +++ b/third_party/move/move-compiler-v2/tests/reference-safety/v1-borrow-tests/writeref_borrow_invalid.old.exp @@ -4,7 +4,7 @@ Diagnostics: bug: bytecode verification failed with unexpected status code `WRITEREF_EXISTS_BORROW_ERROR`. This is a compiler bug, consider reporting it. Error message: none - ┌─ tests/reference-safety/v1-borrow-tests/writeref_borrow_invalid.move:10:18 + ┌─ tests/reference-safety/v1-borrow-tests/writeref_borrow_invalid.move:10:9 │ 10 │ *g_mut = G { v: 0 }; - │ ^^^^^^^^^^ + │ ^^^^^^^^^^^^^^^^^^^ diff --git a/third_party/move/move-ir-compiler/move-bytecode-source-map/src/source_map.rs b/third_party/move/move-ir-compiler/move-bytecode-source-map/src/source_map.rs index ec101a84c7001..4f0b3364d55dc 100644 --- a/third_party/move/move-ir-compiler/move-bytecode-source-map/src/source_map.rs +++ b/third_party/move/move-ir-compiler/move-bytecode-source-map/src/source_map.rs @@ -83,7 +83,7 @@ pub struct SourceMap { // A mapping of `StructDefinitionIndex` to source map for each struct/resource. struct_map: BTreeMap, - // A mapping of `FunctionDefinitionIndex` to the soure map for that function. + // A mapping of `FunctionDefinitionIndex` to the source map for that function. // For scripts, this map has a single element that points to a source map corresponding to the // script's "main" function. function_map: BTreeMap, @@ -202,6 +202,31 @@ impl FunctionSourceMap { }; } + /// Remap the code map based on the given `remap`. + /// If `remap[i] == j`, then the code location associated with code offset `j` + /// will now be associated with code offset `i`. + pub fn remap_code_map(&mut self, remap: Vec) { + let mut prev_loc = None; + let new_code_map = remap + .iter() + .map(|old_offset| self.get_code_location(*old_offset)) + .enumerate() + .filter_map(|(new_offset, loc)| { + if prev_loc == loc { + // optimization: if a series of instructions map to the same location, we only need + // to add code mapping for the first one, because the code map is a segment map. + None + } else if let Some(loc) = loc { + prev_loc = Some(loc); + Some((new_offset as CodeOffset, loc)) + } else { + None + } + }) + .collect(); + self.code_map = new_code_map; + } + /// Record the code offset for an Nop label pub fn add_nop_mapping(&mut self, label: NopLabel, offset: CodeOffset) { assert!(self.nops.insert(label, offset).is_none()) @@ -362,6 +387,22 @@ impl SourceMap { Ok(()) } + /// Remap the code map for the function given by `fdef_idx`, according to `remap`. + /// If `remap[i] == j`, then the code location associated with code offset `j` + /// will now be associated with code offset `i`. + pub fn remap_code_map( + &mut self, + fdef_idx: FunctionDefinitionIndex, + remap: Vec, + ) -> Result<()> { + let func_entry = self + .function_map + .get_mut(&fdef_idx.0) + .ok_or_else(|| format_err!("Tried to remap code map of undefined function index"))?; + func_entry.remap_code_map(remap); + Ok(()) + } + pub fn add_nop_mapping( &mut self, fdef_idx: FunctionDefinitionIndex, diff --git a/third_party/move/move-prover/tests/sources/functional/aborts_if.v2_exp b/third_party/move/move-prover/tests/sources/functional/aborts_if.v2_exp index 8179a66a39bdf..6b904aef88910 100644 --- a/third_party/move/move-prover/tests/sources/functional/aborts_if.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/aborts_if.v2_exp @@ -102,7 +102,7 @@ error: abort not covered by any of the `aborts_if` clauses ┌─ tests/sources/functional/aborts_if.move:139:5 │ 137 │ if (x == 2 || x == 3) abort 1; - │ ----------------------------- abort happened here with code 0x1 + │ ------- abort happened here with code 0x1 138 │ } 139 │ ╭ spec abort_at_2_or_3_total_incorrect { 140 │ │ // Counter check that we get an error message without the pragma. @@ -114,8 +114,6 @@ error: abort not covered by any of the `aborts_if` clauses = at tests/sources/functional/aborts_if.move:136: abort_at_2_or_3_total_incorrect = x = = at tests/sources/functional/aborts_if.move:137: abort_at_2_or_3_total_incorrect - = at tests/sources/functional/aborts_if.move:136: abort_at_2_or_3_total_incorrect - = at tests/sources/functional/aborts_if.move:137: abort_at_2_or_3_total_incorrect = = = at tests/sources/functional/aborts_if.move:137: abort_at_2_or_3_total_incorrect = ABORTED @@ -129,17 +127,16 @@ error: function does not abort under this condition = at tests/sources/functional/aborts_if.move:145: abort_at_2_or_3_spec_incorrect = x = = at tests/sources/functional/aborts_if.move:146: abort_at_2_or_3_spec_incorrect - = at tests/sources/functional/aborts_if.move:145: abort_at_2_or_3_spec_incorrect - = at tests/sources/functional/aborts_if.move:146: abort_at_2_or_3_spec_incorrect = = = at tests/sources/functional/aborts_if.move:146: abort_at_2_or_3_spec_incorrect + = at tests/sources/functional/aborts_if.move:145: abort_at_2_or_3_spec_incorrect = at tests/sources/functional/aborts_if.move:151: abort_at_2_or_3_spec_incorrect (spec) error: abort not covered by any of the `aborts_if` clauses ┌─ tests/sources/functional/aborts_if.move:157:5 │ 155 │ if (x == 2 || x == 3) abort 1; - │ ----------------------------- abort happened here with code 0x1 + │ ------- abort happened here with code 0x1 156 │ } 157 │ ╭ spec abort_at_2_or_3_strict_incorrect { 158 │ │ // When the strict mode is enabled, no aborts_if clause means aborts_if false. @@ -150,8 +147,6 @@ error: abort not covered by any of the `aborts_if` clauses = at tests/sources/functional/aborts_if.move:154: abort_at_2_or_3_strict_incorrect = x = = at tests/sources/functional/aborts_if.move:155: abort_at_2_or_3_strict_incorrect - = at tests/sources/functional/aborts_if.move:154: abort_at_2_or_3_strict_incorrect - = at tests/sources/functional/aborts_if.move:155: abort_at_2_or_3_strict_incorrect = = = at tests/sources/functional/aborts_if.move:155: abort_at_2_or_3_strict_incorrect = ABORTED diff --git a/third_party/move/move-prover/tests/sources/functional/aborts_if_with_code.v2_exp b/third_party/move/move-prover/tests/sources/functional/aborts_if_with_code.v2_exp index 36844a9bf44f1..930280cf0144d 100644 --- a/third_party/move/move-prover/tests/sources/functional/aborts_if_with_code.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/aborts_if_with_code.v2_exp @@ -2,22 +2,21 @@ Move prover returns: exiting with verification errors error: abort code not covered by any of the `aborts_if` or `aborts_with` clauses ┌─ tests/sources/functional/aborts_if_with_code.move:38:5 │ -30 │ ╭ if (x == 1) { -31 │ │ abort 2 -32 │ │ }; - │ ╰─────────' abort happened here with code 0x2 - · │ -38 │ ╭ spec conditional_abort_invalid { -39 │ │ aborts_if x == 1 with 1; // wrong code -40 │ │ aborts_if y == 2 with 3; -41 │ │ ensures result == x; -42 │ │ } - │ ╰───────^ +31 │ abort 2 + │ ------- abort happened here with code 0x2 + · +38 │ ╭ spec conditional_abort_invalid { +39 │ │ aborts_if x == 1 with 1; // wrong code +40 │ │ aborts_if y == 2 with 3; +41 │ │ ensures result == x; +42 │ │ } + │ ╰─────^ │ = at tests/sources/functional/aborts_if_with_code.move:29: conditional_abort_invalid = x = = y = = at tests/sources/functional/aborts_if_with_code.move:30: conditional_abort_invalid + = at tests/sources/functional/aborts_if_with_code.move:31: conditional_abort_invalid = ABORTED error: abort code not covered by any of the `aborts_if` or `aborts_with` clauses @@ -40,8 +39,8 @@ error: abort code not covered by any of the `aborts_if` or `aborts_with` clauses error: abort code not covered by any of the `aborts_if` or `aborts_with` clauses ┌─ tests/sources/functional/aborts_if_with_code.move:77:5 │ -73 │ if (x == 2) { - │ ------ abort happened here with code 0x2 +74 │ abort(2) + │ -------- abort happened here with code 0x2 · 77 │ ╭ spec aborts_if_with_code_mixed_invalid { 78 │ │ aborts_if x == 1; @@ -52,15 +51,15 @@ error: abort code not covered by any of the `aborts_if` or `aborts_with` clauses = at tests/sources/functional/aborts_if_with_code.move:69: aborts_if_with_code_mixed_invalid = x = = at tests/sources/functional/aborts_if_with_code.move:70: aborts_if_with_code_mixed_invalid - = at tests/sources/functional/aborts_if_with_code.move:71: aborts_if_with_code_mixed_invalid = at tests/sources/functional/aborts_if_with_code.move:73: aborts_if_with_code_mixed_invalid + = at tests/sources/functional/aborts_if_with_code.move:74: aborts_if_with_code_mixed_invalid = ABORTED error: abort code not covered by any of the `aborts_if` or `aborts_with` clauses ┌─ tests/sources/functional/aborts_if_with_code.move:105:5 │ -101 │ if (x == 2) { - │ ------ abort happened here with code 0x2 +102 │ abort(2) + │ -------- abort happened here with code 0x2 · 105 │ ╭ spec aborts_with_invalid { 106 │ │ aborts_with 1,3; @@ -70,15 +69,15 @@ error: abort code not covered by any of the `aborts_if` or `aborts_with` clauses = at tests/sources/functional/aborts_if_with_code.move:97: aborts_with_invalid = x = = at tests/sources/functional/aborts_if_with_code.move:98: aborts_with_invalid - = at tests/sources/functional/aborts_if_with_code.move:99: aborts_with_invalid = at tests/sources/functional/aborts_if_with_code.move:101: aborts_with_invalid + = at tests/sources/functional/aborts_if_with_code.move:102: aborts_with_invalid = ABORTED error: abort code not covered by any of the `aborts_if` or `aborts_with` clauses ┌─ tests/sources/functional/aborts_if_with_code.move:131:5 │ -127 │ if (x == 2) { - │ ------ abort happened here with code 0x1 +128 │ abort(1) + │ -------- abort happened here with code 0x1 · 131 │ ╭ spec aborts_with_mixed_invalid { 132 │ │ pragma aborts_if_is_partial = true; @@ -90,6 +89,6 @@ error: abort code not covered by any of the `aborts_if` or `aborts_with` clauses = at tests/sources/functional/aborts_if_with_code.move:123: aborts_with_mixed_invalid = x = = at tests/sources/functional/aborts_if_with_code.move:124: aborts_with_mixed_invalid - = at tests/sources/functional/aborts_if_with_code.move:125: aborts_with_mixed_invalid = at tests/sources/functional/aborts_if_with_code.move:127: aborts_with_mixed_invalid + = at tests/sources/functional/aborts_if_with_code.move:128: aborts_with_mixed_invalid = ABORTED diff --git a/third_party/move/move-prover/tests/sources/functional/bv_aborts.v2_exp b/third_party/move/move-prover/tests/sources/functional/bv_aborts.v2_exp index 2d106370311c2..59c574ebbd267 100644 --- a/third_party/move/move-prover/tests/sources/functional/bv_aborts.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/bv_aborts.v2_exp @@ -8,24 +8,22 @@ error: function does not abort under this condition = at tests/sources/functional/bv_aborts.move:11: assert_with_spec = x = = at tests/sources/functional/bv_aborts.move:12: assert_with_spec + = at tests/sources/functional/bv_aborts.move:11: assert_with_spec = at tests/sources/functional/bv_aborts.move:16: assert_with_spec (spec) error: abort not covered by any of the `aborts_if` clauses ┌─ tests/sources/functional/bv_aborts.move:14:5 │ -11 │ fun assert_with_spec(x: u64) { - │ ╭──────────────────────────────────' -12 │ │ assert!(x > 815); -13 │ │ } - │ ╰─────' abort happened here -14 │ ╭ spec assert_with_spec { -15 │ │ // This will fail -16 │ │ aborts_if x > 815 with std::error::internal(0) | (0xCA26CBD9BE << 24); -17 │ │ } - │ ╰───────^ +12 │ assert!(x > 815); + │ ------ abort happened here +13 │ } +14 │ ╭ spec assert_with_spec { +15 │ │ // This will fail +16 │ │ aborts_if x > 815 with std::error::internal(0) | (0xCA26CBD9BE << 24); +17 │ │ } + │ ╰─────^ │ = at tests/sources/functional/bv_aborts.move:11: assert_with_spec = x = = at tests/sources/functional/bv_aborts.move:12: assert_with_spec - = at tests/sources/functional/bv_aborts.move:11: assert_with_spec = ABORTED diff --git a/third_party/move/move-prover/tests/sources/functional/choice.v2_exp b/third_party/move/move-prover/tests/sources/functional/choice.v2_exp index a5d5c96f98fbe..8717b9158c33c 100644 --- a/third_party/move/move-prover/tests/sources/functional/choice.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/choice.v2_exp @@ -48,8 +48,11 @@ error: post-condition does not hold = at tests/sources/functional/choice.move:78: test_not_using_min_incorrect = at tests/sources/functional/choice.move:79: test_not_using_min_incorrect = at tests/sources/functional/choice.move:80: test_not_using_min_incorrect - = v = = at tests/sources/functional/choice.move:81: test_not_using_min_incorrect + = at tests/sources/functional/choice.move:82: test_not_using_min_incorrect + = v = + = at tests/sources/functional/choice.move:83: test_not_using_min_incorrect + = at tests/sources/functional/choice.move:75: test_not_using_min_incorrect = result = = at tests/sources/functional/choice.move:84: test_not_using_min_incorrect = at tests/sources/functional/choice.move:87: test_not_using_min_incorrect (spec) diff --git a/third_party/move/move-prover/tests/sources/functional/emits.v2_exp b/third_party/move/move-prover/tests/sources/functional/emits.v2_exp index 2387c17c66674..a3c55d75f67cc 100644 --- a/third_party/move/move-prover/tests/sources/functional/emits.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/emits.v2_exp @@ -76,7 +76,6 @@ error: function does not emit the expected event = x = = handle = = at tests/sources/functional/emits.move:106: conditional_wrong_condition_incorrect - = at tests/sources/functional/emits.move:107: conditional_wrong_condition_incorrect = handle = = at tests/sources/functional/emits.move:111: conditional_wrong_condition_incorrect (spec) @@ -90,7 +89,6 @@ error: function does not emit the expected event = x = = handle = = at tests/sources/functional/emits.move:115: conditional_missing_condition_incorrect - = at tests/sources/functional/emits.move:116: conditional_missing_condition_incorrect = handle = = at tests/sources/functional/emits.move:120: conditional_missing_condition_incorrect (spec) diff --git a/third_party/move/move-prover/tests/sources/functional/enum.v2_exp b/third_party/move/move-prover/tests/sources/functional/enum.v2_exp index 737366c5714f8..a4c03bfa8340f 100644 --- a/third_party/move/move-prover/tests/sources/functional/enum.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/enum.v2_exp @@ -33,6 +33,7 @@ error: data invariant does not hold = at tests/sources/functional/enum.move:29: test_data_invariant = = = z = + = at tests/sources/functional/enum.move:30: test_data_invariant = at tests/sources/functional/enum.move:9 = at tests/sources/functional/enum.move:10 diff --git a/third_party/move/move-prover/tests/sources/functional/enum_abort.v2_exp b/third_party/move/move-prover/tests/sources/functional/enum_abort.v2_exp index e913c0054b4b3..d9bd3b8ee1f3b 100644 --- a/third_party/move/move-prover/tests/sources/functional/enum_abort.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/enum_abort.v2_exp @@ -81,7 +81,7 @@ error: abort not covered by any of the `aborts_if` clauses ┌─ tests/sources/functional/enum_abort.move:129:5 │ 123 │ abort 30 // aborts - │ -- abort happened here with code 0x1E + │ -------- abort happened here with code 0x1E · 129 │ ╭ spec test_match_abort { 130 │ │ aborts_if false; @@ -103,7 +103,6 @@ error: abort not covered by any of the `aborts_if` clauses = = = at tests/sources/functional/enum_abort.move:122: test_match_abort = _z = - = at tests/sources/functional/enum_abort.move:123: test_match_abort = at tests/sources/functional/enum_abort.move:122: test_match_abort = = = at tests/sources/functional/enum_abort.move:122: test_match_abort diff --git a/third_party/move/move-prover/tests/sources/functional/loop_unroll.v2_exp b/third_party/move/move-prover/tests/sources/functional/loop_unroll.v2_exp index c561bd90334e9..10d8fa2f9595e 100644 --- a/third_party/move/move-prover/tests/sources/functional/loop_unroll.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/loop_unroll.v2_exp @@ -66,7 +66,7 @@ error: abort not covered by any of the `aborts_if` clauses ┌─ tests/sources/functional/loop_unroll.move:128:5 │ 124 │ assert!(i != 5, 0); - │ ------ abort happened here with code 0x0 + │ ------ abort happened here with code 0x0 · 128 │ ╭ spec t7_failure { 129 │ │ pragma unroll = 6; diff --git a/third_party/move/move-prover/tests/sources/functional/mono.v2_exp b/third_party/move/move-prover/tests/sources/functional/mono.v2_exp index 573e40d78820e..58c8d24e7a195 100644 --- a/third_party/move/move-prover/tests/sources/functional/mono.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/mono.v2_exp @@ -64,6 +64,7 @@ error: post-condition does not hold = x = = at tests/sources/functional/mono.move:80: vec_vec = x = + = at tests/sources/functional/mono.move:79: vec_vec = result = = at tests/sources/functional/mono.move:81: vec_vec = at tests/sources/functional/mono.move:82: vec_vec (spec) diff --git a/third_party/move/move-prover/tests/sources/functional/schema_exp.v2_exp b/third_party/move/move-prover/tests/sources/functional/schema_exp.v2_exp index cdc6c3cede166..b96c99a669024 100644 --- a/third_party/move/move-prover/tests/sources/functional/schema_exp.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/schema_exp.v2_exp @@ -3,7 +3,7 @@ error: abort not covered by any of the `aborts_if` clauses ┌─ tests/sources/functional/schema_exp.move:29:5 │ 26 │ if (!c) abort(1); - │ --- abort happened here with code 0x1 + │ -------- abort happened here with code 0x1 · 29 │ ╭ spec bar_incorrect { 30 │ │ // Once we include a schema with aborts, even conditionally, we need to provide a full spec of the aborts diff --git a/third_party/move/move-prover/tests/sources/functional/verify_custom_table.v2_exp b/third_party/move/move-prover/tests/sources/functional/verify_custom_table.v2_exp index a859dc99038f4..d865f02b759e5 100644 --- a/third_party/move/move-prover/tests/sources/functional/verify_custom_table.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/verify_custom_table.v2_exp @@ -14,6 +14,7 @@ error: post-condition does not hold = at tests/sources/functional/verify_custom_table.move:72: add_fail = t = = at tests/sources/functional/verify_custom_table.move:73: add_fail + = at tests/sources/functional/verify_custom_table.move:68: add_fail = result = = at tests/sources/functional/verify_custom_table.move:74: add_fail = at tests/sources/functional/verify_custom_table.move:76: add_fail (spec) @@ -29,6 +30,7 @@ error: post-condition does not hold = at tests/sources/functional/verify_custom_table.move:203: create_and_insert_fail_due_to_typed_key_encoding = t = = at tests/sources/functional/verify_custom_table.move:204: create_and_insert_fail_due_to_typed_key_encoding + = at tests/sources/functional/verify_custom_table.move:201: create_and_insert_fail_due_to_typed_key_encoding = result = = at tests/sources/functional/verify_custom_table.move:205: create_and_insert_fail_due_to_typed_key_encoding = at tests/sources/functional/verify_custom_table.move:210: create_and_insert_fail_due_to_typed_key_encoding (spec) @@ -44,6 +46,7 @@ error: post-condition does not hold = at tests/sources/functional/verify_custom_table.move:215: create_and_insert_fail1 = t = = at tests/sources/functional/verify_custom_table.move:216: create_and_insert_fail1 + = at tests/sources/functional/verify_custom_table.move:213: create_and_insert_fail1 = result = = at tests/sources/functional/verify_custom_table.move:217: create_and_insert_fail1 = at tests/sources/functional/verify_custom_table.move:219: create_and_insert_fail1 (spec) @@ -59,6 +62,7 @@ error: post-condition does not hold = at tests/sources/functional/verify_custom_table.move:224: create_and_insert_fail2 = t = = at tests/sources/functional/verify_custom_table.move:225: create_and_insert_fail2 + = at tests/sources/functional/verify_custom_table.move:222: create_and_insert_fail2 = result = = at tests/sources/functional/verify_custom_table.move:226: create_and_insert_fail2 = at tests/sources/functional/verify_custom_table.move:228: create_and_insert_fail2 (spec) diff --git a/third_party/move/move-prover/tests/sources/functional/verify_table.v2_exp b/third_party/move/move-prover/tests/sources/functional/verify_table.v2_exp index afae3e87880c6..ba8cc35bd29d7 100644 --- a/third_party/move/move-prover/tests/sources/functional/verify_table.v2_exp +++ b/third_party/move/move-prover/tests/sources/functional/verify_table.v2_exp @@ -14,6 +14,7 @@ error: post-condition does not hold = at tests/sources/functional/verify_table.move:27: add_fail = t = = at tests/sources/functional/verify_table.move:28: add_fail + = at tests/sources/functional/verify_table.move:23: add_fail = result = = at tests/sources/functional/verify_table.move:29: add_fail = at tests/sources/functional/verify_table.move:31: add_fail (spec) diff --git a/third_party/move/tools/move-unit-test/tests/test_sources/abort_location.exp b/third_party/move/tools/move-unit-test/tests/test_sources/abort_location.exp new file mode 100644 index 0000000000000..fd6103f388abd --- /dev/null +++ b/third_party/move/tools/move-unit-test/tests/test_sources/abort_location.exp @@ -0,0 +1,29 @@ +Running Move unit tests +[ FAIL ] 0xc0ffee::Example::test_validate_parameters +0xc0ffee::Example::test_validate_parameters +Output: Ok(Changes { accounts: {} }) + +Test failures: + +Failures in 0xc0ffee::Example: + +┌── test_validate_parameters ────── +│ error[E11001]: test failure +│ ┌─ abort_location.move:36:13 +│ │ +│ 9 │ fun validate_parameters( +│ │ ------------------- In this function in 0xc0ffee::Example +│ · +│ 36 │ ╭ assert!( +│ 37 │ │ is_valid_a, +│ 38 │ │ if (is_positive) E_CONDITION_B else E_CONDITION_C +│ 39 │ │ ); +│ │ ╰─────────────^ Test was not expected to error, but it aborted with code 2 originating in the module 0000000000000000000000000000000000000000000000000000000000c0ffee::Example rooted here +│ +│ +│ stack trace +│ Example::test_validate_parameters(tests/test_sources/abort_location.move:86-92) +│ +└────────────────── + +Test result: FAILED. Total tests: 1; passed: 0; failed: 1 diff --git a/third_party/move/tools/move-unit-test/tests/test_sources/abort_location.move b/third_party/move/tools/move-unit-test/tests/test_sources/abort_location.move new file mode 100644 index 0000000000000..f9f337e2a6005 --- /dev/null +++ b/third_party/move/tools/move-unit-test/tests/test_sources/abort_location.move @@ -0,0 +1,94 @@ +module 0xc0ffee::Example { + use std::option::{Option}; + + const E_CONDITION_A: u64 = 1; + const E_CONDITION_B: u64 = 2; + const E_CONDITION_C: u64 = 3; + const E_CONDITION_D: u64 = 4; + + fun validate_parameters( + param_a: Option, // Optional first parameter to validate + param_b: Option, // Optional second parameter to validate + reference_value: u64, // Reference value for comparisons + threshold_value: u64, // Threshold for additional validation + flag: bool // Indicates a positive or negative condition + ) { + // Determine the context based on the flag + let is_positive = flag; + + // Check and validate param_a if it exists + if (option::is_some(¶m_a)) { + // Extract the value from the Option + let value_a = option::extract(&mut param_a); + + // Ensure the value is non-zero + assert!(value_a > 0, E_CONDITION_A); + + // Validate based on the condition (is_positive) + let is_valid_a = + if (is_positive) { + value_a > reference_value // For positive condition, value_a must be greater than reference_value + } else { + value_a < reference_value // For negative condition, value_a must be less than reference_value + }; + + // Assert that the validation passed + assert!( + is_valid_a, + if (is_positive) E_CONDITION_B else E_CONDITION_C + ); + }; + + // Check and validate param_b if it exists + if (option::is_some(¶m_b)) { + // Extract the value from the Option + let value_b = option::extract(&mut param_b); + + // Ensure the value is non-zero + assert!(value_b > 0, E_CONDITION_A); + + // Validate based on the condition (is_positive) + let is_valid_b = + if (is_positive) { + value_b < reference_value // For positive condition, value_b must be less than reference_value + } else { + value_b > reference_value // For negative condition, value_b must be greater than reference_value + }; + + // Assert that the validation passed + assert!( + is_valid_b, + if (is_positive) E_CONDITION_C else E_CONDITION_D + ); + + // Additional validation against the threshold value if it exists + if (threshold_value > 0) { + let is_valid_threshold = + if (is_positive) { + value_b > threshold_value // For positive condition, value_b must be greater than threshold_value + } else { + value_b < threshold_value // For negative condition, value_b must be less than threshold_value + }; + + // Assert that the threshold validation passed + assert!(is_valid_threshold, E_CONDITION_A); + } + }; + } + + #[test_only] + use std::option; + + #[test] + fun test_validate_parameters() { + // Passing Invalid param_a for positive condition + // This should throw E_CONDITION_B error + validate_parameters( + option::some(40), // Less than reference_value (60) + option::some(50), + 60, + 30, + true + ); + } +} diff --git a/third_party/move/tools/move-unit-test/tests/test_sources/abort_location.v2_exp b/third_party/move/tools/move-unit-test/tests/test_sources/abort_location.v2_exp new file mode 100644 index 0000000000000..ee9ce750daf17 --- /dev/null +++ b/third_party/move/tools/move-unit-test/tests/test_sources/abort_location.v2_exp @@ -0,0 +1,26 @@ +Running Move unit tests +[ FAIL ] 0xc0ffee::Example::test_validate_parameters +0xc0ffee::Example::test_validate_parameters +Output: Ok(Changes { accounts: {} }) + +Test failures: + +Failures in 0xc0ffee::Example: + +┌── test_validate_parameters ────── +│ error[E11001]: test failure +│ ┌─ abort_location.move:36:13 +│ │ +│ 9 │ fun validate_parameters( +│ │ ------------------- In this function in 0xc0ffee::Example +│ · +│ 36 │ assert!( +│ │ ^^^^^^ Test was not expected to error, but it aborted with code 2 originating in the module 0000000000000000000000000000000000000000000000000000000000c0ffee::Example rooted here +│ +│ +│ stack trace +│ Example::test_validate_parameters(tests/test_sources/abort_location.move:86-92) +│ +└────────────────── + +Test result: FAILED. Total tests: 1; passed: 0; failed: 1 diff --git a/third_party/move/tools/move-unit-test/tests/test_sources/native_signer_creation.v2_exp b/third_party/move/tools/move-unit-test/tests/test_sources/native_signer_creation.v2_exp index ee17a0a77e621..9b98bb942e7bf 100644 --- a/third_party/move/tools/move-unit-test/tests/test_sources/native_signer_creation.v2_exp +++ b/third_party/move/tools/move-unit-test/tests/test_sources/native_signer_creation.v2_exp @@ -15,13 +15,13 @@ Failures in 0x1::M: ┌── test_doesnt_exist ────── │ error[E11001]: test failure -│ ┌─ native_signer_creation.move:43:17 +│ ┌─ native_signer_creation.move:47:9 │ │ │ 36 │ fun test_doesnt_exist() { │ │ ----------------- In this function in 0x1::M │ · -│ 43 │ i = i + 1; -│ │ ^^^^^ Test was not expected to error, but it aborted with code 0 originating in the module 0000000000000000000000000000000000000000000000000000000000000001::M rooted here +│ 47 │ abort 0 +│ │ ^^^^^^^ Test was not expected to error, but it aborted with code 0 originating in the module 0000000000000000000000000000000000000000000000000000000000000001::M rooted here │ │ └────────────────── diff --git a/third_party/move/tools/move-unit-test/tests/test_sources/proposal_test.v2_exp b/third_party/move/tools/move-unit-test/tests/test_sources/proposal_test.v2_exp index 7df933167ed6a..83b5ab9ae67e3 100644 --- a/third_party/move/tools/move-unit-test/tests/test_sources/proposal_test.v2_exp +++ b/third_party/move/tools/move-unit-test/tests/test_sources/proposal_test.v2_exp @@ -24,19 +24,13 @@ Failures in 0x1::Module: ┌── tests_d ────── │ error[E11001]: test failure -│ ┌─ proposal_test.move:96:16 -│ │ -│ 95 │ fun tests_d(a1: signer, a2: signer) -│ │ ------- In this function in 0x1::Module -│ 96 │ acquires B { -│ │ ╭────────────────^ -│ 97 │ │ setup_storage_tests_d(&a1, &a2); -│ 98 │ │ assert!(d(@0x1, 5), 0); -│ 99 │ │ assert!(!d(@0x1, 6), 1); -│ · │ -│ 102 │ │ assert!(d(@0x2, 6), 3); -│ 103 │ │ } -│ │ ╰─────^ Test was not expected to error, but it aborted with code 3 originating in the module 0000000000000000000000000000000000000000000000000000000000000001::Module rooted here +│ ┌─ proposal_test.move:102:9 +│ │ +│ 95 │ fun tests_d(a1: signer, a2: signer) +│ │ ------- In this function in 0x1::Module +│ · +│ 102 │ assert!(d(@0x2, 6), 3); +│ │ ^^^^^^ Test was not expected to error, but it aborted with code 3 originating in the module 0000000000000000000000000000000000000000000000000000000000000001::Module rooted here │ │ └──────────────────