[vm_runtime] verify modules while loading them from the store

Modules loaded from the store weren't being verified previously. This diff allows that to happen. The work involves threading verification errors through to the protobuf error messages.

This is a blocker to making `LoadedModule` use `VerifiedModule`.

This found an interesting bug in one of our tests, where a test module we had defined was failing to verify.

common/proptest_helpers/Cargo.toml

@@ -7,5 +7,6 @@ publish = false
 edition = "2018"
 
 [dependencies]
+crossbeam = "0.7.1"
 proptest = "0.9.4"
 proptest-derive = "0.1.2"

common/proptest_helpers/src/lib.rs

@@ -12,13 +12,45 @@ mod repeat_vec;
 
 pub use crate::{growing_subset::GrowingSubset, repeat_vec::RepeatVec};
 
+use crossbeam::thread;
 use proptest::sample::Index as PropIndex;
 use proptest_derive::Arbitrary;
 use std::{
+    any::Any,
     collections::BTreeSet,
     ops::{Deref, Index as OpsIndex},
 };
 
+/// Creates a new thread with a larger stack size.
+///
+/// Generating some proptest values can overflow the stack. This allows test authors to work around
+/// this limitation.
+///
+/// This is expected to be used with closure-style proptest invocations:
+///
+/// ```
+/// use proptest::prelude::*;
+/// use proptest_helpers::with_stack_size;
+///
+/// with_stack_size(4 * 1024 * 1024, || proptest!(|(x in 0usize..128)| {
+///     // assertions go here
+///     prop_assert!(x >= 0 && x < 128);
+/// }));
+/// ```
+pub fn with_stack_size<'a, F, T>(size: usize, f: F) -> Result<T, Box<dyn Any + 'static + Send>>
+where
+    F: FnOnce() -> T + Send + 'a,
+    T: Send + 'a,
+{
+    thread::scope(|s| {
+        let handle = s.builder().stack_size(size).spawn(|_| f()).map_err(|err| {
+            let any: Box<dyn Any + 'static + Send> = Box::new(err);
+            any
+        })?;
+        handle.join()
+    })?
+}
+
 /// 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.
 ///

execution/execution_proto/Cargo.toml

@@ -15,6 +15,7 @@ protobuf = "2.6"
 
 crypto = { path = "../../crypto/legacy_crypto" }
 failure = { path = "../../common/failure_ext", package = "failure_ext" }
+proptest_helpers = { path = "../../common/proptest_helpers" }
 proto_conv = { path = "../../common/proto_conv", features = ["derive"] }
 types = { path = "../../types" }
 

execution/execution_proto/src/protobuf_conversion_test.rs

@@ -6,6 +6,7 @@ use crate::{
     ExecuteChunkRequest, ExecuteChunkResponse,
 };
 use proptest::prelude::*;
+use proptest_helpers::with_stack_size;
 use proto_conv::test_helper::assert_protobuf_encode_decode;
 
 proptest! {
@@ -16,11 +17,6 @@ proptest! {
         assert_protobuf_encode_decode(&execute_block_request);
     }
 
-    #[test]
-    fn test_execute_block_response_roundtrip(execute_block_response in any::<ExecuteBlockResponse>()) {
-        assert_protobuf_encode_decode(&execute_block_response);
-    }
-
     #[test]
     fn test_commit_block_request_roundtrip(commit_block_request in any::<CommitBlockRequest>()) {
         assert_protobuf_encode_decode(&commit_block_request);
@@ -43,3 +39,13 @@ proptest! {
         assert_protobuf_encode_decode(&execute_chunk_response);
     }
 }
+
+#[test]
+fn test_execute_block_response_roundtrip() {
+    with_stack_size(4 * 1024 * 1024, || {
+        proptest!(|(execute_block_response in any::<ExecuteBlockResponse>())| {
+            assert_protobuf_encode_decode(&execute_block_response);
+        })
+    })
+    .unwrap();
+}

language/vm/cost_synthesis/src/stack_generator.rs

@@ -351,7 +351,8 @@ where
         let module = self
             .module_cache
             .get_loaded_module(&module_id)
-            .expect("[Module Lookup] Error while looking up module")
+            .expect("[Module Lookup] Invariant violation while looking up module")
+            .expect("[Module Lookup] Runtime error while looking up module")
             .expect("[Module Lookup] Unable to find module");
         let struct_def_idx = if self.struct_handle_table.contains_key(&module_id) {
             self.struct_handle_table
@@ -400,7 +401,8 @@ where
         let module = self
             .module_cache
             .get_loaded_module(&module_id)
-            .expect("[Module Lookup] Error while looking up module")
+            .expect("[Module Lookup] Invariant violation while looking up module")
+            .expect("[Module Lookup] Runtime error while looking up module")
             .expect("[Module Lookup] Unable to find module");
         let function_def_idx = if self.function_handle_table.contains_key(&module_id) {
             *self

language/vm/cost_synthesis/src/vm_runner.rs

@@ -48,7 +48,8 @@ macro_rules! with_loaded_vm {
         $module_cache.cache_module(root_module.clone());
         let $mod = $module_cache
             .get_loaded_module(&module_id)
-            .expect("[Module Cache] Internal error encountered when fetching module.")
+            .expect("[Module Lookup] Invariant violation while looking up module")
+            .expect("[Module Lookup] Runtime error while looking up module")
             .expect("[Module Cache] Unable to find module in module cache.");
         for m in modules {
             $module_cache.cache_module(m);

language/vm/src/errors.rs

@@ -6,6 +6,7 @@ use failure::Fail;
 use std::{fmt, iter::FromIterator};
 use types::{
     account_address::AccountAddress,
+    language_storage::ModuleId,
     transaction::TransactionStatus,
     vm_error::{VMStatus, VMValidationStatus, VMVerificationError, VMVerificationStatus},
 };
@@ -51,6 +52,7 @@ pub enum VMErrorKind {
     ValueDeserializerError,
     CodeSerializerError(BinaryError),
     CodeDeserializerError(BinaryError),
+    Verification(Vec<VerificationStatus>),
 }
 
 #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
@@ -60,6 +62,8 @@ pub enum VerificationStatus {
     /// A verification error was detected in a module. The first element is the index of the module
     /// in the transaction.
     Module(u16, VerificationError),
+    /// A verification error was detected in a dependency of a module.
+    Dependency(ModuleId, VerificationError),
 }
 
 #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
@@ -702,6 +706,9 @@ impl From<&VerificationStatus> for VMVerificationStatus {
             VerificationStatus::Module(module_idx, err) => {
                 VMVerificationStatus::Module(*module_idx, err.into())
             }
+            VerificationStatus::Dependency(dependency_id, err) => {
+                VMVerificationStatus::Dependency(dependency_id.clone(), err.into())
+            }
         }
     }
 }
@@ -745,8 +752,11 @@ impl From<&VMErrorKind> for VMStatus {
             VMErrorKind::ValueSerializerError => ExecutionStatus::ValueSerializationError,
             VMErrorKind::ValueDeserializerError => ExecutionStatus::ValueDeserializationError,
             VMErrorKind::DuplicateModuleName => ExecutionStatus::DuplicateModuleName,
+            // The below errors already have top-level VMStatus variants associated with them, so
+            // return those.
             VMErrorKind::CodeSerializerError(err) => return VMStatus::from(err),
             VMErrorKind::CodeDeserializerError(err) => return VMStatus::from(err),
+            VMErrorKind::Verification(statuses) => return statuses.iter().collect(),
         };
         VMStatus::Execution(err)
     }

language/vm/vm_runtime/Cargo.toml

@@ -27,6 +27,7 @@ config = { path = "../../../config"}
 logger = { path = "../../../common/logger" }
 
 [dev-dependencies]
+assert_matches = "1.3.0"
 compiler = { path = "../../compiler"}
 
 [dependencies.prometheus]