Skip to content

Commit

Permalink
[release/8.0] [Mono] Fix offset calculation for nested struct, when p…
Browse files Browse the repository at this point in the history
…invoke is enabled (#91417)

* Fix offset calculation for nested struct

* Add a test for nested struct with pinvoke

* Move and update test with real c++ implementation

* Exclude newly added test from wasm

* Disable interop tests on android and apple devices

---------

Co-authored-by: Fan Yang <yangfan@microsoft.com>
  • Loading branch information
github-actions[bot] and fanyang-mono authored Sep 1, 2023
1 parent 399f37c commit cd8b2cb
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/mono/mono/mini/mini-amd64.c
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ collect_field_info_nested (MonoClass *klass, GArray *fields_array, int offset, g
g_assert(info);
for (guint32 i = 0; i < info->num_fields; ++i) {
if (MONO_TYPE_ISSTRUCT (info->fields [i].field->type)) {
collect_field_info_nested (mono_class_from_mono_type_internal (info->fields [i].field->type), fields_array, info->fields [i].offset, pinvoke, unicode);
collect_field_info_nested (mono_class_from_mono_type_internal (info->fields [i].field->type), fields_array, (offset + info->fields [i].offset), pinvoke, unicode);
} else {
guint32 align;
StructFieldInfo f;
Expand All @@ -367,7 +367,7 @@ collect_field_info_nested (MonoClass *klass, GArray *fields_array, int offset, g
info->fields [i].mspec,
&align, TRUE, unicode);
f.offset = offset + info->fields [i].offset;
if (i == info->num_fields - 1 && f.size + f.offset < info->native_size) {
if ((i == info->num_fields - 1) && ((f.size + f.offset) < info->native_size)) {
/* This can happen with .pack directives eg. 'fixed' arrays */
if (MONO_TYPE_IS_PRIMITIVE (f.type)) {
/* Replicate the last field to fill out the remaining place, since the code in add_valuetype () needs type information */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.InteropServices;
using System.Runtime.CompilerServices;
using System.Text;

public unsafe partial struct GameControllerButtonBind
{
public GameControllerButtonBind
(
GameControllerBindType? bindType = null,
GameControllerButtonBindValue? value = null
) : this()
{
if (bindType is not null)
{
BindType = bindType.Value;
}

if (value is not null)
{
Value = value.Value;
}
}

public GameControllerBindType BindType;

public GameControllerButtonBindValue Value;
}

public enum GameControllerBindType : int
{
ControllerBindtypeNone = 0x0,
ControllerBindtypeButton = 0x1,
ControllerBindtypeAxis = 0x2,
ControllerBindtypeHat = 0x3,
None = 0x0,
Button = 0x1,
Axis = 0x2,
Hat = 0x3,
}

[StructLayout(LayoutKind.Explicit)]
public unsafe partial struct GameControllerButtonBindValue
{
[FieldOffset(0)]
public int Button;

[FieldOffset(0)]
public int Axis;

[FieldOffset(0)]
public GameControllerButtonBindValueHat Hat;
}

public unsafe partial struct GameControllerButtonBindValueHat
{
public int Hat;

public int HatMask;
}
Original file line number Diff line number Diff line change
Expand Up @@ -1297,3 +1297,8 @@ extern "C" DLL_EXPORT Int32CLongStruct STDMETHODCALLTYPE AddCLongs(Int32CLongStr
{
return { lhs.i + rhs.i, lhs.l + rhs.l };
}

extern "C" DLL_EXPORT SDL_GameControllerBindType STDMETHODCALLTYPE getBindType(SDL_GameControllerButtonBind button)
{
return button.bindType;
}
Original file line number Diff line number Diff line change
Expand Up @@ -974,3 +974,26 @@ struct Int32CLongStruct
int32_t i;
long l;
};

typedef enum
{
SDL_CONTROLLER_BINDTYPE_NONE = 0,
SDL_CONTROLLER_BINDTYPE_BUTTON,
SDL_CONTROLLER_BINDTYPE_AXIS,
SDL_CONTROLLER_BINDTYPE_HAT
} SDL_GameControllerBindType;

typedef struct SDL_GameControllerButtonBind
{
SDL_GameControllerBindType bindType;
union
{
int button;
int axis;
struct {
int hat;
int hat_mask;
} hat;
} value;

} SDL_GameControllerButtonBind;
27 changes: 27 additions & 0 deletions src/tests/Interop/StructMarshalling/PInvoke/NestedStruct.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Runtime.InteropServices;
using Xunit;

public class Managed
{
[DllImport("MarshalStructAsParam")]
static extern GameControllerBindType getBindType (GameControllerButtonBind button);

public static int Main()
{
GameControllerButtonBind button = new GameControllerButtonBind(GameControllerBindType.ControllerBindtypeAxis, null);
if (getBindType(button) == GameControllerBindType.ControllerBindtypeAxis)
{
Console.WriteLine("\nTEST PASSED!");
return 100;
}
else
{
Console.WriteLine("\nTEST FAILED!");
return 1;
}
}
}
14 changes: 14 additions & 0 deletions src/tests/Interop/StructMarshalling/PInvoke/NestedStruct.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>
<ItemGroup>
<Compile Include="NestedStruct.cs" />
<Compile Include="GameControllerButtonBind.cs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="$(TestSourceDir)Common/CoreCLRTestLibrary/CoreCLRTestLibrary.csproj" />
<CMakeProjectReference Include="CMakeLists.txt" />
</ItemGroup>
</Project>
12 changes: 12 additions & 0 deletions src/tests/issues.targets
Original file line number Diff line number Diff line change
Expand Up @@ -2939,6 +2939,9 @@
<ExcludeList Include="$(XunitTestBinBase)/Interop/PInvoke/SetLastError/**">
<Issue>https://github.com/dotnet/runtime/issues/64127</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Interop/StructMarshalling/PInvoke/NestedStruct/**">
<Issue>https://github.com/dotnet/runtime/issues/64127</Issue>
</ExcludeList>
<ExcludeList Include = "$(XunitTestBinBase)/Interop/SuppressGCTransition/SuppressGCTransitionTest/**">
<Issue>https://github.com/dotnet/runtime/issues/64127</Issue>
</ExcludeList>
Expand Down Expand Up @@ -3492,6 +3495,15 @@
<ExcludeList Include = "$(XunitTestBinBase)/Loader/classloader/StaticVirtualMethods/GenericContext/**">
<Issue>https://github.com/dotnet/runtime/issues/67359</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Interop/StructMarshalling/PInvoke/NestedStruct/**">
<Issue>https://github.com/dotnet/runtime/issues/91388</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Interop/NativeLibrary/MainProgramHandle/MainProgramHandleTests/**">
<Issue>https://github.com/dotnet/runtime/issues/91388</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/Interop/DllImportSearchPaths/DllImportSearchPathsTest/**">
<Issue>https://github.com/dotnet/runtime/issues/91388</Issue>
</ExcludeList>
</ItemGroup>

<ItemGroup Condition="'$(TargetOS)' == 'android' And '$(TargetArchitecture)' == 'arm64'">
Expand Down

0 comments on commit cd8b2cb

Please sign in to comment.