From 4aa7f5fc38e17f75afde3791b5d80e02a554f568 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Wed, 26 Jan 2022 00:24:06 +0100 Subject: [PATCH] Fix #2612 decompilation of newarr with int.MaxValue causes OOME in decompiler. --- .../TestCases/Pretty/InitializerTests.cs | 7 ++ .../IL/Transforms/DynamicCallSiteTransform.cs | 4 +- .../Transforms/TransformArrayInitializers.cs | 86 +++++++++++-------- 3 files changed, 59 insertions(+), 38 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/InitializerTests.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/InitializerTests.cs index 63454a416..ca1298d36 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/InitializerTests.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/InitializerTests.cs @@ -724,6 +724,13 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty.InitializerTests { null, "test", "hello", "world" } }; } + + private static void OutOfMemory() + { + byte[] array = new byte[int.MaxValue]; + array[0] = 1; + Console.WriteLine(array.Length); + } #endregion #region Object initializers diff --git a/ICSharpCode.Decompiler/IL/Transforms/DynamicCallSiteTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/DynamicCallSiteTransform.cs index 7290273a5..2c78d42f9 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/DynamicCallSiteTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/DynamicCallSiteTransform.cs @@ -356,7 +356,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms { if (value is NewArr typeArgsNewArr && typeArgsNewArr.Type.IsKnownType(KnownTypeCode.Type) && typeArgsNewArr.Indices.Count == 1 && typeArgsNewArr.Indices[0].MatchLdcI4(out numberOfTypeArguments)) { - if (!TransformArrayInitializers.HandleSimpleArrayInitializer(context.Function, callSiteInitBlock, 3, variableOrTemporary, typeArgsNewArr.Type, new[] { numberOfTypeArguments }, out var typeArguments, out _)) + if (!TransformArrayInitializers.HandleSimpleArrayInitializer(context.Function, callSiteInitBlock, 3, variableOrTemporary, new[] { numberOfTypeArguments }, out var typeArguments, out _)) return false; int i = 0; callSiteInfo.TypeArguments = new IType[numberOfTypeArguments]; @@ -535,7 +535,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms { if (!(value is NewArr newArr2 && newArr2.Type.FullName == "Microsoft.CSharp.RuntimeBinder.CSharpArgumentInfo" && newArr2.Indices.Count == 1 && newArr2.Indices[0].MatchLdcI4(out var numberOfArguments))) return false; - if (!TransformArrayInitializers.HandleSimpleArrayInitializer(context.Function, callSiteInfo.InitBlock, instructionOffset, variable, newArr2.Type, new[] { numberOfArguments }, out var arguments, out _)) + if (!TransformArrayInitializers.HandleSimpleArrayInitializer(context.Function, callSiteInfo.InitBlock, instructionOffset, variable, new[] { numberOfArguments }, out var arguments, out _)) return false; int i = 0; callSiteInfo.ArgumentInfos = new CSharpArgumentInfo[numberOfArguments]; diff --git a/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs b/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs index ed722b2cb..69e286826 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/TransformArrayInitializers.cs @@ -75,7 +75,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms } if (arrayLength.Length == 1) { - if (HandleSimpleArrayInitializer(function, body, pos + 1, v, elementType, arrayLength, out var arrayValues, out var instructionsToRemove)) + if (HandleSimpleArrayInitializer(function, body, pos + 1, v, arrayLength, out var arrayValues, out var instructionsToRemove)) { context.Step("HandleSimpleArrayInitializer: single-dim", inst); var block = new Block(BlockKind.ArrayInitializer); @@ -172,7 +172,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms ILInlining.InlineIfPossible(body, pos, context); return true; } - if (HandleSimpleArrayInitializer(function, body, pos + 1, v, elementType, length, out var arrayValues, out var instructionsToRemove)) + if (HandleSimpleArrayInitializer(function, body, pos + 1, v, length, out var arrayValues, out var instructionsToRemove)) { context.Step("HandleSimpleArrayInitializer: multi-dim", inst); var block = new Block(BlockKind.ArrayInitializer); @@ -387,12 +387,17 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// /// Handle simple case where RuntimeHelpers.InitializeArray is not used. /// - internal static bool HandleSimpleArrayInitializer(ILFunction function, Block block, int pos, ILVariable store, IType elementType, int[] arrayLength, out (ILInstruction[] Indices, ILInstruction Value)[] values, out int instructionsToRemove) + internal static bool HandleSimpleArrayInitializer(ILFunction function, Block block, int pos, ILVariable store, int[] arrayLength, out (ILInstruction[] Indices, ILInstruction Value)[] values, out int instructionsToRemove) { instructionsToRemove = 0; int elementCount = 0; - var length = arrayLength.Aggregate(1, (t, l) => t * l); - values = new (ILInstruction[] Indices, ILInstruction Value)[length]; + int length = arrayLength.Aggregate(1, (t, l) => t * l); + // Cannot pre-allocate the result array, because we do not know yet, + // whether there is in fact an array initializer. + // To prevent excessive allocations, use min(|block|, arraySize) als initial capacity. + // This should prevent list-resizing as well as out-of-memory errors. + values = null; + var valuesList = new List<(ILInstruction[] Indices, ILInstruction Value)>(Math.Min(block.Instructions.Count, length)); int[] nextMinimumIndex = new int[arrayLength.Length]; @@ -443,7 +448,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms return nextIndices; } - int j = 0; int i = pos; int step; while (i < block.Instructions.Count) @@ -483,7 +487,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (indices.Count != arrayLength.Length) break; bool exact; - if (j >= values.Length) + if (length <= 0) break; do { @@ -492,16 +496,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms return false; if (exact) { - values[j] = (nextIndices, value); + valuesList.Add((nextIndices, value)); elementCount++; instructionsToRemove += step; } else { - values[j] = (nextIndices, null); + valuesList.Add((nextIndices, null)); } - j++; - } while (j < values.Length && !exact); + } while (valuesList.Count < length && !exact); i += step; } if (i < block.Instructions.Count) @@ -514,53 +517,59 @@ namespace ICSharpCode.Decompiler.IL.Transforms return false; } } - while (j < values.Length) + if (pos + instructionsToRemove >= block.Instructions.Count) + return false; + + bool mustTransform = ILInlining.IsCatchWhenBlock(block) || ILInlining.IsInConstructorInitializer(function, block.Instructions[pos]); + // If there are not enough user-defined elements after scanning all instructions, we can abort the transform. + // This avoids unnecessary allocations and OOM crashes (in case of int.MaxValue). + if (elementCount < length / 3 - 5) + { + if (!mustTransform) + return false; + // .NET does not allow allocation of arrays >= 2 GB. + if (length >= int.MaxValue) + return false; + } + + for (int j = valuesList.Count; j < length; j++) { var nextIndices = CalculateNextIndices(null, out _); if (nextIndices == null) return false; - values[j] = (nextIndices, null); - j++; + valuesList.Add((nextIndices, null)); } - if (pos + instructionsToRemove >= block.Instructions.Count) - return false; - return ShouldTransformToInitializer(function, block, pos, elementCount, length); - } - - static bool ShouldTransformToInitializer(ILFunction function, Block block, int startPos, int elementCount, int length) - { - if (elementCount == 0) - return false; - if (elementCount >= length / 3 - 5) - return true; - if (ILInlining.IsCatchWhenBlock(block) || ILInlining.IsInConstructorInitializer(function, block.Instructions[startPos])) - return true; - return false; + values = valuesList.ToArray(); + return true; } bool HandleJaggedArrayInitializer(Block block, int pos, ILVariable store, IType elementType, int length, out ILVariable finalStore, out ILInstruction[] values, out int instructionsToRemove) { instructionsToRemove = 0; finalStore = null; - values = new ILInstruction[length]; + // Cannot pre-allocate the result array, because we do not know yet, + // whether there is in fact an array initializer. + // To prevent excessive allocations, use min(|block|, arraySize) als initial capacity. + // This should prevent list-resizing as well as out-of-memory errors. + values = null; + var valuesList = new List(Math.Min(block.Instructions.Count, length)); - ILInstruction initializer; - IType type; for (int i = 0; i < length; i++) { // 1. Instruction: (optional) temporary copy of store bool hasTemporaryCopy = block.Instructions[pos].MatchStLoc(out var temp, out var storeLoad) && storeLoad.MatchLdLoc(store); + ILInstruction initializer; if (hasTemporaryCopy) { - if (!MatchJaggedArrayStore(block, pos + 1, temp, i, out initializer, out type)) + if (!MatchJaggedArrayStore(block, pos + 1, temp, i, out initializer, out _)) return false; } else { - if (!MatchJaggedArrayStore(block, pos, store, i, out initializer, out type)) + if (!MatchJaggedArrayStore(block, pos, store, i, out initializer, out _)) return false; } - values[i] = initializer; + valuesList.Add(initializer); int inc = hasTemporaryCopy ? 3 : 2; pos += inc; instructionsToRemove += inc; @@ -569,10 +578,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms // Remove it and use its value instead. if (block.Instructions[pos].MatchStLoc(out finalStore, out var array)) { + if (!array.MatchLdLoc(store)) + return false; instructionsToRemove++; - return array.MatchLdLoc(store); } - finalStore = store; + else + { + finalStore = store; + } + values = valuesList.ToArray(); return true; }