-
Notifications
You must be signed in to change notification settings - Fork 198
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
Support arrays of types with simple custom marshalling #379
Support arrays of types with simple custom marshalling #379
Conversation
…lling and gracefully fail when an array of a type with complex custom marshalling (Value property) is introduced.
DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs
Outdated
Show resolved
Hide resolved
DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs
Show resolved
Hide resolved
Could you please share code-snippets that this generates? |
I've added the example from the integration tests into the PR description. The only part that's "custom marshalling"-specific is the body of the for loop. The rest is the same array code as in the main branch. |
What is it going to do if the marshaller has |
If the marshaller has a FreeNative method, the code-gen will have another if (pArray != null)
for (int __i = 0; __i < pArray.Length; ++__i)
{
__pArray_gen_native[__i].FreeNative();
} |
Does this need to account for error handling? What if the initialization of the array failed half-way through? |
If the initialization failed halfway through, the native structs should be default initialized. The implementor of the native struct should handle this scenario. I may have to add handling for that case. |
I thought we were going to assume non-exception cases only at least custom marshalling. In this case that would seem to imply that if the custom marshalling throws, we can either let them handle the possible clean or determine which of the elements were initialized and only free them. @jkotas are you implying the latter? |
…ntify when an element is empty due to not being marshalled.
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
I've added in some basic handling for this case similar to the old system where we zero-init the memory before array marshalling in case we only partially marshal. I also fixed a bug around marshalling arrays of pointers since I found it along the way. |
This works, but it may be more efficient to keep track of how many elements of the array were marshalled successfully and cleanup only those. |
Also, what does would the code look for |
Agree with that. |
I've added a more comprehensive sample to the PR description that covers the I prefer to use the mem-clear method initially since that's what the old system used and it still leaves open the option for switching methods in the future. |
This is prone to buffer overruns. The length should be cached to avoid this problem. |
This is prone to overflow. It should be |
Which loop is this referring to? There's multiple loops with this condition (one in marshal, one in unmarshal). Since both of these comments are relating to general array marshalling, not marshalling of arrays of custom marshaled types, I'm going to put out a separate PR for these fixes. |
The problem is in the loops that operate on arrays passed in via |
This can be just |
…elab into arrays-with-custom-marshalled-elements
DllImportGenerator/DllImportGenerator/Marshalling/MarshallingGenerator.cs
Outdated
Show resolved
Hide resolved
Signed-off-by: Jeremy Koritzinsky <jekoritz@microsoft.com>
…elab into arrays-with-custom-marshalled-elements
Up to date stub example with BoolStruct (taken from the integration tests): namespace DllImportGenerator.IntegrationTests
{
partial class NativeExportsNE
{
public partial class Arrays
{
[System.Runtime.CompilerServices.SkipLocalsInitAttribute]
public static partial bool AndAllMembers(global::SharedTypes.BoolStruct[] pArray, int length)
{
unsafe
{
global::SharedTypes.BoolStructNative*__pArray_gen_native = default;
bool __retVal = default;
byte __retVal_gen_native = default;
//
// Setup
//
bool pArray__allocated = false;
try
{
//
// Marshal
//
if (pArray != null)
{
int pArray__bytelen = checked(sizeof(global::SharedTypes.BoolStructNative) * pArray.Length);
if (pArray__bytelen > 512)
{
__pArray_gen_native = (global::SharedTypes.BoolStructNative*)System.Runtime.InteropServices.Marshal.AllocCoTaskMem(pArray__bytelen);
pArray__allocated = true;
}
else
{
byte *pArray__stackptr = stackalloc byte[pArray__bytelen];
;
__pArray_gen_native = (global::SharedTypes.BoolStructNative*)pArray__stackptr;
}
}
if (pArray != null)
{
new System.Span<global::SharedTypes.BoolStructNative>((global::SharedTypes.BoolStructNative*)__pArray_gen_native, pArray.Length).Clear();
for (int __i = 0; __i < pArray.Length; ++__i)
{
__pArray_gen_native[__i] = new global::SharedTypes.BoolStructNative(pArray[__i]);
}
}
//
// Invoke
//
__retVal_gen_native = AndAllMembers__PInvoke__(__pArray_gen_native, length);
//
// Unmarshal
//
__retVal = __retVal_gen_native != 0;
}
finally
{
//
// Cleanup
//
if (pArray__allocated)
{
System.Runtime.InteropServices.Marshal.FreeCoTaskMem((System.IntPtr)__pArray_gen_native);
}
}
return __retVal;
}
}
[System.Runtime.InteropServices.DllImportAttribute("Microsoft.Interop.Tests.NativeExportsNE", EntryPoint = "and_all_members")]
extern private static unsafe byte AndAllMembers__PInvoke__(global::SharedTypes.BoolStructNative*pArray, int length);
}
}
} |
Here's a more comprehensive one taken from the unit tests: Source: using System.Runtime.InteropServices;
partial class Test
{
[GeneratedDllImport("DoesNotExist")]
[return:MarshalAs(UnmanagedType.LPArray, SizeConst=10)]
public static partial IntStructWrapper[] Method(
IntStructWrapper[] p,
in IntStructWrapper[] pIn,
int pRefSize,
[MarshalAs(UnmanagedType.LPArray, SizeParamIndex=2)] ref IntStructWrapper[] pRef,
[MarshalAs(UnmanagedType.LPArray, SizeParamIndex=5, SizeConst=4)] out IntStructWrapper[] pOut,
out int pOutSize
);
}
[NativeMarshalling(typeof(IntStructWrapperNative))]
public struct IntStructWrapper
{
public int Value;
}
public struct IntStructWrapperNative
{
private int value;
public IntStructWrapperNative(IntStructWrapper managed)
{
value = managed.Value;
}
public IntStructWrapper ToManaged() => new IntStructWrapper { Value = value };
} Generated code: partial class Test
{
[System.Runtime.CompilerServices.SkipLocalsInitAttribute]
public static partial global::IntStructWrapper[] Method(global::IntStructWrapper[] p, in global::IntStructWrapper[] pIn, int pRefSize, ref global::IntStructWrapper[] pRef, out global::IntStructWrapper[] pOut, out int pOutSize)
{
unsafe
{
global::IntStructWrapperNative*__p_gen_native = default;
global::IntStructWrapperNative*__pIn_gen_native = default;
global::IntStructWrapperNative*__pRef_gen_native = default;
pOut = default;
global::IntStructWrapperNative*__pOut_gen_native = default;
pOutSize = default;
global::IntStructWrapper[] __retVal = default;
global::IntStructWrapperNative*__retVal_gen_native = default;
//
// Setup
//
bool p__allocated = false;
bool pIn__allocated = false;
global::IntStructWrapper[] pIn_local = pIn;
global::IntStructWrapper[] pRef_local = pRef;
global::IntStructWrapper[] pOut_local = pOut;
try
{
//
// Marshal
//
if (p != null)
{
int p__bytelen = checked(sizeof(global::IntStructWrapperNative) * p.Length);
if (p__bytelen > 512)
{
__p_gen_native = (global::IntStructWrapperNative*)System.Runtime.InteropServices.Marshal.AllocCoTaskMem(p__bytelen);
p__allocated = true;
}
else
{
byte *p__stackptr = stackalloc byte[p__bytelen];
;
__p_gen_native = (global::IntStructWrapperNative*)p__stackptr;
}
}
if (p != null)
{
new System.Span<global::IntStructWrapperNative>((global::IntStructWrapperNative*)__p_gen_native, p.Length).Clear();
for (int __i = 0; __i < p.Length; ++__i)
{
__p_gen_native[__i] = new global::IntStructWrapperNative(p[__i]);
}
}
if (pIn_local != null)
{
int pIn__bytelen = checked(sizeof(global::IntStructWrapperNative) * pIn_local.Length);
if (pIn__bytelen > 512)
{
__pIn_gen_native = (global::IntStructWrapperNative*)System.Runtime.InteropServices.Marshal.AllocCoTaskMem(pIn__bytelen);
pIn__allocated = true;
}
else
{
byte *pIn__stackptr = stackalloc byte[pIn__bytelen];
;
__pIn_gen_native = (global::IntStructWrapperNative*)pIn__stackptr;
}
}
if (pIn_local != null)
{
new System.Span<global::IntStructWrapperNative>((global::IntStructWrapperNative*)__pIn_gen_native, pIn.Length).Clear();
for (int __i = 0; __i < pIn.Length; ++__i)
{
__pIn_gen_native[__i] = new global::IntStructWrapperNative(pIn_local[__i]);
}
}
__pRef_gen_native = null;
if (pRef_local != null)
{
int pRef__bytelen = checked(sizeof(global::IntStructWrapperNative) * pRef_local.Length);
__pRef_gen_native = (global::IntStructWrapperNative*)System.Runtime.InteropServices.Marshal.AllocCoTaskMem(pRef__bytelen);
}
if (pRef_local != null)
{
new System.Span<global::IntStructWrapperNative>((global::IntStructWrapperNative*)__pRef_gen_native, pRef.Length).Clear();
for (int __i = 0; __i < pRef.Length; ++__i)
{
__pRef_gen_native[__i] = new global::IntStructWrapperNative(pRef_local[__i]);
}
}
//
// Invoke
//
fixed (int *__pOutSize_gen_native = &pOutSize)
__retVal_gen_native = Method__PInvoke__(__p_gen_native, &__pIn_gen_native, pRefSize, &__pRef_gen_native, &__pOut_gen_native, __pOutSize_gen_native);
//
// Unmarshal
//
if (__retVal_gen_native != null)
{
__retVal = new global::IntStructWrapper[10];
for (int __i = 0; __i < __retVal.Length; ++__i)
{
__retVal[__i] = __retVal_gen_native[__i].ToManaged();
}
}
else
__retVal = null;
if (__pRef_gen_native != null)
{
pRef_local = new global::IntStructWrapper[checked((int)pRefSize)];
for (int __i = 0; __i < pRef_local.Length; ++__i)
{
pRef_local[__i] = __pRef_gen_native[__i].ToManaged();
}
}
else
pRef_local = null;
pRef = pRef_local;
if (__pOut_gen_native != null)
{
pOut_local = new global::IntStructWrapper[checked(4 + (int)pOutSize)];
for (int __i = 0; __i < pOut_local.Length; ++__i)
{
pOut_local[__i] = __pOut_gen_native[__i].ToManaged();
}
}
else
pOut_local = null;
pOut = pOut_local;
}
finally
{
//
// Cleanup
//
if (p__allocated)
{
System.Runtime.InteropServices.Marshal.FreeCoTaskMem((System.IntPtr)__p_gen_native);
}
if (pIn__allocated)
{
System.Runtime.InteropServices.Marshal.FreeCoTaskMem((System.IntPtr)__pIn_gen_native);
}
System.Runtime.InteropServices.Marshal.FreeCoTaskMem((System.IntPtr)__pRef_gen_native);
System.Runtime.InteropServices.Marshal.FreeCoTaskMem((System.IntPtr)__pOut_gen_native);
}
return __retVal;
}
}
[System.Runtime.InteropServices.DllImportAttribute("DoesNotExist", EntryPoint = "Method")]
extern private static unsafe global::IntStructWrapperNative*Method__PInvoke__(global::IntStructWrapperNative*p, global::IntStructWrapperNative**pIn, int pRefSize, global::IntStructWrapperNative**pRef, global::IntStructWrapperNative**pOut, int *pOutSize);
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add some notes about arrays to the compat doc - multi-dimensional not supported and support for nested array types?
The compat doc already mentions multidimensional arrays (it says only single dimensional arrays are supported). I've added a little blurb about current support for arrays of arrays. |
Add support for marshalling arrays of types with simple custom marshalling (no Value property) and gracefully fail when an array of a type with complex custom marshalling (Value property) is introduced.
To do so, we create a new
ArrayMarshallingInfo
subtype ofMarshallingInfo
for the default array marshalling case and pre-calculate the element type's marshalling information instead of calculating it in MarshallingGenerator.csGenerated:
Generated: