From 1572ba14fb7cef56e30792bc6f6732590e6bba78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Rylek?= Date: Thu, 27 Sep 2018 23:46:02 +0200 Subject: [PATCH] Static placement fixes (#6363) Static placement fixes Our existing CPAOT implementation of statics placement is incorrect as there's a difference between CoreCLR and CoreRT in statics allocation algorithm - basically CoreRT allocates statics per type whereas CoreCLR does that per module. This change modifies the field layout algorithm to use a loose managed rewrite of the CoreCLR logic. I have also included Michal's addition to the ReadyToRunUnit test demonstrating this behavior. Thanks Tomas --- .../ReadyToRun/FixupConstants.cs | 2 +- .../ReadyToRunCodegenCompilationBuilder.cs | 2 - .../src/Compiler/ReadyToRunCompilerContext.cs | 18 +- .../ReadyToRunMetadataFieldLayoutAlgorithm.cs | 362 ++++++++++++++++-- tests/src/Simple/ReadyToRunUnit/Program.cs | 37 +- .../Simple/ReadyToRunUnit/methodWhiteList.txt | 1 + 6 files changed, 387 insertions(+), 35 deletions(-) diff --git a/src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRun/FixupConstants.cs b/src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRun/FixupConstants.cs index da3ff6f61..d6044a8b7 100644 --- a/src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRun/FixupConstants.cs +++ b/src/ILCompiler.ReadyToRun/src/Compiler/DependencyAnalysis/ReadyToRun/FixupConstants.cs @@ -253,7 +253,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun public enum CorElementType : byte { - Invalid = 0, + ELEMENT_TYPE_END = 0, ELEMENT_TYPE_VOID = 1, ELEMENT_TYPE_BOOLEAN = 2, ELEMENT_TYPE_CHAR = 3, diff --git a/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunCodegenCompilationBuilder.cs b/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunCodegenCompilationBuilder.cs index 094b1eddb..cb33d285a 100644 --- a/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunCodegenCompilationBuilder.cs +++ b/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunCodegenCompilationBuilder.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; -using System.Reflection.Metadata.Ecma335; using ILCompiler.DependencyAnalysis; using ILCompiler.DependencyAnalysis.ReadyToRun; @@ -34,7 +33,6 @@ namespace ILCompiler _devirtualizationManager = new DependencyAnalysis.ReadyToRun.DevirtualizationManager(group); _inputModule = context.GetModuleFromPath(_inputFilePath); - ((ReadyToRunCompilerContext)context).InitializeAlgorithm(_inputModule.MetadataReader.GetTableRowCount(TableIndex.TypeDef)); } public override CompilationBuilder UseBackendOptions(IEnumerable options) diff --git a/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunCompilerContext.cs b/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunCompilerContext.cs index cde62e110..53bed43fb 100644 --- a/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunCompilerContext.cs +++ b/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunCompilerContext.cs @@ -16,12 +16,7 @@ namespace ILCompiler public ReadyToRunCompilerContext(TargetDetails details, SharedGenericsMode genericsMode) : base(details, genericsMode) { - } - - public void InitializeAlgorithm(int numberOfTypesInModule) - { - Debug.Assert(_r2rFieldLayoutAlgorithm == null); - _r2rFieldLayoutAlgorithm = new ReadyToRunMetadataFieldLayoutAlgorithm(Target, numberOfTypesInModule); + _r2rFieldLayoutAlgorithm = new ReadyToRunMetadataFieldLayoutAlgorithm(); } public override FieldLayoutAlgorithm GetLayoutAlgorithmForType(DefType type) @@ -40,5 +35,16 @@ namespace ILCompiler return _r2rFieldLayoutAlgorithm; } } + + protected override bool ComputeHasGCStaticBase(FieldDesc field) + { + Debug.Assert(field.IsStatic); + + TypeDesc fieldType = field.FieldType; + if (fieldType.IsValueType) + return ((DefType)fieldType).ContainsGCPointers; + else + return fieldType.IsGCPointer; + } } } diff --git a/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs b/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs index 0fe7795d6..e14caf28b 100644 --- a/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs +++ b/src/ILCompiler.ReadyToRun/src/Compiler/ReadyToRunMetadataFieldLayoutAlgorithm.cs @@ -3,45 +3,363 @@ // See the LICENSE file in the project root for more information. using System; +using System.Diagnostics; using System.Collections.Generic; +using System.Collections.Immutable; +using System.Reflection; +using System.Reflection.Metadata; +using System.Reflection.Metadata.Ecma335; + +using ILCompiler.DependencyAnalysis.ReadyToRun; using Internal.TypeSystem; +using Internal.TypeSystem.Ecma; namespace ILCompiler { internal class ReadyToRunMetadataFieldLayoutAlgorithm : MetadataFieldLayoutAlgorithm { /// - /// CoreCLR DomainLocalModule::OffsetOfDataBlob() / sizeof(void *) + /// Map from EcmaModule instances to field layouts within the individual modules. /// - private const int DomainLocalModuleDataBlobOffsetAsIntPtrCount = 6; + private ModuleFieldLayoutMap _moduleFieldLayoutMap; + + public ReadyToRunMetadataFieldLayoutAlgorithm() + { + _moduleFieldLayoutMap = new ModuleFieldLayoutMap(); + } + + public override ComputedStaticFieldLayout ComputeStaticFieldLayout(DefType defType, StaticLayoutKind layoutKind) + { + ComputedStaticFieldLayout layout = new ComputedStaticFieldLayout(); + if (defType is EcmaType ecmaType) + { + // ECMA types are the only ones that can have statics + ModuleFieldLayout moduleFieldLayout = _moduleFieldLayoutMap.GetOrCreateValue(ecmaType.EcmaModule); + layout.GcStatics = moduleFieldLayout.GcStatics; + layout.NonGcStatics = moduleFieldLayout.NonGcStatics; + layout.ThreadGcStatics = moduleFieldLayout.ThreadGcStatics; + layout.ThreadNonGcStatics = moduleFieldLayout.ThreadNonGcStatics; + moduleFieldLayout.TypeToFieldMap.TryGetValue(ecmaType.Handle, out layout.Offsets); + } + return layout; + } /// - /// CoreCLR ThreadLocalModule::OffsetOfDataBlob() / sizeof(void *) + /// Map from modules to their static field layouts. /// - private const int ThreadLocalModuleDataBlobOffsetAsIntPtrCount = 3; - - private LayoutInt _initialNonGcStaticsOffset; - - public ReadyToRunMetadataFieldLayoutAlgorithm(TargetDetails target, int numberOfTypesInModule) + private class ModuleFieldLayoutMap : LockFreeReaderHashtable { - _initialNonGcStaticsOffset = new LayoutInt(DomainLocalModuleDataBlobOffsetAsIntPtrCount * target.PointerSize + numberOfTypesInModule); - } + /// + /// CoreCLR DomainLocalModule::OffsetOfDataBlob() / sizeof(void *) + /// + private const int DomainLocalModuleDataBlobOffsetAsIntPtrCount = 6; - protected override void PrepareRuntimeSpecificStaticFieldLayout(TypeSystemContext context, ref ComputedStaticFieldLayout layout) - { - layout.NonGcStatics.Size = _initialNonGcStaticsOffset; - layout.GcStatics.Size = LayoutInt.Zero; - layout.ThreadNonGcStatics.Size = LayoutInt.Zero; - layout.ThreadGcStatics.Size = LayoutInt.Zero; - } + /// + /// CoreCLR ThreadLocalModule::OffsetOfDataBlob() / sizeof(void *) + /// + private const int ThreadLocalModuleDataBlobOffsetAsIntPtrCount = 3; - protected override void FinalizeRuntimeSpecificStaticFieldLayout(TypeSystemContext context, ref ComputedStaticFieldLayout layout) - { - if (layout.NonGcStatics.Size == _initialNonGcStaticsOffset) + protected override bool CompareKeyToValue(EcmaModule key, ModuleFieldLayout value) { - // No non-GC statics, set statics size to 0 - layout.NonGcStatics.Size = LayoutInt.Zero; + return key == value.Module; + } + + protected override bool CompareValueToValue(ModuleFieldLayout value1, ModuleFieldLayout value2) + { + return value1.Module == value2.Module; + } + + protected override ModuleFieldLayout CreateValueFromKey(EcmaModule module) + { + int typeCountInModule = module.MetadataReader.GetTableRowCount(TableIndex.TypeDef); + int pointerSize = module.Context.Target.PointerSize; + + // 0 corresponds to "normal" statics, 1 to thread-local statics + LayoutInt[] gcStatics = new LayoutInt[2] + { + LayoutInt.Zero, + LayoutInt.Zero + }; + LayoutInt[] nonGcStatics = new LayoutInt[2] + { + new LayoutInt(DomainLocalModuleDataBlobOffsetAsIntPtrCount * pointerSize + typeCountInModule), + new LayoutInt(ThreadLocalModuleDataBlobOffsetAsIntPtrCount * pointerSize + typeCountInModule), + }; + Dictionary typeToFieldMap = new Dictionary(); + + foreach (TypeDefinitionHandle typeDefHandle in module.MetadataReader.TypeDefinitions) + { + TypeDefinition typeDef = module.MetadataReader.GetTypeDefinition(typeDefHandle); + List fieldsForType = null; + if (typeDef.GetGenericParameters().Count != 0) + { + // Generic types are exempt from the static field layout algorithm, see + // this check. + continue; + } + foreach (FieldDefinitionHandle fieldDefHandle in typeDef.GetFields()) + { + FieldDefinition fieldDef = module.MetadataReader.GetFieldDefinition(fieldDefHandle); + if ((fieldDef.Attributes & (FieldAttributes.Static | FieldAttributes.Literal)) == FieldAttributes.Static) + { + int index = (IsFieldThreadStatic(in fieldDef, module.MetadataReader) ? 1 : 0); + int alignment = 1; + int size = 0; + bool isGcField = false; + + CorElementType corElementType; + EntityHandle valueTypeHandle; + + GetFieldElementTypeAndValueTypeHandle(in fieldDef, module.MetadataReader, out corElementType, out valueTypeHandle); + FieldDesc fieldDesc = module.GetField(fieldDefHandle); + + switch (corElementType) + { + case CorElementType.ELEMENT_TYPE_I1: + case CorElementType.ELEMENT_TYPE_U1: + case CorElementType.ELEMENT_TYPE_BOOLEAN: + size = 1; + break; + + case CorElementType.ELEMENT_TYPE_I2: + case CorElementType.ELEMENT_TYPE_U2: + case CorElementType.ELEMENT_TYPE_CHAR: + alignment = 2; + size = 2; + break; + + case CorElementType.ELEMENT_TYPE_I4: + case CorElementType.ELEMENT_TYPE_U4: + case CorElementType.ELEMENT_TYPE_R4: + alignment = 4; + size = 4; + break; + + case CorElementType.ELEMENT_TYPE_FNPTR: + case CorElementType.ELEMENT_TYPE_PTR: + case CorElementType.ELEMENT_TYPE_I: + case CorElementType.ELEMENT_TYPE_U: + alignment = pointerSize; + size = pointerSize; + break; + + case CorElementType.ELEMENT_TYPE_I8: + case CorElementType.ELEMENT_TYPE_U8: + case CorElementType.ELEMENT_TYPE_R8: + alignment = 8; + size = 8; + break; + + case CorElementType.ELEMENT_TYPE_VAR: + case CorElementType.ELEMENT_TYPE_MVAR: + case CorElementType.ELEMENT_TYPE_STRING: + case CorElementType.ELEMENT_TYPE_SZARRAY: + case CorElementType.ELEMENT_TYPE_ARRAY: + case CorElementType.ELEMENT_TYPE_CLASS: + case CorElementType.ELEMENT_TYPE_OBJECT: + isGcField = true; + alignment = pointerSize; + size = pointerSize; + break; + + case CorElementType.ELEMENT_TYPE_BYREF: + ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, fieldDesc.OwningType); + break; + + // Statics for valuetypes where the valuetype is defined in this module are handled here. + // Other valuetype statics utilize the pessimistic model below. + case CorElementType.ELEMENT_TYPE_VALUETYPE: + isGcField = true; + alignment = pointerSize; + size = pointerSize; + if (IsTypeByRefLike(valueTypeHandle, module.MetadataReader)) + { + ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, fieldDesc.OwningType); + } + break; + + case CorElementType.ELEMENT_TYPE_END: + default: + isGcField = true; + alignment = pointerSize; + size = pointerSize; + if (!valueTypeHandle.IsNil) + { + // Allocate pessimistic non-GC area for cross-module fields as that's what CoreCLR does + // here + nonGcStatics[index] = LayoutInt.AlignUp(nonGcStatics[index], new LayoutInt(TargetDetails.MaximumPrimitiveSize)) + + new LayoutInt(TargetDetails.MaximumPrimitiveSize); + } + else + { + // Field has an unexpected type + throw new InvalidProgramException(); + } + break; + } + + LayoutInt[] layout = (isGcField ? gcStatics : nonGcStatics); + LayoutInt offset = LayoutInt.AlignUp(layout[index], new LayoutInt(alignment)); + layout[index] = offset + new LayoutInt(size); + if (fieldsForType == null) + { + fieldsForType = new List(); + } + fieldsForType.Add(new FieldAndOffset(fieldDesc, offset)); + } + } + + if (fieldsForType != null) + { + typeToFieldMap.Add(typeDefHandle, fieldsForType.ToArray()); + } + } + + LayoutInt blockAlignment = new LayoutInt(TargetDetails.MaximumPrimitiveSize); + + return new ModuleFieldLayout( + module, + gcStatics: new StaticsBlock() { Size = gcStatics[0], LargestAlignment = blockAlignment }, + nonGcStatics: new StaticsBlock() { Size = nonGcStatics[0], LargestAlignment = blockAlignment }, + threadGcStatics: new StaticsBlock() { Size = gcStatics[1], LargestAlignment = blockAlignment }, + threadNonGcStatics: new StaticsBlock() { Size = nonGcStatics[1], LargestAlignment = blockAlignment }, + typeToFieldMap: typeToFieldMap); + } + + protected override int GetKeyHashCode(EcmaModule key) + { + return key.GetHashCode(); + } + + protected override int GetValueHashCode(ModuleFieldLayout value) + { + return value.Module.GetHashCode(); + } + + /// + /// Try to locate the ThreadStatic custom attribute on the field (much like EcmaField.cs does in the method InitializeFieldFlags). + /// + /// Field definition + /// Metadata reader for the module + /// true when the field is marked with the ThreadStatic custom attribute + private static bool IsFieldThreadStatic(in FieldDefinition fieldDef, MetadataReader metadataReader) + { + return !metadataReader.GetCustomAttributeHandle(fieldDef.GetCustomAttributes(), "System", "ThreadStaticAttribute").IsNil; + } + + /// + /// Try to locate the IsByRefLike attribute on the type (much like EcmaType does in ComputeTypeFlags). + /// + /// Handle to the field type to analyze + /// Metadata reader for the active module + /// + private static bool IsTypeByRefLike(EntityHandle typeDefHandle, MetadataReader metadataReader) + { + return typeDefHandle.Kind == HandleKind.TypeDefinition && + !metadataReader.GetCustomAttributeHandle( + metadataReader.GetTypeDefinition((TypeDefinitionHandle)typeDefHandle).GetCustomAttributes(), + "System.Runtime.CompilerServices", + "IsByRefLikeAttribute").IsNil; + } + + /// + /// Partially decode field signature to obtain CorElementType and optionally the type handle for VALUETYPE fields. + /// + /// Metadata field definition + /// Metadata reader for the active module + /// Output element type decoded from the signature + /// Value type handle decoded from the signature + private static void GetFieldElementTypeAndValueTypeHandle( + in FieldDefinition fieldDef, + MetadataReader metadataReader, + out CorElementType corElementType, + out EntityHandle valueTypeHandle) + { + BlobReader signature = metadataReader.GetBlobReader(fieldDef.Signature); + SignatureHeader signatureHeader = signature.ReadSignatureHeader(); + if (signatureHeader.Kind != SignatureKind.Field) + { + throw new InvalidProgramException(); + } + + corElementType = ReadElementType(ref signature); + valueTypeHandle = default(EntityHandle); + if (corElementType == CorElementType.ELEMENT_TYPE_GENERICINST) + { + corElementType = ReadElementType(ref signature); + } + + if (corElementType == CorElementType.ELEMENT_TYPE_VALUETYPE) + { + valueTypeHandle = signature.ReadTypeHandle(); + } + } + + /// + /// Extract element type from a field signature after skipping various modifiers. + /// + /// Signature byte array + /// On input, index into the signature array. Gets modified to point after the element type on return. + /// + private static CorElementType ReadElementType(ref BlobReader signature) + { + // SigParser::PeekElemType + byte signatureByte = signature.ReadByte(); + if (signatureByte < (byte)CorElementType.ELEMENT_TYPE_CMOD_REQD) + { + // Fast path + return (CorElementType)signatureByte; + } + + // SigParser::SkipCustomModifiers -> SkipAnyVASentinel + if (signatureByte == (byte)CorElementType.ELEMENT_TYPE_SENTINEL) + { + signatureByte = signature.ReadByte(); + } + + // SigParser::SkipCustomModifiers - modifier loop + while (signatureByte == (byte)CorElementType.ELEMENT_TYPE_CMOD_REQD || + signatureByte == (byte)CorElementType.ELEMENT_TYPE_CMOD_OPT) + { + signature.ReadCompressedInteger(); + signatureByte = signature.ReadByte(); + } + return (CorElementType)signatureByte; + } + } + + /// + /// Field layouts for a given EcmaModule. + /// + private class ModuleFieldLayout + { + public EcmaModule Module { get; } + + public StaticsBlock GcStatics { get; } + + public StaticsBlock NonGcStatics { get; } + + public StaticsBlock ThreadGcStatics { get; } + + public StaticsBlock ThreadNonGcStatics { get; } + + public IReadOnlyDictionary TypeToFieldMap { get; } + + public ModuleFieldLayout( + EcmaModule module, + StaticsBlock gcStatics, + StaticsBlock nonGcStatics, + StaticsBlock threadGcStatics, + StaticsBlock threadNonGcStatics, + IReadOnlyDictionary typeToFieldMap) + { + Module = module; + GcStatics = gcStatics; + NonGcStatics = nonGcStatics; + ThreadGcStatics = threadGcStatics; + ThreadNonGcStatics = threadNonGcStatics; + TypeToFieldMap = typeToFieldMap; } } } diff --git a/tests/src/Simple/ReadyToRunUnit/Program.cs b/tests/src/Simple/ReadyToRunUnit/Program.cs index 5bc6ffdb1..0f00f4e5c 100644 --- a/tests/src/Simple/ReadyToRunUnit/Program.cs +++ b/tests/src/Simple/ReadyToRunUnit/Program.cs @@ -7,15 +7,25 @@ using System.Collections.Generic; using System.IO; using System.Text; +internal class ClassWithStatic +{ + public const int StaticValue = 0x666; + + [ThreadStatic] + public static int Static = StaticValue; +} + internal class Program { + const int LineCountInitialValue = 0x12345678; + [ThreadStatic] private static string TextFileName = @"C:\Windows\Microsoft.NET\Framework\v4.0.30319\clientexclusionlist.xml"; [ThreadStatic] - private static int LineCount = 0x12345678; + private static int LineCount = LineCountInitialValue; - private static List _passedTests; + private static volatile List _passedTests; private static List _failedTests; @@ -61,6 +71,24 @@ internal class Program } } + private unsafe static bool CheckNonGCThreadLocalStatic() + { + fixed (int *lineCountPtr = &LineCount) + { + Console.WriteLine($@"LineCount: 0x{LineCount:X8}, @ = 0x{(ulong)lineCountPtr:X8}"); + } + fixed (int *staticPtr = &ClassWithStatic.Static) + { + Console.WriteLine($@"ClassWithStatic.Static: 0x{ClassWithStatic.Static:X8}, @ = 0x{(ulong)staticPtr:X8}"); + } + fixed (int *lineCountPtr = &LineCount) + { + Console.WriteLine($@"LineCount: 0x{LineCount:X8}, @ = 0x{(ulong)lineCountPtr:X8}"); + } + return LineCount == LineCountInitialValue && + ClassWithStatic.Static == ClassWithStatic.StaticValue; + } + private static bool ChkCast() { object obj = TextFileName; @@ -431,6 +459,7 @@ internal class Program RunTest("WriteLine", WriteLine()); RunTest("IsInstanceOf", IsInstanceOf()); RunTest("IsInstanceOfValueType", IsInstanceOfValueType()); + RunTest("CheckNonGCThreadLocalStatic", CheckNonGCThreadLocalStatic()); RunTest("ChkCast", ChkCast()); RunTest("ChkCastValueType", ChkCastValueType()); RunTest("BoxUnbox", BoxUnbox()); @@ -452,8 +481,8 @@ internal class Program RunTest("DisposeEnumeratorTest", DisposeEnumeratorTest()); RunTest("EmptyArrayOfInt", EmptyArrayOfInt()); RunTest("EnumerateEmptyArrayOfInt", EnumerateEmptyArrayOfInt()); - // TODO: RunTest("EmptyArrayOfString", EmptyArrayOfString()); - // TODO: RunTest("EnumerateEmptyArrayOfString", EnumerateEmptyArrayOfString()); + RunTest("EmptyArrayOfString", EmptyArrayOfString()); + RunTest("EnumerateEmptyArrayOfString", EnumerateEmptyArrayOfString()); RunTest("TryCatch", TryCatch()); RunTest("FileStreamNullRefTryCatch", FileStreamNullRefTryCatch()); diff --git a/tests/src/Simple/ReadyToRunUnit/methodWhiteList.txt b/tests/src/Simple/ReadyToRunUnit/methodWhiteList.txt index e8dc6874f..09c418e80 100644 --- a/tests/src/Simple/ReadyToRunUnit/methodWhiteList.txt +++ b/tests/src/Simple/ReadyToRunUnit/methodWhiteList.txt @@ -2,3 +2,4 @@ Program..cctor() Program+DisposeStruct..cctor() Program+DisposeClass.Dispose() Program+DisposeClass..cctor() +ClassWithStatic..cctor() \ No newline at end of file