[vm] move signature checking implementation to bytecode verifier

This is a more natural place to have these checks. Talked with @shaz about this.

language/bytecode_verifier/src/lib.rs

@@ -18,6 +18,8 @@ pub mod resources;
 pub mod signature;
 pub mod stack_usage_verifier;
 pub mod struct_defs;
+#[cfg(test)]
+mod unit_tests;
 pub mod verifier;
 
 pub use check_duplication::DuplicationChecker;

language/bytecode_verifier/src/signature.rs

@@ -5,8 +5,14 @@
 //! parameters, locals, and fields of structs are well-formed. References can only occur at the
 //! top-level in all tokens.  Additionally, references cannot occur at all in field types.
 use vm::{
-    checks::SignatureCheck, errors::VerificationError, file_format::CompiledModule,
-    views::ModuleView, IndexKind,
+    access::ModuleAccess,
+    errors::{VMStaticViolation, VerificationError},
+    file_format::{CompiledModule, SignatureToken},
+    views::{
+        FieldDefinitionView, FunctionSignatureView, LocalsSignatureView, ModuleView,
+        TypeSignatureView, ViewInternals,
+    },
+    IndexKind, SignatureTokenKind,
 };
 
 pub struct SignatureChecker<'a> {
@@ -41,12 +47,11 @@ impl<'a> SignatureChecker<'a> {
             .fields()
             .enumerate()
             .filter_map(move |(idx, view)| {
-                view.check_signature_refs()
-                    .map(move |err| VerificationError {
-                        kind: IndexKind::FieldDefinition,
-                        idx,
-                        err,
-                    })
+                check_signature_refs(&view).map(move |err| VerificationError {
+                    kind: IndexKind::FieldDefinition,
+                    idx,
+                    err,
+                })
             })
             .collect();
         errors.push(signature_ref_errors);
@@ -70,3 +75,73 @@ impl<'a> SignatureChecker<'a> {
             .collect()
     }
 }
+
+trait SignatureCheck {
+    fn check_signatures(&self) -> Vec<VMStaticViolation>;
+}
+
+impl<'a, T: ModuleAccess> SignatureCheck for FunctionSignatureView<'a, T> {
+    fn check_signatures(&self) -> Vec<VMStaticViolation> {
+        self.return_tokens()
+            .filter_map(|token| check_structure(token.as_inner()))
+            .chain(
+                self.arg_tokens()
+                    .filter_map(|token| check_structure(token.as_inner())),
+            )
+            .collect()
+    }
+}
+
+impl<'a, T: ModuleAccess> SignatureCheck for TypeSignatureView<'a, T> {
+    fn check_signatures(&self) -> Vec<VMStaticViolation> {
+        check_structure(self.token().as_inner())
+            .into_iter()
+            .collect()
+    }
+}
+
+impl<'a, T: ModuleAccess> SignatureCheck for LocalsSignatureView<'a, T> {
+    fn check_signatures(&self) -> Vec<VMStaticViolation> {
+        self.tokens()
+            .filter_map(|token| check_structure(token.as_inner()))
+            .collect()
+    }
+}
+
+/// Field definitions have additional constraints on signatures -- field signatures cannot be
+/// references or mutable references.
+pub(crate) fn check_signature_refs(
+    view: &FieldDefinitionView<'_, CompiledModule>,
+) -> Option<VMStaticViolation> {
+    let type_signature = view.type_signature();
+    let token = type_signature.token();
+    let kind = token.kind();
+    match kind {
+        SignatureTokenKind::Reference | SignatureTokenKind::MutableReference => Some(
+            VMStaticViolation::InvalidFieldDefReference(token.as_inner().clone(), kind),
+        ),
+        SignatureTokenKind::Value => None,
+    }
+}
+
+/// Check that this token is structurally correct.
+/// In particular, check that the token has a reference only at the top level.
+pub(crate) fn check_structure(token: &SignatureToken) -> Option<VMStaticViolation> {
+    use SignatureToken::*;
+
+    let inner_token_opt = match token {
+        Reference(token) => Some(token),
+        MutableReference(token) => Some(token),
+        Bool | U64 | String | ByteArray | Address | Struct(_) => None,
+    };
+    if let Some(inner_token) = inner_token_opt {
+        if inner_token.is_reference() {
+            return Some(VMStaticViolation::InvalidSignatureToken(
+                token.clone(),
+                token.kind(),
+                inner_token.kind(),
+            ));
+        }
+    }
+    None
+}

language/vm/src/unit_tests/checks/signature_tests.rs → language/bytecode_verifier/src/unit_tests/signature_tests.rs

@@ -1,7 +1,8 @@
 // Copyright (c) The Libra Core Contributors
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::{
+use crate::signature::check_structure;
+use vm::{
     errors::VMStaticViolation,
     file_format::{SignatureToken, StructHandleIndex},
     SignatureTokenKind,
@@ -11,18 +12,18 @@ use crate::{
 fn test_sig_token_structure() {
     // Valid cases.
     let bool_token = SignatureToken::Bool;
-    assert_eq!(bool_token.check_structure(), None);
+    assert_eq!(check_structure(&bool_token), None);
     let struct_token = SignatureToken::Struct(StructHandleIndex::new(0));
-    assert_eq!(struct_token.check_structure(), None);
+    assert_eq!(check_structure(&struct_token), None);
     let ref_token = SignatureToken::Reference(Box::new(struct_token.clone()));
-    assert_eq!(ref_token.check_structure(), None);
+    assert_eq!(check_structure(&ref_token), None);
     let mut_ref_token = SignatureToken::MutableReference(Box::new(struct_token.clone()));
-    assert_eq!(mut_ref_token.check_structure(), None);
+    assert_eq!(check_structure(&mut_ref_token), None);
 
     // Invalid cases.
     let ref_ref_token = SignatureToken::Reference(Box::new(ref_token.clone()));
     assert_eq!(
-        ref_ref_token.check_structure(),
+        check_structure(&ref_ref_token),
         Some(VMStaticViolation::InvalidSignatureToken(
             ref_ref_token.clone(),
             SignatureTokenKind::Reference,
@@ -31,7 +32,7 @@ fn test_sig_token_structure() {
     );
     let ref_mut_ref_token = SignatureToken::Reference(Box::new(mut_ref_token.clone()));
     assert_eq!(
-        ref_mut_ref_token.check_structure(),
+        check_structure(&ref_mut_ref_token),
         Some(VMStaticViolation::InvalidSignatureToken(
             ref_mut_ref_token.clone(),
             SignatureTokenKind::Reference,

language/vm/src/checks.rs

@@ -2,7 +2,5 @@
 // SPDX-License-Identifier: Apache-2.0
 
 pub mod bounds;
-pub mod signature;
 
 pub use bounds::{BoundsCheck, BoundsChecker};
-pub use signature::SignatureCheck;

language/vm/src/unit_tests/mod.rs

@@ -1,7 +1,6 @@
 // Copyright (c) The Libra Core Contributors
 // SPDX-License-Identifier: Apache-2.0
 
-mod checks;
 mod deserializer_tests;
 mod fixture_tests;
 mod number_tests;