From 6e3f4cb7bdb2a7cf67a9a995ce8bb137e65f9fe2 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 20 Jun 2024 00:12:53 +0800 Subject: [PATCH 1/3] Revert "Separate RVA reflection change out" This reverts commit 089ae127d131790998eac0ffbb815a9a6577f9e2. --- src/coreclr/vm/reflectioninvocation.cpp | 14 +++++---- .../System.Reflection.Tests/FieldInfoTests.cs | 31 +++++++++++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp index a98dafd62a558b..c89407275f81e6 100644 --- a/src/coreclr/vm/reflectioninvocation.cpp +++ b/src/coreclr/vm/reflectioninvocation.cpp @@ -1246,14 +1246,16 @@ FCIMPL1(void*, RuntimeFieldHandle::GetStaticFieldAddress, ReflectFieldObject *pF // IsFastPathSupported needs to checked before calling this method. _ASSERTE(IsFastPathSupportedHelper(pFieldDesc)); - PTR_BYTE base = 0; - if (!pFieldDesc->IsRVA()) + if (pFieldDesc->IsRVA()) { - // For RVA the base is ignored and offset is used. - base = pFieldDesc->GetBase(); + Module* pModule = pFieldDesc->GetModule(); + return pModule->GetRvaField(pFieldDesc->GetOffset()); + } + else + { + PTR_BYTE base = pFieldDesc->GetBase(); + return PTR_VOID(base + pFieldDesc->GetOffset()); } - - return PTR_VOID(base + pFieldDesc->GetOffset()); } FCIMPLEND diff --git a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs index d854e19c71ade7..fbc97e4f17dad0 100644 --- a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs +++ b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs @@ -5,6 +5,8 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using Xunit; #pragma warning disable 0414 @@ -121,6 +123,30 @@ public void GetValueWithFunctionPointers(Type type, string name, object obj, obj Assert.Equal(expected, fieldInfo.GetValue(obj)); } + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/103207")] + public void GetValueFromRvaField() + { + byte[] valueArray = new byte[] { 1, 2, 3, 4, 5 }; + + // Roslyn uses SHA256 of raw data as data field name + FieldInfo fieldInfo = GetField( + typeof(FieldInfoTests).Assembly.GetType(""), + "74F81FE167D99B4CB41D6D0CCDA82278CAEE9F3E2F25D5E5A3936FF3DCEC60D0"); + Assert.NotNull(fieldInfo); + Assert.True(fieldInfo.IsStatic); + Assert.True((fieldInfo.Attributes & FieldAttributes.HasFieldRVA) != 0); + + for (int i = 0; i < 5; i++) + { + // FieldAccessor uses slow path until the class is initialized. + // Make sure subsequent invocations also succeed. + + object value = fieldInfo.GetValue(null); + Assert.Equal(valueArray, MemoryMarshal.CreateReadOnlySpan(ref Unsafe.As(ref value).Data, valueArray.Length)); + } + } + [Fact] public void GetAndSetValueTypeFromStatic() { @@ -839,5 +865,10 @@ public static class SettingsForMyTypeThatThrowsInClassInitializer { public static bool s_shouldThrow = true; } + + private class RawData + { + public byte Data; + } } } From cc0960003e7ec66a22011588a7d95d5b9969fa4d Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 9 Jun 2024 21:44:14 +0800 Subject: [PATCH 2/3] Fix FieldAccessor getting RVA field --- .../src/System/Reflection/FieldAccessor.cs | 7 ++++++- .../tests/System.Reflection.Tests/FieldInfoTests.cs | 1 - 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs index f8d9f6b1ba1761..82e485fab51ff8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/FieldAccessor.cs @@ -48,7 +48,12 @@ private void Initialize() { _addressOrOffset = RuntimeFieldHandle.GetStaticFieldAddress(_fieldInfo); - if (fieldType.IsValueType) + if ((_fieldInfo.Attributes & FieldAttributes.HasFieldRVA) != 0) + { + _methodTable = (MethodTable*)fieldType.TypeHandle.Value; + _fieldAccessType = FieldAccessorType.StaticValueType; + } + else if (fieldType.IsValueType) { if (fieldType.IsEnum) { diff --git a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs index fbc97e4f17dad0..cebc662674b57f 100644 --- a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs +++ b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs @@ -124,7 +124,6 @@ public void GetValueWithFunctionPointers(Type type, string name, object obj, obj } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/103207")] public void GetValueFromRvaField() { byte[] valueArray = new byte[] { 1, 2, 3, 4, 5 }; From 6c72ca0ae4ff7b27bb992a67390a3464101d2c70 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Thu, 20 Jun 2024 22:17:10 +0800 Subject: [PATCH 3/3] Disable test on NativeAot --- .../tests/System.Reflection.Tests/FieldInfoTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs index cebc662674b57f..c3ff97fb342297 100644 --- a/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs +++ b/src/libraries/System.Runtime/tests/System.Reflection.Tests/FieldInfoTests.cs @@ -123,7 +123,8 @@ public void GetValueWithFunctionPointers(Type type, string name, object obj, obj Assert.Equal(expected, fieldInfo.GetValue(obj)); } - [Fact] + // RVA field reflection is not supported in NativeAot + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotNativeAot))] public void GetValueFromRvaField() { byte[] valueArray = new byte[] { 1, 2, 3, 4, 5 };