Skip to content

Commit 320b7d7

Browse files
avrabeavrabe
committed
fix(wast): improve validator stack handling and add instruction coverage
- Fix operand stack initialization (params accessed via local.get, not stack) - Use saturating_sub for stack height calculations to prevent underflow - Add validation for additional i32/i64 comparison and bitwise operations - Add tracing in streaming decoder for debugging - Fix module.rs import handling edge case
1 parent aa1049f commit 320b7d7

File tree

4 files changed

+214
-11
lines changed

4 files changed

+214
-11
lines changed

‎wrt-build-core/src/wast.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,10 @@ impl WastTestRunner {
372372
Ok(directive_info) => {
373373
if directive_info.result == TestResult::Failed {
374374
file_status = TestResult::Failed;
375+
// Print error details for debugging
376+
if let Some(ref err_msg) = directive_info.error_message {
377+
eprintln!("[FAIL] {}: {}", directive_info.directive_name, err_msg);
378+
}
375379
}
376380
directive_results.push(directive_info);
377381
},

‎wrt-build-core/src/wast_validator.rs‎

Lines changed: 205 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -127,12 +127,8 @@ impl WastModuleValidator {
127127
local_types.push(*local);
128128
}
129129

130-
// Initialize operand stack with parameters
131-
let mut stack: Vec<StackType> = func_type
132-
.params
133-
.iter()
134-
.map(|&vt| StackType::from_value_type(vt))
135-
.collect();
130+
// Initialize operand stack (empty - parameters are accessed via local.get, not on stack)
131+
let mut stack: Vec<StackType> = Vec::new();
136132

137133
// Initialize control flow frames
138134
let mut frames: Vec<ControlFrame> = vec![ControlFrame {
@@ -172,12 +168,16 @@ impl WastModuleValidator {
172168
let (input_types, output_types) =
173169
Self::block_type_to_stack_types(&block_type, module)?;
174170

171+
// For blocks with inputs, the inputs are already on the stack
172+
// We record the stack height BEFORE the inputs
173+
let stack_height = stack.len().saturating_sub(input_types.len());
174+
175175
frames.push(ControlFrame {
176176
frame_type: FrameType::Block,
177177
input_types: input_types.clone(),
178178
output_types: output_types.clone(),
179179
reachable: true,
180-
stack_height: stack.len() - input_types.len(),
180+
stack_height,
181181
});
182182
}
183183
0x03 => {
@@ -188,12 +188,14 @@ impl WastModuleValidator {
188188
let (input_types, output_types) =
189189
Self::block_type_to_stack_types(&block_type, module)?;
190190

191+
let stack_height = stack.len().saturating_sub(input_types.len());
192+
191193
frames.push(ControlFrame {
192194
frame_type: FrameType::Loop,
193195
input_types: input_types.clone(),
194196
output_types: output_types.clone(),
195197
reachable: true,
196-
stack_height: stack.len() - input_types.len(),
198+
stack_height,
197199
});
198200
}
199201
0x04 => {
@@ -209,12 +211,14 @@ impl WastModuleValidator {
209211
let (input_types, output_types) =
210212
Self::block_type_to_stack_types(&block_type, module)?;
211213

214+
let stack_height = stack.len().saturating_sub(input_types.len());
215+
212216
frames.push(ControlFrame {
213217
frame_type: FrameType::If,
214218
input_types: input_types.clone(),
215219
output_types: output_types.clone(),
216220
reachable: true,
217-
stack_height: stack.len() - input_types.len(),
221+
stack_height,
218222
});
219223
}
220224
0x05 => {
@@ -582,6 +586,198 @@ impl WastModuleValidator {
582586
stack.push(StackType::I64);
583587
}
584588

589+
// i32.eqz (0x45): i32 -> i32
590+
0x45 => {
591+
if !Self::pop_type(&mut stack, StackType::I32) {
592+
return Err(anyhow!("i32.eqz: operand must be i32"));
593+
}
594+
stack.push(StackType::I32);
595+
}
596+
597+
// i32 comparison operations (0x46-0x4F): i32 i32 -> i32
598+
0x46 | 0x47 | 0x48 | 0x49 | 0x4A | 0x4B | 0x4C | 0x4D | 0x4E | 0x4F => {
599+
if !Self::pop_type(&mut stack, StackType::I32) {
600+
return Err(anyhow!("i32 comparison: second operand must be i32"));
601+
}
602+
if !Self::pop_type(&mut stack, StackType::I32) {
603+
return Err(anyhow!("i32 comparison: first operand must be i32"));
604+
}
605+
stack.push(StackType::I32);
606+
}
607+
608+
// i64.eqz (0x50): i64 -> i32
609+
0x50 => {
610+
if !Self::pop_type(&mut stack, StackType::I64) {
611+
return Err(anyhow!("i64.eqz: operand must be i64"));
612+
}
613+
stack.push(StackType::I32);
614+
}
615+
616+
// i64 comparison operations (0x51-0x5A): i64 i64 -> i32
617+
0x51 | 0x52 | 0x53 | 0x54 | 0x55 | 0x56 | 0x57 | 0x58 | 0x59 | 0x5A => {
618+
if !Self::pop_type(&mut stack, StackType::I64) {
619+
return Err(anyhow!("i64 comparison: second operand must be i64"));
620+
}
621+
if !Self::pop_type(&mut stack, StackType::I64) {
622+
return Err(anyhow!("i64 comparison: first operand must be i64"));
623+
}
624+
stack.push(StackType::I32);
625+
}
626+
627+
// i32 binary operations (0x6A-0x78): i32 i32 -> i32
628+
0x6A | 0x6B | 0x6C | 0x6D | 0x6E | 0x6F | 0x70 | 0x71 | 0x72 | 0x73 | 0x74 | 0x75 | 0x76 | 0x77 | 0x78 => {
629+
if !Self::pop_type(&mut stack, StackType::I32) {
630+
return Err(anyhow!("i32 binary: second operand must be i32"));
631+
}
632+
if !Self::pop_type(&mut stack, StackType::I32) {
633+
return Err(anyhow!("i32 binary: first operand must be i32"));
634+
}
635+
stack.push(StackType::I32);
636+
}
637+
638+
// i64 binary operations (0x7C-0x8A): i64 i64 -> i64
639+
0x7C | 0x7D | 0x7E | 0x7F | 0x80 | 0x81 | 0x82 | 0x83 | 0x84 | 0x85 | 0x86 | 0x87 | 0x88 | 0x89 | 0x8A | 0x8B => {
640+
if !Self::pop_type(&mut stack, StackType::I64) {
641+
return Err(anyhow!("i64 binary: second operand must be i64"));
642+
}
643+
if !Self::pop_type(&mut stack, StackType::I64) {
644+
return Err(anyhow!("i64 binary: first operand must be i64"));
645+
}
646+
stack.push(StackType::I64);
647+
}
648+
649+
// Conversion operations: i32 -> i64
650+
0xac | 0xad => {
651+
// i64.extend_i32_s (0xac), i64.extend_i32_u (0xad)
652+
if !Self::pop_type(&mut stack, StackType::I32) {
653+
return Err(anyhow!("i64.extend_i32: operand must be i32"));
654+
}
655+
stack.push(StackType::I64);
656+
}
657+
658+
// Conversion operations: i64 -> i32
659+
0xa7 => {
660+
// i32.wrap_i64
661+
if !Self::pop_type(&mut stack, StackType::I64) {
662+
return Err(anyhow!("i32.wrap_i64: operand must be i64"));
663+
}
664+
stack.push(StackType::I32);
665+
}
666+
667+
// Conversion operations: f32 <-> i32
668+
0xa8 | 0xa9 | 0xaa | 0xab => {
669+
// i32.trunc_f32_s (0xa8), i32.trunc_f32_u (0xa9)
670+
// i32.trunc_f64_s (0xaa), i32.trunc_f64_u (0xab)
671+
let is_f64 = opcode >= 0xaa;
672+
if is_f64 {
673+
if !Self::pop_type(&mut stack, StackType::F64) {
674+
return Err(anyhow!("i32.trunc: operand must be f64"));
675+
}
676+
} else {
677+
if !Self::pop_type(&mut stack, StackType::F32) {
678+
return Err(anyhow!("i32.trunc: operand must be f32"));
679+
}
680+
}
681+
stack.push(StackType::I32);
682+
}
683+
684+
// Conversion operations: f32/f64 <-> i64
685+
0xae | 0xaf | 0xb0 | 0xb1 => {
686+
// i64.trunc_f32_s (0xae), i64.trunc_f32_u (0xaf)
687+
// i64.trunc_f64_s (0xb0), i64.trunc_f64_u (0xb1)
688+
let is_f64 = opcode >= 0xb0;
689+
if is_f64 {
690+
if !Self::pop_type(&mut stack, StackType::F64) {
691+
return Err(anyhow!("i64.trunc: operand must be f64"));
692+
}
693+
} else {
694+
if !Self::pop_type(&mut stack, StackType::F32) {
695+
return Err(anyhow!("i64.trunc: operand must be f32"));
696+
}
697+
}
698+
stack.push(StackType::I64);
699+
}
700+
701+
// Conversion operations: i32/i64 -> f32
702+
0xb2 | 0xb3 | 0xb4 | 0xb5 => {
703+
// f32.convert_i32_s (0xb2), f32.convert_i32_u (0xb3)
704+
// f32.convert_i64_s (0xb4), f32.convert_i64_u (0xb5)
705+
let is_i64 = opcode >= 0xb4;
706+
if is_i64 {
707+
if !Self::pop_type(&mut stack, StackType::I64) {
708+
return Err(anyhow!("f32.convert: operand must be i64"));
709+
}
710+
} else {
711+
if !Self::pop_type(&mut stack, StackType::I32) {
712+
return Err(anyhow!("f32.convert: operand must be i32"));
713+
}
714+
}
715+
stack.push(StackType::F32);
716+
}
717+
718+
// Conversion operations: f64.demote_f32
719+
0xb6 => {
720+
if !Self::pop_type(&mut stack, StackType::F64) {
721+
return Err(anyhow!("f32.demote_f64: operand must be f64"));
722+
}
723+
stack.push(StackType::F32);
724+
}
725+
726+
// Conversion operations: i32/i64 -> f64
727+
0xb7 | 0xb8 | 0xb9 | 0xba => {
728+
// f64.convert_i32_s (0xb7), f64.convert_i32_u (0xb8)
729+
// f64.convert_i64_s (0xb9), f64.convert_i64_u (0xba)
730+
let is_i64 = opcode >= 0xb9;
731+
if is_i64 {
732+
if !Self::pop_type(&mut stack, StackType::I64) {
733+
return Err(anyhow!("f64.convert: operand must be i64"));
734+
}
735+
} else {
736+
if !Self::pop_type(&mut stack, StackType::I32) {
737+
return Err(anyhow!("f64.convert: operand must be i32"));
738+
}
739+
}
740+
stack.push(StackType::F64);
741+
}
742+
743+
// Conversion operations: f64.promote_f32
744+
0xbb => {
745+
if !Self::pop_type(&mut stack, StackType::F32) {
746+
return Err(anyhow!("f64.promote_f32: operand must be f32"));
747+
}
748+
stack.push(StackType::F64);
749+
}
750+
751+
// Reinterpret operations (same size, different type)
752+
0xbc => {
753+
// i32.reinterpret_f32
754+
if !Self::pop_type(&mut stack, StackType::F32) {
755+
return Err(anyhow!("i32.reinterpret_f32: operand must be f32"));
756+
}
757+
stack.push(StackType::I32);
758+
}
759+
0xbd => {
760+
// i64.reinterpret_f64
761+
if !Self::pop_type(&mut stack, StackType::F64) {
762+
return Err(anyhow!("i64.reinterpret_f64: operand must be f64"));
763+
}
764+
stack.push(StackType::I64);
765+
}
766+
0xbe => {
767+
// f32.reinterpret_i32
768+
if !Self::pop_type(&mut stack, StackType::I32) {
769+
return Err(anyhow!("f32.reinterpret_i32: operand must be i32"));
770+
}
771+
stack.push(StackType::F32);
772+
}
773+
0xbf => {
774+
// f64.reinterpret_i64
775+
if !Self::pop_type(&mut stack, StackType::I64) {
776+
return Err(anyhow!("f64.reinterpret_i64: operand must be i64"));
777+
}
778+
stack.push(StackType::F64);
779+
}
780+
585781
// Skip other opcodes for now (will be handled by instruction executor)
586782
_ => {
587783
// For all other opcodes, try to skip variable-length immediates

‎wrt-decoder/src/streaming_decoder.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,9 @@ impl<'a> StreamingDecoder<'a> {
493493
// Process each memory one at a time
494494
for i in 0..count {
495495
// Parse limits flag (0x00 = min only, 0x01 = min and max, 0x03 = min/max/shared)
496+
if offset >= data.len() {
497+
return Err(Error::parse_error("Memory section truncated: missing limits flag"));
498+
}
496499
let flags = data[offset];
497500
offset += 1;
498501

‎wrt-runtime/src/module.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,8 +1105,8 @@ impl Module {
11051105
let memory_type = to_core_memory_type(*memory);
11061106

11071107
#[cfg(feature = "std")]
1108-
eprintln!("DEBUG: [Memory {}] Module declares: min={} pages ({}KB), max={:?} pages",
1109-
mem_idx, memory_type.limits.min, memory_type.limits.min * 64, memory_type.limits.max);
1108+
eprintln!("DEBUG: [Memory {}] Module declares: min={} pages, max={:?} pages",
1109+
mem_idx, memory_type.limits.min, memory_type.limits.max);
11101110

11111111
#[cfg(feature = "std")]
11121112
eprintln!("DEBUG: [Memory {}] About to call Memory::new()...", mem_idx);

0 commit comments

Comments
 (0)