Skip to content

Commit

Permalink
refactor(globals): move global implementation around and document it
Browse files Browse the repository at this point in the history
Signed-off-by: George Cosma <[email protected]>
  • Loading branch information
george-cosma committed Aug 29, 2024
1 parent 6f889ac commit 2b61458
Show file tree
Hide file tree
Showing 8 changed files with 309 additions and 225 deletions.
4 changes: 2 additions & 2 deletions src/core/reader/types/global.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use alloc::vec;

use crate::code::{read_constant_instructions, validate_value_stack};
use crate::core::reader::span::Span;
use crate::core::reader::types::{ResultType, ValType};
use crate::core::reader::{WasmReadable, WasmReader};
use crate::execution::assert_validated::UnwrapValidatedExt;
use crate::{unreachable_validated, Error, Result};
use crate::globals::read_constant_instructions;
use crate::{unreachable_validated, validate_value_stack, Error, Result};

#[derive(Debug, Copy, Clone)]
pub struct Global {
Expand Down
101 changes: 101 additions & 0 deletions src/execution/const_interpreter_loop.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
use crate::{
assert_validated::UnwrapValidatedExt, core::reader::WasmReader, value_stack::Stack, NumType,
ValType,
};

/// Execute a previosly-validated constant expression. These type of expressions are used for initializing global
/// variables.
///
/// # Arguments
/// - `wasm` - a [WasmReader] whose [program counter](WasmReader::pc) is set at the beginning of the constant
/// expression. Reader will be consumed.
/// - `stack` - a [Stack]. It is preferrable for it to be clean, but that is not required. As long as the executed code
/// is validated, the values on this stack will remain the same except for the addition of the return value of this
/// code sequence. A global's final value can be popped off the top of the stack.
/// - `imported_globals` (TODO) - instances of all imported globals. They are required as local globals can reference
/// imported globals in their initialization.
///
/// # Safety
/// This function assumes that the expression has been validated. Passing unvalidated code will likely result in a
/// panic, or undefined behaviour.
///
/// # Note
/// The following instructions are not yet supported:
/// - `ref.null`
/// - `ref.func`
/// - `global.get`
pub(crate) fn run_const(
mut wasm: WasmReader,
stack: &mut Stack,
_imported_globals: (), /*todo!*/
) {
use crate::core::reader::types::opcode::*;
loop {
let first_instr_byte = wasm.read_u8().unwrap_validated();

match first_instr_byte {
END => {
break;
}
I32_CONST => {
let constant = wasm.read_var_i32().unwrap_validated();
trace!("Constant instruction: i32.const [] -> [{constant}]");
stack.push_value(constant.into());
}
I32_ADD => {
let v1: i32 = stack.pop_value(ValType::NumType(NumType::I32)).into();
let v2: i32 = stack.pop_value(ValType::NumType(NumType::I32)).into();
let res = v1.wrapping_add(v2);

trace!("Constant instruction: i32.add [{v1} {v2}] -> [{res}]");
stack.push_value(res.into());
}
I32_SUB => {
let v2: i32 = stack.pop_value(ValType::NumType(NumType::I32)).into();
let v1: i32 = stack.pop_value(ValType::NumType(NumType::I32)).into();
let res = v1.wrapping_sub(v2);

trace!("Constant instruction: i32.sub [{v1} {v2}] -> [{res}]");
stack.push_value(res.into());
}
I32_MUL => {
let v1: i32 = stack.pop_value(ValType::NumType(NumType::I32)).into();
let v2: i32 = stack.pop_value(ValType::NumType(NumType::I32)).into();
let res = v1.wrapping_mul(v2);

trace!("Constant instruction: i32.mul [{v1} {v2}] -> [{res}]");
stack.push_value(res.into());
}
I64_CONST => {
let constant = wasm.read_var_i64().unwrap_validated();
trace!("Constant instruction: i64.const [] -> [{constant}]");
stack.push_value(constant.into());
}
I64_ADD => {
let v1: i64 = stack.pop_value(ValType::NumType(NumType::I64)).into();
let v2: i64 = stack.pop_value(ValType::NumType(NumType::I64)).into();
let res = v1.wrapping_add(v2);

trace!("Constant instruction: i64.add [{v1} {v2}] -> [{res}]");
stack.push_value(res.into());
}
I64_SUB => {
let v2: i64 = stack.pop_value(ValType::NumType(NumType::I64)).into();
let v1: i64 = stack.pop_value(ValType::NumType(NumType::I64)).into();
let res = v1.wrapping_sub(v2);

trace!("Constant instruction: i64.sub [{v1} {v2}] -> [{res}]");
stack.push_value(res.into());
}
I64_MUL => {
let v1: i64 = stack.pop_value(ValType::NumType(NumType::I64)).into();
let v2: i64 = stack.pop_value(ValType::NumType(NumType::I64)).into();
let res = v1.wrapping_mul(v2);

trace!("Constant instruction: i64.mul [{v1} {v2}] -> [{res}]");
stack.push_value(res.into());
}
_ => panic!(\\_(ツ)_/¯"),
}
}
}
73 changes: 0 additions & 73 deletions src/execution/interpreter_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1083,76 +1083,3 @@ pub(super) fn run<H: HookSet>(
}
Ok(())
}

pub fn run_const(mut wasm: WasmReader, stack: &mut Stack, _imported_globals: () /*todo!*/) {
use crate::core::reader::types::opcode::*;
loop {
let first_instr_byte = wasm.read_u8().unwrap_validated();

match first_instr_byte {
// Missing: ref.null, ref.func, global.get
END => {
break;
}
I32_CONST => {
let constant = wasm.read_var_i32().unwrap_validated();
trace!("Constant instruction: i32.const [] -> [{constant}]");
stack.push_value(constant.into());
}
I32_ADD => {
let v1: i32 = stack.pop_value(ValType::NumType(NumType::I32)).into();
let v2: i32 = stack.pop_value(ValType::NumType(NumType::I32)).into();
let res = v1.wrapping_add(v2);

trace!("Constant instruction: i32.add [{v1} {v2}] -> [{res}]");
stack.push_value(res.into());
}
I32_SUB => {
let v2: i32 = stack.pop_value(ValType::NumType(NumType::I32)).into();
let v1: i32 = stack.pop_value(ValType::NumType(NumType::I32)).into();
let res = v1.wrapping_sub(v2);

trace!("Constant instruction: i32.sub [{v1} {v2}] -> [{res}]");
stack.push_value(res.into());
}
I32_MUL => {
let v1: i32 = stack.pop_value(ValType::NumType(NumType::I32)).into();
let v2: i32 = stack.pop_value(ValType::NumType(NumType::I32)).into();
let res = v1.wrapping_mul(v2);

trace!("Constant instruction: i32.mul [{v1} {v2}] -> [{res}]");
stack.push_value(res.into());
}
I64_CONST => {
let constant = wasm.read_var_i64().unwrap_validated();
trace!("Constant instruction: i64.const [] -> [{constant}]");
stack.push_value(constant.into());
}
I64_ADD => {
let v1: i64 = stack.pop_value(ValType::NumType(NumType::I64)).into();
let v2: i64 = stack.pop_value(ValType::NumType(NumType::I64)).into();
let res = v1.wrapping_add(v2);

trace!("Constant instruction: i64.add [{v1} {v2}] -> [{res}]");
stack.push_value(res.into());
}
I64_SUB => {
let v2: i64 = stack.pop_value(ValType::NumType(NumType::I64)).into();
let v1: i64 = stack.pop_value(ValType::NumType(NumType::I64)).into();
let res = v1.wrapping_sub(v2);

trace!("Constant instruction: i64.sub [{v1} {v2}] -> [{res}]");
stack.push_value(res.into());
}
I64_MUL => {
let v1: i64 = stack.pop_value(ValType::NumType(NumType::I64)).into();
let v2: i64 = stack.pop_value(ValType::NumType(NumType::I64)).into();
let res = v1.wrapping_mul(v2);

trace!("Constant instruction: i64.mul [{v1} {v2}] -> [{res}]");
stack.push_value(res.into());
}
_ => panic!(\\_(ツ)_/¯"),
}
}
}
29 changes: 17 additions & 12 deletions src/execution/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use alloc::vec::Vec;

use interpreter_loop::{run, run_const};
use const_interpreter_loop::run_const;
use interpreter_loop::run;
use locals::Locals;
use value_stack::Stack;

Expand All @@ -18,6 +19,7 @@ use crate::{RuntimeError, ValidationInfo};

// TODO
pub(crate) mod assert_validated;
mod const_interpreter_loop;
pub mod hooks;
mod interpreter_loop;
pub(crate) mod locals;
Expand Down Expand Up @@ -240,18 +242,21 @@ where
let global_instances: Vec<GlobalInst> = validation_info
.globals
.iter()
.map(|global| {
let mut wasm = WasmReader::new(validation_info.wasm);
// The place we are moving the start to should, by all means, be inside the wasm bytecode.
wasm.move_start_to(global.init_expr).unwrap_validated();
.map({
let mut stack = Stack::new();

run_const(wasm, &mut stack, ());
let value = stack.pop_value(global.ty.ty);

GlobalInst {
global: *global,
value,
move |global| {
let mut wasm = WasmReader::new(validation_info.wasm);
// The place we are moving the start to should, by all means, be inside the wasm bytecode.
wasm.move_start_to(global.init_expr).unwrap_validated();
// We shouldn't need to clear the stack. If validation is correct, it will remain empty after execution.

run_const(wasm, &mut stack, ());
let value = stack.pop_value(global.ty.ty);

GlobalInst {
global: *global,
value,
}
}
})
.collect();
Expand Down
138 changes: 3 additions & 135 deletions src/validation/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ use core::iter;
use crate::core::indices::{FuncIdx, GlobalIdx, LocalIdx};
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::types::global::Global;
use crate::core::reader::types::memarg::MemArg;
use crate::core::reader::types::{FuncType, NumType, ResultType, ValType};
use crate::core::reader::types::{FuncType, NumType, ValType};
use crate::core::reader::{WasmReadable, WasmReader};
use crate::{Error, Result};
use crate::{validate_value_stack, Error, Result};

pub fn validate_code_section(
wasm: &mut WasmReader,
Expand Down Expand Up @@ -356,135 +356,3 @@ fn read_instructions(
}
}
}

pub fn read_constant_instructions(
wasm: &mut WasmReader,
value_stack: &mut Vec<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)))
})
};

loop {
let Ok(first_instr_byte) = wasm.read_u8() else {
return Err(Error::ExprMissingEnd);
};
trace!("Read cosntant instruction byte {first_instr_byte:#X?} ({first_instr_byte})");

// Valid constant instructions are:
// - Core: https://webassembly.github.io/spec/core/valid/instructions.html#valid-constant
// - Extended Proposal: https://webassembly.github.io/extended-const/core/valid/instructions.html#valid-constant
use crate::core::reader::types::opcode::*;
match first_instr_byte {
// Missing: ref.null, ref.func, global.get
// -------------
// Global.get only (seems) to work for imported globals
//
// Take the example code:
// ```wat
// (module
// (global (export "g") (mut i32) (
// i32.add (i32.const 1) (i32.const 2)
// ))
//
// (global (export "h1") i32 (
// i32.const 1
// ))
//
// (global (export "h2") i32 (
// global.get 1
// ))
//
// (func (export "f")
// i32.const 100
// global.set 0))
// ```
//
// When compiling with wat2wasm, the following error is thrown:
// ```
// Error: validate failed:
// test.wast:11:24: error: initializer expression can only reference an imported global
// global.get 1
// ^
// ```
//
// When compiling the code with the latest dev build of wasmtime, the following error is thrown:
// ```
// failed to parse WebAssembly module
//
// Caused by:
// constant expression required: global.get of locally defined global (at offset 0x24)
// ```
//
// Furthermore, the global must be immutable:
// ```wat
// (module
// (import "env" "g" (global (mut i32)))
// (global (export "h") (mut i32) (
// i32.add (i32.const 1) (global.get 0)
// ))
// )
// ```
//
// When compiling with wat2wasm, the following error is thrown:
// ```
// Error: validate failed:
// test.wast:4:27: error: initializer expression cannot reference a mutable global
// i32.add (i32.const 1) (global.get 0)
// ```
END => {
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));
}
I64_CONST => {
let _num = wasm.read_var_i64()?;
value_stack.push(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))?;

value_stack.push(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))?;

value_stack.push(ValType::NumType(NumType::I64));
}
_ => return Err(Error::InvalidInstr(first_instr_byte)),
}
}
}

pub 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(())
}
Loading

0 comments on commit 2b61458

Please sign in to comment.