Implement Marshal.GetComObjectData and Marshal.SetComObjectData in .NET Core

[hughbe]

These methods are stubbed out and throw PNSE in .NET Core. Looking at reference source it should be easy to implement (https://github.com/dotnet/coreclr/blob/master/src/System.Private.CoreLib/src/System/__ComObject.cs#L66-L108)

public static Object GetComObjectData(Object obj, Object key)
{
    if (obj == null)
    {
        throw new ArgumentNullException(nameof(obj));
    }
    if (key == null)
    {
        throw new ArgumentNullException(nameof(key));
    }

    __ComObject comObj = null;

    // Make sure the obj is an __ComObject.
    try
    {
        comObj = (__ComObject)obj;
    }
    catch (InvalidCastException)
    {
        throw new ArgumentException(SR.Argument_ObjNotComObject, nameof(obj))
    }

    if (obj.GetType().IsWindowsRuntimeObject)
    {
        throw new ArgumentException(SR.Argument_ObjIsWinRTObject, nameof(obj));
    }

    // Retrieve the data from the __ComObject.
    return comObj.GetData(key);
}

[danmoseley]

Both API's have 2.5% usage in .NET compat lab data.
@ericstj did this show up for you?

[hughbe]

This also breaks the CreateWrapperOfType API which relies on GetComObjectData and SetComObjectData so actual impact may be a little higher

[ericstj]

@hughbe nice find.

None of the apps I brought up hit a codepath where they'd crash due to this, but that doesn't mean its not important.

Did a quick check and here's a couple suspect commits:
dotnet/coreclr@3103a70
dotnet/coreclr@16f5695

For the first I think it just used the wrong ifdef, FEATURE_CORECLR instead of FEATURE_COMINTEROP, then later got removed when folks did define cleanup.

For the latter I think it was a bad change to take. We don't get to remove API just because its confusing. /cc @luqunl @jkotas

[hughbe]

I believe that was temporary and we've now implemented Read*/Write*

e.g. https://github.com/dotnet/coreclr/tree/master/src/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs#L382-L385

[ericstj]

Indeed the right thing was done later on with dotnet/coreclr@88779dc.

I think dotnet/coreclr@3103a70 still introduced throwing APIs where it should have only made them throw on unix.