Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused custom marshaler code and move ICustomMarshaler and Color marshalling handling to managed code #106735

Merged
merged 28 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
ac34579
Remove unused "shared custom marshaler helper" code
jkoritzinsky Aug 19, 2024
3093d4a
Collapse together the custom marshaller helper types
jkoritzinsky Aug 19, 2024
094b807
Remove CustomMarshalerHelper abstraction
jkoritzinsky Aug 19, 2024
3b8a5b9
ICustomMarshaler will never support by-value marshalling, so remove t…
jkoritzinsky Aug 19, 2024
d4e3e08
Remove native size local as we always set it to the same thing
jkoritzinsky Aug 19, 2024
aa0894c
Remove unused methods
jkoritzinsky Aug 19, 2024
f2e869d
Remove more unused code
jkoritzinsky Aug 19, 2024
fd9dfef
Use the same helpers in CustomMarshalerInfo as the IL stubs use
jkoritzinsky Aug 19, 2024
dcfe926
The only method we look up is GetInstance, so clean that up
jkoritzinsky Aug 19, 2024
1841ea5
Move ICustomMarshaler method invocation into the only usage location …
jkoritzinsky Aug 19, 2024
3321129
Remove CustomMarshalerInfo abstraction from P/Invoke IL stubs (inline…
jkoritzinsky Aug 19, 2024
8bdde74
Change default deep copy option to the one we use in the one case we …
jkoritzinsky Aug 19, 2024
0a185b4
Move IEnumerator marshaller support up to managed code only
jkoritzinsky Aug 20, 2024
12c5607
Move reflection lookup and handling for Color marshalling to managed …
jkoritzinsky Aug 20, 2024
539f0eb
Fix tests
jkoritzinsky Aug 20, 2024
1ec3bef
Fix contract
jkoritzinsky Aug 20, 2024
4a33197
Fix non-Windows build
jkoritzinsky Aug 20, 2024
7e9f7db
Remove unused variable
jkoritzinsky Aug 20, 2024
1530b30
Fix GC mode for getting the custom marshaler object
jkoritzinsky Aug 21, 2024
4e4253d
PR feedback
jkoritzinsky Aug 23, 2024
1e238ba
Remove unmanaged definition of System.Drawing.Color from the runtime …
jkoritzinsky Aug 23, 2024
a583aab
Merge branch 'main' of github.com:dotnet/runtime into cm-cleanup
jkoritzinsky Aug 23, 2024
fceb168
Remove the QCall and just call ColorMarshaler directly.
jkoritzinsky Aug 23, 2024
50d251a
Merge branch 'main' of https://github.com/dotnet/runtime into cm-cleanup
jkoritzinsky Aug 28, 2024
cb71d7e
PR feedback
jkoritzinsky Aug 28, 2024
75eecc2
Fix naming
jkoritzinsky Aug 28, 2024
740ccb2
Update contract
jkoritzinsky Aug 28, 2024
8a2bcd4
Fix type for ComVariant.Create call and remove unused qcall
jkoritzinsky Aug 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices.Marshalling;
using System.StubHelpers;

namespace Microsoft.Win32
{
Expand Down Expand Up @@ -73,8 +74,7 @@ internal static unsafe partial class OAVariantLib
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);
result = ColorMarshaler.ConvertToManaged((int)sourceData);
Debug.Assert(result != null);
return result;
}
Expand Down
72 changes: 60 additions & 12 deletions src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,14 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
#if FEATURE_COMINTEROP
using System.Runtime.InteropServices.CustomMarshalers;
#endif
using System.Runtime.InteropServices.Marshalling;
using System.Runtime.Versioning;
using System.Text;

namespace System.StubHelpers
Expand Down Expand Up @@ -792,16 +797,6 @@ internal static void ClearNative(IntPtr pMarshalState, in object pManagedHome, I

internal static unsafe partial class MngdRefCustomMarshaler
{
[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "CustomMarshaler_GetMarshalerObject")]
private static partial void GetMarshaler(IntPtr pCMHelper, ObjectHandleOnStack retMarshaler);

internal static ICustomMarshaler GetMarshaler(IntPtr pCMHelper)
{
ICustomMarshaler? marshaler = null;
GetMarshaler(pCMHelper, ObjectHandleOnStack.Create(ref marshaler));
return marshaler!;
}

internal static unsafe void ConvertContentsToNative(ICustomMarshaler marshaler, in object pManagedHome, IntPtr* pNativeHome)
{
// COMPAT: We never pass null to MarshalManagedToNative.
Expand Down Expand Up @@ -1395,8 +1390,28 @@ internal static void SetPendingExceptionObject(Exception? exception)
s_pendingExceptionObject = exception;
}

[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "StubHelpers_CreateCustomMarshalerHelper")]
internal static partial IntPtr CreateCustomMarshalerHelper(IntPtr pMD, int paramToken, IntPtr hndManagedType);
[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "StubHelpers_CreateCustomMarshaler")]
internal static partial void CreateCustomMarshaler(IntPtr pMD, int paramToken, IntPtr hndManagedType, ObjectHandleOnStack customMarshaler);

#if FEATURE_COMINTEROP
[SupportedOSPlatform("windows")]
internal static object GetIEnumeratorToEnumVariantMarshaler() => EnumeratorToEnumVariantMarshaler.GetInstance(string.Empty);
#endif

internal static object CreateCustomMarshaler(IntPtr pMD, int paramToken, IntPtr hndManagedType)
{
#if FEATURE_COMINTEROP
if (OperatingSystem.IsWindows()
&& hndManagedType == typeof(System.Collections.IEnumerator).TypeHandle.Value)
{
return GetIEnumeratorToEnumVariantMarshaler();
}
#endif

object? retVal = null;
CreateCustomMarshaler(pMD, paramToken, hndManagedType, ObjectHandleOnStack.Create(ref retVal));
return retVal!;
}

//-------------------------------------------------------
// SafeHandle Helpers
Expand Down Expand Up @@ -1575,4 +1590,37 @@ internal static void ValidateObject(object obj, IntPtr pMD)
[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern IntPtr NextCallReturnAddress();
} // class StubHelpers

#if FEATURE_COMINTEROP
internal static class ColorMarshaler
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
{
private static readonly MethodInvoker s_oleColorToDrawingColorMethod;
private static readonly MethodInvoker s_drawingColorToOleColorMethod;

internal static readonly IntPtr s_colorType;

#pragma warning disable CA1810 // explicit static cctor
static ColorMarshaler()
{
Type colorTranslatorType = Type.GetType("System.Drawing.ColorTranslator, System.Drawing.Primitives", throwOnError: true)!;
Type colorType = Type.GetType("System.Drawing.Color, System.Drawing.Primitives", throwOnError: true)!;
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved

s_colorType = colorType.TypeHandle.Value;

s_oleColorToDrawingColorMethod = MethodInvoker.Create(colorTranslatorType.GetMethod("FromOle", [typeof(int)])!);
s_drawingColorToOleColorMethod = MethodInvoker.Create(colorTranslatorType.GetMethod("ToOle", [colorType])!);
}
#pragma warning restore CA1810 // explicit static cctor

internal static object ConvertToManaged(int managedColor)
{
return s_oleColorToDrawingColorMethod.Invoke(null, managedColor)!;
}

internal static int ConvertToNative(object? managedColor)
{
return (int)s_drawingColorToOleColorMethod.Invoke(null, managedColor)!;
}
}
#endif
}
9 changes: 2 additions & 7 deletions src/coreclr/System.Private.CoreLib/src/System/Variant.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices.Marshalling;
using System.StubHelpers;

#pragma warning disable CA1416 // COM interop is only supported on Windows

Expand All @@ -17,12 +18,6 @@ internal static partial class Variant
{
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);

[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "Variant_ConvertValueTypeToRecord")]
private static partial void ConvertValueTypeToRecord(ObjectHandleOnStack obj, out ComVariant pOle);

Expand Down Expand Up @@ -163,7 +158,7 @@ internal static void MarshalHelperConvertObjectToVariant(object? o, out ComVaria

case { } when IsSystemDrawingColor(o.GetType()):
// System.Drawing.Color is converted to UInt32
pOle = ComVariant.Create(ConvertSystemColorToOleColor(ObjectHandleOnStack.Create(ref o)));
pOle = ComVariant.Create((uint)ColorMarshaler.ConvertToNative(o));
break;

// DateTime, decimal handled by IConvertible case
Expand Down
34 changes: 0 additions & 34 deletions src/coreclr/classlibnative/bcltype/variant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,40 +18,6 @@
#include "vars.hpp"
#include "variant.h"

extern "C" uint32_t QCALLTYPE Variant_ConvertSystemColorToOleColor(QCall::ObjectHandleOnStack obj)
{
QCALL_CONTRACT;

uint32_t ret = 0;

BEGIN_QCALL;

GCX_COOP();
OBJECTREF srcObj = obj.Get();

GCPROTECT_BEGIN(srcObj);
ret = ConvertSystemColorToOleColor(&srcObj);
GCPROTECT_END();

END_QCALL;

return ret;
}

extern "C" void QCALLTYPE Variant_ConvertOleColorToSystemColor(QCall::ObjectHandleOnStack objRet, uint32_t oleColor, MethodTable* pMT)
{
QCALL_CONTRACT;

BEGIN_QCALL;

GCX_COOP();
SYSTEMCOLOR systemColor{};
ConvertOleColorToSystemColor(oleColor, &systemColor);
objRet.Set(pMT->Box(&systemColor));

END_QCALL;
}

extern "C" void QCALLTYPE Variant_ConvertValueTypeToRecord(QCall::ObjectHandleOnStack obj, VARIANT * pOle)
{
QCALL_CONTRACT;
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/classlibnative/bcltype/variant.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
#include <cor.h>
#include "olevariant.h"

extern "C" uint32_t QCALLTYPE Variant_ConvertSystemColorToOleColor(QCall::ObjectHandleOnStack obj);
extern "C" void QCALLTYPE Variant_ConvertOleColorToSystemColor(QCall::ObjectHandleOnStack objRet, uint32_t oleColor, MethodTable* pMT);
extern "C" void QCALLTYPE Variant_ConvertValueTypeToRecord(QCall::ObjectHandleOnStack obj, VARIANT* pOle);

#endif // _VARIANT_H_
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/pal/inc/rt/palrt.h
Original file line number Diff line number Diff line change
Expand Up @@ -716,8 +716,6 @@ typename std::remove_reference<T>::type&& move( T&& t );
#define __RPC__inout
#define __RPC__deref_out_ecount_full_opt(x)

typedef DWORD OLE_COLOR;

typedef HANDLE HWND;

typedef struct _LIST_ENTRY {
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/vm/callhelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,6 @@ class MethodDescCallSite
MDCALLDEF_VOID( CallWithValueTypes, TRUE)
MDCALLDEF_ARGSLOT( CallWithValueTypes, _RetArgSlot)
MDCALLDEF_REFTYPE( CallWithValueTypes, TRUE, _RetOBJECTREF, Object*, OBJECTREF)
MDCALLDEF( CallWithValueTypes, TRUE, _RetOleColor, OLE_COLOR, OTHER_ELEMENT_TYPE)
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
#undef OTHER_ELEMENT_TYPE
#undef MDCALL_ARG_SIG_STD_RETTYPES
#undef MDCALLDEF
Expand Down
16 changes: 9 additions & 7 deletions src/coreclr/vm/corelib.h
Original file line number Diff line number Diff line change
Expand Up @@ -384,11 +384,6 @@ DEFINE_CLASS(ICUSTOM_ATTR_PROVIDER, Reflection, ICustomAttributeProv
DEFINE_METHOD(ICUSTOM_ATTR_PROVIDER,GET_CUSTOM_ATTRIBUTES, GetCustomAttributes, IM_Type_RetArrObj)

DEFINE_CLASS(ICUSTOM_MARSHALER, Interop, ICustomMarshaler)
DEFINE_METHOD(ICUSTOM_MARSHALER, MARSHAL_NATIVE_TO_MANAGED,MarshalNativeToManaged, IM_IntPtr_RetObj)
DEFINE_METHOD(ICUSTOM_MARSHALER, MARSHAL_MANAGED_TO_NATIVE,MarshalManagedToNative, IM_Obj_RetIntPtr)
DEFINE_METHOD(ICUSTOM_MARSHALER, CLEANUP_NATIVE_DATA, CleanUpNativeData, IM_IntPtr_RetVoid)
DEFINE_METHOD(ICUSTOM_MARSHALER, CLEANUP_MANAGED_DATA, CleanUpManagedData, IM_Obj_RetVoid)
DEFINE_METHOD(ICUSTOM_MARSHALER, GET_NATIVE_DATA_SIZE, GetNativeDataSize, IM_RetInt)

DEFINE_CLASS(ICUSTOMADAPTER, Interop, ICustomAdapter)

Expand Down Expand Up @@ -929,7 +924,10 @@ DEFINE_METHOD(STUBHELPERS, KEEP_ALIVE_VIA_CLEANUP_LIST, KeepAliveVia
DEFINE_METHOD(STUBHELPERS, DESTROY_CLEANUP_LIST, DestroyCleanupList, SM_RefCleanupWorkListElement_RetVoid)
DEFINE_METHOD(STUBHELPERS, GET_HR_EXCEPTION_OBJECT, GetHRExceptionObject, SM_Int_RetException)
DEFINE_METHOD(STUBHELPERS, GET_PENDING_EXCEPTION_OBJECT, GetPendingExceptionObject, SM_RetException)
DEFINE_METHOD(STUBHELPERS, CREATE_CUSTOM_MARSHALER_HELPER, CreateCustomMarshalerHelper, SM_IntPtr_Int_IntPtr_RetIntPtr)
DEFINE_METHOD(STUBHELPERS, CREATE_CUSTOM_MARSHALER, CreateCustomMarshaler, SM_IntPtr_Int_IntPtr_RetObj)
#ifdef FEATURE_COMINTEROP
DEFINE_METHOD(STUBHELPERS, GET_IENUMERATOR_TO_ENUM_VARIANT_MARSHALER, GetIEnumeratorToEnumVariantMarshaler, SM_RetObj)
#endif // FEATURE_COMINTEROP

DEFINE_METHOD(STUBHELPERS, CHECK_STRING_LENGTH, CheckStringLength, SM_Int_RetVoid)

Expand Down Expand Up @@ -1001,6 +999,11 @@ DEFINE_METHOD(MNGD_SAFE_ARRAY_MARSHALER, CONVERT_CONTENTS_TO_NATIVE, ConvertCon
DEFINE_METHOD(MNGD_SAFE_ARRAY_MARSHALER, CONVERT_SPACE_TO_MANAGED, ConvertSpaceToManaged, SM_IntPtr_RefObj_IntPtr_RetVoid)
DEFINE_METHOD(MNGD_SAFE_ARRAY_MARSHALER, CONVERT_CONTENTS_TO_MANAGED, ConvertContentsToManaged, SM_IntPtr_RefObj_IntPtr_RetVoid)
DEFINE_METHOD(MNGD_SAFE_ARRAY_MARSHALER, CLEAR_NATIVE, ClearNative, SM_IntPtr_RefObj_IntPtr_RetVoid)

DEFINE_CLASS(COLORMARSHALER, StubHelpers, ColorMarshaler)
DEFINE_METHOD(COLORMARSHALER, CONVERT_TO_NATIVE, ConvertToNative, SM_Obj_RetInt)
DEFINE_METHOD(COLORMARSHALER, CONVERT_TO_MANAGED, ConvertToManaged, SM_Int_RetObj)
DEFINE_FIELD(COLORMARSHALER, COLOR_TYPE, s_colorType)
#endif // FEATURE_COMINTEROP
END_ILLINK_FEATURE_SWITCH()

Expand Down Expand Up @@ -1035,7 +1038,6 @@ DEFINE_METHOD(MNGD_REF_CUSTOM_MARSHALER, CONVERT_CONTENTS_TO_NATIVE, ConvertCon
DEFINE_METHOD(MNGD_REF_CUSTOM_MARSHALER, CONVERT_CONTENTS_TO_MANAGED, ConvertContentsToManaged, SM_ICustomMarshaler_RefObj_PtrIntPtr_RetVoid)
DEFINE_METHOD(MNGD_REF_CUSTOM_MARSHALER, CLEAR_NATIVE, ClearNative, SM_ICustomMarshaler_RefObj_PtrIntPtr_RetVoid)
DEFINE_METHOD(MNGD_REF_CUSTOM_MARSHALER, CLEAR_MANAGED, ClearManaged, SM_ICustomMarshaler_RefObj_PtrIntPtr_RetVoid)
DEFINE_METHOD(MNGD_REF_CUSTOM_MARSHALER, GET_MARSHALER, GetMarshaler, SM_IntPtr_RetICustomMarshaler)

DEFINE_CLASS(ASANY_MARSHALER, StubHelpers, AsAnyMarshaler)
DEFINE_METHOD(ASANY_MARSHALER, CTOR, .ctor, IM_IntPtr_RetVoid)
Expand Down
Loading