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

PInvoke calli #5960

Merged
merged 3 commits into from
Jul 2, 2018
Merged

PInvoke calli #5960

merged 3 commits into from
Jul 2, 2018

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Jun 19, 2018

Replace PInvoke calli with a regular call to a marshaling stub as necessary.

Fixes #5587

@jkotas jkotas requested a review from MichalStrehovsky June 19, 2018 00:17

public override MethodIL EmitIL()
{
// TODO: Emit marshaling stub
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing this TODO is about refactoring the marshaling code generator. I am leaving it for follow up PR.

@jkotas
Copy link
Member Author

jkotas commented Jun 19, 2018

Depends on dotnet/coreclr#18534

for (int i = 0; i < _targetSignature.Length; i++)
blittable = blittable && MarshalHelpers.IsBlittableType(_targetSignature[i]);
if (!blittable)
throw new NotSupportedException("PInvoke Calli with non-blittable type");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on how long term this code is expected to stay - maybe this could generate a throwing body. I think throwing this exception type here will lead to a compilation failure.

var emitter = new ILEmitter();
var stream = emitter.NewCodeStream();

for (int i = 0; i < _signature.Length; i++)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably access the property to avoid reading uninitialized value.

namespace Internal.TypeSystem
{
/// <summary>
/// When implemented by a <see cref="TypeDesc"/> or <see cref="MethodDesc"/>, instructs a name mangler to use the same mangled name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the comment should probably say this is only for methods.

Debug.Assert(callConv != CorInfoCallConv.CORINFO_CALLCONV_C ||
callConv != CorInfoCallConv.CORINFO_CALLCONV_STDCALL ||
callConv != CorInfoCallConv.CORINFO_CALLCONV_THISCALL ||
callConv != CorInfoCallConv.CORINFO_CALLCONV_FASTCALL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these be ==? This is always true.

jkotas and others added 3 commits June 28, 2018 17:33
This is a building block to be able to write a repro case for #5587 in C#. With this, it's possible to use `AddrOf` intrinsic in connection with `StdCall` to hit the unimplemented functionality with pure C#.
@jkotas
Copy link
Member Author

jkotas commented Jul 2, 2018

@dotnet-bot test this please

@jkotas
Copy link
Member Author

jkotas commented Jul 2, 2018

All ducks should be in a row now. @MichalStrehovsky Could you please take a look one more time?

@mikedn
Copy link
Contributor

mikedn commented Jul 2, 2018

Please don't shoot the ducks 😁

Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is huge, thanks!

@MichalStrehovsky MichalStrehovsky merged commit ad22792 into dotnet:master Jul 2, 2018
@jkotas jkotas deleted the calli branch July 2, 2018 10:21
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.

4 participants