Convert OAVariant interop to managed (dotnet#100176)

* Convert BoxEnum to managed

* SetFieldsObject to managed

* Handle decimal and other CV_OBJECT

* Managed ToOAVariant and FromOAVariant

* Marshal for IUnknown and IDispatch

* VariantChangeTypeEx interop

* Use managed reflection for System.Drawing.Color conversion

* Use MarshalNative for IDispatch/IUnknown marshalling

* Move Color conversion to Variant

* Respect VTToCV mapping

* Improve test type coverage

* Test for values in ReturnToManaged

---------

Co-authored-by: Aaron Robinson <[email protected]>

Author: 2 people

src/coreclr/System.Private.CoreLib/src/Microsoft/Win32/OAVariantLib.cs

@@ -13,11 +13,13 @@
 ===========================================================*/
 
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.Globalization;
 using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
+using System.Runtime.InteropServices.Marshalling;
 
 namespace Microsoft.Win32
 {
@@ -26,102 +28,162 @@ internal static unsafe partial class OAVariantLib
         #region Constants
 
         // Constants for VariantChangeType from OleAuto.h
-        public const int NoValueProp = 0x01;
-        public const int AlphaBool = 0x02;
-        public const int NoUserOverride = 0x04;
-        public const int CalendarHijri = 0x08;
         public const int LocalBool = 0x10;
 
-        internal static readonly Type?[] ClassTypes = {
-            typeof(Empty),
-            typeof(void),
-            typeof(bool),
-            typeof(char),
-            typeof(sbyte),
-            typeof(byte),
-            typeof(short),
-            typeof(ushort),
-            typeof(int),
-            typeof(uint),
-            typeof(long),
-            typeof(ulong),
-            typeof(float),
-            typeof(double),
-            typeof(string),
-            typeof(void),
-            typeof(DateTime),
-            typeof(TimeSpan),
-            typeof(object),
-            typeof(decimal),
-            null,  // Enums - what do we do here?
-            typeof(Missing),
-            typeof(DBNull),
+        private static readonly Dictionary<Type, VarEnum> ClassTypes = new Dictionary<Type, VarEnum>
+        {
+            { typeof(bool), VarEnum.VT_BOOL },
+            { typeof(char), VarEnum.VT_I2 },
+            { typeof(sbyte), VarEnum.VT_I1 },
+            { typeof(byte), VarEnum.VT_UI1 },
+            { typeof(short), VarEnum.VT_I2 },
+            { typeof(ushort), VarEnum.VT_UI2 },
+            { typeof(int), VarEnum.VT_I4 },
+            { typeof(uint), VarEnum.VT_UI4 },
+            { typeof(long), VarEnum.VT_I8 },
+            { typeof(ulong), VarEnum.VT_UI8 },
+            { typeof(float), VarEnum.VT_R4 },
+            { typeof(double), VarEnum.VT_R8 },
+            { typeof(string), VarEnum.VT_BSTR },
+            { typeof(DateTime), VarEnum.VT_DATE },
+            { typeof(decimal), VarEnum.VT_DECIMAL },
         };
 
-        // Keep these numbers in sync w/ the above array.
-        private const int CV_OBJECT = 0x12;
-
         #endregion
 
 
         #region Internal Methods
 
-#pragma warning disable CS8500
-
         /**
          * Changes a Variant from one type to another, calling the OLE
          * Automation VariantChangeTypeEx routine.  Note the legal types here are
          * restricted to the subset of what can be legally found in a VB
          * Variant and the types that CLR supports explicitly in the
          * CLR Variant class.
          */
-        internal static Variant ChangeType(Variant source, Type targetClass, short options, CultureInfo culture)
+        internal static object? ChangeType(object source, Type targetClass, short options, CultureInfo culture)
         {
             ArgumentNullException.ThrowIfNull(targetClass);
             ArgumentNullException.ThrowIfNull(culture);
 
-            Variant result = default;
-            ChangeType(
-                &result,
-                &source,
-                culture.LCID,
-                targetClass.TypeHandle.Value,
-                GetCVTypeFromClass(targetClass),
-                options);
-            return result;
-        }
+            object? result = null;
 
-        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "OAVariant_ChangeType")]
-        private static partial void ChangeType(Variant* result, Variant* source, int lcid, IntPtr typeHandle, int cvType, short flags);
+            if (Variant.IsSystemDrawingColor(targetClass))
+            {
+                if (source is int || source is uint)
+                {
+                    uint sourceData = source is int ? (uint)(int)source : (uint)source;
+                    // Int32/UInt32 can be converted to System.Drawing.Color
+                    Variant.ConvertOleColorToSystemColor(ObjectHandleOnStack.Create(ref result), sourceData, targetClass.TypeHandle.Value);
+                    Debug.Assert(result != null);
+                    return result;
+                }
+            }
 
-#pragma warning restore CS8500
+            if (!ClassTypes.TryGetValue(targetClass, out VarEnum vt))
+            {
+                throw new NotSupportedException(SR.NotSupported_ChangeType);
+            }
 
-        #endregion
+            ComVariant vOp = ToOAVariant(source);
+            ComVariant ret = default;
 
+            int hr = Interop.OleAut32.VariantChangeTypeEx(&ret, &vOp, culture.LCID, options, (ushort)vt);
 
-        #region Private Helpers
+            using (vOp)
+            using (ret)
+            {
+                if (hr < 0)
+                {
+                    OAFailed(hr);
+                }
 
-        private static int GetCVTypeFromClass(Type ctype)
-        {
-            Debug.Assert(ctype != null);
-            Debug.Assert(ClassTypes[CV_OBJECT] == typeof(object), "OAVariantLib::ClassTypes[CV_OBJECT] == Object.class");
+                result = FromOAVariant(ret);
+                if (targetClass == typeof(char))
+                {
+                    result = (char)(uint)result!;
+                }
+            }
 
-            // OleAut Binder works better if unrecognized
-            // types were changed to Object.
-            int cvtype = CV_OBJECT;
+            return result;
+        }
 
-            for (int i = 0; i < ClassTypes.Length; i++)
+        private static void OAFailed(int hr)
+        {
+            switch (hr)
             {
-                if (ctype.Equals(ClassTypes[i]))
-                {
-                    cvtype = i;
-                    break;
-                }
+                case HResults.COR_E_OUTOFMEMORY:
+                    throw new OutOfMemoryException();
+                case HResults.DISP_E_BADVARTYPE:
+                    throw new NotSupportedException(SR.NotSupported_OleAutBadVarType);
+                case HResults.DISP_E_DIVBYZERO:
+                    throw new DivideByZeroException();
+                case HResults.DISP_E_OVERFLOW:
+                    throw new OverflowException();
+                case HResults.DISP_E_TYPEMISMATCH:
+                    throw new InvalidCastException(SR.InvalidCast_OATypeMismatch);
+                case HResults.E_INVALIDARG:
+                    throw new ArgumentException();
+                default:
+                    Debug.Fail("Unrecognized HResult - OAVariantLib routine failed in an unexpected way!");
+                    throw Marshal.GetExceptionForHR(hr);
             }
+        }
 
-            return cvtype;
+        private static ComVariant ToOAVariant(object input)
+        {
+            return input switch
+            {
+                string str => ComVariant.Create(str),
+                DateTime dateTime => ComVariant.Create(dateTime),
+                bool b => ComVariant.Create(b),
+                decimal d => ComVariant.Create(d),
+                sbyte i1 => ComVariant.Create(i1),
+                byte u1 => ComVariant.Create(u1),
+                short i2 => ComVariant.Create(i2),
+                ushort u2 => ComVariant.Create(u2),
+                int i4 => ComVariant.Create(i4),
+                uint u4 => ComVariant.Create(u4),
+                long i8 => ComVariant.Create(i8),
+                ulong u8 => ComVariant.Create(u8),
+                float r4 => ComVariant.Create(r4),
+                double r8 => ComVariant.Create(r8),
+                null => default,
+                Missing => throw new NotSupportedException(SR.NotSupported_ChangeType),
+                DBNull => ComVariant.Null,
+                _ => GetComIPFromObjectRef(input) // Convert the object to an IDispatch/IUnknown pointer.
+            };
+        }
+
+        private static ComVariant GetComIPFromObjectRef(object? obj)
+        {
+            IntPtr pUnk = GetIUnknownOrIDispatchForObject(ObjectHandleOnStack.Create(ref obj), out bool isIDispatch);
+            return ComVariant.CreateRaw(isIDispatch ? VarEnum.VT_DISPATCH : VarEnum.VT_UNKNOWN, pUnk);
         }
 
+        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "MarshalNative_GetIUnknownOrIDispatchForObject")]
+        private static partial IntPtr GetIUnknownOrIDispatchForObject(ObjectHandleOnStack o, [MarshalAs(UnmanagedType.Bool)] out bool isIDispatch);
+
+        private static object? FromOAVariant(ComVariant input) =>
+            input.VarType switch
+            {
+                VarEnum.VT_BSTR => input.As<string>(),
+                VarEnum.VT_DATE => input.As<DateTime>(),
+                VarEnum.VT_BOOL => input.As<bool>(),
+                VarEnum.VT_DECIMAL => input.As<decimal>(),
+                VarEnum.VT_I1 => input.As<sbyte>(),
+                VarEnum.VT_UI1 => input.As<byte>(),
+                VarEnum.VT_I2 => input.As<short>(),
+                VarEnum.VT_UI2 => input.As<ushort>(),
+                VarEnum.VT_I4 or VarEnum.VT_INT => input.As<int>(),
+                VarEnum.VT_UI4 or VarEnum.VT_UINT => input.As<uint>(),
+                VarEnum.VT_I8 => input.As<long>(),
+                VarEnum.VT_UI8 => input.As<ulong>(),
+                VarEnum.VT_R4 => input.As<float>(),
+                VarEnum.VT_R8 => input.As<double>(),
+                _ => throw new NotSupportedException(SR.NotSupported_ChangeType),
+            };
+
         #endregion
     }
 }

src/coreclr/System.Private.CoreLib/src/System/OleAutBinder.cs

@@ -18,7 +18,6 @@ internal sealed class OleAutBinder : DefaultBinder
         // This binder uses OLEAUT to change the type of the variant.
         public override object ChangeType(object value, Type type, CultureInfo? cultureInfo)
         {
-            Variant myValue = new Variant(value);
             cultureInfo ??= CultureInfo.CurrentCulture;
 
 #if DISPLAY_DEBUG_INFO
@@ -62,7 +61,7 @@ public override object ChangeType(object value, Type type, CultureInfo? cultureI
 #endif
                 // Specify the LocalBool flag to have BOOL values converted to local language rather
                 // than 0 or -1.
-                object RetObj = OAVariantLib.ChangeType(myValue, type, OAVariantLib.LocalBool, cultureInfo).ToObject()!;
+                object RetObj = OAVariantLib.ChangeType(value, type, OAVariantLib.LocalBool, cultureInfo)!;
 
 #if DISPLAY_DEBUG_INFO
                 Console.WriteLine("Object returned from ChangeType is of type: " + RetObj.GetType().Name);

src/coreclr/System.Private.CoreLib/src/System/Variant.cs

@@ -17,7 +17,7 @@
 
 namespace System
 {
-    internal struct Variant
+    internal partial struct Variant
     {
         // Do Not change the order of these fields.
         // They are mapped to the native VariantData * data structure.
@@ -70,6 +70,14 @@ internal struct Variant
         internal static Variant Missing => new Variant(CV_MISSING, Type.Missing, 0);
         internal static Variant DBNull => new Variant(CV_NULL, System.DBNull.Value, 0);
 
+        internal static bool IsSystemDrawingColor(Type type) => type.FullName == "System.Drawing.Color"; // Matches the behavior of IsTypeRefOrDef
+
+        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "Variant_ConvertSystemColorToOleColor")]
+        internal static partial uint ConvertSystemColorToOleColor(ObjectHandleOnStack obj);
+
+        [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "Variant_ConvertOleColorToSystemColor")]
+        internal static partial void ConvertOleColorToSystemColor(ObjectHandleOnStack objret, uint value, IntPtr pMT);
+
         //
         // Native Methods
         //

src/coreclr/classlibnative/bcltype/CMakeLists.txt

@@ -2,7 +2,6 @@ set(CMAKE_INCLUDE_CURRENT_DIR ON)
 
 set(BCLTYPE_SOURCES
     arraynative.cpp
-    oavariant.cpp
     objectnative.cpp
     system.cpp
     varargsnative.cpp