Skip to content

Fix issues in Function.FromCallback() and Linker.DefineFunction() #161

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 20, 2022
Merged

Fix issues in Function.FromCallback() and Linker.DefineFunction() #161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 39 additions & 30 deletions src/Function.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2239,7 +2239,8 @@ internal Function(IStore store, Delegate callback, bool hasReturn)
}
this.store = store;

using var funcType = GetFunctionType(callback.GetType(), hasReturn, this.parameters, this.results, out var hasCaller);
using var funcType = GetFunctionType(callback.GetType(), hasReturn, this.parameters, this.results, out var hasCaller, out var returnsTuple);
var callbackInvokeMethod = callback.GetType().GetMethod(nameof(Action.Invoke))!;

unsafe
{
Expand All @@ -2249,15 +2250,15 @@ internal Function(IStore store, Delegate callback, bool hasReturn)
func = (env, callerPtr, args, nargs, results, nresults) =>
{
using var caller = new Caller(callerPtr);
return InvokeCallback(callback, caller, true, args, (int)nargs, results, (int)nresults, Results);
return InvokeCallback(callback, callbackInvokeMethod, caller, true, args, (int)nargs, results, (int)nresults, Results, returnsTuple);
};
}
else
{
func = (env, callerPtr, args, nargs, results, nresults) =>
{
using var caller = new Caller(callerPtr);
return InvokeCallback(callback, caller, false, args, (int)nargs, results, (int)nresults, Results);
return InvokeCallback(callback, callbackInvokeMethod, caller, false, args, (int)nargs, results, (int)nresults, Results, returnsTuple);
};
}

Expand Down Expand Up @@ -2301,32 +2302,42 @@ internal Function(IStore store, ExternFunc func)
}
}

private static IEnumerable<Type> EnumerateReturnTypes(Type? returnType)
private static IEnumerable<Type> EnumerateReturnTypes(Type? returnType, out bool isTuple)
{
isTuple = false;

if (returnType is null)
{
yield break;
return Array.Empty<Type>();
}

if (IsTuple(returnType))
{
foreach (var type in returnType
.GetGenericArguments()
.SelectMany(type =>
isTuple = true;
return EnumerateTupleTypes(returnType);

static IEnumerable<Type> EnumerateTupleTypes(Type tupleType)
{
foreach (var (typeArgument, idx) in tupleType.GenericTypeArguments.Select((e, idx) => (e, idx)))
{
if (idx is 7 && IsTuple(typeArgument))
{
if (type.IsConstructedGenericType)
// Recursively enumerate the nested tuple's type arguments.
foreach (var type in EnumerateTupleTypes(typeArgument))
{
return type.GenericTypeArguments;
yield return type;
}
return Enumerable.Repeat(type, 1);
}))
{
yield return type;
}
else
{
yield return typeArgument;
}
}
}
}
else
{
yield return returnType;
return new Type[] { returnType };
}
}

Expand All @@ -2350,7 +2361,7 @@ private static bool IsTuple(Type type)
definition == typeof(ValueTuple<,,,,,,,>);
}

internal static TypeHandle GetFunctionType(Type type, bool hasReturn, List<ValueKind> parameters, List<ValueKind> results, out bool hasCaller)
internal static TypeHandle GetFunctionType(Type type, bool hasReturn, List<ValueKind> parameters, List<ValueKind> results, out bool hasCaller, out bool returnsTuple)
{
Span<Type> parameterTypes = null;
Type? returnType = null;
Expand Down Expand Up @@ -2388,7 +2399,7 @@ internal static TypeHandle GetFunctionType(Type type, bool hasReturn, List<Value
parameters.Add(kind);
}

results.AddRange(EnumerateReturnTypes(returnType).Select(t =>
results.AddRange(EnumerateReturnTypes(returnType, out returnsTuple).Select(t =>
{
if (!Value.TryGetKind(t, out var kind))
{
Expand All @@ -2400,7 +2411,7 @@ internal static TypeHandle GetFunctionType(Type type, bool hasReturn, List<Value
return new Function.TypeHandle(Function.Native.wasm_functype_new(new ValueTypeArray(parameters), new ValueTypeArray(results)));
}

internal unsafe static IntPtr InvokeCallback(Delegate callback, Caller caller, bool passCaller, Value* args, int nargs, Value* results, int nresults, IReadOnlyList<ValueKind> resultKinds)
internal unsafe static IntPtr InvokeCallback(Delegate callback, MethodInfo callbackInvokeMethod, Caller caller, bool passCaller, Value* args, int nargs, Value* results, int nresults, IReadOnlyList<ValueKind> resultKinds, bool returnsTuple)
{
try
{
Expand All @@ -2420,23 +2431,21 @@ internal unsafe static IntPtr InvokeCallback(Delegate callback, Caller caller, b

// 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);
var result = callbackInvokeMethod.Invoke(callback, BindingFlags.DoNotWrapExceptions, null, invokeArgs, null);

if (resultKinds.Count > 0)
if (returnsTuple)
{
var tuple = result as ITuple;
if (tuple is null)
{
results[0] = Value.FromObject(result, resultKinds[0]);
}
else
var tuple = (ITuple)result!;

for (int i = 0; i < tuple.Length; ++i)
{
for (int i = 0; i < tuple.Length; ++i)
{
results[i] = Value.FromObject(tuple[i], resultKinds[i]);
}
results[i] = Value.FromObject(tuple[i], resultKinds[i]);
}
}
else if (resultKinds.Count == 1)
{
results[0] = Value.FromObject(result, resultKinds[0]);
}
return IntPtr.Zero;
}
catch (Exception ex)
Expand Down
5 changes: 3 additions & 2 deletions src/Linker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,14 +756,15 @@ private void DefineFunction(string module, string name, Delegate callback, bool
var parameterKinds = new List<ValueKind>();
var resultKinds = new List<ValueKind>();

using var funcType = Function.GetFunctionType(callback.GetType(), hasReturn, parameterKinds, resultKinds, out var hasCaller);
using var funcType = Function.GetFunctionType(callback.GetType(), hasReturn, parameterKinds, resultKinds, out var hasCaller, out var returnsTuple);
var callbackInvokeMethod = callback.GetType().GetMethod(nameof(Action.Invoke))!;

unsafe
{
Function.Native.WasmtimeFuncCallback func = (env, callerPtr, args, nargs, results, nresults) =>
{
using var caller = new Caller(callerPtr);
return Function.InvokeCallback(callback, caller, hasCaller, args, (int)nargs, results, (int)nresults, resultKinds);
return Function.InvokeCallback(callback, callbackInvokeMethod, caller, hasCaller, args, (int)nargs, results, (int)nresults, resultKinds, returnsTuple);
};

var moduleBytes = Encoding.UTF8.GetBytes(module);
Expand Down
13 changes: 13 additions & 0 deletions tests/ExternRefTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,19 @@ public void ItHandlesNullReferences()
(nullref.Invoke()).Should().BeNull();
}

[Fact]
public void ItReturnsBoxedValueTupleAsExternRef()
{
// Test for issue #158
var instance = Linker.Instantiate(Store, Fixture.Module);

var inout = instance.GetFunction<object, object>("inout");
inout.Should().NotBeNull();

var input = (object)(1, 2, 3);
inout(input).Should().BeSameAs(input);
}

unsafe class Value
{
internal Value(int* counter)
Expand Down
55 changes: 55 additions & 0 deletions tests/FunctionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,36 @@ public FunctionTests(FunctionsFixture fixture)
{
caller.GetMemory("mem").ReadString(address, length).Should().Be("Hello World");
}));

Linker.Define("env", "return_i32", Function.FromCallback(Store, GetBoundFuncIntDelegate()));

Linker.Define("env", "return_15_values", Function.FromCallback(Store, () =>
{
return (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15);
}));

Linker.Define("env", "accept_15_values", Function.FromCallback(Store,
(int i1, int i2, int i3, int i4, int i5, int i6, int i7, int i8, int i9, int i10, int i11, int i12, int i13, int i14, int i15) =>
{
(i1, i2, i3, i4, i5, i6, i7, i8, i9, i10, i11, i12, i13, i14, i15)
.Should().Be((1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15));
}));

Func<int> GetBoundFuncIntDelegate()
{
// Get a delegate that is bound over an argument.
// See #159
var getLengthDelegate = GetLength;
var getLengthMethod = getLengthDelegate.Method;

string str = "abc";
return (Func<int>)Delegate.CreateDelegate(typeof(Func<int>), str, getLengthMethod);

int GetLength(string s)
{
return s.Length;
}
}
}

private FunctionsFixture Fixture { get; }
Expand Down Expand Up @@ -316,6 +346,31 @@ public void ItReturnsNullForVeryLongTuples()
.BeNull();
}

[Fact]
public void ItReturnsInt32WithBoundDelegate()
{
// Test for issue #159: It should be possible to defined a Delegate that is bound with
// a first argument.
var instance = Linker.Instantiate(Store, Fixture.Module);
var echo = instance.GetFunction<int>("return_i32");
echo.Should().NotBeNull();

var result = echo.Invoke();
result.Should().Be(3);
}

[Fact]
public void ItReturnsAndAccepts15Values()
{
// Verify that nested levels of ValueTuple are handled correctly. Returning 15
// values means that a ValueTuple<..., ValueTuple<..., ValueTuple<...>>> is used.
var instance = Linker.Instantiate(Store, Fixture.Module);
var action = instance.GetAction("get_and_pass_15_values");
action.Should().NotBeNull();

action.Invoke();
}

public void Dispose()
{
Store.Dispose();
Expand Down
35 changes: 34 additions & 1 deletion tests/LinkerFunctionsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,36 @@ public LinkerFunctionTests(LinkerFunctionsFixture fixture)
{
caller.GetMemory("mem").ReadString(address, length).Should().Be("Hello World");
});

Linker.Define("env", "return_i32", Function.FromCallback(Store, GetBoundFuncIntDelegate()));

Linker.Define("env", "return_15_values", Function.FromCallback(Store, () =>
{
return (1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15);
}));

Linker.Define("env", "accept_15_values", Function.FromCallback(Store,
(int i1, int i2, int i3, int i4, int i5, int i6, int i7, int i8, int i9, int i10, int i11, int i12, int i13, int i14, int i15) =>
{
(i1, i2, i3, i4, i5, i6, i7, i8, i9, i10, i11, i12, i13, i14, i15)
.Should().Be((1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15));
}));

Func<int> GetBoundFuncIntDelegate()
{
// Get a delegate that is bound over an argument.
// See #159
var getLengthDelegate = GetLength;
var getLengthMethod = getLengthDelegate.Method;

string str = "abc";
return (Func<int>)Delegate.CreateDelegate(typeof(Func<int>), str, getLengthMethod);

int GetLength(string s)
{
return s.Length;
}
}
}

private LinkerFunctionsFixture Fixture { get; }
Expand All @@ -40,12 +70,15 @@ public void ItBindsImportMethodsAndCallsThemCorrectly()
var instance = Linker.Instantiate(Store, Fixture.Module);
var add = instance.GetFunction("add");
var swap = instance.GetFunction("swap");
var check = instance.GetFunction("check_string");
var check = instance.GetFunction("check_string"); ;
var getInt32 = instance.GetFunction<int>("return_i32");

int x = (int)add.Invoke(40, 2);
x.Should().Be(42);
x = (int)add.Invoke(22, 5);
x.Should().Be(27);
x = getInt32.Invoke();
x.Should().Be(3);

object[] results = (object[])swap.Invoke(10, 100);
results.Should().Equal(new object[] { 100, 10 });
Expand Down
9 changes: 9 additions & 0 deletions tests/Modules/Functions.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,16 @@
(import "env" "swap" (func $env.swap (param i32 i32) (result i32 i32)))
(import "env" "do_throw" (func $env.do_throw))
(import "env" "check_string" (func $env.check_string (param i32 i32)))
(import "env" "return_i32" (func $env.return_i32 (result i32)))
(import "env" "return_15_values" (func $env.return_15_values (result i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32)))
(import "env" "accept_15_values" (func $env.accept_15_values (param i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32 i32)))
(memory (export "mem") 1)
(export "add" (func $add))
(export "swap" (func $swap))
(export "do_throw" (func $do_throw))
(export "check_string" (func $check_string))
(export "return_i32" (func $env.return_i32))
(export "get_and_pass_15_values" (func $get_and_pass_15_values))
(func $add (param i32 i32) (result i32)
(call $env.add (local.get 0) (local.get 1))
)
Expand All @@ -20,6 +25,10 @@
(func $check_string
(call $env.check_string (i32.const 0) (i32.const 11))
)
(func $get_and_pass_15_values
call $env.return_15_values
call $env.accept_15_values
)
(data (i32.const 0) "Hello World")

(func (export "noop"))
Expand Down