Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Fix reverse marshalling of refs to blittable types#8140

Merged
jkotas merged 1 commit intodotnet:masterfrom
MichalStrehovsky:byrefInterop
May 8, 2020
Merged

Fix reverse marshalling of refs to blittable types#8140
jkotas merged 1 commit intodotnet:masterfrom
MichalStrehovsky:byrefInterop

Conversation

@MichalStrehovsky
Copy link
Member

Hit in selfhosted ilc.exe. In JitInterface we have a couple cases where we do reverse p/invoke with a delegate that has a ref parameter. Sometimes the ref is null (and we don't touch it).

The marshaller was trying to make a copy of the pointed to value, NullRefing in the process because the pointer was null. We don't actually need to make a copy because the value is blittable.

Hit in selfhosted ilc.exe. In JitInterface we have a couple cases where we do reverse p/invoke with a delegate that has a ref parameter. Sometimes the `ref` is null (and we don't touch it).

The marshaller was trying to make a copy of the pointed to value, NullRefing in the process because the pointer was null. We don't actually need to make a copy because the value is blittable.
@jkotas jkotas merged commit 1b7971a into dotnet:master May 8, 2020
@jkotas
Copy link
Member

jkotas commented May 8, 2020

Should this be backported to dotnet/runtime?

@MichalStrehovsky MichalStrehovsky deleted the byrefInterop branch May 8, 2020 14:11
@MichalStrehovsky
Copy link
Member Author

Should this be backported to dotnet/runtime?

Yes, it's a shared file. We would overwrite this on the next update from dotnet/runtime.

This code path is not active in dotnet/runtime though because we don't marshal in the reverse direction (p/invoke doesn't support ref returns and we don't precompile delegates).

@jkotas
Copy link
Member

jkotas commented May 8, 2020

dotnet/runtime#36119

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants