Add simple marshaller which marshal only null COM interfaces #8128

kant2002 · 2020-05-02T17:19:25Z

This idea was proposed by @MichalStrehovsky in #7994

Implemented only marshalling of in parameters, since this is case which I found in the stack trace of the issue. I think other directions can be implemented in same way, I just lack of examples for testing.

Tested code

// [DllImport(Libraries.UiaCore, CharSet = CharSet.Unicode)]
// public static extern IntPtr UiaReturnRawElementProvider(IntPtr hwnd, IntPtr wParam, IntPtr lParam, IRawElementProviderSimple el);
IntPtr result = UiaCore.UiaReturnRawElementProvider(IntPtr.Zero, IntPtr.Zero, IntPtr.Zero, null);

Closes #7994

@MichalStrehovsky

This idea was proposed by @MichalStrehovsky in #7994 Implemented only marshalling of `in` parameters, since this is case which I found in the stack trace of the issue. I think other directions can be implemented in same way, I just lack of examples for testing. Tested code ``` // [DllImport(Libraries.UiaCore, CharSet = CharSet.Unicode)] // public static extern IntPtr UiaReturnRawElementProvider(IntPtr hwnd, IntPtr wParam, IntPtr lParam, IRawElementProviderSimple el); IntPtr result = UiaCore.UiaReturnRawElementProvider(IntPtr.Zero, IntPtr.Zero, IntPtr.Zero, null); ``` Closes #7994

jkotas · 2020-05-02T17:57:08Z

src/Common/src/TypeSystem/Interop/IL/Marshaller.cs

+            ILEmitter emitter = _ilCodeStreams.Emitter;
+
+            // Check if we marshall null pointer
+            ILCodeLabel lNullInterface = emitter.NewCodeLabel();


It would be better to call a helper method to do this instead of inlining the logic here.

Not sure what should I do. If you asking about labels creation, then this is how BlittableArrayMarshaller implemented. In a sense for sure, but they are directly created there. I understand that maybe I can create helper method for reporting that scenario not supported (throwing exception code) but you point to different location. I'm lost. Can you clarify?

I meant add a method to InteropHelpers and just call it from here to do all the work. Many marshalers are implemented like that.

Do I properly understand you, that I add code in the System.Private.CoreLib and then marshaller will just assume that classlib has that method, and just call that method. Am I correctly understand you?

Yes, it is exactly what I meant.

@jkotas It's magic. Much easier then I initially thought how it should be done!

…rary

jkotas · 2020-05-03T02:36:49Z

src/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs

@@ -407,6 +407,26 @@ public static IntPtr GetCurrentCalleeOpenStaticDelegateFunctionPointer()
            return PInvokeMarshal.GetCurrentCalleeDelegate<T>();
        }

+        public static object ConvertManagedComInterfaceToNative(object pUnk)


Suggested change

public static object ConvertManagedComInterfaceToNative(object pUnk)

public static IntPtr ConvertManagedComInterfaceToNative(object pUnk)

The type to use for the unmanaged side should be IntPtr.

(multiple places)

jkotas · 2020-05-03T02:37:25Z

src/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs

+                return null;
+            }
+
+            throw new PlatformNotSupportedException("Marhalling of non null COM interfaces does not supported");


Suggested change

throw new PlatformNotSupportedException("Marhalling of non null COM interfaces does not supported");

throw new PlatformNotSupportedException("Marhalling of non-null COM interfaces is not supported");

MichalStrehovsky

Looks good modulo the type thing, thanks!

Would you mind adding a test for this under tests\src\Simple\PInvoke? There's a CPP file to put unmanaged code into, and a CS file with managed code.

MichalStrehovsky · 2020-05-04T06:23:31Z

src/Common/src/TypeSystem/Interop/IL/MarshalHelpers.cs

@@ -68,6 +68,7 @@ public static partial class MarshalHelpers
                case MarshallerKind.BlittableStruct:
                case MarshallerKind.Decimal:
                case MarshallerKind.VoidReturn:
+                case MarshallerKind.ComInterface:


This should be some kind of a pointer, not type. E.g. return context.GetWellKnownType(WellKnownType.IntPtr);

(This is the native type we marshal into. Leaving this at type would hit RyuJIT asserts added in dotnet/runtime#35026 because type is a GC pointer. Once marshalling is done, this is an unmanaged pointer.)

MichalStrehovsky · 2020-05-04T06:28:43Z

src/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs

+                return IntPtr.Zero;
+            }
+
+            throw new PlatformNotSupportedException("Marhalling of non null COM interfaces is not supported");


Suggested change

throw new PlatformNotSupportedException("Marhalling of non null COM interfaces is not supported");

throw new PlatformNotSupportedException(SR.PlatformNotSupported_ComInterop);

kant2002 · 2020-05-04T22:20:56Z

@MichalStrehovsky I was little bit lazy and do not create specific functions in native code. I take existing sample which I have. Should I implement function for marshalling in native code?

MichalStrehovsky · 2020-05-05T08:03:14Z

Should I implement function for marshalling in native code?

Yes please - it will be even less code that way (the interface doesn't need to have any members, etc.).

I don't know what UI automation would do with this call - I don't want the test to accidentally break screen readers, so it's better to call into "known" code.

jkotas reviewed May 2, 2020

View reviewed changes

Move logic for COM interface transformation into compiler support lib…

27f8fdc

…rary

jkotas reviewed May 3, 2020

View reviewed changes

Address PR feedback

61ad690

jkotas mentioned this pull request May 3, 2020

COM Interop Guidance #4219

Open

MichalStrehovsky approved these changes May 4, 2020

View reviewed changes

MichalStrehovsky reviewed May 4, 2020

View reviewed changes

kant2002 added 2 commits May 5, 2020 03:42

Fix PR comments

cf900ab

Add test for marshalling null values for COM interfaces

24caadd

Run COM test only on Windows

e87e129

Simplify tests

c4f624c

MichalStrehovsky merged commit 3216e1a into dotnet:master May 6, 2020

MichalStrehovsky mentioned this pull request Feb 16, 2022

COM marshalling E2E dotnet/linker#2618

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add simple marshaller which marshal only null COM interfaces #8128

Add simple marshaller which marshal only null COM interfaces #8128

kant2002 commented May 2, 2020

jkotas May 2, 2020

kant2002 May 2, 2020

jkotas May 2, 2020

kant2002 May 2, 2020

jkotas May 2, 2020

kant2002 May 2, 2020

jkotas May 3, 2020

jkotas May 3, 2020

jkotas May 3, 2020

MichalStrehovsky left a comment

MichalStrehovsky May 4, 2020

MichalStrehovsky May 4, 2020

kant2002 commented May 4, 2020

MichalStrehovsky commented May 5, 2020

	public static object ConvertManagedComInterfaceToNative(object pUnk)
	public static IntPtr ConvertManagedComInterfaceToNative(object pUnk)

	throw new PlatformNotSupportedException("Marhalling of non null COM interfaces does not supported");
	throw new PlatformNotSupportedException("Marhalling of non-null COM interfaces is not supported");

Add simple marshaller which marshal only null COM interfaces #8128

Add simple marshaller which marshal only null COM interfaces #8128

Conversation

kant2002 commented May 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichalStrehovsky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kant2002 commented May 4, 2020

MichalStrehovsky commented May 5, 2020