[CoreCLR] Implement support for UnsafeAccessorAttribute
#86161
Comments
dotnet-issue-labeler
bot
added
the
needs-area-label
AaronRobinsonMSFT
added
area-VM-coreclr
and removed
untriaged
AaronRobinsonMSFT
changed the title
UnsafeAccesorAttributeUnsafeAccessorAttribute
|
@AaronRobinsonMSFT do you want us to create a separate issue for NativeAOT support, or do you want to track that here as well? Similar for crossgen - not sure if it needs to do anything, but at the very least it needs testing. |
Possibly? I haven't thought too much about the needs in NativeAOT. My assumption is the compiler should be able to peer through at codegen time and emit the code. However, I suppose the stub needs to be generated. Hmmm yeah, a new issue is probably helpful for tracking at least. |
|
NativeAOT work is probably the same as crossgen work if we want this for crossgen. Some thought for crossgen is needed because R2R only guarantees same versionability as the source IL and accessing privates goes beyond what we consider binary breaks for IL. |
|
Oh I take that back, it's likely not possible in crossgen now because crossgen doesn't (AFAIK) have a way to manufacture new tokens and we'd likely need new ones for this |
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.
|
Open a new tracking issue for the generics support and close this one as completed? |
Yep, that was the plan. Someone is pushing on this earlier than anticipated ;-) |
ghost
locked as resolved and limited conversation to collaborators
Implement #81741 for CoreCLR.
UnsafeAccessorAttributeas a well-known attribute to the runtime [CoreCLR/NativeAOT]UnsafeAccessorAttributenon-generic support #86932We will need to extract a named string-value property from the custom attribute - duplicate work item with mono.
UnsafeAccessorAttributenon-generic support #86932UnsafeAccessorAttributenon-generic support #86932UnsafeAccessorAttributenon-generic support #86932UnsafeAccessorAttributeSee Mono work item - #86040
See NativeAOT work item - #86438
The text was updated successfully, but these errors were encountered: