From: Tomáš Matoušek Date: Mon, 5 Aug 2019 20:49:04 +0000 (-0700) Subject: Fix off-by-one error when calculating GUID handle generation in MetadataAggregator... X-Git-Tag: submit/tizen/20210909.063632~11031^2~775 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a77015d69fa64ae9ae2f5ae81f8e3104d016dfa3;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Fix off-by-one error when calculating GUID handle generation in MetadataAggregator.GetGenerationHandle. (dotnet/corefx#40035) Commit migrated from https://github.com/dotnet/corefx/commit/366c75f583243969a6906410231595523f60bb72 --- diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataAggregator.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataAggregator.cs index 0ae7a0b..8988cd4 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataAggregator.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataAggregator.cs @@ -11,7 +11,7 @@ namespace System.Reflection.Metadata.Ecma335 public sealed class MetadataAggregator { // For each heap handle and each delta contains aggregate heap lengths. - // heapSizes[heap kind][reader index] == Sum { 0..index | reader[i].XxxHeapLength } + // heapSizes[heap kind][reader index] == Sum { 0..index | reader[i].XxxHeap.Block.Length } private readonly ImmutableArray> _heapSizes; private readonly ImmutableArray> _rowCounts; @@ -238,6 +238,13 @@ namespace System.Reflection.Metadata.Ecma335 } } + /// + /// Given a handle of an entity in an aggregate metadata calculates + /// a handle of the entity within the metadata generation it is defined in. + /// + /// Handle of an entity in an aggregate metadata. + /// The generation the entity is defined in. + /// Handle of the entity within the metadata generation . public Handle GetGenerationHandle(Handle handle, out int generation) { if (handle.IsVirtual) @@ -256,17 +263,20 @@ namespace System.Reflection.Metadata.Ecma335 var sizes = _heapSizes[(int)heapIndex]; - generation = sizes.BinarySearch(heapOffset); + // #Guid heap offset is 1-based, other heaps have 0-based offset: + var size = (handle.Type == HandleType.Guid) ? heapOffset - 1 : heapOffset; + + generation = sizes.BinarySearch(size); if (generation >= 0) { - Debug.Assert(sizes[generation] == heapOffset); + Debug.Assert(sizes[generation] == size); // the index points to the start of the next generation that added data to the heap: do { generation++; } - while (generation < sizes.Length && sizes[generation] == heapOffset); + while (generation < sizes.Length && sizes[generation] == size); } else { @@ -278,7 +288,7 @@ namespace System.Reflection.Metadata.Ecma335 throw new ArgumentException(SR.HandleBelongsToFutureGeneration, nameof(handle)); } - // GUID heap accumulates - previous heap is copied to the next generation + // GUID heap accumulates - previous heap is copied to the next generation int relativeHeapOffset = (handle.Type == HandleType.Guid || generation == 0) ? heapOffset : heapOffset - sizes[generation - 1]; return new Handle((byte)handle.Type, relativeHeapOffset); diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataTokens.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataTokens.cs index b86fcea..b05924b 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataTokens.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Metadata/Ecma335/MetadataTokens.cs @@ -164,8 +164,7 @@ namespace System.Reflection.Metadata.Ecma335 /// to the specified . /// /// - /// Zero based offset, or -1 if can only be interpreted in a context of a specific or . - /// See . + /// 1-based index into the #Guid heap. Unlike other heaps, which are essentially byte arrays, the #Guid heap is an array of 16-byte GUIDs. /// public static int GetHeapOffset(GuidHandle handle) => handle.Index; @@ -174,8 +173,7 @@ namespace System.Reflection.Metadata.Ecma335 /// to the specified . /// /// - /// Zero based offset, or -1 if can only be interpreted in a context of a specific or . - /// See . + /// Zero based offset. /// public static int GetHeapOffset(UserStringHandle handle) => handle.GetHeapOffset(); diff --git a/src/libraries/System.Reflection.Metadata/tests/Metadata/Ecma335/MetadataAggregatorTests.cs b/src/libraries/System.Reflection.Metadata/tests/Metadata/Ecma335/MetadataAggregatorTests.cs index e637493..8c6bdbd 100644 --- a/src/libraries/System.Reflection.Metadata/tests/Metadata/Ecma335/MetadataAggregatorTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/Metadata/Ecma335/MetadataAggregatorTests.cs @@ -35,10 +35,10 @@ namespace System.Reflection.Metadata.Ecma335.Tests Assert.Equal(expected, string.Join(" | ", actual)); } - private static void TestGenerationHandle(MetadataAggregator aggregator, Handle handle, Handle expectedHandle, int expectedGeneration) + private static void TestGenerationHandle(MetadataAggregator aggregator, Handle aggregateHandle, Handle expectedHandle, int expectedGeneration) { int actualGeneration; - var actualHandle = aggregator.GetGenerationHandle(handle, out actualGeneration); + var actualHandle = aggregator.GetGenerationHandle(aggregateHandle, out actualGeneration); Assert.Equal(expectedGeneration, actualGeneration); Assert.Equal(expectedHandle, actualHandle); } @@ -152,13 +152,13 @@ namespace System.Reflection.Metadata.Ecma335.Tests 200, // Gen3 400, // Gen4 }, - new int[] // #Guid + new int[] // #Guid (sizes are numbers of GUIDs on the heap, not bytes; GUIDs on the heap accumulate, previous content is copied to the next gen). { - 2, // Gen0 - 4, // Gen1 - 8, // Gen2 - 16, // Gen3 - 32, // Gen4 + 1, // Gen0: Guid #1 + 2, // Gen1: Guid #1, #2 + 2, // Gen2: Guid #1, #2 + 2, // Gen3: Guid #1, #2 + 3, // Gen4: Guid #1, #2, #3 } }; @@ -170,6 +170,11 @@ namespace System.Reflection.Metadata.Ecma335.Tests TestGenerationHandle(aggregator, MetadataTokens.UserStringHandle(12), expectedHandle: MetadataTokens.UserStringHandle(2), expectedGeneration: 2); TestGenerationHandle(aggregator, MetadataTokens.StringHandle(0), expectedHandle: MetadataTokens.StringHandle(0), expectedGeneration: 2); + // GUIDs on the heap accumulate, previous content is copied to the next gen, so the expected handle is the same as the given handle + TestGenerationHandle(aggregator, MetadataTokens.GuidHandle(1), expectedHandle: MetadataTokens.GuidHandle(1), expectedGeneration: 0); + TestGenerationHandle(aggregator, MetadataTokens.GuidHandle(2), expectedHandle: MetadataTokens.GuidHandle(2), expectedGeneration: 1); + TestGenerationHandle(aggregator, MetadataTokens.GuidHandle(3), expectedHandle: MetadataTokens.GuidHandle(3), expectedGeneration: 4); + AssertExtensions.Throws("handle", () => TestGenerationHandle(aggregator, MetadataTokens.StringHandle(22), expectedHandle: MetadataTokens.StringHandle(0), expectedGeneration: 0)); } }