Skip to content

Commit d31ff20

Browse files
authored
Manual .NET 7 backport of #80218 fixing #79327 (#80881)
Description: This change puts HFA calculation in Crossgen2 in sync with native CoreCLR runtime for value types with explicit layout. Previously Crossgen2 had a shortcut in the routine deciding that structs with explicit layouts are never marked as HFA that disagreed with the CoreCLR runtime; consequently, on arm64, Crossgen2 disagreed with the runtime on whether or not a function returning such a type should allocate the stack slot for return value, basically messing up the calling convention and GC refmap, resulting in various random AVs and corruptions. This was first observed by an internal customer in WPF apps where MilRectD is the type in question, later JanK filed the issue 79327 for the same problem. Customer impact: Random runtime crashes on arm64. Regression: Nope, I believe the incomplete implementation was the original one, this change just "improves it" by putting it in better sync with the native runtime. I have also added a code comment mentioning that these two need to be kept in sync. Risk: Low - the error in the previous implementation is obvious, R2RDump and my new runtime diagnostics clearly show the GC refmap mismatch caused by this problem and its fixing after applying the Crossgen2 fix. Link to issue: Link to PR against main: Publishing impact: In the particular case of the WPF app the problem was in the PresentationCore.dll assembly. The assembly (or rather the entire WPF) need to be recompiled with Crossgen2 with the fix applied for this to take effect. For now I assume that is an automated part of the servicing process. Thanks Tomas
1 parent 2cf9538 commit d31ff20

File tree

1 file changed

+39
-20
lines changed

1 file changed

+39
-20
lines changed

src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -945,7 +945,15 @@ public override ValueTypeShapeCharacteristics ComputeValueTypeShapeCharacteristi
945945
return ComputeHomogeneousAggregateCharacteristic(type);
946946
}
947947

948-
private ValueTypeShapeCharacteristics ComputeHomogeneousAggregateCharacteristic(DefType type)
948+
/// <summary>
949+
/// Identify whether a given type is a homogeneous floating-point aggregate. This code must be
950+
/// kept in sync with the CoreCLR runtime method EEClass::CheckForHFA, as of this change it
951+
/// can be found at
952+
/// https://github.com/dotnet/runtime/blob/1928cd2b65c04ebe6fe528d4ebb581e46f1fed47/src/coreclr/vm/class.cpp#L1567
953+
/// </summary>
954+
/// <param name="type">Type to analyze</param>
955+
/// <returns>HFA classification of the type parameter</returns>
956+
private static ValueTypeShapeCharacteristics ComputeHomogeneousAggregateCharacteristic(DefType type)
949957
{
950958
// Use this constant to make the code below more laconic
951959
const ValueTypeShapeCharacteristics NotHA = ValueTypeShapeCharacteristics.None;
@@ -960,12 +968,7 @@ private ValueTypeShapeCharacteristics ComputeHomogeneousAggregateCharacteristic(
960968
return NotHA;
961969

962970
MetadataType metadataType = (MetadataType)type;
963-
964-
// No HAs with explicit layout. There may be cases where explicit layout may be still
965-
// eligible for HA, but it is hard to tell the real intent. Make it simple and just
966-
// unconditionally disable HAs for explicit layout.
967-
if (metadataType.IsExplicitLayout)
968-
return NotHA;
971+
int haElementSize = 0;
969972

970973
switch (metadataType.Category)
971974
{
@@ -978,12 +981,18 @@ private ValueTypeShapeCharacteristics ComputeHomogeneousAggregateCharacteristic(
978981
case TypeFlags.ValueType:
979982
// Find the common HA element type if any
980983
ValueTypeShapeCharacteristics haResultType = NotHA;
984+
bool hasZeroOffsetField = false;
981985

982986
foreach (FieldDesc field in metadataType.GetFields())
983987
{
984988
if (field.IsStatic)
985989
continue;
986990

991+
if (field.Offset == LayoutInt.Zero)
992+
{
993+
hasZeroOffsetField = true;
994+
}
995+
987996
// If a field isn't a DefType, then this type cannot be a HA type
988997
if (!(field.FieldType is DefType fieldType))
989998
return NotHA;
@@ -997,6 +1006,15 @@ private ValueTypeShapeCharacteristics ComputeHomogeneousAggregateCharacteristic(
9971006
{
9981007
// If we hadn't yet figured out what form of HA this type might be, we've now found one case
9991008
haResultType = haFieldType;
1009+
1010+
haElementSize = haResultType switch
1011+
{
1012+
ValueTypeShapeCharacteristics.Float32Aggregate => 4,
1013+
ValueTypeShapeCharacteristics.Float64Aggregate => 8,
1014+
ValueTypeShapeCharacteristics.Vector64Aggregate => 8,
1015+
ValueTypeShapeCharacteristics.Vector128Aggregate => 16,
1016+
_ => throw new ArgumentOutOfRangeException()
1017+
};
10001018
}
10011019
else if (haResultType != haFieldType)
10021020
{
@@ -1005,21 +1023,17 @@ private ValueTypeShapeCharacteristics ComputeHomogeneousAggregateCharacteristic(
10051023
// be a HA type.
10061024
return NotHA;
10071025
}
1026+
1027+
if (field.Offset.IsIndeterminate || field.Offset.AsInt % haElementSize != 0)
1028+
{
1029+
return NotHA;
1030+
}
10081031
}
10091032

1010-
// If there are no instance fields, this is not a HA type
1011-
if (haResultType == NotHA)
1033+
// If the struct doesn't have a zero-offset field, it's not an HFA.
1034+
if (!hasZeroOffsetField)
10121035
return NotHA;
10131036

1014-
int haElementSize = haResultType switch
1015-
{
1016-
ValueTypeShapeCharacteristics.Float32Aggregate => 4,
1017-
ValueTypeShapeCharacteristics.Float64Aggregate => 8,
1018-
ValueTypeShapeCharacteristics.Vector64Aggregate => 8,
1019-
ValueTypeShapeCharacteristics.Vector128Aggregate => 16,
1020-
_ => throw new ArgumentOutOfRangeException()
1021-
};
1022-
10231037
// Types which are indeterminate in field size are not considered to be HA
10241038
if (type.InstanceFieldSize.IsIndeterminate)
10251039
return NotHA;
@@ -1028,8 +1042,13 @@ private ValueTypeShapeCharacteristics ComputeHomogeneousAggregateCharacteristic(
10281042
// - Type of fields can be HA valuetype itself.
10291043
// - Managed C++ HA valuetypes have just one <alignment member> of type float to signal that
10301044
// the valuetype is HA and explicitly specified size.
1031-
int maxSize = haElementSize * type.Context.Target.MaxHomogeneousAggregateElementCount;
1032-
if (type.InstanceFieldSize.AsInt > maxSize)
1045+
int totalSize = type.InstanceFieldSize.AsInt;
1046+
1047+
if (totalSize % haElementSize != 0)
1048+
return NotHA;
1049+
1050+
// On ARM, HFAs can have a maximum of four fields regardless of whether those are float or double.
1051+
if (totalSize > haElementSize * type.Context.Target.MaxHomogeneousAggregateElementCount)
10331052
return NotHA;
10341053

10351054
// All the tests passed. This is a HA type.

0 commit comments

Comments
 (0)