Fix explicit layout validator memory alloc problems (#56757)
authorDavid Wrighton <davidwr@microsoft.com>
Wed, 4 Aug 2021 18:57:03 +0000 (11:57 -0700)
committerGitHub <noreply@github.com>
Wed, 4 Aug 2021 18:57:03 +0000 (11:57 -0700)
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

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

index 9066f54..07489f8 100644 (file)
@@ -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<FieldLayoutInterval>
+        {
+            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<FieldLayoutInterval> _fieldLayout = new List<FieldLayoutInterval>();
 
         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<FieldLayoutInterval> fieldORefMap = new List<FieldLayoutInterval>();
                     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<FieldLayoutInterval> 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> 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> 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());