refactor(globals): refactor to work with merged validation stack and …

…moved global validation entirely to validation crate

Signed-off-by: George Cosma <[email protected]>

src/core/reader/types/global.rs

@@ -1,11 +1,8 @@
-use alloc::vec;
-
 use crate::core::reader::span::Span;
-use crate::core::reader::types::{ResultType, ValType};
+use crate::core::reader::types::ValType;
 use crate::core::reader::{WasmReadable, WasmReader};
 use crate::execution::assert_validated::UnwrapValidatedExt;
-use crate::globals::read_constant_instructions;
-use crate::{unreachable_validated, validate_value_stack, Error, Result};
+use crate::{unreachable_validated, Error, Result};
 
 #[derive(Debug, Copy, Clone)]
 pub struct Global {
@@ -14,37 +11,6 @@ pub struct Global {
     pub init_expr: Span,
 }
 
-impl WasmReadable for Global {
-    fn read(wasm: &mut WasmReader) -> Result<Self> {
-        let ty = GlobalType::read(wasm)?;
-        let mut init_expr = None;
-
-        let expected_type = ResultType {
-            valtypes: vec![ty.ty],
-        };
-        validate_value_stack(expected_type, |value_stack| {
-            init_expr = Some(read_constant_instructions(
-                wasm,
-                value_stack,
-                &[/* todo!(imported globals tpyes) */],
-            )?);
-
-            Ok(())
-        })?;
-
-        // At this point, we can assume that `init_expr` is `Some(_)`. `read_constant_instructions` returns a Span or an
-        // Error. If an Error is returned it is pushed up the call stack.
-        Ok(Self {
-            ty,
-            init_expr: init_expr.expect("expected gobal init expression to be initialized on successful value stack validation"),
-        })
-    }
-
-    fn read_unvalidated(wasm: &mut WasmReader) -> Self {
-        Self::read(wasm).unwrap_validated()
-    }
-}
-
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub struct GlobalType {
     pub ty: ValType,

src/execution/const_interpreter_loop.rs

@@ -95,7 +95,9 @@ pub(crate) fn run_const(
                 trace!("Constant instruction: i64.mul [{v1} {v2}] -> [{res}]");
                 stack.push_value(res.into());
             }
-            _ => panic!("¯\\_(ツ)_/¯"),
+            other => {
+                panic!("Unknown constant instruction {other:#x}, validation allowed an unimplemented instruction.");
+            }
         }
     }
 }

src/validation/code.rs

@@ -8,7 +8,8 @@ use crate::core::reader::types::global::Global;
 use crate::core::reader::types::memarg::MemArg;
 use crate::core::reader::types::{FuncType, NumType, ValType};
 use crate::core::reader::{WasmReadable, WasmReader};
-use crate::{validate_value_stack, Error, Result};
+use crate::validation_stack::ValidationStack;
+use crate::{Error, Result};
 
 pub fn validate_code_section(
     wasm: &mut WasmReader,
@@ -133,7 +134,7 @@ fn read_instructions(
                 } else {
                     // This is the last end of a function
 
-                    // The stack must only contain the functions return valtypes
+                    // The stack must only contain the function's return valtypes
                     let this_func_ty = &fn_types[type_idx_of_fn[this_function_idx]];
                     stack.assert_val_types(&this_func_ty.returns.valtypes)?;
                     return Ok(());

src/validation/globals.rs

@@ -1,15 +1,38 @@
 use alloc::vec::Vec;
 
-use crate::{
-    core::reader::{span::Span, types::global::GlobalType, WasmReader},
-    Error, NumType, Result, ValType,
-};
+use crate::core::reader::section_header::{SectionHeader, SectionTy};
+use crate::core::reader::span::Span;
+use crate::core::reader::types::global::{Global, GlobalType};
+use crate::core::reader::{WasmReadable, WasmReader};
+use crate::{Error, NumType, Result, ValType};
+
+use super::validation_stack::ValidationStack;
+
+/// Validate the global section.
+///
+/// The global section is a vector of global variables. Each [Global] variable is composed of a [GlobalType] and an
+/// initialization expression represented by a constant expression.
+///
+/// See [read_constant_instructions] for more information.
+pub(super) fn validate_global_section(
+    wasm: &mut WasmReader,
+    section_header: SectionHeader,
+) -> Result<Vec<Global>> {
+    assert_eq!(section_header.ty, SectionTy::Global);
+
+    wasm.read_vec(|wasm| {
+        let ty = GlobalType::read(wasm)?;
+        let init_expr =
+            read_constant_instructions(wasm, ty.ty, &[/* todo!(imported globals tpyes) */])?;
+
+        Ok(Global { ty, init_expr })
+    })
+}
 
 /// Read and validate constant expressions.
 ///
-/// This function, alongside [`validate_value_stack()`](crate::validation::validate_value_stack) can be used to validate
-/// that a constant expression produces the expected result. The main use case for this is to validate that an
-/// initialization expression for a global returns the correct value.
+/// This function is used to validate that a constant expression produces the expected result. The main use case for
+/// this is to validate that an initialization expression for a global returns the correct value.
 ///
 /// Note: to be valid, constant expressions may not leave garbage data on the stack. It may leave only what is expected
 /// and nothing more.
@@ -82,23 +105,15 @@ use crate::{
 /// - `ref.null`
 /// - `ref.func`
 /// - `global.get`
-pub(crate) fn read_constant_instructions(
+pub(super) fn read_constant_instructions(
     wasm: &mut WasmReader,
-    value_stack: &mut Vec<ValType>,
+    this_global_valtype: ValType,
     _globals_ty: &[GlobalType],
 ) -> Result<Span> {
     let start_pc = wasm.pc;
 
-    let assert_pop_value_stack = |value_stack: &mut Vec<ValType>, expected_ty: ValType| {
-        value_stack
-            .pop()
-            .ok_or(Error::InvalidValueStackType(None))
-            .and_then(|ty| {
-                (ty == expected_ty)
-                    .then_some(())
-                    .ok_or(Error::InvalidValueStackType(Some(ty)))
-            })
-    };
+    // Compared to the code validation, we create the validation stack here as opposed to taking it as an argument.
+    let mut stack = ValidationStack::new();
 
     loop {
         let Ok(first_instr_byte) = wasm.read_u8() else {
@@ -110,27 +125,29 @@ pub(crate) fn read_constant_instructions(
         match first_instr_byte {
             // Missing: ref.null, ref.func, global.get
             END => {
+                // The stack must only contain the global's valtype
+                stack.assert_val_types(&[this_global_valtype])?;
                 return Ok(Span::new(start_pc, wasm.pc - start_pc + 1));
             }
             I32_CONST => {
                 let _num = wasm.read_var_i32()?;
-                value_stack.push(ValType::NumType(NumType::I32));
+                stack.push_valtype(ValType::NumType(NumType::I32));
             }
             I64_CONST => {
                 let _num = wasm.read_var_i64()?;
-                value_stack.push(ValType::NumType(NumType::I64));
+                stack.push_valtype(ValType::NumType(NumType::I64));
             }
             I32_ADD | I32_SUB | I32_MUL => {
-                assert_pop_value_stack(value_stack, ValType::NumType(NumType::I32))?;
-                assert_pop_value_stack(value_stack, ValType::NumType(NumType::I32))?;
+                stack.assert_pop_val_type(ValType::NumType(NumType::I32))?;
+                stack.assert_pop_val_type(ValType::NumType(NumType::I32))?;
 
-                value_stack.push(ValType::NumType(NumType::I32));
+                stack.push_valtype(ValType::NumType(NumType::I32));
             }
             I64_ADD | I64_SUB | I64_MUL => {
-                assert_pop_value_stack(value_stack, ValType::NumType(NumType::I64))?;
-                assert_pop_value_stack(value_stack, ValType::NumType(NumType::I64))?;
+                stack.assert_pop_val_type(ValType::NumType(NumType::I64))?;
+                stack.assert_pop_val_type(ValType::NumType(NumType::I64))?;
 
-                value_stack.push(ValType::NumType(NumType::I64));
+                stack.push_valtype(ValType::NumType(NumType::I64));
             }
             _ => return Err(Error::InvalidInstr(first_instr_byte)),
         }

src/validation/mod.rs

@@ -6,9 +6,9 @@ use crate::core::reader::span::Span;
 use crate::core::reader::types::export::Export;
 use crate::core::reader::types::global::Global;
 use crate::core::reader::types::import::Import;
-use crate::core::reader::types::{FuncType, MemType, ResultType, TableType};
+use crate::core::reader::types::{FuncType, MemType, TableType};
 use crate::core::reader::{WasmReadable, WasmReader};
-use crate::{Error, Result, ValType};
+use crate::{Error, Result};
 
 pub(crate) mod code;
 pub(crate) mod globals;
@@ -97,13 +97,8 @@ pub fn validate(wasm: &[u8]) -> Result<ValidationInfo> {
 
     while (skip_section(&mut wasm, &mut header)?).is_some() {}
 
-    let globals = handle_section(&mut wasm, &mut header, SectionTy::Global, |wasm, _| {
-        wasm.read_vec(|wasm| {
-            // TODO validate instructions in `global.init_expr`. Furthermore all of these instructions need to be constant.
-            //  See https://webassembly.github.io/spec/core/valid/instructions.html#valid-constant
-            //  Maybe we can also execute constant expressions right here so they do not even exist in the runtime environment. <-- Needs further research to check if this is even possible
-            Global::read(wasm)
-        })
+    let globals = handle_section(&mut wasm, &mut header, SectionTy::Global, |wasm, h| {
+        globals::validate_global_section(wasm, h)
     })?
     .unwrap_or_default();
 
@@ -194,22 +189,3 @@ fn handle_section<T, F: FnOnce(&mut WasmReader, SectionHeader) -> Result<T>>(
         _ => Ok(None),
     }
 }
-
-pub(crate) fn validate_value_stack<F>(return_ty: ResultType, f: F) -> Result<()>
-where
-    F: FnOnce(&mut Vec<ValType>) -> Result<()>,
-{
-    let mut value_stack: Vec<ValType> = Vec::new();
-
-    f(&mut value_stack)?;
-
-    // TODO also check here if correct order
-    if value_stack != return_ty.valtypes {
-        error!(
-            "Expected types {:?} on stack, got {:?}",
-            return_ty.valtypes, value_stack
-        );
-        return Err(Error::EndInvalidValueStack);
-    }
-    Ok(())
-}