[move-vm] separate error code for vector ops

The `ABORTED` error code is reserved for the Move `abort` operations. Runtime errors originated from vector operations should have their own error category (similar to `ARITHMETIC_ERROR`). This commit adds the new error category `VECTOR_OPERATION_ERROR`. Closes: aptos-labs#9698
Thomas-guo007 · Nov 10, 2021 · 1a8bbb8 · 1a8bbb8
1 parent c7dc142
commit 1a8bbb8
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 23 deletions.
diff --git a/language/move-core/types/src/vm_status.rs b/language/move-core/types/src/vm_status.rs
@@ -643,6 +643,7 @@ pub enum StatusCode {
     DATA_FORMAT_ERROR = 4009,
     ABORTED = 4016,
     ARITHMETIC_ERROR = 4017,
+    VECTOR_OPERATION_ERROR = 4018,
     EXECUTION_STACK_OVERFLOW = 4020,
     CALL_STACK_OVERFLOW = 4021,
     VM_MAX_TYPE_DEPTH_REACHED = 4024,

diff --git a/language/move-stdlib/src/natives/vector.rs b/language/move-stdlib/src/natives/vector.rs
@@ -1,8 +1,8 @@
 // Copyright (c) The Diem Core Contributors
 // SPDX-License-Identifier: Apache-2.0
 
-use move_binary_format::errors::PartialVMResult;
-use move_core_types::gas_schedule::GasAlgebra;
+use move_binary_format::errors::{PartialVMError, PartialVMResult};
+use move_core_types::{gas_schedule::GasAlgebra, vm_status::StatusCode};
 use move_vm_runtime::native_functions::NativeContext;
 use move_vm_types::{
     gas_schedule::NativeCostIndex,
@@ -68,7 +68,11 @@ pub fn native_borrow(
     let idx = pop_arg!(args, u64) as usize;
     let r = pop_arg!(args, VectorRef);
     let cost = native_gas(context.cost_table(), NativeCostIndex::BORROW, 1);
-    NativeResult::map_partial_vm_result_one(cost, r.borrow_elem(idx, &ty_args[0]))
+    NativeResult::map_partial_vm_result_one(
+        cost,
+        r.borrow_elem(idx, &ty_args[0])
+            .map_err(native_error_to_abort),
+    )
 }
 
 pub fn native_pop(
@@ -81,7 +85,7 @@ pub fn native_pop(
 
     let r = pop_arg!(args, VectorRef);
     let cost = native_gas(context.cost_table(), NativeCostIndex::POP_BACK, 1);
-    NativeResult::map_partial_vm_result_one(cost, r.pop(&ty_args[0]))
+    NativeResult::map_partial_vm_result_one(cost, r.pop(&ty_args[0]).map_err(native_error_to_abort))
 }
 
 pub fn native_destroy_empty(
@@ -94,7 +98,10 @@ pub fn native_destroy_empty(
 
     let v = pop_arg!(args, Vector);
     let cost = native_gas(context.cost_table(), NativeCostIndex::DESTROY_EMPTY, 1);
-    NativeResult::map_partial_vm_result_empty(cost, v.destroy_empty(&ty_args[0]))
+    NativeResult::map_partial_vm_result_empty(
+        cost,
+        v.destroy_empty(&ty_args[0]).map_err(native_error_to_abort),
+    )
 }
 
 pub fn native_swap(
@@ -109,5 +116,26 @@ pub fn native_swap(
     let idx1 = pop_arg!(args, u64) as usize;
     let r = pop_arg!(args, VectorRef);
     let cost = native_gas(context.cost_table(), NativeCostIndex::SWAP, 1);
-    NativeResult::map_partial_vm_result_empty(cost, r.swap(idx1, idx2, &ty_args[0]))
+    NativeResult::map_partial_vm_result_empty(
+        cost,
+        r.swap(idx1, idx2, &ty_args[0])
+            .map_err(native_error_to_abort),
+    )
+}
+
+fn native_error_to_abort(err: PartialVMError) -> PartialVMError {
+    let (major_status, sub_status_opt, message_opt, indices, offsets) = err.all_data();
+    let new_err = match major_status {
+        StatusCode::VECTOR_OPERATION_ERROR => PartialVMError::new(StatusCode::ABORTED),
+        _ => PartialVMError::new(major_status),
+    };
+    let new_err = match sub_status_opt {
+        None => new_err,
+        Some(code) => new_err.with_sub_status(code),
+    };
+    let new_err = match message_opt {
+        None => new_err,
+        Some(message) => new_err.with_message(message),
+    };
+    new_err.at_indices(indices).at_code_offsets(offsets)
 }
diff --git a/language/move-vm/transactional-tests/tests/builtins/vector_ops_out_of_bound.exp b/language/move-vm/transactional-tests/tests/builtins/vector_ops_out_of_bound.exp
@@ -2,7 +2,7 @@ processed 3 tasks
 
 task 0 'run'. lines 1-10:
 Error: Script execution failed with VMError: {
-    major_status: ABORTED,
+    major_status: VECTOR_OPERATION_ERROR,
     sub_status: Some(1),
     location: script,
     indices: [],
@@ -11,7 +11,7 @@ Error: Script execution failed with VMError: {
 
 task 1 'run'. lines 12-21:
 Error: Script execution failed with VMError: {
-    major_status: ABORTED,
+    major_status: VECTOR_OPERATION_ERROR,
     sub_status: Some(1),
     location: script,
     indices: [],
@@ -20,7 +20,7 @@ Error: Script execution failed with VMError: {
 
 task 2 'run'. lines 23-31:
 Error: Script execution failed with VMError: {
-    major_status: ABORTED,
+    major_status: VECTOR_OPERATION_ERROR,
     sub_status: Some(1),
     location: script,
     indices: [],

diff --git a/language/move-vm/transactional-tests/tests/builtins/vector_ops_pop_empty.exp b/language/move-vm/transactional-tests/tests/builtins/vector_ops_pop_empty.exp
@@ -2,7 +2,7 @@ processed 2 tasks
 
 task 0 'run'. lines 1-9:
 Error: Script execution failed with VMError: {
-    major_status: ABORTED,
+    major_status: VECTOR_OPERATION_ERROR,
     sub_status: Some(2),
     location: script,
     indices: [],
@@ -11,7 +11,7 @@ Error: Script execution failed with VMError: {
 
 task 1 'run'. lines 11-20:
 Error: Script execution failed with VMError: {
-    major_status: ABORTED,
+    major_status: VECTOR_OPERATION_ERROR,
     sub_status: Some(2),
     location: script,
     indices: [],

diff --git a/language/move-vm/transactional-tests/tests/builtins/vector_ops_unpack_less.exp b/language/move-vm/transactional-tests/tests/builtins/vector_ops_unpack_less.exp
@@ -2,7 +2,7 @@ processed 3 tasks
 
 task 0 'run'. lines 1-9:
 Error: Script execution failed with VMError: {
-    major_status: ABORTED,
+    major_status: VECTOR_OPERATION_ERROR,
     sub_status: Some(3),
     location: script,
     indices: [],
@@ -11,7 +11,7 @@ Error: Script execution failed with VMError: {
 
 task 1 'run'. lines 11-19:
 Error: Script execution failed with VMError: {
-    major_status: ABORTED,
+    major_status: VECTOR_OPERATION_ERROR,
     sub_status: Some(3),
     location: script,
     indices: [],
@@ -20,7 +20,7 @@ Error: Script execution failed with VMError: {
 
 task 2 'run'. lines 21-30:
 Error: Script execution failed with VMError: {
-    major_status: ABORTED,
+    major_status: VECTOR_OPERATION_ERROR,
     sub_status: Some(3),
     location: script,
     indices: [],

diff --git a/language/move-vm/transactional-tests/tests/builtins/vector_ops_unpack_more.exp b/language/move-vm/transactional-tests/tests/builtins/vector_ops_unpack_more.exp
@@ -2,7 +2,7 @@ processed 3 tasks
 
 task 0 'run'. lines 1-9:
 Error: Script execution failed with VMError: {
-    major_status: ABORTED,
+    major_status: VECTOR_OPERATION_ERROR,
     sub_status: Some(3),
     location: script,
     indices: [],
@@ -11,7 +11,7 @@ Error: Script execution failed with VMError: {
 
 task 1 'run'. lines 11-19:
 Error: Script execution failed with VMError: {
-    major_status: ABORTED,
+    major_status: VECTOR_OPERATION_ERROR,
     sub_status: Some(3),
     location: script,
     indices: [],
@@ -20,7 +20,7 @@ Error: Script execution failed with VMError: {
 
 task 2 'run'. lines 21-30:
 Error: Script execution failed with VMError: {
-    major_status: ABORTED,
+    major_status: VECTOR_OPERATION_ERROR,
     sub_status: Some(3),
     location: script,
     indices: [],

diff --git a/language/move-vm/types/src/values/values_impl.rs b/language/move-vm/types/src/values/values_impl.rs
@@ -1602,9 +1602,8 @@ impl VectorRef {
         let c = self.0.container();
         check_elem_layout(type_param, c)?;
         if idx >= c.len() {
-            return Err(
-                PartialVMError::new(StatusCode::ABORTED).with_sub_status(INDEX_OUT_OF_BOUNDS)
-            );
+            return Err(PartialVMError::new(StatusCode::VECTOR_OPERATION_ERROR)
+                .with_sub_status(INDEX_OUT_OF_BOUNDS));
         }
         Ok(Value(self.0.borrow_elem(idx)?))
     }
@@ -1615,7 +1614,8 @@ impl VectorRef {
 
         macro_rules! err_pop_empty_vec {
             () => {
-                return Err(PartialVMError::new(StatusCode::ABORTED).with_sub_status(POP_EMPTY_VEC))
+                return Err(PartialVMError::new(StatusCode::VECTOR_OPERATION_ERROR)
+                    .with_sub_status(POP_EMPTY_VEC))
             };
         }
 
@@ -1659,7 +1659,7 @@ impl VectorRef {
             ($v: expr) => {{
                 let mut v = $v.borrow_mut();
                 if idx1 >= v.len() || idx2 >= v.len() {
-                    return Err(PartialVMError::new(StatusCode::ABORTED)
+                    return Err(PartialVMError::new(StatusCode::VECTOR_OPERATION_ERROR)
                         .with_sub_status(INDEX_OUT_OF_BOUNDS));
                 }
                 v.swap(idx1, idx2);
@@ -1765,7 +1765,7 @@ impl Vector {
         if expected_num as usize == elements.len() {
             Ok(elements)
         } else {
-            Err(PartialVMError::new(StatusCode::ABORTED)
+            Err(PartialVMError::new(StatusCode::VECTOR_OPERATION_ERROR)
                 .with_sub_status(VEC_UNPACK_PARITY_MISMATCH))
         }
     }

diff --git a/language/tools/move-cli/src/sandbox/utils/mod.rs b/language/tools/move-cli/src/sandbox/utils/mod.rs
@@ -448,6 +448,10 @@ pub(crate) fn explain_execution_error(
                 ARITHMETIC_ERROR => "an arithmetic error (i.e., integer overflow/underflow, \
                                      div/mod by zero, or invalid shift)"
                     .to_string(),
+                VECTOR_OPERATION_ERROR => "an error originated from vector operations (i.e., \
+                                           index out of bound, pop an empty vector, or unpack a \
+                                           vector with a wrong parity)"
+                    .to_string(),
                 EXECUTION_STACK_OVERFLOW => "an execution stack overflow".to_string(),
                 CALL_STACK_OVERFLOW => "a call stack overflow".to_string(),
                 OUT_OF_GAS => "an out of gas error".to_string(),