-
Notifications
You must be signed in to change notification settings - Fork 56
Description
Hi, I noticed the an issue in Function.InvokeCallback() using Wasmtime 1.0.0. Consider the following C# program (.NET 6.0):
using Wasmtime;
using var engine = new Engine();
using var module = Module.FromText(
engine,
"hello",
@"
(module
(func $getExternref (import """" ""getExternref"") (result externref))
(func (export ""run"")
call $getExternref
drop
)
)");
using var linker = new Linker(engine);
using var store = new Store(engine);
linker.Define(
"",
"getExternref",
Function.FromCallback(store, () => (object)(1, 2, 3)));
var instance = linker.Instantiate(store, module);
var run = instance.GetAction("run")!;
run();Expected behavior: Program runs successfully (the boxed ValueTuple<int, int, int> should be returned as externref to WASM, as the Delegate returns an object and the return type's value kind is ValueKind.ExternRef).
Actual behavior: An ArgumentOutOfRangeException occurs in Function.InvokeCallback():
System.ArgumentOutOfRangeException
HResult=0x80131502
Message=Index was out of range. Must be non-negative and less than the size of the collection.
Source=System.Private.CoreLib
StackTrace:
at System.ThrowHelper.ThrowArgumentOutOfRange_IndexMustBeLessException()
at System.Collections.Generic.List`1.get_Item(Int32 index)
at Wasmtime.Function.InvokeCallback(Delegate callback, Caller caller, Boolean passCaller, Value* args, Int32 nargs, Value* results, Int32 nresults, IReadOnlyList`1 resultKinds) in C:\Users\VPC\Desktop\WebAssembly-Test\wasmtime-dotnet\src\Function.cs:line 2436
This is because Function.InvokeCallback() unconditionally checks whether the returned value is a ITuple, regardless of whether Function.IsTuple() previously returned true when determining the function type:
wasmtime-dotnet/src/Function.cs
Lines 2421 to 2439 in f3a383a
| // NOTE: reflection is extremely slow for invoking methods. in the future, perhaps this could be replaced with | |
| // source generators, system.linq.expressions, or generate IL with DynamicMethods or something | |
| var result = callback.Method.Invoke(callback.Target, BindingFlags.DoNotWrapExceptions, null, invokeArgs, null); | |
| if (resultKinds.Count > 0) | |
| { | |
| var tuple = result as ITuple; | |
| if (tuple is null) | |
| { | |
| results[0] = Value.FromObject(result, resultKinds[0]); | |
| } | |
| else | |
| { | |
| for (int i = 0; i < tuple.Length; ++i) | |
| { | |
| results[i] = Value.FromObject(tuple[i], resultKinds[i]); | |
| } | |
| } | |
| } |
I think it should probably cast the value as ITuple only when Function.IsTuple() returned true previously, so that in the above case, the ValueTuple<int, int, int> is returned as externref.
Additionally, I was thinking to try to implement an approch using ILGenerator and DynamicMethod to generate IL code that can directly call the method without reflection, as it is also noted in the comment that calling the callback using reflection is very slow. Would you accept such a change as PR?
Thank you!