Skip to content

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