structured ctrl

[cemonem]

Pull Request Overview

• implementations for block...end, br, br_if, br_table, return, loop...end, if..else...end instructions.
• ctrl stack and type stack are separated during validation to avoid linear look up for ctrl labels.
• sidetable generation per function.
• integration of sidetable and sidetable pointer in the interpreter loop for all ctrl structures.
• execution tests for structured_ctrl.
• assertion of types in the type stack during validation might mutate the stack. Any error during validation may leave the stack in a modified, invalid state.
• validation tests for cases where assertion of types that do mutate the stack, and this change is required.
• preparation for the type polymorphic select instruction (Polymorphic type NumOrVecType for select instruction on the type stack)

TODO

• Per module sidetables instead of per function sidetables.
• Backpatching sidetable entries without extra memory / without vectors in the CtrlStackEntries (using the linked list trick in wizard)

This pull request still needs...

Formatting

• Ran cargo fmt
• Ran cargo check
• Ran cargo build
• Ran cargo doc
• Ran nix fmt

[codecov]

Codecov Report

Attention: Patch coverage is 89.52802% with 71 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/validation/code.rs	80.00%	9 Missing and 28 partials ⚠️
src/validation/validation_stack.rs	93.60%	12 Missing and 7 partials ⚠️
src/core/error.rs	0.00%	6 Missing ⚠️
src/core/reader/types/values.rs	64.28%	2 Missing and 3 partials ⚠️
src/core/reader/types/mod.rs	94.11%	0 Missing and 3 partials ⚠️
src/validation/mod.rs	95.23%	1 Missing ⚠️

Files with missing lines	Coverage Δ
src/core/reader/mod.rs	98.53% <100.00%> (+0.02%)	⬆️
src/core/reader/types/element.rs	82.29% <100.00%> (ø)
src/execution/interpreter_loop.rs	91.95% <100.00%> (+0.55%)	⬆️
src/execution/mod.rs	90.52% <100.00%> (+0.04%)	⬆️
src/execution/store.rs	81.81% <ø> (ø)
src/execution/value_stack.rs	80.59% <100.00%> (-2.22%)	⬇️
src/validation/mod.rs	79.05% <95.23%> (+0.58%)	⬆️
src/core/reader/types/mod.rs	57.26% <94.11%> (+9.89%)	⬆️
src/core/reader/types/values.rs	83.33% <64.28%> (-2.67%)	⬇️
src/core/error.rs	2.79% <0.00%> (-0.06%)	⬇️
... and 2 more

... and 1 file with indirect coverage changes

[george-cosma]

My review is not yet done, I still have to check the validation stack changes

[george-cosma]

Is there a reason why we continue and we don't break?

Suggested change

	// TODO there is definitely a better to write this
	let current_func_span = store
	.funcs
	.get(stack.current_stackframe().func_idx)
	.unwrap_validated()
	.code_expr;
	if wasm.pc != current_func_span.from() + current_func_span.len() {
	continue;
	}
	let current_func_span = store
	.funcs
	.get(stack.current_stackframe().func_idx)
	.unwrap_validated()
	.code_expr;
	if wasm.pc != current_func_span.from() + current_func_span.len() {
	continue;
	}

[cemonem]

if this is not the very last END instr, END does not do anything, so we read the next instr (wasm.pc < last_end_idx). The dodgy case here is when the pc is erroneously after the very last END (wasm.pc > last_end_idx). I let it continue here, but we could also give an error or trap saying "instruction pointer is out of bounds"