Implement support for UnsafeAccessor in the trimmer

[vitek-karas]

The core of the change is that UnsafeAccessor creates a code dependency from the accessor method to the target specified by the attribute. The trimmer needs to follow this dependency and preserve the target. Additionally, because the trimmer operates at the IL level, it needs to make sure that the target will keep its name and some other properties intact (so that the runtime implementation of the UnsafeAccessor can still work).

Implementation choices:

• The trimmer will mark the target as "accessed via reflection", this is a simple way to make sure that name and other properties about the target are preserved. This could be optimized in the future, but the savings are probably not that interesting.
• The implementation ran into a problem when trying to precisely match the signature overload resolution. Due to Cecil issues and the fact that Cecil's resolution algorithm is not extensible, it was not possible to match the runtime's behavior without adding lot more complexity (currently it seems we would have to reimplement method resolution in the trimmer). So, to simplify the implementation, trimmer will mark all methods of a given name. This means it will mark more than necessary. This is fixable by adding more complexity to the code base if we think there's a good reason for it.
• Due to the above choices, there are some behavioral differences:
  • Trimmer will warn if the target has data flow annotations, always. There's no way to "fix" this in the code without a suppression.
  • Trimmer will produce different warning codes even if there is a true data flow mismatch - this is because it treats the access as "reflection access" which produces different warning codes from direct access.
  • These differences are fixable, but it was not deemed necessary right now.
• We decided that analyzer will not react to the attribute at all, and thus will not produce any diagnostics around it.

The guiding reason to keep the implementation simple is that we don't expect the unsafe accessor to be used by developers directly, instead we assume that vast majority of its usages will be from source generators. So, developer UX is not as important.

Test changes:

• Adds directed tests for the marking behavior
• Adds tests to verify that Requires* attributes behave correctly
• Adds tests to verify that data flow annotations behave as expected (described above)
• The tests are effectively a second validation of the NativeAOT implementation as they cover NativeAOT as well.

Fixes in CoreCLR/NativeAOT:
This change fixes one bug in the CoreCLR/NativeAOT implementation, unsafe accessor on a instance method of a value type must use "by-ref" parameter for the this parameter. Without the "by-ref" the accessor is considered invalid and will throw.
This change also adds some tests to the CoreCLR/NativeAOT test suite.

Part of #86161.
Related to #86438.
Feature design in #81741.

[ghost]

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

The core of the change is that UnsafeAccessor creates a code dependency from the accessor method to the target specified by the attribute. The trimmer needs to follow this dependency and preserve the target. Additionally, because the trimmer operates at the IL level, it needs to make sure that the target will keep its name and some other properties intact (so that the runtime implementation of the UnsafeAccessor can still work).

Implementation choices:

• The trimmer will mark the target as "accessed via reflection", this is a simple way to make sure that name and other properties about the target are preserved. This could be optimized in the future, but the savings are probably not that interesting.
• The implementation ran into a problem when trying to precisely match the signature overload resolution. Due to Cecil issues and the fact that Cecil's resolution algorithm is not extensible, it was not possible to match the runtime's behavior without adding lot more complexity (currently it seems we would have to reimplement method resolution in the trimmer). So, to simplify the implementation, trimmer will mark all methods of a given name. This means it will mark more than necessary. This is fixable by adding more complexity to the code base if we think there's a good reason for it.
• Due to the above choices, there are some behavioral differences:
  • Trimmer will warn if the target has data flow annotations, always. There's no way to "fix" this in the code without a suppression.
  • Trimmer will produce different warning codes even if there is a true data flow mismatch - this is because it treats the access as "reflection access" which produces different warning codes from direct access.
  • These differences are fixable, but it was not deemed necessary right now.
• We decided that analyzer will not react to the attribute at all, and thus will not produce any diagnostics around it.

The guiding reason to keep the implementation simple is that we don't expect the unsafe accessor to be used by developers directly, instead we assume that vast majority of its usages will be from source generators. So, developer UX is not as important.

Test changes:

• Adds directed tests for the marking behavior
• Adds tests to verify that Requires* attributes behave correctly
• Adds tests to verify that data flow annotations behave as expected (described above)
• The tests are effectively a second validation of the NativeAOT implementation as they cover NativeAOT as well.

Fixes in CoreCLR/NativeAOT:
This change fixes one bug in the CoreCLR/NativeAOT implementation, unsafe accessor on a instance method of a value type must use "by-ref" parameter for the this parameter. Without the "by-ref" the accessor is considered invalid and will throw.
This change also adds some tests to the CoreCLR/NativeAOT test suite.

Part of #86161.
Related to #86438.
Feature design in #81741.

Author:	vitek-karas
Assignees:	vitek-karas
Labels:	area-Tools-ILLink
Milestone:	8.0.0

[AaronRobinsonMSFT]

NativeAOT/VM changes and new runtime tests LGTM.

Thank you!

[lambdageek]

@vitek-karas I resolved the merge conflict

[sbomer]

ILLink changes LGTM!

[sbomer]

Based on the build failures I think this can be removed now

[vitek-karas]

Yes - I'm testing this locally - there are also some merge conflicts in the runtime test. I need to test it on all 3 runtimes now :-)

[vitek-karas]

Test failures are unrelated.