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

Align up structs to IntPtr.Size if they have gc pointers #100289

Merged
merged 20 commits into from
Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -965,33 +965,37 @@ private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt
{
SizeAndAlignment result;

// Pad the length of structs to be 1 if they are empty so we have no zero-length structures
// Pad the length of structs to be 1 if they are empty, so we have no zero-length structures
if (type.IsValueType && instanceSize == LayoutInt.Zero)
{
instanceSize = LayoutInt.One;
}

TargetDetails target = type.Context.Target;

if (classLayoutSize != 0)
if (type.IsValueType)
{
LayoutInt parentSize;
if (type.IsValueType)
parentSize = new LayoutInt(0);
else
parentSize = type.BaseType.InstanceByteCountUnaligned;

LayoutInt specifiedInstanceSize = parentSize + new LayoutInt(classLayoutSize);
if (classLayoutSize != 0)
{
instanceSize = LayoutInt.Max(new LayoutInt(classLayoutSize), instanceSize);

instanceSize = LayoutInt.Max(specifiedInstanceSize, instanceSize);
}
else
{
if (type.IsValueType)
// Size of a struct with gc references is always expected to be aligned to pointer size
if (type.ContainsGCPointers)
{
instanceSize = LayoutInt.AlignUp(instanceSize, new LayoutInt(target.PointerSize), target);
}
}
else
{
instanceSize = LayoutInt.AlignUp(instanceSize, alignment, target);
}
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
}
else if (classLayoutSize != 0 && type.IsSequentialLayout && !type.ContainsGCPointers)
{
// For classes, we respect classLayoutSize only for SequentialLayout + no gc fields
LayoutInt specifiedInstanceSize = type.BaseType.InstanceByteCountUnaligned + new LayoutInt(classLayoutSize);
instanceSize = LayoutInt.Max(specifiedInstanceSize, instanceSize);
}

if (type.IsValueType)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public void TestExplicitLayoutThatIsEmpty()
public void TestExplicitTypeLayoutWithSize()
{
var explicitSizeType = _testModule.GetType("Explicit", "ExplicitSize");
Assert.Equal(48, explicitSizeType.InstanceByteCount.AsInt);
Assert.Equal(32, explicitSizeType.InstanceByteCount.AsInt);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,190 @@
// 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.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using Xunit;

public static class Runtime_100220
{
[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/100220", TestRuntimes.Mono)]
// Also, Mono needs RuntimeHelpers.GetRawObjectDataSize or equivalent to get an object size
public static int TestEntryPoint()
{
if (IntPtr.Size == 8)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this maybe not have the if and multiply the pointer size appropriately in each case?

Copy link
Member Author

@EgorBo EgorBo Apr 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer expected value to be a constant rather than computed expression for Asserts

{
// Classes
Assert.Equal(16, ObjectSize<MyClassAuto>());
Assert.Equal(16, ObjectSize<MyClass1000Exp>());
Assert.Equal(16, ObjectSize<MyClass1000Seq>());
Assert.Equal(16, ObjectSize<MyClass1000NoGcExp>());
Assert.Equal(1000, ObjectSize<MyClass1000NoGcSeq>());
Assert.Equal(24, ObjectSize<BaseClassSeq>());
Assert.Equal(40, ObjectSize<SubclassSeq>());
Assert.Equal(48, ObjectSize<SubclassSubclassSeq>());
Assert.Equal(32, ObjectSize<SubclassWithGcSeq>());
Assert.Equal(16, ObjectSize<SubclassOfBaseWithGcSeq>());

// Structs
Assert.Equal(16, Unsafe.SizeOf<MyStructAuto>());
Assert.Equal(16, Unsafe.SizeOf<MyStruct9Seq>());
Assert.Equal(16, Unsafe.SizeOf<MyStruct9Exp>());
Assert.Equal(16, Unsafe.SizeOf<MyStruct1000Seq>());
Assert.Equal(1000, Unsafe.SizeOf<MyStruct1000Exp>());
}
else
{
// Classes
Assert.Equal(8, ObjectSize<MyClassAuto>());
Assert.Equal(12, ObjectSize<MyClass1000Exp>());
Assert.Equal(8, ObjectSize<MyClass1000Seq>());
Assert.Equal(12, ObjectSize<MyClass1000NoGcExp>());
Assert.Equal(1000, ObjectSize<MyClass1000NoGcSeq>());
Assert.Equal(20, ObjectSize<BaseClassSeq>());
Assert.Equal(36, ObjectSize<SubclassSeq>());
Assert.Equal(44, ObjectSize<SubclassSubclassSeq>());
Assert.Equal(28, ObjectSize<SubclassWithGcSeq>());
Assert.Equal(8, ObjectSize<SubclassOfBaseWithGcSeq>());

// Structs
Assert.Equal(8, Unsafe.SizeOf<MyStructAuto>());
Assert.Equal(8, Unsafe.SizeOf<MyStruct9Seq>());
Assert.Equal(12, Unsafe.SizeOf<MyStruct9Exp>());
Assert.Equal(8, Unsafe.SizeOf<MyStruct1000Seq>());
Assert.Equal(1000, Unsafe.SizeOf<MyStruct1000Exp>());
}

// Field offsets:
Assert.Equal("(5, 3, 4, 1, 2, 6)", FieldOffsets(new SubclassSubclassSeq { A = 1, B = 2, C = 3, D = 4, E = 5, F = 6 }).ToString());
return 100;
}

[MethodImpl(MethodImplOptions.NoInlining)]
static object FieldOffsets(SubclassSubclassSeq f) => (f.E, f.C, f.D, f.A, f.B, f.F);

[StructLayout(LayoutKind.Sequential, Size = 20)]
public class BaseClassSeq
{
public byte A;
public nint B;
}

[StructLayout(LayoutKind.Sequential, Size = 20)]
public class BaseClassWithGcSeq
{
public byte A;
public string B;
}

public class SubclassOfBaseWithGcSeq : BaseClassWithGcSeq
{
public byte C;
}

[StructLayout(LayoutKind.Sequential, Size = 16)]
public class SubclassSeq : BaseClassSeq
{
public byte C;
public nint D;
}

[StructLayout(LayoutKind.Sequential, Size = 32)]
public class SubclassWithGcSeq : BaseClassSeq
{
public byte C;
public object D;
}

public class SubclassSubclassSeq : SubclassSeq
{
public byte E;
public nint F;
}

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)]
private static int ObjectSize<T>() where T : new()
{
return (int)(nuint)typeof(RuntimeHelpers)
.GetMethod("GetRawObjectDataSize", BindingFlags.Static | BindingFlags.NonPublic)
.Invoke(null, [new T()]);
}

private class MyClassAuto
{
private byte[] a;
private byte b;
}

[StructLayout(LayoutKind.Explicit, Size = 1000)]
private class MyClass1000Exp
{
[FieldOffset(0)]
private byte[] a;
[FieldOffset(8)]
private byte b;
}

[StructLayout(LayoutKind.Sequential, Size = 1000)]
private class MyClass1000Seq
{
private byte[] a;
private byte b;
}

[StructLayout(LayoutKind.Explicit, Size = 1000)]
private class MyClass1000NoGcExp
{
[FieldOffset(0)]
private nint a;
[FieldOffset(8)]
private byte b;
}

[StructLayout(LayoutKind.Sequential, Size = 1000)]
private class MyClass1000NoGcSeq
{
private nint a;
private byte b;
}

private struct MyStructAuto
{
private byte[] a;
private byte b;
}

[StructLayout(LayoutKind.Explicit, Size = 1000)]
private struct MyStruct1000Exp
{
[FieldOffset(0)]
private byte[] a;
[FieldOffset(8)]
private byte b;
}

[StructLayout(LayoutKind.Sequential, Size = 1000)]
private struct MyStruct1000Seq
{
private byte[] a;
private byte b;
}

[StructLayout(LayoutKind.Explicit, Size = 9)]
private struct MyStruct9Exp
{
[FieldOffset(0)]
private byte[] a;
[FieldOffset(8)]
private byte b;
}

[StructLayout(LayoutKind.Sequential, Size = 9)]
private struct MyStruct9Seq
{
private byte[] a;
private byte b;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>