Skip to content

Commit 8ccc22a

Browse files
refactor: add labelidx readers
Signed-off-by: Florian Hartung <florian.hartung@dlr.de>
1 parent b0b1f36 commit 8ccc22a

File tree

5 files changed

+64
-24
lines changed

5 files changed

+64
-24
lines changed

src/core/error.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::RefType;
33
use core::fmt::{Display, Formatter};
44
use core::str::Utf8Error;
55

6-
use super::indices::{FuncIdx, LabelIdx};
6+
use super::indices::FuncIdx;
77
use crate::core::reader::section_header::SectionTy;
88
use crate::core::reader::types::ValType;
99

@@ -67,7 +67,7 @@ pub enum ValidationError {
6767
/// An index for a local is invalid.
6868
InvalidLocalIdx(u32),
6969
/// An index for a label is invalid.
70-
InvalidLabelIdx(LabelIdx),
70+
InvalidLabelIdx(u32),
7171
/// An index for a lane of some vector type is invalid.
7272
InvalidLaneIdx(u8),
7373

src/core/indices.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,4 +687,18 @@ impl LocalIdx {
687687
}
688688
}
689689

690-
pub type LabelIdx = usize;
690+
/// Reads a label index from Wasm code without validating it.
691+
pub fn read_label_idx(wasm: &mut WasmReader) -> Result<u32, ValidationError> {
692+
wasm.read_var_u32()
693+
}
694+
695+
/// Reads a label index from Wasm code without validating it.
696+
///
697+
/// # Safety
698+
///
699+
/// The caller must ensure that there is a valid label index in the
700+
/// [`WasmReader`].
701+
pub unsafe fn read_label_idx_unchecked(wasm: &mut WasmReader) -> u32 {
702+
// TODO use `unwrap_unchecked` instead
703+
wasm.read_var_u32().unwrap()
704+
}

src/execution/interpreter_loop.rs

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ use crate::{
1919
addrs::{AddrVec, DataAddr, ElemAddr, MemAddr, ModuleAddr, TableAddr},
2020
assert_validated::UnwrapValidatedExt,
2121
core::{
22-
indices::{DataIdx, ElemIdx, FuncIdx, GlobalIdx, Idx, LocalIdx, MemIdx, TableIdx, TypeIdx},
22+
indices::{
23+
read_label_idx_unchecked, DataIdx, ElemIdx, FuncIdx, GlobalIdx, Idx, LocalIdx, MemIdx,
24+
TableIdx, TypeIdx,
25+
},
2326
reader::{
2427
types::{memarg::MemArg, BlockType},
2528
WasmReader,
@@ -163,7 +166,10 @@ pub(super) fn run<T: Config>(
163166
}
164167
BR_IF => {
165168
decrement_fuel!(T::get_flat_cost(BR_IF));
166-
wasm.read_var_u32().unwrap_validated();
169+
170+
// SAFETY: Validation guarantees there to be a valid label index
171+
// next.
172+
let _label_idx = unsafe { read_label_idx_unchecked(wasm) };
167173

168174
let test_val: i32 = stack.pop_value().try_into().unwrap_validated();
169175

@@ -177,9 +183,16 @@ pub(super) fn run<T: Config>(
177183
BR_TABLE => {
178184
decrement_fuel!(T::get_flat_cost(BR_TABLE));
179185
let label_vec = wasm
180-
.read_vec(|wasm| wasm.read_var_u32().map(|v| v.into_usize()))
186+
.read_vec(|wasm| {
187+
// SAFETY: Validation guarantees that there is a
188+
// valid vec of label indices.
189+
Ok(unsafe { read_label_idx_unchecked(wasm) })
190+
})
181191
.unwrap_validated();
182-
wasm.read_var_u32().unwrap_validated();
192+
193+
// SAFETY: Validation guarantees there to be another label index
194+
// for the default case.
195+
let _default_label_idx = unsafe { read_label_idx_unchecked(wasm) };
183196

184197
// TODO is this correct?
185198
let case_val_i32: i32 = stack.pop_value().try_into().unwrap_validated();
@@ -195,8 +208,9 @@ pub(super) fn run<T: Config>(
195208
}
196209
BR => {
197210
decrement_fuel!(T::get_flat_cost(BR));
198-
//skip n of BR n
199-
wasm.read_var_u32().unwrap_validated();
211+
// SAFETY: Validation guarantees there to be a valid label index
212+
// next.
213+
let _label_idx = unsafe { read_label_idx_unchecked(wasm) };
200214
do_sidetable_control_transfer(wasm, stack, &mut stp, current_sidetable)?;
201215
}
202216
BLOCK => {
@@ -213,7 +227,7 @@ pub(super) fn run<T: Config>(
213227
}
214228
RETURN => {
215229
decrement_fuel!(T::get_flat_cost(RETURN));
216-
//same as BR, except no need to skip n of BR n
230+
// same as BR
217231
do_sidetable_control_transfer(wasm, stack, &mut stp, current_sidetable)?;
218232
}
219233
CALL => {

src/validation/code.rs

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use alloc::collections::btree_set::BTreeSet;
44
use alloc::vec::Vec;
55

66
use crate::core::indices::{
7-
DataIdx, ElemIdx, ExtendedIdxVec, FuncIdx, GlobalIdx, IdxVec, LocalIdx, MemIdx, TableIdx,
8-
TypeIdx,
7+
read_label_idx, DataIdx, ElemIdx, ExtendedIdxVec, FuncIdx, GlobalIdx, IdxVec, LocalIdx, MemIdx,
8+
TableIdx, TypeIdx,
99
};
1010
use crate::core::reader::section_header::{SectionHeader, SectionTy};
1111
use crate::core::reader::span::Span;
@@ -147,7 +147,7 @@ pub fn read_declared_locals(wasm: &mut WasmReader) -> Result<Vec<ValType>, Valid
147147
/// Branches are generated by branching instructions and some can even generate multiple branches.
148148
fn validate_branch_and_generate_sidetable_entry(
149149
wasm: &WasmReader,
150-
label_idx: usize,
150+
label_idx: u32,
151151
stack: &mut ValidationStack,
152152
sidetable: &mut Sidetable,
153153
unify_to_expected_types: bool,
@@ -160,7 +160,7 @@ fn validate_branch_and_generate_sidetable_entry(
160160
let index_of_label_in_ctrl_stack = stack
161161
.ctrl_stack
162162
.len()
163-
.checked_sub(label_idx)
163+
.checked_sub(label_idx.into_usize())
164164
.and_then(|i| i.checked_sub(1));
165165

166166
let targeted_ctrl_block_entry = index_of_label_in_ctrl_stack
@@ -349,14 +349,14 @@ unsafe fn read_instructions(
349349
}
350350
}
351351
BR => {
352-
let label_idx = wasm.read_var_u32()?.into_usize();
352+
let label_idx = read_label_idx(wasm)?;
353353
validate_branch_and_generate_sidetable_entry(
354354
wasm, label_idx, stack, sidetable, false,
355355
)?;
356356
stack.make_unspecified()?;
357357
}
358358
BR_IF => {
359-
let label_idx = wasm.read_var_u32()?.into_usize();
359+
let label_idx = read_label_idx(wasm)?;
360360
stack.assert_pop_val_type(ValType::NumType(NumType::I32))?;
361361
// The types are explicitly popped and pushed in
362362
// https://webassembly.github.io/spec/core/appendix/algorithm.html
@@ -366,9 +366,8 @@ unsafe fn read_instructions(
366366
)?;
367367
}
368368
BR_TABLE => {
369-
let label_vec =
370-
wasm.read_vec(|wasm| wasm.read_var_u32().map(|v| v.into_usize()))?;
371-
let max_label_idx = wasm.read_var_u32()?.into_usize();
369+
let label_vec = wasm.read_vec(read_label_idx)?;
370+
let max_label_idx = read_label_idx(wasm)?;
372371
stack.assert_pop_val_type(ValType::NumType(NumType::I32))?;
373372
for label_idx in &label_vec {
374373
validate_branch_and_generate_sidetable_entry(
@@ -390,16 +389,19 @@ unsafe fn read_instructions(
390389
// in which the smaller arity type is a suffix in the label type list of the larger arity function
391390

392391
// stack includes all labels, that check is made in the above fn already
392+
let max_label_idx = max_label_idx.into_usize();
393+
393394
let max_label_arity = stack
394395
.ctrl_stack
395396
.get(stack.ctrl_stack.len() - max_label_idx - 1)
396397
.unwrap()
397398
.label_types()
398399
.len();
399400
for label_idx in &label_vec {
401+
let label_idx_as_usize = label_idx.into_usize();
400402
let label_arity = stack
401403
.ctrl_stack
402-
.get(stack.ctrl_stack.len() - *label_idx - 1)
404+
.get(stack.ctrl_stack.len() - label_idx_as_usize - 1)
403405
.unwrap()
404406
.label_types()
405407
.len();
@@ -466,7 +468,14 @@ unsafe fn read_instructions(
466468
}
467469
}
468470
RETURN => {
469-
let label_idx = stack.ctrl_stack.len() - 1; // return behaves the same as br <most_outer>
471+
// Generally `return` behaves the same as `br <most_outer>`,
472+
// except when there are more than [`u32::MAX`] labels. In that
473+
// case `return` would still be valid, while `br (u32::MAX + n)`
474+
// is invalid.
475+
//
476+
// TODO Find a way to get rid of this limitation
477+
let label_idx = u32::try_from(stack.ctrl_stack.len() - 1)
478+
.expect("that there are never so many nested control blocks that the index to the outermost label exceeds u32::MAX");
470479
validate_branch_and_generate_sidetable_entry(
471480
wasm, label_idx, stack, sidetable, false,
472481
)?;

src/validation/validation_stack.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ use alloc::vec;
44
use alloc::vec::Vec;
55

66
use crate::{
7-
core::reader::types::{FuncType, ResultType},
7+
core::{
8+
reader::types::{FuncType, ResultType},
9+
utils::ToUsizeExt,
10+
},
811
NumType, RefType, ValType, ValidationError,
912
};
1013

@@ -288,13 +291,13 @@ impl ValidationStack {
288291
///
289292
pub fn assert_val_types_of_label_jump_types_on_top(
290293
&mut self,
291-
label_idx: usize,
294+
label_idx: u32,
292295
unify_to_expected_types: bool,
293296
) -> Result<(), ValidationError> {
294297
let index_of_label_in_ctrl_stack = self
295298
.ctrl_stack
296299
.len()
297-
.checked_sub(label_idx)
300+
.checked_sub(label_idx.into_usize())
298301
.and_then(|i| i.checked_sub(1));
299302

300303
let label_types = index_of_label_in_ctrl_stack

0 commit comments

Comments
 (0)