From abd59fd87ae969fe9aabdfcaff6542d9237e6748 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Wed, 7 Aug 2024 15:46:22 -0700 Subject: [PATCH 1/9] Check HFA alignment for valuetype fields --- src/coreclr/vm/class.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index a96bcffae4fe1a..6b02bca81fa14d 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -1863,6 +1863,29 @@ EEClass::CheckForHFA() fieldHFAType = pFD->LookupApproxFieldTypeHandle().AsMethodTable()->GetHFAType(); #endif } + + int requiredAlignment; + switch (fieldHFAType) + { + case CORINFO_HFA_ELEM_FLOAT: + requiredAlignment = 4; + break; + case CORINFO_HFA_ELEM_VECTOR64: + case CORINFO_HFA_ELEM_DOUBLE: + requiredAlignment = 8; + break; + case CORINFO_HFA_ELEM_VECTOR128: + requiredAlignment = 16; + break; + default: + requiredAlignment = 0; + break; + } + + if (requiredAlignment && (pFD->GetOffset() % requiredAlignment != 0)) + { + return false; + } } break; From 0dd0b1a3fae5da401866a67a1e620fa0fcb19431 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Mon, 12 Aug 2024 15:29:59 -0700 Subject: [PATCH 2/9] Attempt to add test for Issue 80393 --- src/tests/JIT/Directed/StructABI/StructABI.c | 28 ++++ src/tests/JIT/Directed/StructABI/StructABI.cs | 121 +++++++++++------- 2 files changed, 105 insertions(+), 44 deletions(-) diff --git a/src/tests/JIT/Directed/StructABI/StructABI.c b/src/tests/JIT/Directed/StructABI/StructABI.c index 959ca9afda52d0..6ae416e4d16cad 100644 --- a/src/tests/JIT/Directed/StructABI/StructABI.c +++ b/src/tests/JIT/Directed/StructABI/StructABI.c @@ -221,6 +221,34 @@ struct Nested9 struct InlineArray4 Field2; }; +#pragma pack(push, 1) +struct Issue80393_F2 +{ + double value; +}; + +struct Issue80393_S_Doubles +{ + double f1; + double f3; +}; + +struct Issue80393_F2_Offset { + char padding[3]; + struct Issue80393_F2 F2; +}; + +union Issue80393_S { + struct Issue80393_S_Doubles f1_f3; + struct Issue80393_F2_Offset f2; +}; +#pragma pack(pop) + +DLLEXPORT union Issue80393_S Issue80393_HFA(union Issue80393_S value) +{ + return value; +} + DLLEXPORT struct SingleByte EchoSingleByte(struct SingleByte value) { return value; diff --git a/src/tests/JIT/Directed/StructABI/StructABI.cs b/src/tests/JIT/Directed/StructABI/StructABI.cs index 2f74d49e75853f..f044176f1694a6 100644 --- a/src/tests/JIT/Directed/StructABI/StructABI.cs +++ b/src/tests/JIT/Directed/StructABI/StructABI.cs @@ -668,8 +668,24 @@ public bool Equals(Nested9 other) } } +struct Issue80393_F2 +{ + public double value; +} + +[StructLayout(LayoutKind.Explicit)] +struct Issue80393_S +{ + [FieldOffset(0)] public double f1; + [FieldOffset(3)] public Issue80393_F2 f2; + [FieldOffset(8)] public double f3; +} + public static partial class StructABI { + [DllImport("StructABILib")] + static extern Issue80393_S Issue80393_HFA(Issue80393_S value); + [DllImport("StructABILib")] static extern SingleByte EchoSingleByte(SingleByte value); @@ -1075,7 +1091,7 @@ static bool EchoSingleByteWrapper() SingleByte expectedSingleByte = SingleByte.Get(); SingleByte nativeSingleByte = EchoSingleByte(expectedSingleByte); SingleByte managedSingleByte = EchoSingleByteManaged(expectedSingleByte); - + if (!expectedSingleByte.Equals(nativeSingleByte)) { Console.WriteLine("Native call for EchoSingleByte failed"); @@ -1093,7 +1109,7 @@ static bool EchoSingleByteWrapper() nativeSingleByte = EchoSingleByte(expectedSingleByte2); managedSingleByte = EchoSingleByteManaged(expectedSingleByte2); - + if (!expectedSingleByte2.Equals(nativeSingleByte)) { Console.WriteLine("Native call for EchoSingleByte failed"); @@ -1115,7 +1131,7 @@ static bool EchoSingleLongWrapper() SingleLong expectedSingleLong = SingleLong.Get(); SingleLong nativeSingleLong = EchoSingleLong(expectedSingleLong); SingleLong managedSingleLong = EchoSingleLongManaged(expectedSingleLong); - + if (!expectedSingleLong.Equals(nativeSingleLong)) { Console.WriteLine("Native call for EchoSingleLong failed"); @@ -1133,7 +1149,7 @@ static bool EchoSingleLongWrapper() nativeSingleLong = EchoSingleLong(expectedSingleLong2); managedSingleLong = EchoSingleLongManaged(expectedSingleLong2); - + if (!expectedSingleLong2.Equals(nativeSingleLong)) { Console.WriteLine("Native call for EchoSingleByte failed"); @@ -1155,7 +1171,7 @@ static bool EchoSingleFloatWrapper() SingleFloat expectedSingleFloat = SingleFloat.Get(); SingleFloat nativeSingleFloat = EchoSingleFloat(expectedSingleFloat); SingleFloat managedSingleFloat = EchoSingleFloatManaged(expectedSingleFloat); - + if (!expectedSingleFloat.Equals(nativeSingleFloat)) { Console.WriteLine("Native call for EchoSingleFloat failed"); @@ -1173,7 +1189,7 @@ static bool EchoSingleFloatWrapper() nativeSingleFloat = EchoSingleFloat(expectedSingleFloat2); managedSingleFloat = EchoSingleFloatManaged(expectedSingleFloat2); - + if (!expectedSingleFloat2.Equals(nativeSingleFloat)) { Console.WriteLine("Native call for EchoSingleFloat failed"); @@ -1195,7 +1211,7 @@ static bool EchoSingleDoubleWrapper() SingleDouble expectedSingleDouble = SingleDouble.Get(); SingleDouble nativeSingleDouble = EchoSingleDouble(expectedSingleDouble); SingleDouble managedSingleDouble = EchoSingleDoubleManaged(expectedSingleDouble); - + if (!expectedSingleDouble.Equals(nativeSingleDouble)) { Console.WriteLine("Native call for EchoSingleDouble failed"); @@ -1213,7 +1229,7 @@ static bool EchoSingleDoubleWrapper() nativeSingleDouble = EchoSingleDouble(expectedSingleDouble2); managedSingleDouble = EchoSingleDoubleManaged(expectedSingleDouble2); - + if (!expectedSingleDouble2.Equals(nativeSingleDouble)) { Console.WriteLine("Native call for EchoSingleDouble failed"); @@ -1235,7 +1251,7 @@ static bool EchoByteAndFloatWrapper() ByteAndFloat expectedByteAndFloat = ByteAndFloat.Get(); ByteAndFloat nativeByteAndFloat = EchoByteAndFloat(expectedByteAndFloat); ByteAndFloat managedByteAndFloat = EchoByteAndFloatManaged(expectedByteAndFloat); - + if (!expectedByteAndFloat.Equals(nativeByteAndFloat)) { Console.WriteLine("Native call for EchoByteAndFloat failed"); @@ -1254,7 +1270,7 @@ static bool EchoByteAndFloatWrapper() nativeByteAndFloat = EchoByteAndFloat(expectedByteAndFloat2); managedByteAndFloat = EchoByteAndFloatManaged(expectedByteAndFloat2); - + if (!expectedByteAndFloat2.Equals(nativeByteAndFloat)) { Console.WriteLine("Native call for EchoByteAndFloat failed"); @@ -1276,7 +1292,7 @@ static bool EchoLongAndFloatWrapper() LongAndFloat expectedLongAndFloat = LongAndFloat.Get(); LongAndFloat nativeLongAndFloat = EchoLongAndFloat(expectedLongAndFloat); LongAndFloat managedLongAndFloat = EchoLongAndFloatManaged(expectedLongAndFloat); - + if (!expectedLongAndFloat.Equals(nativeLongAndFloat)) { Console.WriteLine("Native call for EchoLongAndFloat failed"); @@ -1295,7 +1311,7 @@ static bool EchoLongAndFloatWrapper() nativeLongAndFloat = EchoLongAndFloat(expectedLongAndFloat2); managedLongAndFloat = EchoLongAndFloatManaged(expectedLongAndFloat2); - + if (!expectedLongAndFloat2.Equals(nativeLongAndFloat)) { Console.WriteLine("Native call for EchoLongAndFloat failed"); @@ -1317,7 +1333,7 @@ static bool EchoByteAndDoubleWrapper() ByteAndDouble expectedByteAndDouble = ByteAndDouble.Get(); ByteAndDouble nativeByteAndDouble = EchoByteAndDouble(expectedByteAndDouble); ByteAndDouble managedByteAndDouble = EchoByteAndDoubleManaged(expectedByteAndDouble); - + if (!expectedByteAndDouble.Equals(nativeByteAndDouble)) { Console.WriteLine("Native call for EchoByteAndDouble failed"); @@ -1336,7 +1352,7 @@ static bool EchoByteAndDoubleWrapper() nativeByteAndDouble = EchoByteAndDouble(expectedByteAndDouble2); managedByteAndDouble = EchoByteAndDoubleManaged(expectedByteAndDouble2); - + if (!expectedByteAndDouble2.Equals(nativeByteAndDouble)) { Console.WriteLine("Native call for EchoByteAndDouble failed"); @@ -1358,7 +1374,7 @@ static bool EchoDoubleAndByteWrapper() DoubleAndByte expectedDoubleAndByte = DoubleAndByte.Get(); DoubleAndByte nativeDoubleAndByte = EchoDoubleAndByte(expectedDoubleAndByte); DoubleAndByte managedDoubleAndByte = EchoDoubleAndByteManaged(expectedDoubleAndByte); - + if (!expectedDoubleAndByte.Equals(nativeDoubleAndByte)) { Console.WriteLine("Native call for EchoDoubleAndByte failed"); @@ -1380,7 +1396,7 @@ static bool EchoPointerAndByteWrapper() PointerAndByte expectedPointerAndByte = PointerAndByte.Get(); PointerAndByte nativePointerAndByte = EchoPointerAndByte(expectedPointerAndByte); PointerAndByte managedPointerAndByte = EchoPointerAndByteManaged(expectedPointerAndByte); - + if (!expectedPointerAndByte.Equals(nativePointerAndByte)) { Console.WriteLine("Native call for EchoPointerAndByte failed"); @@ -1402,7 +1418,7 @@ static bool EchoByteAndPointerWrapper() ByteAndPointer expectedByteAndPointer = ByteAndPointer.Get(); ByteAndPointer nativeByteAndPointer = EchoByteAndPointer(expectedByteAndPointer); ByteAndPointer managedByteAndPointer = EchoByteAndPointerManaged(expectedByteAndPointer); - + if (!expectedByteAndPointer.Equals(nativeByteAndPointer)) { Console.WriteLine("Native call for EchoByteAndPointer failed"); @@ -1424,7 +1440,7 @@ static bool EchoByteFloatAndPointerWrapper() ByteFloatAndPointer expectedByteFloatAndPointer = ByteFloatAndPointer.Get(); ByteFloatAndPointer nativeByteFloatAndPointer = EchoByteFloatAndPointer(expectedByteFloatAndPointer); ByteFloatAndPointer managedByteFloatAndPointer = EchoByteFloatAndPointerManaged(expectedByteFloatAndPointer); - + if (!expectedByteFloatAndPointer.Equals(nativeByteFloatAndPointer)) { Console.WriteLine("Native call for EchoByteFloatAndPointer failed"); @@ -1446,7 +1462,7 @@ static bool EchoPointerFloatAndByteWrapper() PointerFloatAndByte expectedPointerFloatAndByte = PointerFloatAndByte.Get(); PointerFloatAndByte nativePointerFloatAndByte = EchoPointerFloatAndByte(expectedPointerFloatAndByte); PointerFloatAndByte managedPointerFloatAndByte = EchoPointerFloatAndByteManaged(expectedPointerFloatAndByte); - + if (!expectedPointerFloatAndByte.Equals(nativePointerFloatAndByte)) { Console.WriteLine("Native call for EchoPointerFloatAndByte failed"); @@ -1468,7 +1484,7 @@ static bool EchoShortIntFloatIntPtrWrapper() ShortIntFloatIntPtr expectedShortIntFloatIntPtr = ShortIntFloatIntPtr.Get(); ShortIntFloatIntPtr nativeShortIntFloatIntPtr = EchoShortIntFloatIntPtr(expectedShortIntFloatIntPtr); ShortIntFloatIntPtr managedShortIntFloatIntPtr = EchoShortIntFloatIntPtrManaged(expectedShortIntFloatIntPtr); - + if (!expectedShortIntFloatIntPtr.Equals(nativeShortIntFloatIntPtr)) { Console.WriteLine("Native call for EchoShortIntFloatIntPtr failed"); @@ -1490,7 +1506,7 @@ static bool EchoTwoLongsWrapper() TwoLongs expectedTwoLongs = TwoLongs.Get(); TwoLongs nativeTwoLongs = EchoTwoLongs(expectedTwoLongs); TwoLongs managedTwoLongs = EchoTwoLongsManaged(expectedTwoLongs); - + if (!expectedTwoLongs.Equals(nativeTwoLongs)) { Console.WriteLine("Native call for EchoTwoLongs failed"); @@ -1512,7 +1528,7 @@ static bool EchoTwoFloatsWrapper() TwoFloats expectedTwoFloats = TwoFloats.Get(); TwoFloats nativeTwoFloats = EchoTwoFloats(expectedTwoFloats); TwoFloats managedTwoFloats = EchoTwoFloatsManaged(expectedTwoFloats); - + if (!expectedTwoFloats.Equals(nativeTwoFloats)) { Console.WriteLine("Native call for EchoTwoFloats failed"); @@ -1534,7 +1550,7 @@ static bool EchoTwoDoublesWrapper() TwoDoubles expectedTwoDoubles = TwoDoubles.Get(); TwoDoubles nativeTwoDoubles = EchoTwoDoubles(expectedTwoDoubles); TwoDoubles managedTwoDoubles = EchoTwoDoublesManaged(expectedTwoDoubles); - + if (!expectedTwoDoubles.Equals(nativeTwoDoubles)) { Console.WriteLine("Native call for EchoTwoDoubles failed"); @@ -1556,7 +1572,7 @@ static bool EchoFourLongsWrapper() FourLongs expectedFourLongs = FourLongs.Get(); FourLongs nativeFourLongs = EchoFourLongs(expectedFourLongs); FourLongs managedFourLongs = EchoFourLongsManaged(expectedFourLongs); - + if (!expectedFourLongs.Equals(nativeFourLongs)) { Console.WriteLine("Native call for EchoFourLongs failed"); @@ -1578,7 +1594,7 @@ static bool EchoFourDoublesWrapper() FourDoubles expectedFourDoubles = FourDoubles.Get(); FourDoubles nativeFourDoubles = EchoFourDoubles(expectedFourDoubles); FourDoubles managedFourDoubles = EchoFourDoublesManaged(expectedFourDoubles); - + if (!expectedFourDoubles.Equals(nativeFourDoubles)) { Console.WriteLine("Native call for EchoFourDoubles failed"); @@ -1600,7 +1616,7 @@ static bool EchoInlineArray1Wrapper() InlineArray1 expectedInlineArray1 = InlineArray1.Get(); InlineArray1 nativeInlineArray1 = EchoInlineArray1(expectedInlineArray1); InlineArray1 managedInlineArray1 = EchoInlineArray1Managed(expectedInlineArray1); - + if (!expectedInlineArray1.Equals(nativeInlineArray1)) { Console.WriteLine("Native call for EchoInlineArray1 failed"); @@ -1622,7 +1638,7 @@ static bool EchoInlineArray2Wrapper() InlineArray2 expectedInlineArray2 = InlineArray2.Get(); InlineArray2 nativeInlineArray2 = EchoInlineArray2(expectedInlineArray2); InlineArray2 managedInlineArray2 = EchoInlineArray2Managed(expectedInlineArray2); - + if (!expectedInlineArray2.Equals(nativeInlineArray2)) { Console.WriteLine("Native call for EchoInlineArray2 failed"); @@ -1644,7 +1660,7 @@ static bool EchoInlineArray3Wrapper() InlineArray3 expectedInlineArray3 = InlineArray3.Get(); InlineArray3 nativeInlineArray3 = EchoInlineArray3(expectedInlineArray3); InlineArray3 managedInlineArray3 = EchoInlineArray3Managed(expectedInlineArray3); - + if (!expectedInlineArray3.Equals(nativeInlineArray3)) { Console.WriteLine("Native call for EchoInlineArray3 failed"); @@ -1666,7 +1682,7 @@ static bool EchoInlineArray4Wrapper() InlineArray4 expectedInlineArray4 = InlineArray4.Get(); InlineArray4 nativeInlineArray4 = EchoInlineArray4(expectedInlineArray4); InlineArray4 managedInlineArray4 = EchoInlineArray4Managed(expectedInlineArray4); - + if (!expectedInlineArray4.Equals(nativeInlineArray4)) { Console.WriteLine("Native call for EchoInlineArray4 failed"); @@ -1688,7 +1704,7 @@ static bool EchoInlineArray5Wrapper() InlineArray5 expectedInlineArray5 = InlineArray5.Get(); InlineArray5 nativeInlineArray5 = EchoInlineArray5(expectedInlineArray5); InlineArray5 managedInlineArray5 = EchoInlineArray5Managed(expectedInlineArray5); - + if (!expectedInlineArray5.Equals(nativeInlineArray5)) { Console.WriteLine("Native call for EchoInlineArray5 failed"); @@ -1710,7 +1726,7 @@ static bool EchoInlineArray6Wrapper() InlineArray6 expectedInlineArray6 = InlineArray6.Get(); InlineArray6 nativeInlineArray6 = EchoInlineArray6(expectedInlineArray6); InlineArray6 managedInlineArray6 = EchoInlineArray6Managed(expectedInlineArray6); - + if (!expectedInlineArray6.Equals(nativeInlineArray6)) { Console.WriteLine("Native call for EchoInlineArray6 failed"); @@ -1732,7 +1748,7 @@ static bool EchoNested1Wrapper() Nested1 expectedNested1 = Nested1.Get(); Nested1 nativeNested1 = EchoNested1(expectedNested1); Nested1 managedNested1 = EchoNested1Managed(expectedNested1); - + if (!expectedNested1.Equals(nativeNested1)) { Console.WriteLine("Native call for EchoNested1 failed"); @@ -1754,7 +1770,7 @@ static bool EchoNested2Wrapper() Nested2 expectedNested2 = Nested2.Get(); Nested2 nativeNested2 = EchoNested2(expectedNested2); Nested2 managedNested2 = EchoNested2Managed(expectedNested2); - + if (!expectedNested2.Equals(nativeNested2)) { Console.WriteLine("Native call for EchoNested2 failed"); @@ -1776,7 +1792,7 @@ static bool EchoNested3Wrapper() Nested3 expectedNested3 = Nested3.Get(); Nested3 nativeNested3 = EchoNested3(expectedNested3); Nested3 managedNested3 = EchoNested3Managed(expectedNested3); - + if (!expectedNested3.Equals(nativeNested3)) { Console.WriteLine("Native call for EchoNested3 failed"); @@ -1798,7 +1814,7 @@ static bool EchoNested4Wrapper() Nested4 expectedNested4 = Nested4.Get(); Nested4 nativeNested4 = EchoNested4(expectedNested4); Nested4 managedNested4 = EchoNested4Managed(expectedNested4); - + if (!expectedNested4.Equals(nativeNested4)) { Console.WriteLine("Native call for EchoNested4 failed"); @@ -1820,7 +1836,7 @@ static bool EchoNested5Wrapper() Nested5 expectedNested5 = Nested5.Get(); Nested5 nativeNested5 = EchoNested5(expectedNested5); Nested5 managedNested5 = EchoNested5Managed(expectedNested5); - + if (!expectedNested5.Equals(nativeNested5)) { Console.WriteLine("Native call for EchoNested5 failed"); @@ -1842,7 +1858,7 @@ static bool EchoNested6Wrapper() Nested6 expectedNested6 = Nested6.Get(); Nested6 nativeNested6 = EchoNested6(expectedNested6); Nested6 managedNested6 = EchoNested6Managed(expectedNested6); - + if (!expectedNested6.Equals(nativeNested6)) { Console.WriteLine("Native call for EchoNested6 failed"); @@ -1864,7 +1880,7 @@ static bool EchoNested7Wrapper() Nested7 expectedNested7 = Nested7.Get(); Nested7 nativeNested7 = EchoNested7(expectedNested7); Nested7 managedNested7 = EchoNested7Managed(expectedNested7); - + if (!expectedNested7.Equals(nativeNested7)) { Console.WriteLine("Native call for EchoNested7 failed"); @@ -1886,7 +1902,7 @@ static bool EchoNested8Wrapper() Nested8 expectedNested8 = Nested8.Get(); Nested8 nativeNested8 = EchoNested8(expectedNested8); Nested8 managedNested8 = EchoNested8Managed(expectedNested8); - + if (!expectedNested8.Equals(nativeNested8)) { Console.WriteLine("Native call for EchoNested8 failed"); @@ -1908,7 +1924,7 @@ static bool EchoNested9Wrapper() Nested9 expectedNested9 = Nested9.Get(); Nested9 nativeNested9 = EchoNested9(expectedNested9); Nested9 managedNested9 = EchoNested9Managed(expectedNested9); - + if (!expectedNested9.Equals(nativeNested9)) { Console.WriteLine("Native call for EchoNested9 failed"); @@ -1973,7 +1989,7 @@ static bool NotEnoughRegistersSysV2Wrapper() static bool NotEnoughRegistersSysV3Wrapper() { bool ok = true; - + DoubleAndByte expectedNotEnoughRegistersSysV3 = DoubleAndByte.Get(); DoubleAndByte nativeNotEnoughRegistersSysV3 = NotEnoughRegistersSysV3(1, 2, 3, 4, 5, 6, expectedNotEnoughRegistersSysV3); DoubleAndByte managedNotEnoughRegistersSysV3 = NotEnoughRegistersSysV3Managed(1, 2, 3, 4, 5, 6, expectedNotEnoughRegistersSysV3); @@ -2042,7 +2058,7 @@ static bool NotEnoughRegistersSysV5Wrapper() static bool NotEnoughRegistersSysV6Wrapper() { bool ok = true; - + DoubleAndByte expectedNotEnoughRegistersSysV6 = DoubleAndByte.Get(); DoubleAndByte nativeNotEnoughRegistersSysV6 = NotEnoughRegistersSysV6(1.0d, 2.0d, 3.0d, 4.0d, 5.0d, 6.0d, 7.0d, 8.0d, expectedNotEnoughRegistersSysV6); DoubleAndByte managedNotEnoughRegistersSysV6 = NotEnoughRegistersSysV6Managed(1.0d, 2.0d, 3.0d, 4.0d, 5.0d, 6.0d, 7.0d, 8.0d, expectedNotEnoughRegistersSysV6); @@ -2154,11 +2170,27 @@ static bool EnoughRegistersSysV4Wrapper() return ok; } + static bool Issue80393Wrapper() + { + bool ok = true; + Issue80393_S s = default; + s.f1 = 37.1; + s.f3 = 21.9; + Issue80393_S ret = Issue80393_HFA(s); + + if (!s.f1.Equals(ret.f1) || !s.f3.Equals(ret.f3) || !s.f2.value.Equals(ret.f2.value)) { + Console.WriteLine($"Issue80393_HFA failed. Retval: f1={ret.f1} f2={ret.f2} f3={ret.f3}"); + ok = false; + } + + return ok; + } + [Fact] public static int TestEntryPoint() { var ok = true; - + if (!EchoSingleByteWrapper()) ok = false; if (!EchoSingleLongWrapper()) ok = false; if (!EchoSingleFloatWrapper()) ok = false; @@ -2202,7 +2234,8 @@ public static int TestEntryPoint() if (!EnoughRegistersSysV2Wrapper()) ok = false; if (!EnoughRegistersSysV3Wrapper()) ok = false; if (!EnoughRegistersSysV4Wrapper()) ok = false; - + if (!Issue80393Wrapper()) ok = false; + return ok ? 100 : -1; } } From addbcdb95991cdcd8ad78126991aa9c5e9ef6cf3 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Tue, 13 Aug 2024 18:44:17 -0700 Subject: [PATCH 3/9] Make test more 'real' --- src/tests/JIT/Directed/StructABI/StructABI.c | 5 ++++- src/tests/JIT/Directed/StructABI/StructABI.cs | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/tests/JIT/Directed/StructABI/StructABI.c b/src/tests/JIT/Directed/StructABI/StructABI.c index 6ae416e4d16cad..4a04c2229eda23 100644 --- a/src/tests/JIT/Directed/StructABI/StructABI.c +++ b/src/tests/JIT/Directed/StructABI/StructABI.c @@ -246,7 +246,10 @@ union Issue80393_S { DLLEXPORT union Issue80393_S Issue80393_HFA(union Issue80393_S value) { - return value; + union Issue80393_S result; + result.f1_f3.f1 = 1.0 + value.f1_f3.f1; + result.f1_f3.f3 = 3.0 + value.f1_f3.f3; + return result; } DLLEXPORT struct SingleByte EchoSingleByte(struct SingleByte value) diff --git a/src/tests/JIT/Directed/StructABI/StructABI.cs b/src/tests/JIT/Directed/StructABI/StructABI.cs index f044176f1694a6..e0ca455d7b36f6 100644 --- a/src/tests/JIT/Directed/StructABI/StructABI.cs +++ b/src/tests/JIT/Directed/StructABI/StructABI.cs @@ -2174,12 +2174,12 @@ static bool Issue80393Wrapper() { bool ok = true; Issue80393_S s = default; - s.f1 = 37.1; - s.f3 = 21.9; + s.f1 = 1.0d; + s.f3 = 1.0d; Issue80393_S ret = Issue80393_HFA(s); - if (!s.f1.Equals(ret.f1) || !s.f3.Equals(ret.f3) || !s.f2.value.Equals(ret.f2.value)) { - Console.WriteLine($"Issue80393_HFA failed. Retval: f1={ret.f1} f2={ret.f2} f3={ret.f3}"); + if (!ret.f1.Equals(2.0d) || !ret.f3.Equals(4.0d)) { + Console.WriteLine($"Issue80393_HFA failed. Retval: f1={ret.f1} f3={ret.f3}"); ok = false; } From 37818707c8e9030e08fe821d105135a20b922e61 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Wed, 14 Aug 2024 14:11:17 -0700 Subject: [PATCH 4/9] Clarifying comments --- src/tests/JIT/Directed/StructABI/StructABI.c | 8 ++++++++ src/tests/JIT/Directed/StructABI/StructABI.cs | 1 + 2 files changed, 9 insertions(+) diff --git a/src/tests/JIT/Directed/StructABI/StructABI.c b/src/tests/JIT/Directed/StructABI/StructABI.c index 4a04c2229eda23..2d9ed1ef8f5d21 100644 --- a/src/tests/JIT/Directed/StructABI/StructABI.c +++ b/src/tests/JIT/Directed/StructABI/StructABI.c @@ -234,6 +234,8 @@ struct Issue80393_S_Doubles }; struct Issue80393_F2_Offset { + // 3 padding bytes to approximate C# FieldOffset of 3. + // This padding prevents the outer union from being treated as an HVA/HFA by clang for either arm32 or arm64. char padding[3]; struct Issue80393_F2 F2; }; @@ -244,9 +246,15 @@ union Issue80393_S { }; #pragma pack(pop) +// NOTE: If investigating this in isolation, make sure you set -mfloat-abi=hard -mfpu=neon when building for arm32 DLLEXPORT union Issue80393_S Issue80393_HFA(union Issue80393_S value) { + // Simply doing 'return value' like most of these other functions isn't enough to exercise everything, because + // depending on the calling convention it can turn the whole function into a no-op, where 'value' flows in + // via the same registers that the result flows out through. union Issue80393_S result; + // Use the value argument as part of the result so we can tell whether it was passed in correctly, in addition + // to checking whether the return value was passed correctly back to C#. result.f1_f3.f1 = 1.0 + value.f1_f3.f1; result.f1_f3.f3 = 3.0 + value.f1_f3.f3; return result; diff --git a/src/tests/JIT/Directed/StructABI/StructABI.cs b/src/tests/JIT/Directed/StructABI/StructABI.cs index e0ca455d7b36f6..1f15db541ed664 100644 --- a/src/tests/JIT/Directed/StructABI/StructABI.cs +++ b/src/tests/JIT/Directed/StructABI/StructABI.cs @@ -677,6 +677,7 @@ struct Issue80393_F2 struct Issue80393_S { [FieldOffset(0)] public double f1; + // This FieldOffset of 3 causes this struct to no longer be a HVA or HFA according to ARM32 or ARM64 ABI [FieldOffset(3)] public Issue80393_F2 f2; [FieldOffset(8)] public double f3; } From 5c39f85e93ff26a559e141331882595e55322967 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Wed, 14 Aug 2024 17:55:15 -0700 Subject: [PATCH 5/9] Possible fix --- src/coreclr/vm/class.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/vm/class.cpp b/src/coreclr/vm/class.cpp index 6b02bca81fa14d..71a03c6cf3d767 100644 --- a/src/coreclr/vm/class.cpp +++ b/src/coreclr/vm/class.cpp @@ -1878,11 +1878,11 @@ EEClass::CheckForHFA() requiredAlignment = 16; break; default: - requiredAlignment = 0; - break; + // VT without a valid HFA type inside of this struct means this struct is not an HFA + return false; } - if (requiredAlignment && (pFD->GetOffset() % requiredAlignment != 0)) + if (requiredAlignment && (pFD->GetOffset() % requiredAlignment != 0)) // HFAs don't have unaligned fields. { return false; } From 496b12ac788785540c1bc8c554ddc1b0fe3e18a0 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Tue, 20 Aug 2024 17:38:08 -0700 Subject: [PATCH 6/9] Add rough documentation on how to run arm32 tests --- .../testing/coreclr/running-arm32-tests.md | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 docs/workflow/testing/coreclr/running-arm32-tests.md diff --git a/docs/workflow/testing/coreclr/running-arm32-tests.md b/docs/workflow/testing/coreclr/running-arm32-tests.md new file mode 100644 index 00000000000000..5c833bd3d9a223 --- /dev/null +++ b/docs/workflow/testing/coreclr/running-arm32-tests.md @@ -0,0 +1,88 @@ +# Running ARM32 tests on modern hardware + +One of our supported targets is 32-bit ARM. It can be quite challenging to construct a realistic ARM32 environment where you can build or run tests in a reasonable amount of time. Thankfully, it's possible to take some shortcuts, and do your build in a different environment, which reduces your need to 'somewhere I can run ARM32 binaries'. This is a rough explanation of one way to do that - building your runtime+tests in a cross-build container, and then running them in an ARM64 linux environment. + +## Setting up a cross-build environment targeting ARM32 + +https://github.com/dotnet/runtime/blob/main/docs/workflow/building/coreclr/cross-building.md#linux-cross-building provides detailed instructions on how to cross-build, but if you try to cross-build for ARM32 in particular, you're likely to encounter issues due to latent problems in our build scripts and dependencies. As such, here's a set of steps that worked for me: + +1. Pull the `mcr.microsoft.com/dotnet-buildtools/prereqs:azurelinux-3.0-cross-arm-net9.0` docker image onto an x64 host. +2. Spawn a persistent container from the image. regular `docker run` won't work right, so you want to spawn a shell in the container and leave it running, i.e. `docker run -dit mcr.microsoft.com/dotnet-buildtools/prereqs:azurelinux-3.0-cross-arm-net9.0 bash` +3. Attach to the now-running container. You can find it with `docker container list` and then `docker attach `. You are now in a cross-build ready environment that has an ARM32 cross-build root at `/crossrootfs/arm`. +4. `git clone` the runtime and check out the branch you wish to build. +5. Perform a cross build of the runtime with the right flags set to be able to also cross-build tests. The details are important here; if you miss one of these switches the test build will probably go looking for x64 linux binaries or things like that. + +```bash +$ export ROOTFS_DIR=/crossrootfs/arm + +$ ./build.sh clr+libs+host+packs -rc Release -lc Release -c Debug -arch arm -os linux -cross +Build succeeded. + 0 Warning(s) + 0 Error(s) + +Time Elapsed 00:07:59.69 + +$ ./src/tests/build.sh release arm -cross /p:LibrariesConfiguration=Release +Test build successful. +Test binaries are available at /home/runtime/artifacts/tests/coreclr/linux.arm.Release +To run all the tests use: + src/tests/run.sh Release +To run a single test use: + bash /home/runtime/artifacts/tests/coreclr/linux.arm.Release/__TEST_PATH__/__TEST_NAME__.sh -coreroot= +``` + +Make sure to match the `LibrariesConfiguration` for the tests build to the `-lc` configuration from the runtime build. + +Now that you've successfully built the runtime and tests, you're going to want to grab the entire artifacts directory, and migrate it from this build environment to your execution environment. First, archive the whole artifacts directory from inside the container, in the existing shell you opened: + +```bash +$ tar -acf /home/artifacts.tar.gz ./artifacts +``` + +This will take a while, the artifacts folder is big. Now outside of the container, grab the artifacts tarball from its filesystem using `docker cp`, and migrate it to the ARM64 environment where you will be running tests - i.e.: +```bash +$ docker cp :/home/artifacts.tar.gz ./artifacts.tar.gz + +$ scp -i ~/.ssh/.pem ./artifacts.tar.gz @:/home//artifacts.tar.gz +``` + +This will take a while, the tarball is probably a few gigabytes. Once it's in the right place, you can migrate over to the ARM64 environment and get set up, i.e.: + +```bash +$ git clone https://github.com/dotnet/runtime.git + +$ cd runtime + +$ tar -xzvf ~/artifacts.tar.gz +``` + +This will also take a while since cloning the runtime repo isn't fast and neither is unpacking a 4GB tarball, but once it's done, you're almost ready to run tests! + +## Configuring your ARM64 environment to run ARM32 binaries + +By default your ARM64 Linux install probably doesn't have support for ARM32 binaries enabled, which will cause running the binaries to fail with a cryptic error. So you'll need to add the architecture to dpkg and install some core userspace libraries that CoreCLR will need to actually run your tests, i.e.: + +```bash +$ sudo dpkg --add-architecture armhf + +$ sudo apt-get update +Reading package lists... Done + +$ sudo apt-get install libc6:armhf libstdc++6:armhf libicu74:armhf +The following additional packages will be installed: +``` + +Note that when installing a package for another architecture, you need to suffix the package name with the architecture name. For me, the three packages above were sufficient to run an ARM32 JIT test. + +## Running an ARM32 test in your ARM64 environment + +We're finally ready to go, probably. Export an environment variable to point to your ARM32 core root, and then run your test, i.e.: + +```bash +$ export CORE_ROOT=/home/kg/runtime/artifacts/tests/coreclr/linux.arm.Release/Tests/Core_Root/ + +$ bash artifacts/tests/coreclr/linux.arm.Release/JIT/Directed/StructABI/StructABI/StructABI.sh +BEGIN EXECUTION +/home/kg/runtime/artifacts/tests/coreclr/linux.arm.Release/Tests/Core_Root//corerun -p System.Reflection.Metadata.MetadataUpdater.IsSupported=false -p System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization=true StructABI.dll '' +Issue80393_HFA failed. Retval: f1=1 f3=3 +``` From 12d42c5cc8c45fc848b68a2019555b1be984cd1d Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Wed, 11 Sep 2024 13:02:50 -0700 Subject: [PATCH 7/9] Fix C side of HFA test --- src/tests/JIT/Directed/StructABI/StructABI.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/tests/JIT/Directed/StructABI/StructABI.c b/src/tests/JIT/Directed/StructABI/StructABI.c index 2d9ed1ef8f5d21..77d8d1ee8da156 100644 --- a/src/tests/JIT/Directed/StructABI/StructABI.c +++ b/src/tests/JIT/Directed/StructABI/StructABI.c @@ -221,30 +221,34 @@ struct Nested9 struct InlineArray4 Field2; }; -#pragma pack(push, 1) -struct Issue80393_F2 -{ - double value; -}; - struct Issue80393_S_Doubles { double f1; double f3; }; +// We need to apply 1-byte packing to these structs to get the exact alignment we want, but we +// don't want to apply packing to the union or 2-doubles struct because it will change the natural +// alignment of the union and as a result alter which registers it's assigned to by clang, which +// won't match what CoreCLR does. +#pragma pack(push, 1) +struct Issue80393_F2 +{ + double value; +}; + struct Issue80393_F2_Offset { // 3 padding bytes to approximate C# FieldOffset of 3. // This padding prevents the outer union from being treated as an HVA/HFA by clang for either arm32 or arm64. char padding[3]; struct Issue80393_F2 F2; }; +#pragma pack(pop) union Issue80393_S { struct Issue80393_S_Doubles f1_f3; struct Issue80393_F2_Offset f2; }; -#pragma pack(pop) // NOTE: If investigating this in isolation, make sure you set -mfloat-abi=hard -mfpu=neon when building for arm32 DLLEXPORT union Issue80393_S Issue80393_HFA(union Issue80393_S value) From aeb43d4a16e62d0ec4140bbd35735a32620dde46 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Thu, 24 Oct 2024 11:15:02 -0700 Subject: [PATCH 8/9] Update docs page --- .../testing/coreclr/running-arm32-tests.md | 62 ++----------------- 1 file changed, 5 insertions(+), 57 deletions(-) diff --git a/docs/workflow/testing/coreclr/running-arm32-tests.md b/docs/workflow/testing/coreclr/running-arm32-tests.md index 5c833bd3d9a223..437c8825dc89b2 100644 --- a/docs/workflow/testing/coreclr/running-arm32-tests.md +++ b/docs/workflow/testing/coreclr/running-arm32-tests.md @@ -1,62 +1,6 @@ # Running ARM32 tests on modern hardware -One of our supported targets is 32-bit ARM. It can be quite challenging to construct a realistic ARM32 environment where you can build or run tests in a reasonable amount of time. Thankfully, it's possible to take some shortcuts, and do your build in a different environment, which reduces your need to 'somewhere I can run ARM32 binaries'. This is a rough explanation of one way to do that - building your runtime+tests in a cross-build container, and then running them in an ARM64 linux environment. - -## Setting up a cross-build environment targeting ARM32 - -https://github.com/dotnet/runtime/blob/main/docs/workflow/building/coreclr/cross-building.md#linux-cross-building provides detailed instructions on how to cross-build, but if you try to cross-build for ARM32 in particular, you're likely to encounter issues due to latent problems in our build scripts and dependencies. As such, here's a set of steps that worked for me: - -1. Pull the `mcr.microsoft.com/dotnet-buildtools/prereqs:azurelinux-3.0-cross-arm-net9.0` docker image onto an x64 host. -2. Spawn a persistent container from the image. regular `docker run` won't work right, so you want to spawn a shell in the container and leave it running, i.e. `docker run -dit mcr.microsoft.com/dotnet-buildtools/prereqs:azurelinux-3.0-cross-arm-net9.0 bash` -3. Attach to the now-running container. You can find it with `docker container list` and then `docker attach `. You are now in a cross-build ready environment that has an ARM32 cross-build root at `/crossrootfs/arm`. -4. `git clone` the runtime and check out the branch you wish to build. -5. Perform a cross build of the runtime with the right flags set to be able to also cross-build tests. The details are important here; if you miss one of these switches the test build will probably go looking for x64 linux binaries or things like that. - -```bash -$ export ROOTFS_DIR=/crossrootfs/arm - -$ ./build.sh clr+libs+host+packs -rc Release -lc Release -c Debug -arch arm -os linux -cross -Build succeeded. - 0 Warning(s) - 0 Error(s) - -Time Elapsed 00:07:59.69 - -$ ./src/tests/build.sh release arm -cross /p:LibrariesConfiguration=Release -Test build successful. -Test binaries are available at /home/runtime/artifacts/tests/coreclr/linux.arm.Release -To run all the tests use: - src/tests/run.sh Release -To run a single test use: - bash /home/runtime/artifacts/tests/coreclr/linux.arm.Release/__TEST_PATH__/__TEST_NAME__.sh -coreroot= -``` - -Make sure to match the `LibrariesConfiguration` for the tests build to the `-lc` configuration from the runtime build. - -Now that you've successfully built the runtime and tests, you're going to want to grab the entire artifacts directory, and migrate it from this build environment to your execution environment. First, archive the whole artifacts directory from inside the container, in the existing shell you opened: - -```bash -$ tar -acf /home/artifacts.tar.gz ./artifacts -``` - -This will take a while, the artifacts folder is big. Now outside of the container, grab the artifacts tarball from its filesystem using `docker cp`, and migrate it to the ARM64 environment where you will be running tests - i.e.: -```bash -$ docker cp :/home/artifacts.tar.gz ./artifacts.tar.gz - -$ scp -i ~/.ssh/.pem ./artifacts.tar.gz @:/home//artifacts.tar.gz -``` - -This will take a while, the tarball is probably a few gigabytes. Once it's in the right place, you can migrate over to the ARM64 environment and get set up, i.e.: - -```bash -$ git clone https://github.com/dotnet/runtime.git - -$ cd runtime - -$ tar -xzvf ~/artifacts.tar.gz -``` - -This will also take a while since cloning the runtime repo isn't fast and neither is unpacking a 4GB tarball, but once it's done, you're almost ready to run tests! +One of our supported targets is 32-bit ARM. It can be quite challenging to construct a realistic ARM32 environment where you can build or run tests in a reasonable amount of time. Thankfully, it's possible to configure an ARM64 linux environment so that you can cross-build from ARM64 to ARM32, and run tests there using native hardware support instead of software emulation. This is not possible on ARM64-based Windows (this functionality is not offered by the OS). ## Configuring your ARM64 environment to run ARM32 binaries @@ -74,6 +18,10 @@ The following additional packages will be installed: Note that when installing a package for another architecture, you need to suffix the package name with the architecture name. For me, the three packages above were sufficient to run an ARM32 JIT test. +## Cross-building for ARM32 on ARM64 + +Follow the steps from https://github.com/dotnet/runtime/blob/main/docs/workflow/building/coreclr/cross-building.md#linux-cross-building as-is. You should end up with a `linux.arm` `Core_Root` and test artifacts. + ## Running an ARM32 test in your ARM64 environment We're finally ready to go, probably. Export an environment variable to point to your ARM32 core root, and then run your test, i.e.: From fca188f23ca9549aaa054b89dc67060da14a5f18 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Mon, 18 Aug 2025 16:21:26 -0700 Subject: [PATCH 9/9] Address PR feedback --- docs/workflow/testing/coreclr/running-arm32-tests.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/workflow/testing/coreclr/running-arm32-tests.md b/docs/workflow/testing/coreclr/running-arm32-tests.md index 437c8825dc89b2..c7bac99960c921 100644 --- a/docs/workflow/testing/coreclr/running-arm32-tests.md +++ b/docs/workflow/testing/coreclr/running-arm32-tests.md @@ -4,7 +4,7 @@ One of our supported targets is 32-bit ARM. It can be quite challenging to const ## Configuring your ARM64 environment to run ARM32 binaries -By default your ARM64 Linux install probably doesn't have support for ARM32 binaries enabled, which will cause running the binaries to fail with a cryptic error. So you'll need to add the architecture to dpkg and install some core userspace libraries that CoreCLR will need to actually run your tests, i.e.: +By default your ARM64 Linux install probably doesn't have support for ARM32 binaries enabled, which will cause running the binaries to fail with a cryptic error. So you'll need to add the architecture to dpkg and install some core userspace libraries that CoreCLR will need to actually run your tests, which on Debian or Ubuntu will look like: ```bash $ sudo dpkg --add-architecture armhf