From 1dcec2e99536536d705500e5a068d62c41a1899d Mon Sep 17 00:00:00 2001 From: Jimmy Date: Tue, 6 Aug 2024 11:02:53 +0800 Subject: [PATCH] [Neo Core Bug] fix compound type reference issue (#3334) * fix compound type reference issue * fix warning * add benchmark * throw exception instead * Update benchmarks/Neo.VM.Benchmarks/Benchmarks.Types.cs * Update src/Neo.VM/Types/Map.cs * Apply suggestions from code review * Update src/Neo.VM/Types/Map.cs Co-authored-by: Shargon * update accessibality. --------- Co-authored-by: Shargon Co-authored-by: NGD Admin <154295625+NGDAdmin@users.noreply.github.com> --- .../Neo.VM.Benchmarks/Benchmarks.Types.cs | 122 +++++++++++++++ benchmarks/Neo.VM.Benchmarks/TestArray.cs | 141 ++++++++++++++++++ benchmarks/Neo.VM.Benchmarks/TestStruct.cs | 129 ++++++++++++++++ src/Neo.VM/EvaluationStack.cs | 2 + src/Neo.VM/Neo.VM.csproj | 1 + src/Neo.VM/Types/Array.cs | 29 +++- src/Neo.VM/Types/CompoundType.cs | 2 +- src/Neo.VM/Types/Map.cs | 4 + tests/Neo.VM.Tests/UT_ReferenceCounter.cs | 19 +++ 9 files changed, 444 insertions(+), 5 deletions(-) create mode 100644 benchmarks/Neo.VM.Benchmarks/Benchmarks.Types.cs create mode 100644 benchmarks/Neo.VM.Benchmarks/TestArray.cs create mode 100644 benchmarks/Neo.VM.Benchmarks/TestStruct.cs diff --git a/benchmarks/Neo.VM.Benchmarks/Benchmarks.Types.cs b/benchmarks/Neo.VM.Benchmarks/Benchmarks.Types.cs new file mode 100644 index 0000000000..3685e45f12 --- /dev/null +++ b/benchmarks/Neo.VM.Benchmarks/Benchmarks.Types.cs @@ -0,0 +1,122 @@ +// Copyright (C) 2015-2024 The Neo Project. +// +// Benchmarks.Types.cs file belongs to the neo project and is free +// software distributed under the MIT software license, see the +// accompanying file LICENSE in the main directory of the +// repository or http://www.opensource.org/licenses/mit-license.php +// for more details. +// +// Redistribution and use in source and binary forms with or without +// modifications are permitted. + +using BenchmarkDotNet.Attributes; +using Array = Neo.VM.Types.Array; + +namespace Neo.VM.Benchmark; + +public class Benchmarks_Types +{ + public IEnumerable<(int Depth, int ElementsPerLevel)> ParamSource() + { + int[] depths = [2, 4]; + int[] elementsPerLevel = [2, 4, 6]; + + foreach (var depth in depths) + { + foreach (var elements in elementsPerLevel) + { + if (depth <= 8 || elements <= 2) + { + yield return (depth, elements); + } + } + } + } + + [ParamsSource(nameof(ParamSource))] + public (int Depth, int ElementsPerLevel) Params; + + [Benchmark] + public void BenchNestedArrayDeepCopy() + { + var root = new Array(new ReferenceCounter()); + CreateNestedArray(root, Params.Depth, Params.ElementsPerLevel); + _ = root.DeepCopy(); + } + + [Benchmark] + public void BenchNestedArrayDeepCopyWithReferenceCounter() + { + var referenceCounter = new ReferenceCounter(); + var root = new Array(referenceCounter); + CreateNestedArray(root, Params.Depth, Params.ElementsPerLevel, referenceCounter); + _ = root.DeepCopy(); + } + + [Benchmark] + public void BenchNestedTestArrayDeepCopy() + { + var root = new TestArray(new ReferenceCounter()); + CreateNestedTestArray(root, Params.Depth, Params.ElementsPerLevel); + _ = root.DeepCopy(); + } + + [Benchmark] + public void BenchNestedTestArrayDeepCopyWithReferenceCounter() + { + var referenceCounter = new ReferenceCounter(); + var root = new TestArray(referenceCounter); + CreateNestedTestArray(root, Params.Depth, Params.ElementsPerLevel, referenceCounter); + _ = root.DeepCopy(); + } + + private static void CreateNestedArray(Array? rootArray, int depth, int elementsPerLevel = 1, ReferenceCounter? referenceCounter = null) + { + if (depth < 0) + { + throw new ArgumentException("Depth must be non-negative", nameof(depth)); + } + + if (rootArray == null) + { + throw new ArgumentNullException(nameof(rootArray)); + } + + if (depth == 0) + { + return; + } + + for (var i = 0; i < elementsPerLevel; i++) + { + var childArray = new Array(referenceCounter); + rootArray.Add(childArray); + CreateNestedArray(childArray, depth - 1, elementsPerLevel, referenceCounter); + } + } + + private static void CreateNestedTestArray(TestArray rootArray, int depth, int elementsPerLevel = 1, ReferenceCounter referenceCounter = null) + { + if (depth < 0) + { + throw new ArgumentException("Depth must be non-negative", nameof(depth)); + } + + if (rootArray == null) + { + throw new ArgumentNullException(nameof(rootArray)); + } + + if (depth == 0) + { + return; + } + + for (var i = 0; i < elementsPerLevel; i++) + { + var childArray = new TestArray(referenceCounter); + rootArray.Add(childArray); + CreateNestedTestArray(childArray, depth - 1, elementsPerLevel, referenceCounter); + } + } +} diff --git a/benchmarks/Neo.VM.Benchmarks/TestArray.cs b/benchmarks/Neo.VM.Benchmarks/TestArray.cs new file mode 100644 index 0000000000..62fedfed11 --- /dev/null +++ b/benchmarks/Neo.VM.Benchmarks/TestArray.cs @@ -0,0 +1,141 @@ +// Copyright (C) 2015-2024 The Neo Project. +// +// TestArray.cs file belongs to the neo project and is free +// software distributed under the MIT software license, see the +// accompanying file LICENSE in the main directory of the +// repository or http://www.opensource.org/licenses/mit-license.php +// for more details. +// +// Redistribution and use in source and binary forms with or without +// modifications are permitted. + +using Neo.VM.Types; +using System.Collections; + +namespace Neo.VM.Benchmark; + +public class TestArray : CompoundType, IReadOnlyList +{ + protected readonly List _array; + + /// + /// Get or set item in the array. + /// + /// The index of the item in the array. + /// The item at the specified index. + public StackItem this[int index] + { + get => _array[index]; + set + { + if (IsReadOnly) throw new InvalidOperationException("The object is readonly."); + ReferenceCounter?.RemoveReference(_array[index], this); + _array[index] = value; + ReferenceCounter?.AddReference(value, this); + } + } + + /// + /// The number of items in the array. + /// + public override int Count => _array.Count; + public override IEnumerable SubItems => _array; + public override int SubItemsCount => _array.Count; + public override StackItemType Type => StackItemType.Array; + + /// + /// Create an array containing the specified items. + /// + /// The items to be included in the array. + public TestArray(IEnumerable? items = null) + : this(null, items) + { + } + + /// + /// Create an array containing the specified items. And make the array use the specified . + /// + /// The to be used by this array. + /// The items to be included in the array. + public TestArray(ReferenceCounter? referenceCounter, IEnumerable? items = null) + : base(referenceCounter) + { + _array = items switch + { + null => new List(), + List list => list, + _ => new List(items) + }; + if (referenceCounter != null) + foreach (StackItem item in _array) + referenceCounter.AddReference(item, this); + } + + /// + /// Add a new item at the end of the array. + /// + /// The item to be added. + public void Add(StackItem item) + { + if (IsReadOnly) throw new InvalidOperationException("The object is readonly."); + _array.Add(item); + ReferenceCounter?.AddReference(item, this); + } + + public override void Clear() + { + if (IsReadOnly) throw new InvalidOperationException("The object is readonly."); + if (ReferenceCounter != null) + foreach (StackItem item in _array) + ReferenceCounter.RemoveReference(item, this); + _array.Clear(); + } + + public override StackItem ConvertTo(StackItemType type) + { + if (Type == StackItemType.Array && type == StackItemType.Struct) + return new Struct(ReferenceCounter, new List(_array)); + return base.ConvertTo(type); + } + + internal sealed override StackItem DeepCopy(Dictionary refMap, bool asImmutable) + { + if (refMap.TryGetValue(this, out StackItem? mappedItem)) return mappedItem; + var result = this is TestStruct ? new TestStruct(ReferenceCounter) : new TestArray(ReferenceCounter); + refMap.Add(this, result); + foreach (StackItem item in _array) + result.Add(item.DeepCopy(refMap, asImmutable)); + result.IsReadOnly = true; + return result; + } + + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + + public IEnumerator GetEnumerator() + { + return _array.GetEnumerator(); + } + + /// + /// Remove the item at the specified index. + /// + /// The index of the item to be removed. + public void RemoveAt(int index) + { + if (IsReadOnly) throw new InvalidOperationException("The object is readonly."); + ReferenceCounter?.RemoveReference(_array[index], this); + _array.RemoveAt(index); + } + + /// + /// Reverse all items in the array. + /// + public void Reverse() + { + if (IsReadOnly) throw new InvalidOperationException("The object is readonly."); + _array.Reverse(); + } +} diff --git a/benchmarks/Neo.VM.Benchmarks/TestStruct.cs b/benchmarks/Neo.VM.Benchmarks/TestStruct.cs new file mode 100644 index 0000000000..5a9541f1e0 --- /dev/null +++ b/benchmarks/Neo.VM.Benchmarks/TestStruct.cs @@ -0,0 +1,129 @@ +// Copyright (C) 2015-2024 The Neo Project. +// +// TestStruct.cs file belongs to the neo project and is free +// software distributed under the MIT software license, see the +// accompanying file LICENSE in the main directory of the +// repository or http://www.opensource.org/licenses/mit-license.php +// for more details. +// +// Redistribution and use in source and binary forms with or without +// modifications are permitted. + +using Neo.VM.Types; + +namespace Neo.VM.Benchmark; + +public class TestStruct : TestArray +{ + public override StackItemType Type => StackItemType.Struct; + + /// + /// Create a structure with the specified fields. + /// + /// The fields to be included in the structure. + public TestStruct(IEnumerable? fields = null) + : this(null, fields) + { + } + + /// + /// Create a structure with the specified fields. And make the structure use the specified . + /// + /// The to be used by this structure. + /// The fields to be included in the structure. + public TestStruct(ReferenceCounter? referenceCounter, IEnumerable? fields = null) + : base(referenceCounter, fields) + { + } + + /// + /// Create a new structure with the same content as this structure. All nested structures will be copied by value. + /// + /// Execution engine limits + /// The copied structure. + public TestStruct Clone(ExecutionEngineLimits limits) + { + int count = (int)(limits.MaxStackSize - 1); + TestStruct result = new(ReferenceCounter); + Queue queue = new(); + queue.Enqueue(result); + queue.Enqueue(this); + while (queue.Count > 0) + { + TestStruct a = queue.Dequeue(); + TestStruct b = queue.Dequeue(); + foreach (StackItem item in b) + { + count--; + if (count < 0) throw new InvalidOperationException("Beyond clone limits!"); + if (item is TestStruct sb) + { + TestStruct sa = new(ReferenceCounter); + a.Add(sa); + queue.Enqueue(sa); + queue.Enqueue(sb); + } + else + { + a.Add(item); + } + } + } + return result; + } + + public override StackItem ConvertTo(StackItemType type) + { + if (type == StackItemType.Array) + return new TestArray(ReferenceCounter, new List(_array)); + return base.ConvertTo(type); + } + + public override bool Equals(StackItem? other) + { + throw new NotSupportedException(); + } + + internal override bool Equals(StackItem? other, ExecutionEngineLimits limits) + { + if (other is not TestStruct s) return false; + Stack stack1 = new(); + Stack stack2 = new(); + stack1.Push(this); + stack2.Push(s); + uint count = limits.MaxStackSize; + uint maxComparableSize = limits.MaxComparableSize; + while (stack1.Count > 0) + { + if (count-- == 0) + throw new InvalidOperationException("Too many struct items to compare."); + StackItem a = stack1.Pop(); + StackItem b = stack2.Pop(); + if (a is ByteString byteString) + { + if (!byteString.Equals(b, ref maxComparableSize)) return false; + } + else + { + if (maxComparableSize == 0) + throw new InvalidOperationException("The operand exceeds the maximum comparable size."); + maxComparableSize -= 1; + if (a is TestStruct sa) + { + if (ReferenceEquals(a, b)) continue; + if (b is not TestStruct sb) return false; + if (sa.Count != sb.Count) return false; + foreach (StackItem item in sa) + stack1.Push(item); + foreach (StackItem item in sb) + stack2.Push(item); + } + else + { + if (!a.Equals(b)) return false; + } + } + } + return true; + } +} diff --git a/src/Neo.VM/EvaluationStack.cs b/src/Neo.VM/EvaluationStack.cs index 34517b2197..571cb6b2db 100644 --- a/src/Neo.VM/EvaluationStack.cs +++ b/src/Neo.VM/EvaluationStack.cs @@ -26,6 +26,8 @@ public sealed class EvaluationStack : IReadOnlyList private readonly List innerList = new(); private readonly ReferenceCounter referenceCounter; + internal ReferenceCounter ReferenceCounter => referenceCounter; + internal EvaluationStack(ReferenceCounter referenceCounter) { this.referenceCounter = referenceCounter; diff --git a/src/Neo.VM/Neo.VM.csproj b/src/Neo.VM/Neo.VM.csproj index eb137b623a..cc6fb4a3a9 100644 --- a/src/Neo.VM/Neo.VM.csproj +++ b/src/Neo.VM/Neo.VM.csproj @@ -11,6 +11,7 @@ + diff --git a/src/Neo.VM/Types/Array.cs b/src/Neo.VM/Types/Array.cs index e15358a881..76c1486778 100644 --- a/src/Neo.VM/Types/Array.cs +++ b/src/Neo.VM/Types/Array.cs @@ -35,6 +35,11 @@ public StackItem this[int index] if (IsReadOnly) throw new InvalidOperationException("The object is readonly."); ReferenceCounter?.RemoveReference(_array[index], this); _array[index] = value; + if (ReferenceCounter != null && value is CompoundType { ReferenceCounter: null }) + { + throw new InvalidOperationException("Can not set a CompoundType without a ReferenceCounter."); + } + ReferenceCounter?.AddReference(value, this); } } @@ -70,9 +75,18 @@ public Array(ReferenceCounter? referenceCounter, IEnumerable? items = List list => list, _ => new List(items) }; - if (referenceCounter != null) - foreach (StackItem item in _array) - referenceCounter.AddReference(item, this); + + if (referenceCounter == null) return; + + foreach (var item in _array) + { + if (item is CompoundType { ReferenceCounter: null }) + { + throw new InvalidOperationException("Can not set a CompoundType without a ReferenceCounter."); + } + + referenceCounter.AddReference(item, this); + } } /// @@ -83,7 +97,14 @@ public void Add(StackItem item) { if (IsReadOnly) throw new InvalidOperationException("The object is readonly."); _array.Add(item); - ReferenceCounter?.AddReference(item, this); + + if (ReferenceCounter == null) return; + + if (item is CompoundType { ReferenceCounter: null }) + { + throw new InvalidOperationException("Can not set a CompoundType without a ReferenceCounter."); + } + ReferenceCounter.AddReference(item, this); } public override void Clear() diff --git a/src/Neo.VM/Types/CompoundType.cs b/src/Neo.VM/Types/CompoundType.cs index ede743b881..daa9b05be6 100644 --- a/src/Neo.VM/Types/CompoundType.cs +++ b/src/Neo.VM/Types/CompoundType.cs @@ -24,7 +24,7 @@ public abstract class CompoundType : StackItem /// /// The reference counter used to count the items in the VM object. /// - protected readonly ReferenceCounter? ReferenceCounter; + protected internal readonly ReferenceCounter? ReferenceCounter; /// /// Create a new with the specified reference counter. diff --git a/src/Neo.VM/Types/Map.cs b/src/Neo.VM/Types/Map.cs index 5df66fb897..48155b5207 100644 --- a/src/Neo.VM/Types/Map.cs +++ b/src/Neo.VM/Types/Map.cs @@ -54,6 +54,10 @@ public StackItem this[PrimitiveType key] ReferenceCounter.RemoveReference(old_value, this); else ReferenceCounter.AddReference(key, this); + if (value is CompoundType { ReferenceCounter: null }) + { + throw new InvalidOperationException("Can not set a Map without a ReferenceCounter."); + } ReferenceCounter.AddReference(value, this); } dictionary[key] = value; diff --git a/tests/Neo.VM.Tests/UT_ReferenceCounter.cs b/tests/Neo.VM.Tests/UT_ReferenceCounter.cs index f974670805..61f7788dd8 100644 --- a/tests/Neo.VM.Tests/UT_ReferenceCounter.cs +++ b/tests/Neo.VM.Tests/UT_ReferenceCounter.cs @@ -12,6 +12,8 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using Neo.VM; using Neo.VM.Types; +using System; +using Array = Neo.VM.Types.Array; namespace Neo.Test { @@ -240,5 +242,22 @@ public void TestArrayNoPush() Assert.AreEqual(VMState.HALT, engine.Execute()); Assert.AreEqual(array.Count, engine.ReferenceCounter.Count); } + + [TestMethod] + [ExpectedException(typeof(InvalidOperationException))] + public void TestInvalidReferenceStackItem() + { + var reference = new ReferenceCounter(); + var arr = new Array(reference); + var arr2 = new Array(); + + for (var i = 0; i < 10; i++) + { + arr2.Add(i); + } + + arr.Add(arr2); + Assert.AreEqual(11, reference.Count); + } } }