Skip to content
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.

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

[wasm] improve memory access and marshaling range checks #64845

Merged
merged 13 commits into from
May 20, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ internal static MarshalType GetMarshalTypeFromType(Type? type)

switch (typeCode)
{
case TypeCode.Byte:
case TypeCode.SByte:
case TypeCode.Int16:
case TypeCode.UInt16:
case TypeCode.Int32:
return MarshalType.INT;
case TypeCode.Byte:
case TypeCode.UInt16:
pavelsavara marked this conversation as resolved.
Show resolved Hide resolved
case TypeCode.UInt32:
return MarshalType.UINT32;
case TypeCode.Boolean:
Expand Down Expand Up @@ -232,11 +232,14 @@ internal static char GetCallSignatureCharacterForMarshalType(MarshalType t, char
switch (t)
{
case MarshalType.BOOL:
case MarshalType.INT:
return 'b';
case MarshalType.UINT32:
case MarshalType.POINTER:
return 'I';
case MarshalType.INT:
return 'i';
case MarshalType.UINT64:
return 'L';
case MarshalType.INT64:
return 'l';
case MarshalType.FP32:
Expand All @@ -250,9 +253,9 @@ internal static char GetCallSignatureCharacterForMarshalType(MarshalType t, char
case MarshalType.SAFEHANDLE:
return 'h';
case MarshalType.ENUM:
return 'j';
return 'j'; // this is wrong for uint enums
case MarshalType.ENUM64:
return 'k';
return 'k'; // this is wrong for ulong enums
case MarshalType.TASK:
case MarshalType.DELEGATE:
case MarshalType.OBJECT:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
<ItemGroup>
<Compile Include="System\Runtime\InteropServices\JavaScript\JavaScriptTests.cs" />
<Compile Include="System\Runtime\InteropServices\JavaScript\DataViewTests.cs" />
<Compile Include="System\Runtime\InteropServices\JavaScript\MemoryTests.cs" />
<Compile Include="System\Runtime\InteropServices\JavaScript\TypedArrayTests.cs" />
<Compile Include="System\Runtime\InteropServices\JavaScript\ArrayTests.cs" />
<Compile Include="System\Runtime\InteropServices\JavaScript\MarshalTests.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,13 @@ public static void InvokeActionIntInt()
public static void InvokeActionFloatIntToIntInt()
{
HelperMarshal._actionResultValue = 0;
Runtime.InvokeJS(@"
var ex = Assert.Throws<JSException>(()=>Runtime.InvokeJS(@"
var actionDelegate = App.call_test_method (""CreateActionDelegate"", [ ]);
actionDelegate(3.14,40);
");
"));

Assert.Equal(43, HelperMarshal._actionResultValue);
Assert.Contains("Value is not integer but float", ex.Message);
Assert.Equal(0, HelperMarshal._actionResultValue);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,21 @@ private static object InvokeReturnMarshalObj()
return _marshaledObject;
}

private static int InvokeReturnInt()
{
return 42;
}

private static long InvokeReturnLong()
{
return 42L;
}

private static double InvokeReturnDouble()
{
return double.Pi;
}

internal static int _valOne, _valTwo;
private static void ManipulateObject(JSObject obj)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,35 @@ public static void InvokeUnboxNumber(object o, object expected = null)
Assert.Equal(expected ?? o, HelperMarshal._object1);
}

[Fact]
public static void InvokeUnboxInt()
{
Runtime.InvokeJS(@"
var obj = App.call_test_method (""InvokeReturnInt"");
var res = App.call_test_method (""InvokeObj1"", [ obj ]);
");

Assert.Equal(42, HelperMarshal._object1);
}

[Fact]
public static void InvokeUnboxDouble()
{
Runtime.InvokeJS(@"
var obj = App.call_test_method (""InvokeReturnDouble"");
var res = App.call_test_method (""InvokeObj1"", [ obj ]);
");

Assert.Equal(double.Pi, HelperMarshal._object1);
}

[Fact]
public static void InvokeUnboxLongFail()
{
var ex = Assert.Throws<JSException>(() => Runtime.InvokeJS(@"App.call_test_method (""InvokeReturnLong"");"));
Assert.Contains("int64 not available", ex.Message);
}

[Theory]
[InlineData(byte.MinValue, 0)]
[InlineData(byte.MaxValue, 255)]
Expand Down Expand Up @@ -421,16 +450,13 @@ public static void TestFunctionApply()
[Fact]
public static void BoundStaticMethodMissingArgs()
{
// TODO: We currently have code that relies on this behavior (missing args default to 0) but
// it would be better if it threw an exception about the missing arguments. This test is here
// to ensure we do not break things by accidentally changing this behavior -kg

HelperMarshal._intValue = 1;
Runtime.InvokeJS(@$"
var ex = Assert.Throws<JSException>(() => Runtime.InvokeJS(@$"
var invoke_int = INTERNAL.mono_bind_static_method (""{HelperMarshal.INTEROP_CLASS}InvokeInt"");
invoke_int ();
");
Assert.Equal(0, HelperMarshal._intValue);
"));
Assert.Contains("Value is not integer but undefined", ex.Message);
Assert.Equal(1, HelperMarshal._intValue);
}

[Fact]
Expand All @@ -445,40 +471,41 @@ public static void BoundStaticMethodExtraArgs()
}

[Fact]
public static void BoundStaticMethodArgumentTypeCoercion()
public static void RangeCheckInt()
{
// TODO: As above, the type coercion behavior on display in this test is not ideal, but
// changing it risks breakage in existing code so for now it is verified by a test -kg

HelperMarshal._intValue = 0;
Runtime.InvokeJS(@$"
// no numbers bigger than 32 bits
var ex = Assert.Throws<JSException>(() => Runtime.InvokeJS(@$"
var invoke_int = INTERNAL.mono_bind_static_method (""{HelperMarshal.INTEROP_CLASS}InvokeInt"");
invoke_int (""200"");
");
Assert.Equal(200, HelperMarshal._intValue);

Runtime.InvokeJS(@$"
var invoke_int = INTERNAL.mono_bind_static_method (""{HelperMarshal.INTEROP_CLASS}InvokeInt"");
invoke_int (400.5);
");
Assert.Equal(400, HelperMarshal._intValue);
invoke_int (Number.MAX_SAFE_INTEGER);
"));
Assert.Contains("Overflow: value 9007199254740991 is out of -2147483648 2147483647 range", ex.Message);
Assert.Equal(0, HelperMarshal._intValue);
}

[Fact]
public static void BoundStaticMethodUnpleasantArgumentTypeCoercion()
public static void IntegerCheckInt()
{
HelperMarshal._intValue = 100;
Runtime.InvokeJS(@$"
HelperMarshal._intValue = 0;
// no floating point rounding
var ex = Assert.Throws<JSException>(() => Runtime.InvokeJS(@$"
var invoke_int = INTERNAL.mono_bind_static_method (""{HelperMarshal.INTEROP_CLASS}InvokeInt"");
invoke_int (""hello"");
");
invoke_int (3.14);
"));
Assert.Contains("Value is not integer but float", ex.Message);
Assert.Equal(0, HelperMarshal._intValue);
}

// In this case at the very least, the leading "7" is not turned into the number 7
Runtime.InvokeJS(@$"
[Fact]
public static void TypeCheckInt()
{
HelperMarshal._intValue = 0;
// no string conversion
var ex = Assert.Throws<JSException>(() => Runtime.InvokeJS(@$"
var invoke_int = INTERNAL.mono_bind_static_method (""{HelperMarshal.INTEROP_CLASS}InvokeInt"");
invoke_int (""7apples"");
");
invoke_int (""200"");
"));
Assert.Contains("Value is not integer but string", ex.Message);
Assert.Equal(0, HelperMarshal._intValue);
}

Expand Down Expand Up @@ -519,19 +546,6 @@ public static void PassUintEnumByValue()
Assert.Equal(TestEnum.BigValue, HelperMarshal._enumValue);
}

[Fact]
public static void PassUintEnumByValueMasqueradingAsInt()
{
HelperMarshal._enumValue = TestEnum.Zero;
// HACK: We're explicitly telling the bindings layer to pass an int here, not an enum
// Because we know the enum is : uint, this is compatible, so it works.
Runtime.InvokeJS(@$"
var set_enum = INTERNAL.mono_bind_static_method (""{HelperMarshal.INTEROP_CLASS}SetEnumValue"", ""i"");
set_enum (0xFFFFFFFE);
");
Assert.Equal(TestEnum.BigValue, HelperMarshal._enumValue);
}

[Fact]
public static void PassUintEnumByNameIsNotImplemented()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.CompilerServices;
using Xunit;

namespace System.Runtime.InteropServices.JavaScript.Tests
{
public class MemoryTests
{
[Theory]
[InlineData(-1L)]
[InlineData(-42L)]
[InlineData(int.MinValue)]
[InlineData(-9007199254740991L)]//MIN_SAFE_INTEGER
[InlineData(1L)]
[InlineData(0L)]
[InlineData(42L)]
[InlineData(int.MaxValue)]
[InlineData(0xF_FFFF_FFFFL)]
[InlineData(9007199254740991L)]//MAX_SAFE_INTEGER
public static unsafe void Int52TestOK(long value)
{
long expected = value;
long actual2 = value;
var bagFn = new Function("ptr", "ptr2", @"
const value=globalThis.App.MONO.getI52(ptr);
globalThis.App.MONO.setI52(ptr2, value);
return value;");

uint ptr = (uint)Unsafe.AsPointer(ref expected);
uint ptr2 = (uint)Unsafe.AsPointer(ref actual2);

object o = bagFn.Call(null, ptr, ptr2);
if (value < int.MaxValue && value > int.MinValue)
{
Assert.IsType<int>(o);
long actual = (int)o;
Assert.Equal(expected, actual);
}
Assert.Equal(expected, actual2);
}

[Theory]
[InlineData(uint.MinValue)]
[InlineData(1L)]
[InlineData(0L)]
[InlineData(42L)]
[InlineData(uint.MaxValue)]
[InlineData(0xF_FFFF_FFFFL)]
[InlineData(9007199254740991L)]//MAX_SAFE_INTEGER
public static unsafe void UInt52TestOK(ulong value)
{
ulong expected = value;
ulong actual2 = value;
var bagFn = new Function("ptr", "ptr2", @"
const value=globalThis.App.MONO.getI52(ptr);
globalThis.App.MONO.setU52(ptr2, value);
return value;");

uint ptr = (uint)Unsafe.AsPointer(ref expected);
uint ptr2 = (uint)Unsafe.AsPointer(ref actual2);

object o = bagFn.Call(null, ptr, ptr2);
if (value < int.MaxValue)
{
Assert.IsType<int>(o);
ulong actual = (ulong)(long)(int)o;
Assert.Equal(expected, actual);
}
Assert.Equal(expected, actual2);
}

[Theory]
[InlineData(double.NegativeInfinity)]
[InlineData(double.PositiveInfinity)]
[InlineData(double.MinValue)]
[InlineData(double.MaxValue)]
[InlineData(double.Pi)]
[InlineData(9007199254740993.0)]//MAX_SAFE_INTEGER +2
public static unsafe void Int52TestRange(double value)
{
long actual = 0;
uint ptr = (uint)Unsafe.AsPointer(ref actual);
var bagFn = new Function("ptr", "value", @"
globalThis.App.MONO.setI52(ptr, value);");
var ex=Assert.Throws<JSException>(() => bagFn.Call(null, ptr, value));
Assert.Contains("Overflow: value out of Number.isSafeInteger range", ex.Message);

double expectedD = value;
uint ptrD = (uint)Unsafe.AsPointer(ref expectedD);
var bagFnD = new Function("ptr", "value", @"
globalThis.App.MONO.getI52(ptr);");
var exD = Assert.Throws<JSException>(() => bagFn.Call(null, ptr, value));
Assert.Contains("Overflow: value out of Number.isSafeInteger range", ex.Message);
}

[Theory]
[InlineData(-1.0)]
public static unsafe void UInt52TestRange(double value)
{
long actual = 0;
uint ptr = (uint)Unsafe.AsPointer(ref actual);
var bagFn = new Function("ptr", "value", @"
globalThis.App.MONO.setU52(ptr, value);");
var ex=Assert.Throws<JSException>(() => bagFn.Call(null, ptr, value));
Assert.Contains("Can't convert negative Number into UInt64", ex.Message);

double expectedD = value;
uint ptrD = (uint)Unsafe.AsPointer(ref expectedD);
var bagFnD = new Function("ptr", "value", @"
globalThis.App.MONO.getU52(ptr);");
var exD = Assert.Throws<JSException>(() => bagFn.Call(null, ptr, value));
Assert.Contains("Can't convert negative Number into UInt64", ex.Message);
}

[Fact]
public static unsafe void Int52TestNaN()
{
long actual = 0;
uint ptr = (uint)Unsafe.AsPointer(ref actual);
var bagFn = new Function("ptr", "value", @"
globalThis.App.MONO.setI52(ptr, value);");
var ex=Assert.Throws<JSException>(() => bagFn.Call(null, ptr, double.NaN));
Assert.Contains("Can't convert Number.Nan into Int64", ex.Message);
}
}
}
3 changes: 2 additions & 1 deletion src/mono/wasm/runtime/cjs/dotnet.cjs.lib.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ const DotnetSupportLib = {
// we replace implementation of readAsync and fetch
// replacement of require is there for consistency with ES6 code
$DOTNET__postset: `
let __dotnet_replacements = {readAsync, fetch: globalThis.fetch, require};
let __dotnet_replacements = {readAsync, fetch: globalThis.fetch, require, updateGlobalBufferAndViews};
let __dotnet_exportedAPI = __dotnet_runtime.__initializeImportsAndExports(
{ isESM:false, isGlobal:ENVIRONMENT_IS_GLOBAL, isNode:ENVIRONMENT_IS_NODE, isShell:ENVIRONMENT_IS_SHELL, isWeb:ENVIRONMENT_IS_WEB, locateFile, quit_, ExitStatus, requirePromise:Promise.resolve(require)},
{ mono:MONO, binding:BINDING, internal:INTERNAL, module:Module },
__dotnet_replacements);
updateGlobalBufferAndViews = __dotnet_replacements.updateGlobalBufferAndViews;
readAsync = __dotnet_replacements.readAsync;
var fetch = __dotnet_replacements.fetch;
require = __dotnet_replacements.requireOut;
Expand Down
4 changes: 2 additions & 2 deletions src/mono/wasm/runtime/cwraps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

import {
assert,
mono_assert,
MonoArray, MonoAssembly, MonoClass,
MonoMethod, MonoObject, MonoString,
MonoType, MonoObjectRef, MonoStringRef
Expand Down Expand Up @@ -197,7 +197,7 @@ export default wrapped_c_functions;
export function wrap_c_function(name: string): Function {
const wf: any = wrapped_c_functions;
const sig = fn_signatures.find(s => s[0] === name);
assert(sig, () => `Function ${name} not found`);
mono_assert(sig, () => `Function ${name} not found`);
const fce = Module.cwrap(sig[0], sig[1], sig[2], sig[3]);
wf[sig[0]] = fce;
return fce;
Expand Down
Loading