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.

Sign up for GitHub

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

Jump to bottom

UnmanagedCallersOnlyAttribute returning non-primitive value types do not work #35928

Closed
jkotas opened this issue May 7, 2020 · 5 comments · Fixed by #45625
Closed

UnmanagedCallersOnlyAttribute returning non-primitive value types do not work #35928

jkotas opened this issue May 7, 2020 · 5 comments · Fixed by #45625
Labels
area-Interop-coreclr
Milestone
6.0.0

Comments

@jkotas
Copy link
Member

jkotas commented May 7, 2020 

[UnmanagedCallersOnly]
static Vector2 MyCallback() { ... }

Expected: works fine
Actual: fails

More context: #35926 (comment)
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label May 7, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label.

@jkotas jkotas mentioned this issue May 7, 2020
Merged
@jkotas jkotas added this to the 5.0 milestone May 7, 2020
@AaronRobinsonMSFT AaronRobinsonMSFT removed the untriaged New issue has not been triaged by the area owner label May 11, 2020
@jkoritzinsky
Copy link
Member

Since this depends on a fix for #12375 and #39294 isn't going to make .NET 5, I'm going to move this to .NET 6.

@jkoritzinsky jkoritzinsky modified the milestones: 5.0.0, 6.0.0 Aug 7, 2020
jkoritzinsky added a commit to jkoritzinsky/runtime that referenced this issue Dec 4, 2020
@jkoritzinsky
Allow non-primitive struct returns to not require a stub. Fixes dotne…
0302e18 
…t#35928.
@jkoritzinsky jkoritzinsky mentioned this issue Dec 5, 2020
Merged
@marek-safar
Copy link
Contributor

/cc @vargaz

jkoritzinsky added a commit that referenced this issue Dec 15, 2020
@jkoritzinsky @jkotas
Enable non-blittable struct returns on UnmanagedCallersOnly (#45625)
c64861b 
* Allow non-primitive struct returns to not require a stub. Fixes #35928.

* Support propagating the UnmanagedCallersOnly calling convention to the JIT. Block UnmanagedCallersOnly in crossgen1 since the attribute parsing code isn't included.

* Support passing through the calling convention for UnmanagedCallersOnly in crossgen2

* Fix clang errors.

* Fix stack calculation.

* Fix usings

* Clean up jitinterface.

* Remove invalid assert.

* Fix up stdcall name mangling lookup.

* Fix flag condition.

* Use the register var type when copying from the register to the stack.

* Change flag check for readability.

* Rename variables to remove shadowing.

* Fix formatting.

* Create new getEntryPointCallConv method on the EE-JIT interface to handle all calling convention resolution and support extensible calling conventions.

* Remove unreachable code.

* Remove now unused getUnmanagedCallConv jitinterface method (replaced by getEntryPointCallConv).

* Fix formatting.

* Rename getEntryPointCallConv and only call it with a method when it's a P/Invoke or Reverse P/Invoke.

* Pass SuppressGCTransition through the getUnmanagedCallConv JIT-EE interface API.

* Refactor callconv handling so we can handle reverse P/Invokes with the callconv in the signature (and not in an UnmanagedCallersOnly attribute).

* Clean up whitespace.

* Pass MethodIL as the scope for the signature to enable propagating down annotations for calli's in P/Invokes.

* Remove usages of CORINFO_CALLCONV_FOO where FOO is an unmanaged callconv. move the definitions of those ones to the interpreter since that's the only place they're used.

* SuppressGC cleanup.

* Rename superpmi struct

* Add default condition to make clang happy.

* change enums to make clang happy.

* Remove CORINFO_CALLCONV_C and family from interpreter.

* Fix up handling of managed function pointers and remove invalid assert.

* Continue to use sigflag for suppressgc workaround.

* Clean up comment wording.

Signed-off-by: Jeremy Koritzinsky <[email protected]>

* Remove more MethodIL passing we don't need any more

* Update src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs

Co-authored-by: Jan Kotas <[email protected]>

* Fix SigTypeContext creation.

* Pass context by ptr.

* Fix formatting.

* Clear the Reverse P/Invoke flag when compiling inlinees. It's only needed on the entry-point functions.

Co-authored-by: Jan Kotas <[email protected]>
@vargaz
Copy link
Contributor

vargaz commented Dec 15, 2020

How is this going to work without marshal attributes ?

@jkoritzinsky
Copy link
Member

The returned value type must be a blittable value type. As a result, it doesn’t require marshalling and the marshal attributes aren’t needed.

@ghost ghost locked as resolved and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area-Interop-coreclr
Projects
None yet
Milestone
6.0.0
Development

Successfully merging a pull request may close this issue.

Enable non-blittable struct returns on UnmanagedCallersOnly jkoritzinsky/runtime
6 participants
@vargaz @marek-safar @jkoritzinsky @jkotas @AaronRobinsonMSFT @Dotnet-GitSync-Bot