From 5965268398ccb248bfce7ae29a83a1a090c3fac9 Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Fri, 8 Jan 2021 22:01:27 +0200 Subject: [PATCH 1/4] Add tests for MarshalUtils.IsBillitableType --- .../CoreTestAssembly/Marshalling.cs | 146 ++++++++++++++++++ ...ompiler.TypeSystem.ReadyToRun.Tests.csproj | 2 + .../MarshalUtilsTests.cs | 83 ++++++++++ 3 files changed, 231 insertions(+) create mode 100644 src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/CoreTestAssembly/Marshalling.cs create mode 100644 src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/MarshalUtilsTests.cs diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/CoreTestAssembly/Marshalling.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/CoreTestAssembly/Marshalling.cs new file mode 100644 index 00000000000000..f9d2d14949dd97 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/CoreTestAssembly/Marshalling.cs @@ -0,0 +1,146 @@ +// 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.InteropServices; + +namespace Marshalling +{ + [StructLayout(LayoutKind.Explicit)] + public class ExplicitEmptyBase + { + } + + [StructLayout(LayoutKind.Explicit)] + public class ClassWithExplicitEmptyBase : ExplicitEmptyBase + { + [FieldOffset(0)] + public int i; + } + + [StructLayout(LayoutKind.Explicit, Size = 0)] + public class ExplicitEmptySizeZeroBase + { + } + + [StructLayout(LayoutKind.Explicit)] + public class ClassWithExplicitSizeZeroEmptyBase : ExplicitEmptySizeZeroBase + { + [FieldOffset(0)] + public int i; + } + + [StructLayout(LayoutKind.Explicit)] + public class ExplicitByteBase + { + [FieldOffset(0)] + public byte x; + } + + [StructLayout(LayoutKind.Explicit)] + public class ClassWithExplicitByteBase : ExplicitByteBase + { + [FieldOffset(1)] + public int i; + } + + [StructLayout(LayoutKind.Explicit)] + public class ExplicitInt16Base + { + [FieldOffset(0)] + public short x; + } + + [StructLayout(LayoutKind.Explicit)] + public class ClassWithExplicitInt16Base : ExplicitInt16Base + { + [FieldOffset(1)] + public int i; + } + + [StructLayout(LayoutKind.Explicit)] + public class ExplicitInt32Base + { + [FieldOffset(0)] + public int x; + } + + [StructLayout(LayoutKind.Explicit)] + public class ClassWithExplicitInt32Base : ExplicitInt32Base + { + [FieldOffset(1)] + public int i; + } + + [StructLayout(LayoutKind.Explicit)] + public class ExplicitInt64Base + { + [FieldOffset(0)] + public long x; + } + + [StructLayout(LayoutKind.Explicit)] + public class ClassWithExplicitInt64Base : ExplicitInt64Base + { + [FieldOffset(1)] + public int i; + } + + [StructLayout(LayoutKind.Sequential)] + public class SequentialEmptyBase + { + } + + [StructLayout(LayoutKind.Sequential)] + public class ClassWithSequentialEmptyBase : SequentialEmptyBase + { + public int i; + } + + [StructLayout(LayoutKind.Sequential)] + public class SequentialByteBase + { + public byte x; + } + + [StructLayout(LayoutKind.Sequential)] + public class ClassWithSequentialByteBase : SequentialByteBase + { + public int i; + } + + [StructLayout(LayoutKind.Sequential)] + public class SequentialInt16Base + { + public short x; + } + + [StructLayout(LayoutKind.Sequential)] + public class ClassWithSequentialInt16Base : SequentialInt16Base + { + public int i; + } + + [StructLayout(LayoutKind.Sequential)] + public class SequentialInt32Base + { + public int x; + } + + [StructLayout(LayoutKind.Sequential)] + public class ClassWithSequentialInt32Base : SequentialInt32Base + { + public int i; + } + + [StructLayout(LayoutKind.Sequential)] + public class SequentialInt64Base + { + public long x; + } + + [StructLayout(LayoutKind.Sequential)] + public class ClassWithSequentialInt64Base : SequentialInt64Base + { + public int i; + } +} diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj index 21f6f8a1370edd..07b94962001184 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj @@ -21,6 +21,7 @@ + @@ -58,5 +59,6 @@ + diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/MarshalUtilsTests.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/MarshalUtilsTests.cs new file mode 100644 index 00000000000000..121c6aad27dbf1 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/MarshalUtilsTests.cs @@ -0,0 +1,83 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Internal.TypeSystem; +using Internal.TypeSystem.Interop; + +using Xunit; + +namespace TypeSystemTests +{ + public class MarshalUtilsTests + { + private TestTypeSystemContext _context; + private ModuleDesc _testModule; + + public MarshalUtilsTests() + { + _context = new TestTypeSystemContext(TargetArchitecture.X64); + var systemModule = _context.CreateModuleForSimpleName("CoreTestAssembly"); + _context.SetSystemModule(systemModule); + + _testModule = systemModule; + } + + [Theory] + [InlineData(WellKnownType.Void)] + [InlineData(WellKnownType.Boolean)] + [InlineData(WellKnownType.Char)] + [InlineData(WellKnownType.SByte)] + [InlineData(WellKnownType.Byte)] + [InlineData(WellKnownType.Int16)] + [InlineData(WellKnownType.UInt16)] + [InlineData(WellKnownType.Int32)] + [InlineData(WellKnownType.UInt32)] + [InlineData(WellKnownType.Int64)] + [InlineData(WellKnownType.UInt64)] + [InlineData(WellKnownType.IntPtr)] + [InlineData(WellKnownType.UIntPtr)] + [InlineData(WellKnownType.Single)] + [InlineData(WellKnownType.Double)] + [InlineData(WellKnownType.RuntimeFieldHandle)] + [InlineData(WellKnownType.RuntimeTypeHandle)] + [InlineData(WellKnownType.RuntimeMethodHandle)] + public void IsBlittableType_BilittableWellKnownTypes_ReturnsTrue(WellKnownType type) => + Assert.True(MarshalUtils.IsBlittableType(_context.GetWellKnownType(type))); + + [Theory] + [InlineData(WellKnownType.String)] + [InlineData(WellKnownType.ValueType)] + [InlineData(WellKnownType.Enum)] + [InlineData(WellKnownType.Array)] + [InlineData(WellKnownType.MulticastDelegate)] + [InlineData(WellKnownType.Exception)] + [InlineData(WellKnownType.Object)] + public void IsBlittableType_NonBilittableWellKnownTypes_ReturnsFalse(WellKnownType type) => + Assert.False(MarshalUtils.IsBlittableType(_context.GetWellKnownType(type))); + + [Theory] + [InlineData("ClassWithExplicitByteBase")] + [InlineData("ClassWithExplicitInt16Base")] + [InlineData("ClassWithExplicitInt32Base")] + [InlineData("ClassWithExplicitInt64Base")] + [InlineData("ClassWithSequentialByteBase")] + [InlineData("ClassWithSequentialInt16Base")] + [InlineData("ClassWithSequentialInt32Base")] + [InlineData("ClassWithSequentialInt64Base")] + public void IsBlittableType_TypeWithBlittableBase_ReturnsTrue(string className) + { + TypeDesc classWithSequentialBase = _testModule.GetType("Marshalling", className); + Assert.True(MarshalUtils.IsBlittableType(classWithSequentialBase)); + } + + [Theory] + [InlineData("ClassWithExplicitEmptyBase")] + [InlineData("ClassWithExplicitSizeZeroEmptyBase")] + [InlineData("ClassWithSequentialEmptyBase")] + public void IsBlittableType_TypeWithEmptyBase_ReturnsFalse(string className) + { + TypeDesc classWithEmptyBase = _testModule.GetType("Marshalling", className); + Assert.False(MarshalUtils.IsBlittableType(classWithEmptyBase)); + } + } +} From f088baa4a743d3ba90416454be222a607b5b0ae4 Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Tue, 12 Jan 2021 14:34:04 +0200 Subject: [PATCH 2/4] Classify type with empty base as non-blittable --- .../Common/TypeSystem/Common/DefType.FieldLayout.cs | 12 ++++++++++++ .../Common/TypeSystem/Interop/IL/MarshalUtils.cs | 5 +++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs b/src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs index 6c6c7ddecd9e6d..6de46e5ee353f3 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs @@ -161,6 +161,18 @@ public LayoutInt InstanceByteAlignment } } + public bool IsZeroSized + { + get + { + if (!_fieldLayoutFlags.HasFlags(FieldLayoutFlags.ComputedInstanceTypeLayout)) + { + ComputeInstanceLayout(InstanceLayoutKind.TypeOnly); + } + return _instanceByteCountUnaligned == _instanceByteAlignment; + } + } + /// /// The type has stable Abi layout /// diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalUtils.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalUtils.cs index f713f9cb0c29f6..760a233dcd963a 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalUtils.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalUtils.cs @@ -18,12 +18,13 @@ public static bool IsBlittableType(TypeDesc type) return false; } - TypeDesc baseType = type.BaseType; + DefType baseType = type.BaseType; bool hasNonTrivialParent = baseType != null && !baseType.IsWellKnownType(WellKnownType.Object) && !baseType.IsWellKnownType(WellKnownType.ValueType); - if (hasNonTrivialParent && !IsBlittableType(baseType)) + // Type is blittable only if parent is also blittable and is not empty. + if (hasNonTrivialParent && (!IsBlittableType(baseType) || baseType.IsZeroSized)) { return false; } From 856de75dbcb2bc180c10cffbe6c16b45b9858c9e Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Wed, 20 Jan 2021 00:15:40 +0200 Subject: [PATCH 3/4] Add tests for IsZeroSizedReferenceType --- .../TypeSystem/Common/DefType.FieldLayout.cs | 11 +++- .../TypeSystem/Interop/IL/MarshalUtils.cs | 2 +- .../CoreTestAssembly/Marshalling.cs | 39 +++++++++++++- .../DefType.FieldLayoutTests.cs | 54 +++++++++++++++++++ ...ompiler.TypeSystem.ReadyToRun.Tests.csproj | 1 + .../MarshalUtilsTests.cs | 2 +- 6 files changed, 105 insertions(+), 4 deletions(-) create mode 100644 src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/DefType.FieldLayoutTests.cs diff --git a/src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs b/src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs index 6de46e5ee353f3..a92cca26699994 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/DefType.FieldLayout.cs @@ -161,14 +161,23 @@ public LayoutInt InstanceByteAlignment } } - public bool IsZeroSized + public bool IsZeroSizedReferenceType { get { + if (Category != TypeFlags.Class) + { + throw new InvalidOperationException("Only reference types are allowed."); + } + if (!_fieldLayoutFlags.HasFlags(FieldLayoutFlags.ComputedInstanceTypeLayout)) { ComputeInstanceLayout(InstanceLayoutKind.TypeOnly); } + + // test that size without padding is zero: + // _instanceByteCountUnaligned - _instanceByteAlignment == LayoutInt.Zero + // simplified to: return _instanceByteCountUnaligned == _instanceByteAlignment; } } diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalUtils.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalUtils.cs index 760a233dcd963a..84dcc80be1323e 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalUtils.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/MarshalUtils.cs @@ -24,7 +24,7 @@ public static bool IsBlittableType(TypeDesc type) && !baseType.IsWellKnownType(WellKnownType.ValueType); // Type is blittable only if parent is also blittable and is not empty. - if (hasNonTrivialParent && (!IsBlittableType(baseType) || baseType.IsZeroSized)) + if (hasNonTrivialParent && (!IsBlittableType(baseType) || baseType.IsZeroSizedReferenceType)) { return false; } diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/CoreTestAssembly/Marshalling.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/CoreTestAssembly/Marshalling.cs index f9d2d14949dd97..4ddf5702b2b46c 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/CoreTestAssembly/Marshalling.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/CoreTestAssembly/Marshalling.cs @@ -5,6 +5,37 @@ namespace Marshalling { + + #region Simple classes + + public class SimpleEmptyClass + { + } + + public class SimpleByteClass + { + public byte x; + } + + public class SimpleInt16Class + { + public short x; + } + + public class SimpleInt32Class + { + public int x; + } + + public class SimpleInt64Class + { + public long x; + } + + #endregion + + #region LayoutKind.Explicit classes + [StructLayout(LayoutKind.Explicit)] public class ExplicitEmptyBase { @@ -23,7 +54,7 @@ public class ExplicitEmptySizeZeroBase } [StructLayout(LayoutKind.Explicit)] - public class ClassWithExplicitSizeZeroEmptyBase : ExplicitEmptySizeZeroBase + public class ClassWithExplicitEmptySizeZeroBase : ExplicitEmptySizeZeroBase { [FieldOffset(0)] public int i; @@ -90,6 +121,10 @@ public class SequentialEmptyBase { } + #endregion + + #region LayoutKind.Sequential classes + [StructLayout(LayoutKind.Sequential)] public class ClassWithSequentialEmptyBase : SequentialEmptyBase { @@ -143,4 +178,6 @@ public class ClassWithSequentialInt64Base : SequentialInt64Base { public int i; } + + #endregion } diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/DefType.FieldLayoutTests.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/DefType.FieldLayoutTests.cs new file mode 100644 index 00000000000000..ad896ad596b904 --- /dev/null +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/DefType.FieldLayoutTests.cs @@ -0,0 +1,54 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Internal.TypeSystem; +using Internal.TypeSystem.Interop; + +using Xunit; + +namespace TypeSystemTests +{ + public partial class DefTypeTests + { + private ModuleDesc _testModule; + + public DefTypeTests() + { + var context = new TestTypeSystemContext(TargetArchitecture.X64); + var systemModule = context.CreateModuleForSimpleName("CoreTestAssembly"); + context.SetSystemModule(systemModule); + + _testModule = systemModule; + } + + [Theory] + [InlineData("SimpleByteClass")] + [InlineData("SimpleInt16Class")] + [InlineData("SimpleInt32Class")] + [InlineData("SimpleInt64Class")] + [InlineData("ExplicitByteBase")] + [InlineData("ExplicitInt16Base")] + [InlineData("ExplicitInt32Base")] + [InlineData("ExplicitInt64Base")] + [InlineData("SequentialByteBase")] + [InlineData("SequentialInt16Base")] + [InlineData("SequentialInt32Base")] + [InlineData("SequentialInt64Base")] + public void IsZeroSizedReferenceType_NonEmptyType_ReturnsFalse(string className) + { + DefType classWithSequentialBase = _testModule.GetType("Marshalling", className); + Assert.False(classWithSequentialBase.IsZeroSizedReferenceType); + } + + [Theory] + [InlineData("SimpleEmptyClass")] + [InlineData("ExplicitEmptyBase")] + [InlineData("ExplicitEmptySizeZeroBase")] + [InlineData("SequentialEmptyBase")] + public void IsZeroSizedReferenceType_EmptyType_ReturnsTrue(string className) + { + DefType classWithEmptyBase = _testModule.GetType("Marshalling", className); + Assert.True(classWithEmptyBase.IsZeroSizedReferenceType); + } + } +} diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj index 07b94962001184..9b5fec9b380651 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/ILCompiler.TypeSystem.ReadyToRun.Tests.csproj @@ -42,6 +42,7 @@ + diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/MarshalUtilsTests.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/MarshalUtilsTests.cs index 121c6aad27dbf1..34c169c42c33af 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/MarshalUtilsTests.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/MarshalUtilsTests.cs @@ -72,7 +72,7 @@ public void IsBlittableType_TypeWithBlittableBase_ReturnsTrue(string className) [Theory] [InlineData("ClassWithExplicitEmptyBase")] - [InlineData("ClassWithExplicitSizeZeroEmptyBase")] + [InlineData("ClassWithExplicitEmptySizeZeroBase")] [InlineData("ClassWithSequentialEmptyBase")] public void IsBlittableType_TypeWithEmptyBase_ReturnsFalse(string className) { From abfeb637427b148f2ac35fbbcd20d01fda135ec9 Mon Sep 17 00:00:00 2001 From: Adeel <3840695+am11@users.noreply.github.com> Date: Wed, 20 Jan 2021 00:37:35 +0200 Subject: [PATCH 4/4] Rename local variables in tests --- .../DefType.FieldLayoutTests.cs | 8 ++++---- .../MarshalUtilsTests.cs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/DefType.FieldLayoutTests.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/DefType.FieldLayoutTests.cs index ad896ad596b904..c82f6c21a85be8 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/DefType.FieldLayoutTests.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/DefType.FieldLayoutTests.cs @@ -36,8 +36,8 @@ public DefTypeTests() [InlineData("SequentialInt64Base")] public void IsZeroSizedReferenceType_NonEmptyType_ReturnsFalse(string className) { - DefType classWithSequentialBase = _testModule.GetType("Marshalling", className); - Assert.False(classWithSequentialBase.IsZeroSizedReferenceType); + DefType nonEmptyClass = _testModule.GetType("Marshalling", className); + Assert.False(nonEmptyClass.IsZeroSizedReferenceType); } [Theory] @@ -47,8 +47,8 @@ public void IsZeroSizedReferenceType_NonEmptyType_ReturnsFalse(string className) [InlineData("SequentialEmptyBase")] public void IsZeroSizedReferenceType_EmptyType_ReturnsTrue(string className) { - DefType classWithEmptyBase = _testModule.GetType("Marshalling", className); - Assert.True(classWithEmptyBase.IsZeroSizedReferenceType); + DefType emptyClass = _testModule.GetType("Marshalling", className); + Assert.True(emptyClass.IsZeroSizedReferenceType); } } } diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/MarshalUtilsTests.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/MarshalUtilsTests.cs index 34c169c42c33af..c3befa87af72d4 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/MarshalUtilsTests.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.ReadyToRun.Tests/MarshalUtilsTests.cs @@ -66,8 +66,8 @@ public void IsBlittableType_NonBilittableWellKnownTypes_ReturnsFalse(WellKnownTy [InlineData("ClassWithSequentialInt64Base")] public void IsBlittableType_TypeWithBlittableBase_ReturnsTrue(string className) { - TypeDesc classWithSequentialBase = _testModule.GetType("Marshalling", className); - Assert.True(MarshalUtils.IsBlittableType(classWithSequentialBase)); + TypeDesc classWithBlittableBase = _testModule.GetType("Marshalling", className); + Assert.True(MarshalUtils.IsBlittableType(classWithBlittableBase)); } [Theory]