-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix field accessor for RVA field #103705
Fix field accessor for RVA field #103705
Conversation
This reverts commit 089ae12.
Tagging subscribers to this area: @dotnet/area-system-reflection |
{ | ||
byte[] valueArray = new byte[] { 1, 2, 3, 4, 5 }; | ||
|
||
// Roslyn uses SHA256 of raw data as data field name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting; this is likely why the original RVA tests added with the fast field feature didn't fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a test failure on macOS-12.7.2. Does Mono do things differently?
[FAIL] System.Reflection.Tests.FieldInfoTests.GetValueFromRvaField
System.NotSupportedException : This object cannot be invoked because no code was generated for it: '<PrivateImplementationDetails>.74F81FE167D99B4CB41D6D0CCDA82278CAEE9F3E2F25D5E5A3936FF3DCEC60D0'.
at System.Reflection.Runtime.FieldInfos.RuntimeFieldInfo.get_FieldAccessor() + 0xe8
at System.Reflection.Runtime.FieldInfos.RuntimeFieldInfo.GetValue(Object) + 0x14
at System.Reflection.Tests.FieldInfoTests.GetValueFromRvaField() + 0x120
at System.Reflection!<BaseAddress>+0xb29b40
at System.Reflection.DynamicInvokeInfo.Invoke(Object, IntPtr, Object[], BinderBundle, Boolean) + 0x10c
It's NativeAOT. Seems that NativeAOT doesn't support reflection on RVA field. Let's see if it can easily be added or just disable the test. @dotnet/ilc-contrib |
I do not think that it is something we want enable. Could you please disable the test for NAOT? |
Agreed with Jan. This would only be useful for two things: C++/CLI support and support for (non-intrinsic) RuntimeHelpers.InitializeArray. Native AOT has both listed as unplanned in #69919. The whole reflection with RVA static field business would ideally be just blocked everywhere, it has always been buggy - calling SetValue on RVA statics on .NET Framework crashes the runtime badly, and accessing RVA statics in the TLS range doesn't work as expected (accessing fields marked |
The failures look unrelated now. |
Fixes #103207