From 54f6b8824500b122788026ba0a1f433b4eda0a5c Mon Sep 17 00:00:00 2001 From: Konstantin Baladurin Date: Mon, 16 Apr 2018 21:11:41 +0300 Subject: [PATCH] [x86/Linux] Fix marshalling struct with 64-bit types (dotnet/coreclr#17455) * [x86/Linux] Fix marshalling struct with 64-bit types The System V ABI for i386 defines 4-byte alignment for 64-bit types. * [Linux/x86] Fix marshalling tests in the case of System V i386 ABI Commit migrated from https://github.com/dotnet/coreclr/commit/af74af2f4a01341fce78bcaac370d497f4f511b1 --- src/coreclr/src/vm/fieldmarshaler.cpp | 20 +++++++ src/coreclr/src/vm/fieldmarshaler.h | 6 ++ .../interopservices/marshal/marshalsizeof1.cs | 12 +++- .../interopservices/marshal/marshalsizeof2.cs | 12 +++- .../src/Interop/MarshalAPI/OffsetOf/OffsetOf.cs | 69 +++++++++++++++++----- .../src/Interop/StructPacking/StructPacking.cs | 36 +++++------ 6 files changed, 120 insertions(+), 35 deletions(-) diff --git a/src/coreclr/src/vm/fieldmarshaler.cpp b/src/coreclr/src/vm/fieldmarshaler.cpp index 1e24399..dfbbb9a 100644 --- a/src/coreclr/src/vm/fieldmarshaler.cpp +++ b/src/coreclr/src/vm/fieldmarshaler.cpp @@ -285,7 +285,27 @@ do \ if (CorTypeInfo::IsPrimitiveType(corElemType)) { pfwalk->m_managedSize = ((UINT32)CorTypeInfo::Size(corElemType)); // Safe cast - no primitive type is larger than 4gb! +#if defined(_TARGET_X86_) && defined(UNIX_X86_ABI) + switch (corElemType) + { + // The System V ABI for i386 defines different packing for these types. + case ELEMENT_TYPE_I8: + case ELEMENT_TYPE_U8: + case ELEMENT_TYPE_R8: + { + pfwalk->m_managedAlignmentReq = 4; + break; + } + + default: + { + pfwalk->m_managedAlignmentReq = pfwalk->m_managedSize; + break; + } + } +#else // _TARGET_X86_ && UNIX_X86_ABI pfwalk->m_managedAlignmentReq = pfwalk->m_managedSize; +#endif } else if (corElemType == ELEMENT_TYPE_PTR) { diff --git a/src/coreclr/src/vm/fieldmarshaler.h b/src/coreclr/src/vm/fieldmarshaler.h index 8c1f8fe..934f058 100644 --- a/src/coreclr/src/vm/fieldmarshaler.h +++ b/src/coreclr/src/vm/fieldmarshaler.h @@ -1626,7 +1626,13 @@ public: UNUSED_METHOD_IMPL(VOID UpdateNativeImpl(OBJECTREF* pCLRValue, LPVOID pNativeValue, OBJECTREF *ppCleanupWorkListOnStack) const) UNUSED_METHOD_IMPL(VOID UpdateCLRImpl(const VOID *pNativeValue, OBJECTREF *ppProtectedCLRValue, OBJECTREF *ppProtectedOldCLRValue) const) +#if defined(_TARGET_X86_) && defined(UNIX_X86_ABI) + // The System V ABI for i386 defines 4-byte alignment for 64-bit types. + SCALAR_MARSHALER_IMPL(8, 4) +#else SCALAR_MARSHALER_IMPL(8, 8) +#endif // _TARGET_X86_ + COPY_TO_IMPL_BASE_STRUCT_ONLY() VOID ScalarUpdateNativeImpl(LPVOID pCLR, LPVOID pNative) const diff --git a/src/coreclr/tests/src/CoreMangLib/cti/system/runtime/interopservices/marshal/marshalsizeof1.cs b/src/coreclr/tests/src/CoreMangLib/cti/system/runtime/interopservices/marshal/marshalsizeof1.cs index 030e767..c0d16b8 100644 --- a/src/coreclr/tests/src/CoreMangLib/cti/system/runtime/interopservices/marshal/marshalsizeof1.cs +++ b/src/coreclr/tests/src/CoreMangLib/cti/system/runtime/interopservices/marshal/marshalsizeof1.cs @@ -225,7 +225,17 @@ public class MarshalSizeOf1 TestMultiMemberStruct1 obj = new TestMultiMemberStruct1(); obj.TestInt = TestLibrary.Generator.GetInt32(-55); obj.TestDouble = TestLibrary.Generator.GetDouble(-55); - int expectedSize = 16; // sizeof(double) + sizeof(int) + padding + int expectedSize; + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || (RuntimeInformation.ProcessArchitecture != Architecture.X86)) + { + expectedSize = 16; // sizeof(double) + sizeof(int) + padding + } + else + { + // The System V ABI for i386 defines double as having 4-byte alignment + expectedSize = 12; // sizeof(double) + sizeof(int) + } int actualSize = Marshal.SizeOf(obj); diff --git a/src/coreclr/tests/src/CoreMangLib/cti/system/runtime/interopservices/marshal/marshalsizeof2.cs b/src/coreclr/tests/src/CoreMangLib/cti/system/runtime/interopservices/marshal/marshalsizeof2.cs index eac0ab6..6882ad0 100644 --- a/src/coreclr/tests/src/CoreMangLib/cti/system/runtime/interopservices/marshal/marshalsizeof2.cs +++ b/src/coreclr/tests/src/CoreMangLib/cti/system/runtime/interopservices/marshal/marshalsizeof2.cs @@ -210,7 +210,17 @@ public class MarshalSizeOf2 try { Type obj = typeof(TestMultiMemberStruct1); - int expectedSize = 16; // sizeof(double) + sizeof(int) + padding + int expectedSize; + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || (RuntimeInformation.ProcessArchitecture != Architecture.X86)) + { + expectedSize = 16; // sizeof(double) + sizeof(int) + padding + } + else + { + // The System V ABI for i386 defines double as having 4-byte alignment + expectedSize = 12; // sizeof(double) + sizeof(int) + } int actualSize = Marshal.SizeOf(obj); diff --git a/src/coreclr/tests/src/Interop/MarshalAPI/OffsetOf/OffsetOf.cs b/src/coreclr/tests/src/Interop/MarshalAPI/OffsetOf/OffsetOf.cs index 5bb4715..6b28401 100644 --- a/src/coreclr/tests/src/Interop/MarshalAPI/OffsetOf/OffsetOf.cs +++ b/src/coreclr/tests/src/Interop/MarshalAPI/OffsetOf/OffsetOf.cs @@ -109,13 +109,13 @@ internal struct FieldAlignementTest // 3 bytes of padding public Int32 m_int2; // 4 bytes - // 4 bytes of padding + // 4 bytes of padding (0 bytes on x86/Unix according System V ABI as double 4-byte aligned) public double m_double1; // 8 bytes public char m_char1; // 1 byte public char m_char2; // 1 byte public char m_char3; // 1 byte - // 5 bytes of padding + // 5 bytes of padding (1 byte on x86/Unix according System V ABI as double 4-byte aligned) public double m_double2; // 8 bytes public byte m_byte3; // 1 byte @@ -137,7 +137,7 @@ struct FieldAlignementTest_Decimal // This is because unlike fields of other types well known to mcg (like long, char etc.) // which need to be aligned according to their byte size, decimal is really a struct // with 8 byte alignment requirement. - public FieldAlignementTest p; // 80 bytes + public FieldAlignementTest p; // 80 bytes (72 bytes on x86/Unix) public short s; // 2 bytes // 6 bytes of padding @@ -251,7 +251,14 @@ public class OffsetTest public static void TestFieldAlignment() { var t = typeof(FieldAlignementTest); - Assert.AreEqual(80, Marshal.SizeOf(t)); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || (RuntimeInformation.ProcessArchitecture != Architecture.X86)) + { + Assert.AreEqual(80, Marshal.SizeOf(t)); + } + else + { + Assert.AreEqual(72, Marshal.SizeOf(t)); + } Assert.AreEqual(new IntPtr(0), Marshal.OffsetOf(t, "m_byte1")); Assert.AreEqual(new IntPtr(2), Marshal.OffsetOf(t, "m_short1")); @@ -259,26 +266,58 @@ public class OffsetTest Assert.AreEqual(new IntPtr(8), Marshal.OffsetOf(t, "m_int1")); Assert.AreEqual(new IntPtr(12), Marshal.OffsetOf(t, "m_byte2")); Assert.AreEqual(new IntPtr(16), Marshal.OffsetOf(t, "m_int2")); - Assert.AreEqual(new IntPtr(24), Marshal.OffsetOf(t, "m_double1")); - Assert.AreEqual(new IntPtr(32), Marshal.OffsetOf(t, "m_char1")); - Assert.AreEqual(new IntPtr(33), Marshal.OffsetOf(t, "m_char2")); - Assert.AreEqual(new IntPtr(34), Marshal.OffsetOf(t, "m_char3")); - Assert.AreEqual(new IntPtr(40), Marshal.OffsetOf(t, "m_double2")); - Assert.AreEqual(new IntPtr(48), Marshal.OffsetOf(t, "m_byte3")); - Assert.AreEqual(new IntPtr(49), Marshal.OffsetOf(t, "m_byte4")); - Assert.AreEqual(new IntPtr(56), Marshal.OffsetOf(t, "m_decimal1")); - Assert.AreEqual(new IntPtr(72), Marshal.OffsetOf(t, "m_char4")); + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || (RuntimeInformation.ProcessArchitecture != Architecture.X86)) + { + Assert.AreEqual(new IntPtr(24), Marshal.OffsetOf(t, "m_double1")); + Assert.AreEqual(new IntPtr(32), Marshal.OffsetOf(t, "m_char1")); + Assert.AreEqual(new IntPtr(33), Marshal.OffsetOf(t, "m_char2")); + Assert.AreEqual(new IntPtr(34), Marshal.OffsetOf(t, "m_char3")); + Assert.AreEqual(new IntPtr(40), Marshal.OffsetOf(t, "m_double2")); + Assert.AreEqual(new IntPtr(48), Marshal.OffsetOf(t, "m_byte3")); + Assert.AreEqual(new IntPtr(49), Marshal.OffsetOf(t, "m_byte4")); + Assert.AreEqual(new IntPtr(56), Marshal.OffsetOf(t, "m_decimal1")); + Assert.AreEqual(new IntPtr(72), Marshal.OffsetOf(t, "m_char4")); + } + else + { + Assert.AreEqual(new IntPtr(20), Marshal.OffsetOf(t, "m_double1")); + Assert.AreEqual(new IntPtr(28), Marshal.OffsetOf(t, "m_char1")); + Assert.AreEqual(new IntPtr(29), Marshal.OffsetOf(t, "m_char2")); + Assert.AreEqual(new IntPtr(30), Marshal.OffsetOf(t, "m_char3")); + Assert.AreEqual(new IntPtr(32), Marshal.OffsetOf(t, "m_double2")); + Assert.AreEqual(new IntPtr(40), Marshal.OffsetOf(t, "m_byte3")); + Assert.AreEqual(new IntPtr(41), Marshal.OffsetOf(t, "m_byte4")); + Assert.AreEqual(new IntPtr(48), Marshal.OffsetOf(t, "m_decimal1")); + Assert.AreEqual(new IntPtr(64), Marshal.OffsetOf(t, "m_char4")); + } } public static void TestFieldAlignment_Decimal() { var t = typeof(FieldAlignementTest_Decimal); - Assert.AreEqual(96, Marshal.SizeOf(t)); + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || (RuntimeInformation.ProcessArchitecture != Architecture.X86)) + { + Assert.AreEqual(96, Marshal.SizeOf(t)); + } + else + { + Assert.AreEqual(88, Marshal.SizeOf(t)); + } Assert.AreEqual(new IntPtr(0), Marshal.OffsetOf(t, "b")); Assert.AreEqual(new IntPtr(8), Marshal.OffsetOf(t, "p")); - Assert.AreEqual(new IntPtr(88), Marshal.OffsetOf(t, "s")); + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || (RuntimeInformation.ProcessArchitecture != Architecture.X86)) + { + Assert.AreEqual(new IntPtr(88), Marshal.OffsetOf(t, "s")); + } + else + { + Assert.AreEqual(new IntPtr(80), Marshal.OffsetOf(t, "s")); + } } diff --git a/src/coreclr/tests/src/Interop/StructPacking/StructPacking.cs b/src/coreclr/tests/src/Interop/StructPacking/StructPacking.cs index ad27ef6..704552d 100644 --- a/src/coreclr/tests/src/Interop/StructPacking/StructPacking.cs +++ b/src/coreclr/tests/src/Interop/StructPacking/StructPacking.cs @@ -384,20 +384,20 @@ unsafe class Program succeeded &= Test>( expectedSize: 12, - expectedOffsetByte: 0, - expectedOffsetValue: 4 + expectedOffsetByte: 8, + expectedOffsetValue: 0 ); succeeded &= Test>( expectedSize: 12, - expectedOffsetByte: 0, - expectedOffsetValue: 4 + expectedOffsetByte: 8, + expectedOffsetValue: 0 ); succeeded &= Test>( expectedSize: 12, - expectedOffsetByte: 0, - expectedOffsetValue: 4 + expectedOffsetByte: 8, + expectedOffsetValue: 0 ); } @@ -603,20 +603,20 @@ unsafe class Program succeeded &= Test>( expectedSize: 12, - expectedOffsetByte: 0, - expectedOffsetValue: 4 + expectedOffsetByte: 8, + expectedOffsetValue: 0 ); succeeded &= Test>( expectedSize: 12, - expectedOffsetByte: 0, - expectedOffsetValue: 4 + expectedOffsetByte: 8, + expectedOffsetValue: 0 ); succeeded &= Test>( expectedSize: 12, - expectedOffsetByte: 0, - expectedOffsetValue: 4 + expectedOffsetByte: 8, + expectedOffsetValue: 0 ); } @@ -1016,20 +1016,20 @@ unsafe class Program succeeded &= Test>( expectedSize: 12, - expectedOffsetByte: 0, - expectedOffsetValue: 4 + expectedOffsetByte: 8, + expectedOffsetValue: 0 ); succeeded &= Test>( expectedSize: 12, - expectedOffsetByte: 0, - expectedOffsetValue: 4 + expectedOffsetByte: 8, + expectedOffsetValue: 0 ); succeeded &= Test>( expectedSize: 12, - expectedOffsetByte: 0, - expectedOffsetValue: 4 + expectedOffsetByte: 8, + expectedOffsetValue: 0 ); } -- 2.7.4