From 8db3511dd6216c204e2073968cc6fa6defe81781 Mon Sep 17 00:00:00 2001 From: Tomas Date: Sun, 4 Apr 2021 17:43:26 +0200 Subject: [PATCH] More fixes regarding FieldBaseOffset vs. the actual field base offset --- .../Common/MetadataFieldLayoutAlgorithm.cs | 12 +++++++----- .../Compiler/ReadyToRunCompilerContext.cs | 7 ++++++- .../ReadyToRunMetadataFieldLayoutAlgorithm.cs | 13 ++++--------- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 0a0a33464757c..9f7e5d51cc2e0 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -393,7 +393,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType // For types inheriting from another type, field offsets continue on from where they left off // For reference types, we calculate field alignment as if the address after the method table pointer // has offset 0 (on 32-bit platforms, this location is guaranteed to be 8-aligned). - LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8: false) - offsetBias; + LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8: false, requiresAlignedBase: false) - offsetBias; var layoutMetadata = type.GetClassLayout(); @@ -434,7 +434,7 @@ protected ComputedInstanceFieldLayout ComputeSequentialFieldLayout(MetadataType return computedLayout; } - protected virtual void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8) + protected virtual void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8, bool requiresAlignedBase) { } @@ -541,7 +541,9 @@ protected ComputedInstanceFieldLayout ComputeAutoFieldLayout(MetadataType type, LayoutInt offsetBias = OffsetBias(type); // For types inheriting from another type, field offsets continue on from where they left off - LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8); + // Base alignment is not always required, it's only applied when there's a version bubble boundary + // between base type and the current type. + LayoutInt cumulativeInstanceFieldPos = CalculateFieldBaseOffset(type, requiresAlign8, requiresAlignedBase: false); if (!cumulativeInstanceFieldPos.IsIndeterminate) { cumulativeInstanceFieldPos -= offsetBias; @@ -758,7 +760,7 @@ private static bool IsByValueClass(TypeDesc type) return type.IsValueType && !type.IsPrimitive && !type.IsEnum; } - public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8) + public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8, bool requiresAlignedBase) { LayoutInt cumulativeInstanceFieldPos = LayoutInt.Zero; @@ -773,7 +775,7 @@ public LayoutInt CalculateFieldBaseOffset(MetadataType type, bool requiresAlign8 { cumulativeInstanceFieldPos += LayoutInt.One; } - AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos, requiresAlign8); + AlignBaseOffsetIfNecessary(type, ref cumulativeInstanceFieldPos, requiresAlign8, requiresAlignedBase); cumulativeInstanceFieldPos += offsetBias; } } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs index 8d0a1fff5c29b..4c8f9770332ca 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilerContext.cs @@ -68,7 +68,12 @@ public override FieldLayoutAlgorithm GetLayoutAlgorithmForType(DefType type) } } - public LayoutInt CalculateFieldBaseOffset(MetadataType type) => _r2rFieldLayoutAlgorithm.CalculateFieldBaseOffset(type, type.RequiresAlign8()); + /// + /// This is a rough equivalent of the CoreCLR runtime method ReadyToRunInfo::GetFieldBaseOffset. + /// In contrast to the auto field layout algorithm, this method unconditionally applies alignment + /// between base and derived class (even when they reside in the same version bubble). + /// + public LayoutInt CalculateFieldBaseOffset(MetadataType type) => _r2rFieldLayoutAlgorithm.CalculateFieldBaseOffset(type, type.RequiresAlign8(), requiresAlignedBase: true); public void SetCompilationGroup(ReadyToRunCompilationModuleGroupBase compilationModuleGroup) { diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs index c723c0f8525ff..4f754f895e35c 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs @@ -817,18 +817,13 @@ protected override ComputedInstanceFieldLayout ComputeInstanceFieldLayout(Metada /// This method decides whether the type needs aligned base offset in order to have layout resilient to /// base class layout changes. /// - protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8) + protected override void AlignBaseOffsetIfNecessary(MetadataType type, ref LayoutInt baseOffset, bool requiresAlign8, bool requiresAlignedBase) { - DefType baseType = type.BaseType; - - if (!_compilationGroup.NeedsAlignmentBetweenBaseTypeAndDerived(baseType: (MetadataType)baseType, derivedType: type)) + if (requiresAlignedBase || _compilationGroup.NeedsAlignmentBetweenBaseTypeAndDerived(baseType: (MetadataType)type.BaseType, derivedType: type)) { - // The type is defined in the module that's currently being compiled and the type layout doesn't depend on other modules - return; + LayoutInt alignment = new LayoutInt(requiresAlign8 ? 8 : type.Context.Target.PointerSize); + baseOffset = LayoutInt.AlignUp(baseOffset, alignment, type.Context.Target); } - - LayoutInt alignment = new LayoutInt(requiresAlign8 ? 8 : type.Context.Target.PointerSize); - baseOffset = LayoutInt.AlignUp(baseOffset, alignment, type.Context.Target); } public static bool IsManagedSequentialType(TypeDesc type)