Skip to content
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

Conversation

AaronRobinsonMSFT
Copy link
Member

See #35798

Issues were introduced in #35763

/cc @tannergooding @jkotas @jkoritzinsky @elinor-fung

// 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())
Copy link
Member

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?

Copy link
Member Author

@AaronRobinsonMSFT AaronRobinsonMSFT May 7, 2020

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,

Copy link
Member

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:

// 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.

Copy link
Member

@jkotas jkotas May 7, 2020

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.

@AaronRobinsonMSFT
Copy link
Member Author

AaronRobinsonMSFT commented May 7, 2020

The Windows_NT x86 checked failure was fixed in #35904

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants