From: David Wrighton Date: Wed, 4 Aug 2021 18:57:03 +0000 (-0700) Subject: Fix explicit layout validator memory alloc problems (#56757) X-Git-Tag: accepted/tizen/unified/20220110.054933~654 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=9d9aed68df99a4dd4c65b513f76bea1fd5a7b286;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Fix explicit layout validator memory alloc problems (#56757) The ExplicitLayoutValidator currently used an array of bytes one to one with the size of the layout of the class. However, we have some test cases for outrageously large explicit layout structures and classes which cause this approach to OOM in crossgen2. The fix is to move to a data structure defined in terms of intervals. This is probably a bit slower, but it requires vastly less memory for pathological situations. Fixes #55164 Fixes #53559 --- diff --git a/src/coreclr/tools/Common/TypeSystem/Common/ExplicitLayoutValidator.cs b/src/coreclr/tools/Common/TypeSystem/Common/ExplicitLayoutValidator.cs index 9066f54..07489f8 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/ExplicitLayoutValidator.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/ExplicitLayoutValidator.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.Diagnostics; namespace Internal.TypeSystem @@ -15,9 +16,44 @@ namespace Internal.TypeSystem ORef, } + private struct FieldLayoutInterval : IComparable + { + public FieldLayoutInterval(int start, int size, FieldLayoutTag tag) + { + Start = start; + Size = size; + Tag = tag; + } + + public int Start; + public int Size; + + public int EndSentinel + { + get + { + return Start + Size; + } + set + { + Size = value - Start; + Debug.Assert(Size >= 0); + } + } + + public FieldLayoutTag Tag; + + public int CompareTo(FieldLayoutInterval other) + { + return Start.CompareTo(other.Start); + } + } + private readonly int _pointerSize; - private readonly FieldLayoutTag[] _fieldLayout; + // Represent field layout bits as as a series of intervals to prevent pathological bad behavior + // involving excessively large explicit layout structures. + private readonly List _fieldLayout = new List(); private readonly MetadataType _typeBeingValidated; @@ -25,12 +61,13 @@ namespace Internal.TypeSystem { _typeBeingValidated = type; _pointerSize = type.Context.Target.PointerSize; - _fieldLayout = new FieldLayoutTag[typeSizeInBytes]; } public static void Validate(MetadataType type, ComputedInstanceFieldLayout layout) { - ExplicitLayoutValidator validator = new ExplicitLayoutValidator(type, layout.ByteCountUnaligned.AsInt); + int typeSizeInBytes = layout.ByteCountUnaligned.AsInt; + ExplicitLayoutValidator validator = new ExplicitLayoutValidator(type, typeSizeInBytes); + foreach (FieldAndOffset fieldAndOffset in layout.Offsets) { validator.AddToFieldLayout(fieldAndOffset.Offset.AsInt, fieldAndOffset.Field.FieldType); @@ -75,11 +112,25 @@ namespace Internal.TypeSystem ThrowFieldLayoutError(offset); } - bool[] fieldORefMap = new bool[fieldSize]; + List fieldORefMap = new List(); MarkORefLocations(mdType, fieldORefMap, offset: 0); - for (int index = 0; index < fieldSize; index++) + + // Merge in fieldORefMap from structure specifying not attributed intervals as NonORef + int lastGCRegionReportedEnd = 0; + + foreach (var gcRegion in fieldORefMap) { - SetFieldLayout(offset + index, fieldORefMap[index] ? FieldLayoutTag.ORef : FieldLayoutTag.NonORef); + SetFieldLayout(offset + lastGCRegionReportedEnd, gcRegion.Start - lastGCRegionReportedEnd, FieldLayoutTag.NonORef); + Debug.Assert(gcRegion.Tag == FieldLayoutTag.ORef); + SetFieldLayout(offset + gcRegion.Start, gcRegion.Size, gcRegion.Tag); + lastGCRegionReportedEnd = gcRegion.EndSentinel; + } + + if (fieldORefMap.Count > 0) + { + int trailingRegionStart = fieldORefMap[fieldORefMap.Count - 1].EndSentinel; + int trailingRegionSize = fieldSize - trailingRegionStart; + SetFieldLayout(offset + trailingRegionStart, trailingRegionSize, FieldLayoutTag.NonORef); } } } @@ -98,7 +149,7 @@ namespace Internal.TypeSystem } } - private void MarkORefLocations(MetadataType type, bool[] orefMap, int offset) + private void MarkORefLocations(MetadataType type, List orefMap, int offset) { // Recurse into struct fields foreach (FieldDesc field in type.GetFields()) @@ -108,10 +159,7 @@ namespace Internal.TypeSystem int fieldOffset = offset + field.Offset.AsInt; if (field.FieldType.IsGCPointer) { - for (int index = 0; index < _pointerSize; index++) - { - orefMap[fieldOffset + index] = true; - } + SetFieldLayout(orefMap, offset, _pointerSize, FieldLayoutTag.ORef); } else if (field.FieldType.IsValueType) { @@ -125,27 +173,133 @@ namespace Internal.TypeSystem } } - private void SetFieldLayout(int offset, int count, FieldLayoutTag tag) + private void SetFieldLayout(List fieldLayoutInterval, int offset, int count, FieldLayoutTag tag) { - for (int index = 0; index < count; index++) + if (count == 0) + return; + + var newInterval = new FieldLayoutInterval(offset, count, tag); + + int binarySearchIndex = fieldLayoutInterval.BinarySearch(newInterval); + + if (binarySearchIndex >= 0) + { + var existingInterval = fieldLayoutInterval[binarySearchIndex]; + + // Exact match found for start of interval. + if (tag != existingInterval.Tag) + { + ThrowFieldLayoutError(offset); + } + + if (existingInterval.Size >= count) + { + // Existing interval is big enough. + } + else + { + // Expand existing interval, and then check to see if that's valid. + existingInterval.Size = count; + fieldLayoutInterval[binarySearchIndex] = existingInterval; + + ValidateAndMergeIntervalWithFollowingIntervals(fieldLayoutInterval, binarySearchIndex); + } + } + else { - SetFieldLayout(offset + index, tag); + // No exact start match found. + + int newIntervalLocation = ~binarySearchIndex; + + // Check for previous interval overlaps cases + if (newIntervalLocation > 0) + { + var previousInterval = fieldLayoutInterval[newIntervalLocation - 1]; + bool tagMatches = previousInterval.Tag == tag; + + if (previousInterval.EndSentinel > offset) + { + // Previous interval overlaps. + if (!tagMatches) + { + ThrowFieldLayoutError(offset); + } + } + + if (previousInterval.EndSentinel > offset || (tagMatches && previousInterval.EndSentinel == offset)) + { + // Previous interval overlaps, or exactly matches up with new interval and tag matches. Instead + // of expanding interval set, simply expand the previous interval. + previousInterval.EndSentinel = newInterval.EndSentinel; + + fieldLayoutInterval[newIntervalLocation - 1] = previousInterval; + newIntervalLocation = newIntervalLocation - 1; + } + else + { + fieldLayoutInterval.Insert(newIntervalLocation, newInterval); + } + } + else + { + // New interval added at start + fieldLayoutInterval.Insert(newIntervalLocation, newInterval); + } + + ValidateAndMergeIntervalWithFollowingIntervals(fieldLayoutInterval, newIntervalLocation); } } - private void SetFieldLayout(int offset, FieldLayoutTag tag) + private void ValidateAndMergeIntervalWithFollowingIntervals(List fieldLayoutInterval, int intervalIndex) { - FieldLayoutTag existingTag = _fieldLayout[offset]; - if (existingTag != tag) + while(true) { - if (existingTag != FieldLayoutTag.Empty) + if (intervalIndex + 1 == fieldLayoutInterval.Count) { - ThrowFieldLayoutError(offset); + // existing interval is last interval. Expansion always succeeds + break; + } + else + { + var nextInterval = fieldLayoutInterval[intervalIndex + 1]; + var expandedInterval = fieldLayoutInterval[intervalIndex]; + var tag = expandedInterval.Tag; + + if (nextInterval.Start > expandedInterval.EndSentinel) + { + // Next interval does not contact existing interval. Expansion succeeded + break; + } + + if ((nextInterval.Start == expandedInterval.EndSentinel) && nextInterval.Tag != tag) + { + // Next interval starts just after existing interval, but does not match tag. Expansion succeeded + break; + } + + Debug.Assert(nextInterval.Start <= expandedInterval.EndSentinel); + // Next interval overlaps with expanded interval. + + if (nextInterval.Tag != tag) + { + ThrowFieldLayoutError(nextInterval.Start); + } + + // Expand existing interval to cover region of next interval + expandedInterval.EndSentinel = nextInterval.EndSentinel; + fieldLayoutInterval[intervalIndex] = expandedInterval; + + // Remove next interval + fieldLayoutInterval.RemoveAt(intervalIndex + 1); } - _fieldLayout[offset] = tag; } } + private void SetFieldLayout(int offset, int count, FieldLayoutTag tag) + { + SetFieldLayout(_fieldLayout, offset, count, tag); + } + private void ThrowFieldLayoutError(int offset) { ThrowHelper.ThrowTypeLoadException(ExceptionStringID.ClassLoadExplicitLayout, _typeBeingValidated, offset.ToStringInvariant());