Skip to content

Commit 822e134

Browse files
Fix dead code validation for multivalue block (bytecodealliance#193)
* Fix dead code validation for multivalue block Avoid assert/numeric overflow assotiated with "borrowing" unknown stack types from polymorphyc stack when multivalue block is created. * Update src/operators_validator.rs Co-Authored-By: Nick Fitzgerald <fitzgen@gmail.com> * Add wast tests, get wasm features flags from wast Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>
1 parent 9faf21e commit 822e134

4 files changed

Lines changed: 73 additions & 3 deletions

File tree

src/operators_validator.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,15 @@ pub(crate) fn is_subtype_supertype(subtype: Type, supertype: Type) -> bool {
3636
struct BlockState {
3737
start_types: Vec<Type>,
3838
return_types: Vec<Type>,
39+
// Position in `FuncState::stack_types` array where block values
40+
// start.
3941
stack_starts_at: usize,
4042
jump_to_top: bool,
4143
is_else_allowed: bool,
4244
is_dead_code: bool,
45+
// Amount of the required polymorphic values at the stack_starts_at
46+
// position in `FuncState::stack_types` array. These values are
47+
// fictitious and are not actually present in the stack_types.
4348
polymorphic_values: Option<usize>,
4449
}
4550

@@ -154,6 +159,7 @@ impl FuncState {
154159
}
155160
};
156161
if block_type == BlockType::If {
162+
// Collect conditional value from the stack_types.
157163
let last_block = self.blocks.last().unwrap();
158164
if !last_block.is_stack_polymorphic()
159165
|| self.stack_types.len() > last_block.stack_starts_at
@@ -167,15 +173,28 @@ impl FuncState {
167173
return Err(OperatorValidatorError::new("stack operand type mismatch"));
168174
}
169175
}
170-
let stack_starts_at = self.stack_types.len() - start_types.len();
176+
let (stack_starts_at, polymorphic_values) = {
177+
// When stack for last block is polymorphic, ensure that
178+
// the polymorphic_values matches, and next block is informed about that.
179+
let last_block = self.blocks.last_mut().unwrap();
180+
if !last_block.is_stack_polymorphic()
181+
|| last_block.stack_starts_at + start_types.len() <= self.stack_types.len()
182+
{
183+
(self.stack_types.len() - start_types.len(), None)
184+
} else {
185+
let unknown_stack_types_len =
186+
last_block.stack_starts_at + start_types.len() - self.stack_types.len();
187+
(last_block.stack_starts_at, Some(unknown_stack_types_len))
188+
}
189+
};
171190
self.blocks.push(BlockState {
172191
start_types,
173192
return_types,
174193
stack_starts_at,
175194
jump_to_top: block_type == BlockType::Loop,
176195
is_else_allowed: block_type == BlockType::If,
177196
is_dead_code: false,
178-
polymorphic_values: None,
197+
polymorphic_values,
179198
});
180199
Ok(())
181200
}

src/tests.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,26 @@ mod wast_tests {
342342
}
343343
}
344344

345+
fn extract_config(wast: &str) -> ValidatingParserConfig {
346+
let first = wast.split('\n').next();
347+
if first.is_none() || !first.unwrap().starts_with(";;; ") {
348+
return default_config();
349+
}
350+
let first = first.unwrap();
351+
ValidatingParserConfig {
352+
operator_config: OperatorValidatorConfig {
353+
enable_threads: first.contains("--enable-threads"),
354+
enable_reference_types: first.contains("--enable-reference-types"),
355+
enable_simd: first.contains("--enable-simd"),
356+
enable_bulk_memory: first.contains("--enable-bulk-memory"),
357+
enable_multi_value: first.contains("--enable-multi-value"),
358+
359+
#[cfg(feature = "deterministic")]
360+
deterministic_only: true,
361+
},
362+
}
363+
}
364+
345365
fn validate_module(
346366
mut module: wast::Module,
347367
config: ValidatingParserConfig,
@@ -545,10 +565,11 @@ mod wast_tests {
545565
}
546566

547567
let data = read(&dir.path()).expect("wast data");
568+
let config = extract_config(&String::from_utf8_lossy(&data));
548569
run_wabt_scripts(
549570
dir.file_name().to_str().expect("name"),
550571
&data,
551-
default_config(),
572+
config,
552573
|_, _| false,
553574
);
554575
}

tests/invalid/issue192.wasm

145 Bytes
Binary file not shown.

tests/wast/issue192.wast

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
;;; --enable-multi-value
2+
(module
3+
(func
4+
(unreachable)
5+
(block (param i32)
6+
(drop)
7+
)
8+
)
9+
)
10+
11+
(assert_invalid
12+
(module
13+
(func
14+
(unreachable)
15+
(block (param i32))
16+
)
17+
)
18+
"type mismatch: stack size does not match block type"
19+
)
20+
21+
(assert_invalid
22+
(module
23+
(func
24+
(block (param i32)
25+
(drop)
26+
)
27+
)
28+
)
29+
"type mismatch: not enough operands"
30+
)

0 commit comments

Comments
 (0)