From 82167b42d98369c38e5ff0da7199b8f8003eb032 Mon Sep 17 00:00:00 2001 From: Dimension4 Date: Mon, 21 Sep 2020 19:27:52 +0200 Subject: [PATCH 1/6] Equals and GetHashCode for Reflection.Pointer If a pointer is accessed via reflection it is boxed into a `System.Reflection.Pointer`. This type didn't override `Equals` and `GetHashCode`, meaning instances are equal by reference. On the other hand, pointers are compared by their value. Since value types are equal by either memcmp or memberwise equality (via reflection), the outcome of an equality test of a pointer itself depends on the type that holds that pointer. Fix #42457 --- .../src/System/Reflection/Pointer.cs | 12 ++++ .../System.Reflection/tests/PointerTests.cs | 60 +++++++++++++++++++ .../tests/System.Reflection.Tests.csproj | 1 + 3 files changed, 73 insertions(+) create mode 100644 src/libraries/System.Reflection/tests/PointerTests.cs diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/Pointer.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/Pointer.cs index 098ddfdcebaff1..ef8e82a9b0f18f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/Pointer.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/Pointer.cs @@ -39,6 +39,18 @@ public static object Box(void* ptr, Type type) return ((Pointer)ptr)._ptr; } + public override unsafe bool Equals(object? obj) + { + if (obj is Pointer pointer) + { + return _ptr == pointer._ptr; + } + + return false; + } + + public override unsafe int GetHashCode() => ((nuint)_ptr).GetHashCode(); + void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context) { throw new PlatformNotSupportedException(); diff --git a/src/libraries/System.Reflection/tests/PointerTests.cs b/src/libraries/System.Reflection/tests/PointerTests.cs new file mode 100644 index 00000000000000..e8e076ee318689 --- /dev/null +++ b/src/libraries/System.Reflection/tests/PointerTests.cs @@ -0,0 +1,60 @@ +using Xunit; + +namespace System.Reflection.Tests +{ + public class PointerTests + { + public unsafe struct BitwiseComparable + { + public int* PublicInt; + } + + public unsafe struct MemberwiseComparable + { + public string ReferenceType; + public int* PublicInt; + } + + [Fact] + public unsafe void BitwiseEquality_AreEqual() + { + int someNumber = 1; + var a = new BitwiseComparable(); + a.PublicInt = &someNumber; + var b = a; + + Assert.True(a.Equals(b)); + } + + [Fact] + public unsafe void BitwiseEquality_EqualWithSelf() + { + int someNumber = 1; + var a = new BitwiseComparable(); + a.PublicInt = &someNumber; + + Assert.True(a.Equals(a)); + } + + [Fact] + public unsafe void MemberwiseEquality_AreEqual() + { + int someNumber = 1; + var a = new MemberwiseComparable(); + a.PublicInt = &someNumber; + var b = a; + + Assert.True(a.Equals(b)); + } + + [Fact] + public unsafe void MemberwiseEquality_EqualWithSelf() + { + int someNumber = 1; + var a = new MemberwiseComparable(); + a.PublicInt = &someNumber; + + Assert.True(a.Equals(a)); + } + } +} diff --git a/src/libraries/System.Reflection/tests/System.Reflection.Tests.csproj b/src/libraries/System.Reflection/tests/System.Reflection.Tests.csproj index 3d22e60dfd35c5..f480b9783c53d9 100644 --- a/src/libraries/System.Reflection/tests/System.Reflection.Tests.csproj +++ b/src/libraries/System.Reflection/tests/System.Reflection.Tests.csproj @@ -30,6 +30,7 @@ + From 61c1ead2690713f4d8fdac21e832b958df324d68 Mon Sep 17 00:00:00 2001 From: Dimension4 Date: Mon, 21 Sep 2020 20:48:03 +0200 Subject: [PATCH 2/6] Added license header and more tests --- .../System.Reflection/tests/PointerTests.cs | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Reflection/tests/PointerTests.cs b/src/libraries/System.Reflection/tests/PointerTests.cs index e8e076ee318689..4de5399c53913b 100644 --- a/src/libraries/System.Reflection/tests/PointerTests.cs +++ b/src/libraries/System.Reflection/tests/PointerTests.cs @@ -1,4 +1,8 @@ -using Xunit; +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Xunit; +using System.Reflection; namespace System.Reflection.Tests { @@ -56,5 +60,23 @@ public unsafe void MemberwiseEquality_EqualWithSelf() Assert.True(a.Equals(a)); } + + [Fact] + public unsafe void Nullptrs_AreEqual() + { + var a = Pointer.Box(null, typeof(int*)); + var b = Pointer.Box(null, typeof(int*)); + + Assert.True(a.Equals(b)); + } + + [Fact] + public unsafe void DifferentPointerTypes_AreEqual() + { + var a = Pointer.Box((void*)0x12340000, typeof(int*)); + var b = Pointer.Box((void*)0x12340000, typeof(uint*)); + + Assert.True(a.Equals(b)); + } } } From bec0386b3782ec0be8c3672f4fa18106ee1f9164 Mon Sep 17 00:00:00 2001 From: Dimension4 Date: Fri, 2 Oct 2020 08:11:06 +0200 Subject: [PATCH 3/6] Use explicit type instead of var --- .../System.Reflection/tests/PointerTests.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Reflection/tests/PointerTests.cs b/src/libraries/System.Reflection/tests/PointerTests.cs index 4de5399c53913b..32a82e28ee4237 100644 --- a/src/libraries/System.Reflection/tests/PointerTests.cs +++ b/src/libraries/System.Reflection/tests/PointerTests.cs @@ -25,7 +25,7 @@ public unsafe void BitwiseEquality_AreEqual() int someNumber = 1; var a = new BitwiseComparable(); a.PublicInt = &someNumber; - var b = a; + BitwiseComparable b = a; Assert.True(a.Equals(b)); } @@ -46,7 +46,7 @@ public unsafe void MemberwiseEquality_AreEqual() int someNumber = 1; var a = new MemberwiseComparable(); a.PublicInt = &someNumber; - var b = a; + MemberwiseComparable b = a; Assert.True(a.Equals(b)); } @@ -64,8 +64,8 @@ public unsafe void MemberwiseEquality_EqualWithSelf() [Fact] public unsafe void Nullptrs_AreEqual() { - var a = Pointer.Box(null, typeof(int*)); - var b = Pointer.Box(null, typeof(int*)); + object a = Pointer.Box(null, typeof(int*)); + object b = Pointer.Box(null, typeof(int*)); Assert.True(a.Equals(b)); } @@ -73,8 +73,8 @@ public unsafe void Nullptrs_AreEqual() [Fact] public unsafe void DifferentPointerTypes_AreEqual() { - var a = Pointer.Box((void*)0x12340000, typeof(int*)); - var b = Pointer.Box((void*)0x12340000, typeof(uint*)); + object a = Pointer.Box((void*)0x12340000, typeof(int*)); + object b = Pointer.Box((void*)0x12340000, typeof(uint*)); Assert.True(a.Equals(b)); } From d959680189103f43f988f52cf4983deb43b2bdce Mon Sep 17 00:00:00 2001 From: Dimension4 Date: Fri, 2 Oct 2020 08:42:41 +0200 Subject: [PATCH 4/6] Reflect changes in the System.Runtime reference assembly --- src/libraries/System.Runtime/ref/System.Runtime.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 2a0a3921bee9e7..7a44865a21f3ce 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -8551,6 +8551,8 @@ internal Pointer() { } public unsafe static object Box(void* ptr, System.Type type) { throw null; } void System.Runtime.Serialization.ISerializable.GetObjectData(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { } public unsafe static void* Unbox(object ptr) { throw null; } + public override bool Equals(object? obj) { throw null; } + public override int GetHashCode() { throw null; } } [System.FlagsAttribute] public enum PortableExecutableKinds From 60ecf5ded7261f054f08d5eb34197fd6de21a64f Mon Sep 17 00:00:00 2001 From: Dimension4 Date: Fri, 2 Oct 2020 08:44:51 +0200 Subject: [PATCH 5/6] Added more Pointer tests Added tests for GetHashCode Renamed tests Added negative tests --- .../System.Reflection/tests/PointerTests.cs | 53 +++++++++++++++++-- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Reflection/tests/PointerTests.cs b/src/libraries/System.Reflection/tests/PointerTests.cs index 32a82e28ee4237..55379ee6075916 100644 --- a/src/libraries/System.Reflection/tests/PointerTests.cs +++ b/src/libraries/System.Reflection/tests/PointerTests.cs @@ -20,7 +20,7 @@ public unsafe struct MemberwiseComparable } [Fact] - public unsafe void BitwiseEquality_AreEqual() + public unsafe void EqualBitwiseComparables_AreEqual() { int someNumber = 1; var a = new BitwiseComparable(); @@ -28,10 +28,22 @@ public unsafe void BitwiseEquality_AreEqual() BitwiseComparable b = a; Assert.True(a.Equals(b)); + Assert.Equal(a.GetHashCode(), b.GetHashCode()); } [Fact] - public unsafe void BitwiseEquality_EqualWithSelf() + public unsafe void UnequalBitwiseComparables_AreUnequal() + { + int someNumber = 1; + var a = new BitwiseComparable(); + a.PublicInt = &someNumber; + var b = new BitwiseComparable(); + + Assert.False(a.Equals(b)); + } + + [Fact] + public unsafe void SameBitwiseComparable_EqualWithSelf() { int someNumber = 1; var a = new BitwiseComparable(); @@ -41,7 +53,7 @@ public unsafe void BitwiseEquality_EqualWithSelf() } [Fact] - public unsafe void MemberwiseEquality_AreEqual() + public unsafe void EqualMemberwiseComparables_AreEqual() { int someNumber = 1; var a = new MemberwiseComparable(); @@ -49,10 +61,22 @@ public unsafe void MemberwiseEquality_AreEqual() MemberwiseComparable b = a; Assert.True(a.Equals(b)); + Assert.Equal(a.GetHashCode(), b.GetHashCode()); } [Fact] - public unsafe void MemberwiseEquality_EqualWithSelf() + public unsafe void UnequalMemberwiseComparables_AreUnequal() + { + int someNumber = 1; + var a = new MemberwiseComparable(); + a.PublicInt = &someNumber; + var b = new MemberwiseComparable(); + + Assert.False(a.Equals(b)); + } + + [Fact] + public unsafe void SameMemberwiseComparable_EqualWithSelf() { int someNumber = 1; var a = new MemberwiseComparable(); @@ -61,6 +85,16 @@ public unsafe void MemberwiseEquality_EqualWithSelf() Assert.True(a.Equals(a)); } + [Fact] + public unsafe void EqualPointers_AreEqual() + { + object a = Pointer.Box((void*)0x12340000, typeof(int*)); + object b = Pointer.Box((void*)0x12340000, typeof(int*)); + + Assert.True(a.Equals(b)); + Assert.Equal(a.GetHashCode(), b.GetHashCode()); + } + [Fact] public unsafe void Nullptrs_AreEqual() { @@ -68,6 +102,7 @@ public unsafe void Nullptrs_AreEqual() object b = Pointer.Box(null, typeof(int*)); Assert.True(a.Equals(b)); + Assert.Equal(a.GetHashCode(), b.GetHashCode()); } [Fact] @@ -77,6 +112,16 @@ public unsafe void DifferentPointerTypes_AreEqual() object b = Pointer.Box((void*)0x12340000, typeof(uint*)); Assert.True(a.Equals(b)); + Assert.Equal(a.GetHashCode(), b.GetHashCode()); + } + + [Fact] + public unsafe void DifferentPointers_AreUnequal() + { + object a = Pointer.Box((void*)0x12340000, typeof(int*)); + object b = Pointer.Box((void*)0x56780000, typeof(int*)); + + Assert.False(a.Equals(b)); } } } From e67a31af9df1c2cea2703de935716314c43c09c7 Mon Sep 17 00:00:00 2001 From: Dimension4 Date: Sat, 17 Oct 2020 12:44:10 +0200 Subject: [PATCH 6/6] Added another Pointer null test --- src/libraries/System.Reflection/tests/PointerTests.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libraries/System.Reflection/tests/PointerTests.cs b/src/libraries/System.Reflection/tests/PointerTests.cs index 55379ee6075916..cbcf7828df59ce 100644 --- a/src/libraries/System.Reflection/tests/PointerTests.cs +++ b/src/libraries/System.Reflection/tests/PointerTests.cs @@ -123,5 +123,14 @@ public unsafe void DifferentPointers_AreUnequal() Assert.False(a.Equals(b)); } + + [Fact] + public unsafe void DifferentPointerTypes_Null_AreEqual() + { + object a = Pointer.Box(null, typeof(int*)); + object b = Pointer.Box(null, typeof(long*)); + + Assert.True(a.Equals(b)); + } } }