[vm] introduce CompiledScriptMut to provide separation between mutabl…

…e and immutable scripts A `CompiledScriptMut` is typically in the middle of being built, while a `CompiledScript` is a frozen version that has been bounds checked. The next diff will do the same for `CompiledModule`.
tnasu · Jun 18, 2019 · 5fb4075 · 5fb4075
1 parent 2fa6a3e
commit 5fb4075
Show file tree

Hide file tree

Showing 15 changed files with 299 additions and 149 deletions.
diff --git a/language/bytecode_verifier/src/verifier.rs b/language/bytecode_verifier/src/verifier.rs
@@ -10,6 +10,7 @@ use crate::{
 use std::collections::BTreeMap;
 use types::language_storage::CodeKey;
 use vm::{
+    access::{BaseAccess, ScriptAccess},
     checks::BoundsChecker,
     errors::{VMStaticViolation, VerificationError},
     file_format::{CompiledModule, CompiledScript},
@@ -66,8 +67,8 @@ pub fn verify_script(script: CompiledScript) -> (CompiledScript, Vec<Verificatio
 
 /// This function checks the extra requirements on the signature of the main function of a script.
 pub fn verify_main_signature(script: &CompiledScript) -> Vec<VMStaticViolation> {
-    let function_handle = &script.function_handles[script.main.function.0 as usize];
-    let function_signature = &script.function_signatures[function_handle.signature.0 as usize];
+    let function_handle = &script.function_handle_at(script.main().function);
+    let function_signature = &script.function_signature_at(function_handle.signature);
     if !function_signature.return_types.is_empty() {
         return vec![VMStaticViolation::InvalidMainFunctionSignature];
     }

diff --git a/language/compiler/src/compiler.rs b/language/compiler/src/compiler.rs
@@ -1,11 +1,14 @@
 // Copyright (c) The Libra Core Contributors
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::parser::ast::{
-    BinOp, Block, Builtin, Cmd, CopyableVal, Exp, Field, Fields, Function, FunctionBody,
-    FunctionCall, FunctionSignature as AstFunctionSignature, FunctionVisibility, IfElse, Kind,
-    Loop, ModuleDefinition, ModuleIdent, ModuleName, Program, Statement,
-    StructDefinition as MoveStruct, Tag, Type, UnaryOp, Var, Var_, While,
+use crate::{
+    errors::*,
+    parser::ast::{
+        BinOp, Block, Builtin, Cmd, CopyableVal, Exp, Field, Fields, Function, FunctionBody,
+        FunctionCall, FunctionSignature as AstFunctionSignature, FunctionVisibility, IfElse, Kind,
+        Loop, ModuleDefinition, ModuleIdent, ModuleName, Program, Statement,
+        StructDefinition as MoveStruct, Tag, Type, UnaryOp, Var, Var_, While,
+    },
 };
 use bytecode_verifier::verifier::{verify_module, verify_module_dependencies};
 use failure::*;
@@ -21,7 +24,7 @@ use vm::{
     errors::VerificationError,
     file_format::{
         AddressPoolIndex, ByteArrayPoolIndex, Bytecode, CodeUnit, CompiledModule, CompiledProgram,
-        CompiledScript, FieldDefinition, FieldDefinitionIndex, FunctionDefinition,
+        CompiledScriptMut, FieldDefinition, FieldDefinitionIndex, FunctionDefinition,
         FunctionDefinitionIndex, FunctionHandle, FunctionHandleIndex, FunctionSignature,
         FunctionSignatureIndex, LocalsSignature, LocalsSignatureIndex, MemberCount, ModuleHandle,
         ModuleHandleIndex, SignatureToken, StringPoolIndex, StructDefinition,
@@ -404,11 +407,11 @@ impl<'a> ModuleScope<'a> {
 struct ScriptScope<'a> {
     // parent scope, the global module scope
     compilation_scope: CompilationScope<'a>,
-    pub script: CompiledScript,
+    pub script: CompiledScriptMut,
 }
 
 impl<'a> ScriptScope<'a> {
-    fn new(script: CompiledScript, modules: &[CompiledModule]) -> ScriptScope {
+    fn new(script: CompiledScriptMut, modules: &[CompiledModule]) -> ScriptScope {
         ScriptScope {
             compilation_scope: CompilationScope::new(modules),
             script,
@@ -1036,7 +1039,7 @@ pub fn compile_program(
 
     // Compile transaction script
     let func_def: FunctionDefinition;
-    let compiled_script = CompiledScript::default();
+    let compiled_script = CompiledScriptMut::default();
     let scope = ScriptScope::new(compiled_script, &deps);
     let mut compiler = Compiler::new(scope);
     let addr_idx = compiler.make_address(&address)?;
@@ -1059,6 +1062,10 @@ pub fn compile_program(
 
     let mut script = compiler.scope.script;
     script.main = func_def;
+    let script = match script.freeze() {
+        Ok(script) => script,
+        Err(errs) => bail_err!(InternalCompilerError::BoundsCheckErrors(errs)),
+    };
 
     Ok(CompiledProgram::new(
         deps[n_external_deps..].to_vec(),

diff --git a/language/compiler/src/errors.rs b/language/compiler/src/errors.rs
@@ -0,0 +1,11 @@
+// Copyright (c) The Libra Core Contributors
+// SPDX-License-Identifier: Apache-2.0
+
+use failure::Fail;
+use vm::errors::VerificationError;
+
+#[derive(Clone, Debug, Eq, Fail, Ord, PartialEq, PartialOrd)]
+pub enum InternalCompilerError {
+    #[fail(display = "Post-compile bounds check errors: {:?}", _0)]
+    BoundsCheckErrors(Vec<VerificationError>),
+}
diff --git a/language/compiler/src/lib.rs b/language/compiler/src/lib.rs
@@ -5,5 +5,6 @@
 extern crate log;
 
 pub mod compiler;
+pub mod errors;
 pub mod parser;
 pub mod util;
diff --git a/language/compiler/src/unit_tests/cfg_tests.rs b/language/compiler/src/unit_tests/cfg_tests.rs
@@ -5,6 +5,7 @@ mod testutils;
 use super::*;
 use bytecode_verifier::control_flow_graph::{ControlFlowGraph, VMControlFlowGraph};
 use testutils::compile_script_string;
+use vm::access::ScriptAccess;
 
 #[test]
 fn cfg_compile_script_ret() {
@@ -17,7 +18,7 @@ fn cfg_compile_script_ret() {
     );
     let compiled_script_res = compile_script_string(&code);
     let compiled_script = compiled_script_res.unwrap();
-    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main.code.code).unwrap();
+    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main().code.code).unwrap();
     cfg.display();
     assert!(cfg.blocks.len() == 1);
     assert!(cfg.num_blocks() == 1);
@@ -41,7 +42,7 @@ fn cfg_compile_script_let() {
     );
     let compiled_script_res = compile_script_string(&code);
     let compiled_script = compiled_script_res.unwrap();
-    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main.code.code).unwrap();
+    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main().code.code).unwrap();
     println!("SCRIPT:\n {:?}", compiled_script);
     cfg.display();
     assert!(cfg.blocks.len() == 1);
@@ -66,7 +67,7 @@ fn cfg_compile_if() {
 
     let compiled_script_res = compile_script_string(&code);
     let compiled_script = compiled_script_res.unwrap();
-    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main.code.code).unwrap();
+    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main().code.code).unwrap();
     println!("SCRIPT:\n {:?}", compiled_script);
     cfg.display();
     assert!(cfg.blocks.len() == 3);
@@ -94,7 +95,7 @@ fn cfg_compile_if_else() {
     );
     let compiled_script_res = compile_script_string(&code);
     let compiled_script = compiled_script_res.unwrap();
-    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main.code.code).unwrap();
+    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main().code.code).unwrap();
     println!("SCRIPT:\n {:?}", compiled_script);
     cfg.display();
     assert!(cfg.blocks.len() == 4);
@@ -120,7 +121,7 @@ fn cfg_compile_if_else_with_else_return() {
 
     let compiled_script_res = compile_script_string(&code);
     let compiled_script = compiled_script_res.unwrap();
-    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main.code.code).unwrap();
+    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main().code.code).unwrap();
     println!("SCRIPT:\n {:?}", compiled_script);
     cfg.display();
     assert!(cfg.blocks.len() == 4);
@@ -149,7 +150,7 @@ fn cfg_compile_nested_if() {
     );
     let compiled_script_res = compile_script_string(&code);
     let compiled_script = compiled_script_res.unwrap();
-    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main.code.code).unwrap();
+    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main().code.code).unwrap();
     println!("SCRIPT:\n {:?}", compiled_script);
     cfg.display();
     assert!(cfg.blocks.len() == 6);
@@ -174,7 +175,7 @@ fn cfg_compile_if_else_with_if_return() {
     );
     let compiled_script_res = compile_script_string(&code);
     let compiled_script = compiled_script_res.unwrap();
-    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main.code.code).unwrap();
+    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main().code.code).unwrap();
     println!("SCRIPT:\n {}", compiled_script);
     cfg.display();
     assert!(cfg.blocks.len() == 3);
@@ -200,7 +201,7 @@ fn cfg_compile_if_else_with_two_returns() {
     );
     let compiled_script_res = compile_script_string(&code);
     let compiled_script = compiled_script_res.unwrap();
-    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main.code.code).unwrap();
+    let cfg: VMControlFlowGraph = ControlFlowGraph::new(&compiled_script.main().code.code).unwrap();
     println!("SCRIPT:\n {}", compiled_script);
     cfg.display();
     assert!(cfg.blocks.len() == 4);

diff --git a/language/compiler/src/unit_tests/expression_tests.rs b/language/compiler/src/unit_tests/expression_tests.rs
@@ -8,7 +8,10 @@ use testutils::{
     compile_module_string, compile_script_string, compile_script_string_and_assert_error,
     count_locals,
 };
-use vm::file_format::Bytecode::*;
+use vm::{
+    access::{BaseAccess, ScriptAccess},
+    file_format::Bytecode::*,
+};
 
 #[test]
 fn compile_script_expr_addition() {
@@ -27,17 +30,17 @@ fn compile_script_expr_addition() {
     );
     let compiled_script_res = compile_script_string(&code);
     let compiled_script = compiled_script_res.unwrap();
-    assert!(compiled_script.main.code.max_stack_size == 2);
+    assert!(compiled_script.main().code.max_stack_size == 2);
     assert!(count_locals(&compiled_script) == 3);
-    assert!(compiled_script.main.code.code.len() == 9);
-    assert!(compiled_script.struct_handles.is_empty());
-    assert!(compiled_script.function_handles.len() == 1);
-    assert!(compiled_script.type_signatures.is_empty());
-    assert!(compiled_script.function_signatures.len() == 1); // method sig
-    assert!(compiled_script.locals_signatures.len() == 1); // local variables sig
-    assert!(compiled_script.module_handles.len() == 1); // the <SELF> module
-    assert!(compiled_script.string_pool.len() == 2); // the name of `main()` + the name of the "<SELF>" module
-    assert!(compiled_script.address_pool.len() == 1); // the empty address of <SELF> module
+    assert!(compiled_script.main().code.code.len() == 9);
+    assert!(compiled_script.struct_handles().len() == 0);
+    assert!(compiled_script.function_handles().len() == 1);
+    assert!(compiled_script.type_signatures().len() == 0);
+    assert!(compiled_script.function_signatures().len() == 1); // method sig
+    assert!(compiled_script.locals_signatures().len() == 1); // local variables sig
+    assert!(compiled_script.module_handles().len() == 1); // the <SELF> module
+    assert!(compiled_script.string_pool().len() == 2); // the name of `main()` + the name of the "<SELF>" module
+    assert!(compiled_script.address_pool().len() == 1); // the empty address of <SELF> module
 }
 
 #[test]
@@ -57,17 +60,17 @@ fn compile_script_expr_combined() {
     );
     let compiled_script_res = compile_script_string(&code);
     let compiled_script = compiled_script_res.unwrap();
-    assert!(compiled_script.main.code.max_stack_size == 3);
+    assert!(compiled_script.main().code.max_stack_size == 3);
     assert!(count_locals(&compiled_script) == 3);
-    assert!(compiled_script.main.code.code.len() == 13);
-    assert!(compiled_script.struct_handles.is_empty());
-    assert!(compiled_script.function_handles.len() == 1);
-    assert!(compiled_script.type_signatures.is_empty());
-    assert!(compiled_script.function_signatures.len() == 1); // method sig
-    assert!(compiled_script.locals_signatures.len() == 1); // local variables sig
-    assert!(compiled_script.module_handles.len() == 1); // the <SELF> module
-    assert!(compiled_script.string_pool.len() == 2); // the name of `main()` + the name of the "<SELF>" module
-    assert!(compiled_script.address_pool.len() == 1); // the empty address of <SELF> module
+    assert!(compiled_script.main().code.code.len() == 13);
+    assert!(compiled_script.struct_handles().len() == 0);
+    assert!(compiled_script.function_handles().len() == 1);
+    assert!(compiled_script.type_signatures().len() == 0);
+    assert!(compiled_script.function_signatures().len() == 1); // method sig
+    assert!(compiled_script.locals_signatures().len() == 1); // local variables sig
+    assert!(compiled_script.module_handles().len() == 1); // the <SELF> module
+    assert!(compiled_script.string_pool().len() == 2); // the name of `main()` + the name of the "<SELF>" module
+    assert!(compiled_script.address_pool().len() == 1); // the empty address of <SELF> module
 }
 
 #[test]
@@ -87,14 +90,14 @@ fn compile_script_borrow_local() {
     let compiled_script_res = compile_script_string(&code);
     let compiled_script = compiled_script_res.unwrap();
     assert!(count_locals(&compiled_script) == 2);
-    assert!(compiled_script.struct_handles.is_empty());
-    assert!(compiled_script.function_handles.len() == 1);
-    assert!(compiled_script.type_signatures.is_empty());
-    assert!(compiled_script.function_signatures.len() == 1); // method sig
-    assert!(compiled_script.locals_signatures.len() == 1); // local variables sig
-    assert!(compiled_script.module_handles.len() == 1); // the <SELF> module
-    assert!(compiled_script.string_pool.len() == 2); // the name of `main()` + the name of the "<SELF>" module
-    assert!(compiled_script.address_pool.len() == 1); // the empty address of <SELF> module
+    assert!(compiled_script.struct_handles().len() == 0);
+    assert!(compiled_script.function_handles().len() == 1);
+    assert!(compiled_script.type_signatures().len() == 0);
+    assert!(compiled_script.function_signatures().len() == 1); // method sig
+    assert!(compiled_script.locals_signatures().len() == 1); // local variables sig
+    assert!(compiled_script.module_handles().len() == 1); // the <SELF> module
+    assert!(compiled_script.string_pool().len() == 2); // the name of `main()` + the name of the "<SELF>" module
+    assert!(compiled_script.address_pool().len() == 1); // the empty address of <SELF> module
 }
 
 #[test]
@@ -113,15 +116,15 @@ fn compile_script_borrow_local_mutable() {
     );
     let compiled_script_res = compile_script_string(&code);
     let compiled_script = compiled_script_res.unwrap();
-    assert!(count_locals(&&compiled_script) == 2);
-    assert!(compiled_script.struct_handles.is_empty());
-    assert!(compiled_script.function_handles.len() == 1);
-    assert!(compiled_script.type_signatures.is_empty());
-    assert!(compiled_script.function_signatures.len() == 1); // method sig
-    assert!(compiled_script.locals_signatures.len() == 1); // local variables sig
-    assert!(compiled_script.module_handles.len() == 1); // the <SELF> module
-    assert!(compiled_script.string_pool.len() == 2); // the name of `main()` + the name of the "<SELF>" module
-    assert!(compiled_script.address_pool.len() == 1); // the empty address of <SELF> module
+    assert!(count_locals(&compiled_script) == 2);
+    assert!(compiled_script.struct_handles().len() == 0);
+    assert!(compiled_script.function_handles().len() == 1);
+    assert!(compiled_script.type_signatures().len() == 0);
+    assert!(compiled_script.function_signatures().len() == 1); // method sig
+    assert!(compiled_script.locals_signatures().len() == 1); // local variables sig
+    assert!(compiled_script.module_handles().len() == 1); // the <SELF> module
+    assert!(compiled_script.string_pool().len() == 2); // the name of `main()` + the name of the "<SELF>" module
+    assert!(compiled_script.address_pool().len() == 1); // the empty address of <SELF> module
 }
 
 #[test]
@@ -141,15 +144,15 @@ fn compile_script_borrow_reference() {
     );
     let compiled_script_res = compile_script_string_and_assert_error(&code, None);
     let compiled_script = compiled_script_res.unwrap();
-    assert!(count_locals(&&compiled_script) == 3);
-    assert!(compiled_script.struct_handles.is_empty());
-    assert!(compiled_script.function_handles.len() == 1);
-    assert!(compiled_script.type_signatures.is_empty());
-    assert!(compiled_script.function_signatures.len() == 1); // method sig
-    assert!(compiled_script.locals_signatures.len() == 1); // local variables sig
-    assert!(compiled_script.module_handles.len() == 1); // the <SELF> module
-    assert!(compiled_script.string_pool.len() == 2); // the name of `main()` + the name of the "<SELF>" module
-    assert!(compiled_script.address_pool.len() == 1); // the empty address of <SELF> module
+    assert!(count_locals(&compiled_script) == 3);
+    assert!(compiled_script.struct_handles().len() == 0);
+    assert!(compiled_script.function_handles().len() == 1);
+    assert!(compiled_script.type_signatures().len() == 0);
+    assert!(compiled_script.function_signatures().len() == 1); // method sig
+    assert!(compiled_script.locals_signatures().len() == 1); // local variables sig
+    assert!(compiled_script.module_handles().len() == 1); // the <SELF> module
+    assert!(compiled_script.string_pool().len() == 2); // the name of `main()` + the name of the "<SELF>" module
+    assert!(compiled_script.address_pool().len() == 1); // the empty address of <SELF> module
 }
 
 #[test]

diff --git a/language/compiler/src/unit_tests/testutils.rs b/language/compiler/src/unit_tests/testutils.rs
@@ -5,6 +5,7 @@ use super::*;
 use crate::parser::{parse_module, parse_program};
 use bytecode_verifier::verifier::{verify_module, verify_script};
 use vm::{
+    access::{BaseAccess, ScriptAccess},
     errors::VerificationError,
     file_format::{CompiledModule, CompiledScript},
 };
@@ -13,6 +14,7 @@ use vm::{
 macro_rules! instr_count {
     ($compiled: expr, $instr: pat) => {
         $compiled
+            .as_inner()
             .main
             .code
             .code
@@ -147,9 +149,7 @@ pub fn compile_module_string_and_assert_error(
 #[allow(dead_code)]
 pub fn count_locals(script: &CompiledScript) -> usize {
     script
-        .locals_signatures
-        .get(script.main.code.locals.0 as usize)
-        .unwrap()
+        .locals_signature_at(script.main().code.locals)
         .0
         .len()
 }