Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Implement support for copy constructors when marshalling in IJW #22805

Merged
merged 7 commits into from
Mar 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/dlls/mscorrc/mscorrc.rc
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ BEGIN
IDS_EE_BADMARSHAL_DECIMALARRAY "Invalid managed/unmanaged type combination (Decimal[] must be paired with an ArraySubType of Struct or Currency)."
IDS_EE_BADMARSHAL_WINRT_MARSHAL_AS "Invalid managed/unmanaged type combination (Windows Runtime parameters and fields must not have a MarshalAs attribute set)."
IDS_EE_BADMARSHAL_WINRT_MISSING_GUID "Invalid managed/unmanaged type combination (Windows Runtime interfaces, classes and delegates must have a Guid or a default interface)."
IDS_EE_BADMARSHAL_WINRT_COPYCTOR "Windows Runtime marshaler does not support types with copy constructor."
IDS_EE_BADMARSHAL_WINRT_DELEGATE "Invalid managed/unmanaged type combination (Delegates must be Windows Runtime delegates)."
IDS_EE_BADMARSHAL_DEFAULTIFACE_NOT_WINRT_IFACE "The default interface must refer to a Windows Runtime interface with a GUID."
IDS_EE_BADMARSHAL_DEFAULTIFACE_NOT_SUBTYPE "The default interface must refer to an interface that is implemented by the type."
Expand Down Expand Up @@ -611,6 +612,7 @@ BEGIN
IDS_EE_BADMARSHAL_ASANYRESTRICTION "AsAny cannot be used on return types, ByRef parameters, ArrayWithOffset, or parameters passed from unmanaged to managed."
IDS_EE_BADMARSHAL_VBBYVALSTRRESTRICTION "VBByRefStr can only be used in combination with in/out, ByRef managed-to-unmanaged strings."
IDS_EE_BADMARSHAL_AWORESTRICTION "ArrayWithOffsets can only be marshaled as inout, non-ByRef, managed-to-unmanaged parameters."
IDS_EE_BADMARSHAL_COPYCTORRESTRICTION "Classes with copy-ctors can only be marshaled by value."
IDS_EE_BADMARSHAL_ARGITERATORRESTRICTION "ArgIterators cannot be marshaled ByRef."
IDS_EE_BADMARSHAL_HANDLEREFRESTRICTION "HandleRefs cannot be marshaled ByRef or from unmanaged to managed."
IDS_EE_BADMARSHAL_SAFEHANDLENATIVETOCOM "SafeHandles cannot be marshaled from unmanaged to managed."
Expand Down
2 changes: 2 additions & 0 deletions src/dlls/mscorrc/resource.h
Original file line number Diff line number Diff line change
Expand Up @@ -718,3 +718,5 @@
#define IDS_EE_NDIRECT_GETPROCADDR_WIN_DLL 0x2644
#define IDS_EE_NDIRECT_GETPROCADDR_UNIX_SO 0x2645
#define IDS_EE_BADMARSHAL_STRING_OUT 0x2646
#define IDS_EE_BADMARSHAL_COPYCTORRESTRICTION 0x2647
#define IDS_EE_BADMARSHAL_WINRT_COPYCTOR 0x2648
1 change: 1 addition & 0 deletions src/vm/dllimport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4139,6 +4139,7 @@ static void CreateNDirectStubWorker(StubState* pss,
COMPlusThrow(kMarshalDirectiveException, IDS_EE_NDIRECT_BADNATL_THISCALL);
}

fHasCopyCtorArgs = info.GetMarshalType() == MarshalInfo::MARSHAL_TYPE_BLITTABLEVALUECLASSWITHCOPYCTOR ? TRUE : FALSE;

argidx++;
}
Expand Down
86 changes: 86 additions & 0 deletions src/vm/ilmarshalers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3292,6 +3292,92 @@ ILCriticalHandleMarshaler::ReturnOverride(
return OVERRIDDEN;
} // ILCriticalHandleMarshaler::ReturnOverride

MarshalerOverrideStatus ILBlittableValueClassWithCopyCtorMarshaler::ArgumentOverride(NDirectStubLinker* psl,
BOOL byref,
BOOL fin,
BOOL fout,
BOOL fManagedToNative,
OverrideProcArgs* pargs,
UINT* pResID,
UINT argidx,
UINT nativeStackOffset)
{
CONTRACTL
{
THROWS;
GC_TRIGGERS;
MODE_ANY;
}
CONTRACTL_END;

ILCodeStream* pslIL = psl->GetMarshalCodeStream();
ILCodeStream* pslILDispatch = psl->GetDispatchCodeStream();

if (byref)
{
*pResID = IDS_EE_BADMARSHAL_COPYCTORRESTRICTION;
return DISALLOWED;
}

if (fManagedToNative)
{
// 1) create new native value type local
// 2) run new->CopyCtor(old)
// 3) run old->Dtor()

LocalDesc locDesc(pargs->mm.m_pMT);

DWORD dwNewValueTypeLocal;

// Step 1
dwNewValueTypeLocal = pslIL->NewLocal(locDesc);

// Step 2
if (pargs->mm.m_pCopyCtor)
{
// Managed copy constructor has signature of CopyCtor(T* new, T old);
pslIL->EmitLDLOCA(dwNewValueTypeLocal);
pslIL->EmitLDARG(argidx);
pslIL->EmitCALL(pslIL->GetToken(pargs->mm.m_pCopyCtor), 2, 0);
jkoritzinsky marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
pslIL->EmitLDARG(argidx);
pslIL->EmitLDOBJ(pslIL->GetToken(pargs->mm.m_pMT));
pslIL->EmitSTLOC(dwNewValueTypeLocal);
}

// Step 3
if (pargs->mm.m_pDtor)
{
// Managed destructor has signature of Destructor(T old);
pslIL->EmitLDARG(argidx);
pslIL->EmitCALL(pslIL->GetToken(pargs->mm.m_pDtor), 1, 0);
}
#ifdef _TARGET_X86_
pslIL->SetStubTargetArgType(&locDesc); // native type is the value type
pslILDispatch->EmitLDLOC(dwNewValueTypeLocal); // we load the local directly
#else
pslIL->SetStubTargetArgType(ELEMENT_TYPE_I); // native type is a pointer
pslILDispatch->EmitLDLOCA(dwNewValueTypeLocal);
#endif

return OVERRIDDEN;
}
else
{
// nothing to do but pass the value along
// note that on x86 the argument comes by-value but is converted to pointer by the UM thunk
// so that we don't make copies that would not be accounted for by copy ctors
LocalDesc locDesc(pargs->mm.m_pMT);
locDesc.MakeCopyConstructedPointer();

pslIL->SetStubTargetArgType(&locDesc); // native type is a pointer
pslILDispatch->EmitLDARG(argidx);

return OVERRIDDEN;
}
}

LocalDesc ILArgIteratorMarshaler::GetNativeType()
{
Expand Down
32 changes: 32 additions & 0 deletions src/vm/ilmarshalers.h
Original file line number Diff line number Diff line change
Expand Up @@ -2725,8 +2725,40 @@ class ILBlittablePtrMarshaler : public ILLayoutClassPtrMarshalerBase
virtual void EmitConvertContentsNativeToCLR(ILCodeStream* pslILEmit);
};

class ILBlittableValueClassWithCopyCtorMarshaler : public ILMarshaler
{
public:
enum
{
c_fInOnly = TRUE,
c_nativeSize = VARIABLESIZE,
c_CLRSize = sizeof(OBJECTREF),
};

LocalDesc GetManagedType()
{
LIMITED_METHOD_CONTRACT;
return LocalDesc();
}

LocalDesc GetNativeType()
{
LIMITED_METHOD_CONTRACT;
return LocalDesc();
}

static MarshalerOverrideStatus ArgumentOverride(NDirectStubLinker* psl,
BOOL byref,
BOOL fin,
BOOL fout,
BOOL fManagedToNative,
OverrideProcArgs* pargs,
UINT* pResID,
UINT argidx,
UINT nativeStackOffset);


};

class ILArgIteratorMarshaler : public ILMarshaler
{
Expand Down
57 changes: 57 additions & 0 deletions src/vm/mlinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1438,6 +1438,7 @@ MarshalInfo::MarshalInfo(Module* pModule,

CorNativeType nativeType = NATIVE_TYPE_DEFAULT;
Assembly *pAssembly = pModule->GetAssembly();
BOOL fNeedsCopyCtor = FALSE;
m_BestFit = BestFit;
m_ThrowOnUnmappableChar = ThrowOnUnmappableChar;
m_ms = ms;
Expand Down Expand Up @@ -1548,6 +1549,21 @@ MarshalInfo::MarshalInfo(Module* pModule,
IfFailGoto(sig.GetElemType(NULL), lFail);
mtype = sig.PeekElemTypeNormalized(pModule, pTypeContext);

// Check for Copy Constructor Modifier - peek closed elem type here to prevent ELEMENT_TYPE_VALUETYPE
// turning into a primitive.
if (sig.PeekElemTypeClosed(pModule, pTypeContext) == ELEMENT_TYPE_VALUETYPE)
{
// Skip ET_BYREF
IfFailGoto(sigtmp.GetByte(NULL), lFail);

if (sigtmp.HasCustomModifier(pModule, "Microsoft.VisualC.NeedsCopyConstructorModifier", ELEMENT_TYPE_CMOD_REQD) ||
sigtmp.HasCustomModifier(pModule, "System.Runtime.CompilerServices.IsCopyConstructed", ELEMENT_TYPE_CMOD_REQD) )
{
mtype = ELEMENT_TYPE_VALUETYPE;
fNeedsCopyCtor = TRUE;
m_byref = FALSE;
}
}
}
else
{
Expand Down Expand Up @@ -1590,6 +1606,19 @@ MarshalInfo::MarshalInfo(Module* pModule,
IfFailGoto(E_FAIL, lFail);
}

// Check for Copy Constructor Modifier
if (sigtmp.HasCustomModifier(pModule, "Microsoft.VisualC.NeedsCopyConstructorModifier", ELEMENT_TYPE_CMOD_REQD) ||
sigtmp.HasCustomModifier(pModule, "System.Runtime.CompilerServices.IsCopyConstructed", ELEMENT_TYPE_CMOD_REQD) )
{
mtype = mtype2;

// Keep the sig pointer in sync with mtype (skip ELEMENT_TYPE_PTR) because for the rest
// of this method we are pretending that the parameter is a value type passed by-value.
IfFailGoto(sig.GetElemType(NULL), lFail);

fNeedsCopyCtor = TRUE;
m_byref = FALSE;
}
}
}
else
Expand Down Expand Up @@ -2684,6 +2713,29 @@ MarshalInfo::MarshalInfo(Module* pModule,
}
else
{
if (fNeedsCopyCtor)
{
#ifdef FEATURE_COMINTEROP
if (m_ms == MARSHAL_SCENARIO_WINRT)
{
// our WinRT-optimized GetCOMIPFromRCW helpers don't support copy
// constructor stubs so make sure that this marshaler will not be used
m_resID = IDS_EE_BADMARSHAL_WINRT_COPYCTOR;
IfFailGoto(E_FAIL, lFail);
}
#endif

MethodDesc *pCopyCtor;
MethodDesc *pDtor;
FindCopyCtor(pModule, m_pMT, &pCopyCtor);
FindDtor(pModule, m_pMT, &pDtor);

m_args.mm.m_pMT = m_pMT;
m_args.mm.m_pCopyCtor = pCopyCtor;
m_args.mm.m_pDtor = pDtor;
m_type = MARSHAL_TYPE_BLITTABLEVALUECLASSWITHCOPYCTOR;
}
else
#ifdef _TARGET_X86_
// JIT64 is not aware of normalized value types and this optimization
// (returning small value types by value in registers) is already done in JIT64.
Expand Down Expand Up @@ -3398,6 +3450,7 @@ UINT16 MarshalInfo::GetNativeSize(MarshalType mtype, MarshalScenario ms)
{
case MARSHAL_TYPE_BLITTABLEVALUECLASS:
case MARSHAL_TYPE_VALUECLASS:
case MARSHAL_TYPE_BLITTABLEVALUECLASSWITHCOPYCTOR:
return (UINT16) m_pMT->GetNativeSize();

default:
Expand Down Expand Up @@ -4049,6 +4102,7 @@ DispParamMarshaler *MarshalInfo::GenerateDispParamMarshaler()
case MARSHAL_TYPE_BLITTABLEVALUECLASS:
case MARSHAL_TYPE_BLITTABLEPTR:
case MARSHAL_TYPE_LAYOUTCLASSPTR:
case MARSHAL_TYPE_BLITTABLEVALUECLASSWITHCOPYCTOR:
pDispParamMarshaler = new DispParamRecordMarshaler(m_pMT);
break;

Expand Down Expand Up @@ -4350,6 +4404,9 @@ VOID MarshalInfo::MarshalTypeToString(SString& strMarshalType, BOOL fSizeIsSpeci
case MARSHAL_TYPE_ARGITERATOR:
strRetVal = W("ArgIterator");
break;
case MARSHAL_TYPE_BLITTABLEVALUECLASSWITHCOPYCTOR:
strRetVal = W("blittable value class with copy constructor");
break;
#ifdef FEATURE_COMINTEROP
case MARSHAL_TYPE_OBJECT:
strRetVal = W("VARIANT");
Expand Down
2 changes: 1 addition & 1 deletion src/vm/mtypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ DEFINE_MARSHALER_TYPE(MARSHAL_TYPE_VALUECLASS, ValueClassMa

DEFINE_MARSHALER_TYPE(MARSHAL_TYPE_REFERENCECUSTOMMARSHALER, ReferenceCustomMarshaler, false)
DEFINE_MARSHALER_TYPE(MARSHAL_TYPE_ARGITERATOR, ArgIteratorMarshaler, false)

DEFINE_MARSHALER_TYPE(MARSHAL_TYPE_BLITTABLEVALUECLASSWITHCOPYCTOR, BlittableValueClassWithCopyCtorMarshaler, false)

#ifdef FEATURE_COMINTEROP
DEFINE_MARSHALER_TYPE(MARSHAL_TYPE_OBJECT, ObjectMarshaler, false)
Expand Down
1 change: 1 addition & 0 deletions tests/src/Interop/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,6 @@ if(WIN32)
if(NOT CLR_CMAKE_PLATFORM_ARCH_ARM64)
add_subdirectory(IJW/ManagedCallingNative/IjwNativeDll)
add_subdirectory(IJW/NativeCallingManaged/IjwNativeCallingManagedDll)
add_subdirectory(IJW/CopyConstructorMarshaler)
endif()
endif(WIN32)
42 changes: 42 additions & 0 deletions tests/src/Interop/IJW/CopyConstructorMarshaler/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
cmake_minimum_required (VERSION 2.6)
project (CopyConstructorMarshaler)
include_directories( ${INC_PLATFORM_DIR} )
set(SOURCES IjwCopyConstructorMarshaler.cpp)

if (WIN32)
# 4365 - signed/unsigned mismatch
add_compile_options(/wd4365)

# IJW
add_compile_options(/clr)

# IJW requires the CRT as a dll, not linked in
add_compile_options(/MD$<$<OR:$<CONFIG:Debug>,$<CONFIG:Checked>>:d>)

# CMake enables /RTC1 and /EHsc by default, but they're not compatible with /clr, so remove them
if(CMAKE_CXX_FLAGS_DEBUG MATCHES "/RTC1")
string(REPLACE "/RTC1" " " CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}")
endif()

if(CMAKE_CXX_FLAGS MATCHES "/EHsc")
string(REPLACE "/EHsc" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
endif()

# IJW isn't compatible with CFG
if(CMAKE_CXX_FLAGS MATCHES "/guard:cf")
string(REPLACE "/guard:cf" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
endif()

# IJW isn't compatible with GR-
if(CMAKE_CXX_FLAGS MATCHES "/GR-")
string(REPLACE "/GR-" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
endif()

endif()

# add the shared library
add_library (IjwCopyConstructorMarshaler SHARED ${SOURCES})
target_link_libraries(IjwCopyConstructorMarshaler ${LINK_LIBRARIES_ADDITIONAL})

# add the install targets
install (TARGETS IjwCopyConstructorMarshaler DESTINATION bin)
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.IO;
using System.Reflection;
using System.Runtime.InteropServices;
using TestLibrary;

namespace CopyConstructorMarshaler
{
class CopyConstructorMarshaler
{
static int Main(string[] args)
{
if(Environment.OSVersion.Platform != PlatformID.Win32NT || TestLibrary.Utilities.IsWindows7)
{
return 100;
}

try
{
// Load a fake mscoree.dll to avoid starting desktop
LoadLibraryEx(Path.Combine(Environment.CurrentDirectory, "mscoree.dll"), IntPtr.Zero, 0);

Assembly ijwNativeDll = Assembly.Load("IjwCopyConstructorMarshaler");
Type testType = ijwNativeDll.GetType("TestClass");
object testInstance = Activator.CreateInstance(testType);
MethodInfo testMethod = testType.GetMethod("PInvokeNumCopies");

// PInvoke will copy twice. Once from argument to parameter, and once from the managed to native parameter.
Assert.AreEqual(2, (int)testMethod.Invoke(testInstance, null));

testMethod = testType.GetMethod("ReversePInvokeNumCopies");

// Reverse PInvoke will copy 3 times. Two are from the same paths as the PInvoke,
// and the third is from the reverse P/Invoke call.
Assert.AreEqual(3, (int)testMethod.Invoke(testInstance, null));

testMethod = testType.GetMethod("PInvokeNumCopiesDerivedType");

// PInvoke will copy twice. Once from argument to parameter, and once from the managed to native parameter.
Assert.AreEqual(2, (int)testMethod.Invoke(testInstance, null));

testMethod = testType.GetMethod("ReversePInvokeNumCopiesDerivedType");

// Reverse PInvoke will copy 3 times. Two are from the same paths as the PInvoke,
// and the third is from the reverse P/Invoke call.
Assert.AreEqual(3, (int)testMethod.Invoke(testInstance, null));
}
catch (Exception ex)
{
Console.WriteLine(ex);
return 101;
}
return 100;
}

[DllImport("kernel32.dll")]
static extern IntPtr LoadLibraryEx(string lpFileName, IntPtr hReservedNull, int dwFlags);

[DllImport("kernel32.dll")]
static extern IntPtr GetModuleHandle(string lpModuleName);
}
}
Loading