Interpreter P/Invoke support

[kg]

• Implements the static field intrinsic for IntPtr.Zero
• Adds a CALL_PINVOKE opcode that is similar to CALL but has a second data item containing the address-of-address or address the target native function to call
• Modified InvokeCompiledMethod to accept the target address separately instead of fetching it from the MethodDesc unconditionally, so we can reuse it for p/invoke
• Adds a custom C++ library with some functions to call via pinvoke for testing
• Fixes CEE_LDC_R8 (it was loading nans sometimes for reasons I can't identify)
• Adds a couple basic tests for p/invoke, with more commented out that don't work yet
• Fixes some whitespace errors left over from my boxing PR

[dotnet-policy-service]

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

[kg]

cc @BrzVlad @janvorli
Main remaining problem here is the check inside the ndirect import worker. I don't really know what the right fix is, other than updating it to be happy seeing an interpreter frame.

[janvorli]

@AaronRobinsonMSFT all the things you've commented on are from my WIP PR that I am still cleaning up, Katelyn has built her stuff on top of that.

[jkotas]

This is an optimization (JIT calls it PInvoke inlining) that is allowed only when pInvokeMarshalingRequired returns false.

When pInvokeMarshalingRequired returns true, the PInvoke method should be called like a regular method. It is fine to call the PInvoke method like a regular method even when pInvokeMarshalingRequired returns false. The regular JIT does that when optimizations are off, in cold code, or to resolve problematic interactions with exception handling.

[jkotas]

The regular JIT does that

impCanPInvokeInline and impCanPInvokeInlineCallSite are the methods that decide whether to do PInvoke inlining in regular JIT

[kg]

Makes sense. So if marshaling is required I should update this to go through the normal call path, and the target method will have IL? I will try to cook up a test for that.

[jkotas]

Right.

Or do not worry about PInvoke inlining at all in the interpreter and call all PInvokes as regular methods. PInvoke inlining is an optimization. Interpreter JIT does not do any optimizations today, and building an optimizing interpreter JIT should be a separate project.

the target method will have IL?

Yes, the target method will have IL.

[kg]

I tried calling the pinvoke target as a regular method and it asserted, but maybe it will work now that the native call PR is in. I'll test it.

[jkotas]

I expect that we will need to do something about the hidden IL stub arguments and PInvoke IL stub sharing to make the marshalling stubs work. The easiest path may be to disable PInvoke IL stub sharing in interpreter mode (or get rid of it completely - it may not be as important as it used to be).

[kg]

Assert failure(PID 56980 [0x0000de94], Thread: 57112 [0xdf18]): IsIL() || IsNoMetadata()

CORECLR! MethodDesc::PrepareCode + 0x1F8 (0x00007ffa`16b59ee8)
CORECLR! MethodDesc::PrepareInitialCode + 0x231 (0x00007ffa`16b5a991)
CORECLR! InterpExecMethod + 0x67C3 (0x00007ffa`16dbb313)
CORECLR! ExecuteInterpretedMethod + 0xE1 (0x00007ffa`16b5dbc1)

This is what happens if I do a normal call to invoke a non-marshaled PInvoke method. Am I doing something wrong? When I first encountered it I went "That makes sense, it's not an IL method".

[kg]

This assert also happens if I try to invoke a marshaled pinvoke method as if it were a regular IL method.

[jkotas]

The interpreter related code in prestub.cpp (and other places) is not prepared to handle PInvokes.

We should agree on how we are going to deal with sharing of marshalling stubs first; and then do the required changes.

[jkotas]

cc @AaronRobinsonMSFT

[kg]

The working directory change is controversial, so I'm open to changing it around and making InterpreterTester be the one that builds the dll instead.

IMO Interpreter.dll should be built alongside pinvoke.dll since that's the actual dependency, and we should be running it where-it-was-built instead of in InterpreterTester's artifacts dir. IIRC msbuild has never been able to express 'when foo.csproj is referenced by bar.csproj, copy these satellite files along with foo.dll'.

I like being able to directly run Interpreter.dll with corerun in the debugger and have it work correctly without having to know to set a special cwd. But I can see the argument for doing it the other way. This will fail in a less confusing way once we have EH, at least - at that point you would see the dll not found error and be able to figure out what's wrong pretty quickly.