Fix issue#2398 #2462
Fix issue#2398 #2462
Conversation
|
I wonder if we could figure out a test case for this. Maybe there is a missing test case upstream? |
|
Hi @sbc100, I'm not quite sure about the test case, I use the POC to trigger it in a related issue. I'm glad to help if needed. |
|
Ah, yes, I guess we can use the POC to craft a new test. I don't have time to do that right now but I can maybe help out next week with that. |
|
I uploaded poc.wasm and poc.wat all in poc.zip Also, do you think this patch is OK at this time? I'm glad to make it better, if needed |
|
The patch seems OK but I'd like to see how the POC causes the index here to be out of range. There might be a better fix elsewhere in the codebase that avoid this being out-of-range in the first place (and perhaps we can instead assert here that the index is in-range?) |
|
this one's a tricky one, we've managed to minify it to this and still reproduce the original issue: (module
(type (;3;) $reti32 (func (result i32)))
(type (;4;) $externref (func (param i64 f64 f64 i64) (result externref)))
;;(type (;4;) $externref (func (result externref)))
(type (;5;) $basic_func (func))
(func (;4;) $start (type $basic_func)
call $funny_recursion_1
drop
)
(func (;6;) $grow_funcs_by_256 (type $reti32) (result i32)
ref.null func
i32.const 256
table.grow 0
)
(func (;8;) $funny_recursion_1 (type $reti32) (result i32)
i32.const 1
ref.func $funny_recursion_2
table.set 0
i64.const -432345564227567366
f64.const nan (;=nan;)
f64.const inf (;=inf;)
i64.const 65535
i32.const 1
call_indirect (type $externref)
i32.const 0
table.grow 1
)
(func (;9;) $funny_recursion_2 (type $externref) (param i64 f64 f64 i64) (result externref)
call $grow_funcs_by_256
drop
call $funny_recursion_1
unreachable
)
(table (;0;) 16 funcref)
(table (;1;) 16 externref)
(start $start)
(elem (;0;) declare func $grow_funcs_by_256 $funny_recursion_1 $funny_recursion_2)
)but we are struggling to minify it further... |
|
correct fix: diff --git a/src/interp/interp.cc b/src/interp/interp.cc
index 74f33eb8..cb5623d8 100644
--- a/src/interp/interp.cc
+++ b/src/interp/interp.cc
@@ -1989,7 +1989,7 @@ RunResult Thread::DoCall(const Func::Ptr& func, Trap::Ptr* out_trap) {
PushValues(func_type.results, results);
} else {
if (PushCall(*cast<DefinedFunc>(func.get()), out_trap) == RunResult::Trap) {
- return RunResult::Ok;
+ return RunResult::Trap;
}
}
return RunResult::Ok; |
|
(half tempted to go putting a |
fix issue: #2398