-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Reenable unmanagedcallersonly related tests #35926
Reenable unmanagedcallersonly related tests #35926
Conversation
// it for UnmanagedCallersOnlyAttribute usage. There are cases | ||
// where the validation doesn't handle all of the cases we can | ||
// permit during stub generation (e.g. Vector2 returns). | ||
if (!ftn->IsILStub()) |
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.
Could you elaborate why this is a problem today? I'm not quite connecting how or why we have special handling for Vector2/3/4
?
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.
It was always a problem given our existing checking mechanism. We recently moved this check to catch all cases instead of just cases that result from a query by the JIT to getCallInfo()
. Now we don't miss any and this deficiency in our validation was discovered. The Vector*
types were just the thing that popped,
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.
MarshalingRequired rejects non-primitive value types here:
runtime/src/coreclr/src/vm/dllimport.cpp
Line 3506 in ddd458e
// return value is fine as long as it can be normalized to an integer |
The problem is that the JIT did not always get the return buffers right for non-primitive value types and so the interop IL stub generation compensated for it via a very complex hack.
This should go away once #12375 is fixed.
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.
I have opened #35928 so that we make the unmanaged function pointers returning non-primitive value types work for .NET 5 at least.
The Windows_NT x86 checked failure was fixed in #35904 |
See #35798
Issues were introduced in #35763
/cc @tannergooding @jkotas @jkoritzinsky @elinor-fung