Skip to content

Commit 3686fea

Browse files
manodasanWCopilotSergio0694
authored
Fix constructor marshalling for non-blittable array element types (#2474)
* Fix constructor marshalling for non-blittable array element types Activation-factory constructors taking arrays of non-blittable structs, mapped value types (DateTime/TimeSpan), or HResult/Exception elements hardcoded nint InlineArray storage and void** CopyToUnmanaged/Dispose data pointers, so the array marshaller's UnsafeAccessor failed to bind at runtime (Method not found: ...ArrayMarshaller.Dispose). Route the constructor's PassArray storage, CopyToUnmanaged, and finally cleanup through the element-kind-aware helpers (GetArrayElementStorageType and typed data-pointer/cast selection), mirroring the RCW caller. Add a TestComponentCSharp constructor and TestNonBlittableArrayConstructor test covering all three element kinds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR feedback: simplify constructor array CopyToUnmanaged cast Drop the separate dataCastType and derive the cast inline from dataParamType. Keep the if/else style (matching the RCW caller's equivalent logic) with the same IDE0045 suppression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Refactor ConstructorFactory comments and suppress IDE0045 Replace SuppressMessage attribute with a file-level pragma to disable IDE0045 and adjust comment wording/formatting across ConstructorFactory.FactoryCallbacks.cs. Changes are purely cosmetic (comments, quoting, punctuation) and do not modify runtime behavior or logic. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Sergio Pedri <sergio0694@live.com>
1 parent fe8dbf4 commit 3686fea

5 files changed

Lines changed: 132 additions & 20 deletions

File tree

src/Tests/TestComponentCSharp/Class.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,23 @@ namespace winrt::TestComponentCSharp::implementation
230230
_nonBlittableStruct = nonBlittableStruct;
231231
}
232232

233+
Class::Class(int32_t intProperty, array_view<TestComponentCSharp::ComposedNonBlittableStruct const> nonBlittableStructs, array_view<winrt::Windows::Foundation::DateTime const> dateTimes, array_view<winrt::hresult const> hresults) :
234+
Class(intProperty, L"")
235+
{
236+
if (nonBlittableStructs.size() > 0)
237+
{
238+
_nonBlittableStruct = nonBlittableStructs[0];
239+
}
240+
if (dateTimes.size() > 0)
241+
{
242+
_dateTime = dateTimes[0];
243+
}
244+
if (hresults.size() > 0)
245+
{
246+
_hr = hresults[0];
247+
}
248+
}
249+
233250
void Class::TypeProperty(Windows::UI::Xaml::Interop::TypeName val)
234251
{
235252
_typeProperty = val;

src/Tests/TestComponentCSharp/Class.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ namespace winrt::TestComponentCSharp::implementation
100100
Class(int32_t intProperty);
101101
Class(int32_t intProperty, hstring const& stringProperty);
102102
Class(int32_t intProperty, hstring const& stringProperty, ComposedNonBlittableStruct const& nonBlittableStruct);
103+
Class(int32_t intProperty, array_view<TestComponentCSharp::ComposedNonBlittableStruct const> nonBlittableStructs, array_view<winrt::Windows::Foundation::DateTime const> dateTimes, array_view<winrt::hresult const> hresults);
103104
static int32_t StaticIntProperty();
104105
static void StaticIntProperty(int32_t value);
105106
static winrt::event_token StaticIntPropertyChanged(Windows::Foundation::EventHandler<int32_t> const& handler);

src/Tests/TestComponentCSharp/TestComponentCSharp.idl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,8 @@ namespace TestComponentCSharp
213213
Class(Int32 intProperty, String stringProperty);
214214
// Activation factory taking a non-blittable struct by value.
215215
Class(Int32 intProperty, String stringProperty, ComposedNonBlittableStruct nonBlittableStruct);
216+
// Activation factory taking arrays of non-blittable struct, mapped value-type, and HResult elements.
217+
Class(Int32 intProperty, ComposedNonBlittableStruct[] nonBlittableStructs, Windows.Foundation.DateTime[] dateTimes, Windows.Foundation.HResult[] hresults);
216218

217219
Windows.UI.Xaml.Interop.TypeName TypeProperty{ get; set; };
218220
String GetTypePropertyAbiName();

src/Tests/UnitTest/TestComponentCSharp_Tests.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2306,6 +2306,33 @@ public void TestNonBlittableStructConstructor()
23062306
Assert.AreEqual(5, instance.IntProperty);
23072307
}
23082308

2309+
[TestMethod]
2310+
public void TestNonBlittableArrayConstructor()
2311+
{
2312+
// Activation factory taking arrays of non-blittable struct, mapped value-type, and
2313+
// HResult/Exception elements by value (constructor PassArray marshalling).
2314+
var structVal = new ComposedNonBlittableStruct()
2315+
{
2316+
blittable = new BlittableStruct() { i32 = 42 },
2317+
strings = new NonBlittableStringStruct() { str = "I like tacos" },
2318+
bools = new NonBlittableBoolStruct() { w = true, x = false, y = true, z = false },
2319+
refs = TestObject.NonBlittableRefStructProperty
2320+
};
2321+
var dateTime = new DateTimeOffset(2021, 1, 1, 0, 0, 0, TimeSpan.Zero);
2322+
var exception = new ArgumentException();
2323+
2324+
var instance = new Class(
2325+
9,
2326+
new[] { structVal },
2327+
new[] { dateTime },
2328+
new Exception[] { exception });
2329+
2330+
Assert.AreEqual(9, instance.IntProperty);
2331+
Assert.AreEqual("I like tacos", instance.ComposedNonBlittableStructProperty.strings.str);
2332+
Assert.AreEqual(dateTime, instance.DateTimeProperty);
2333+
Assert.IsNotNull(instance.HResultProperty);
2334+
}
2335+
23092336
[TestMethod]
23102337
public void TestBlittableArrays()
23112338
{

src/WinRT.Projection.Writer/Factories/ConstructorFactory.FactoryCallbacks.cs

Lines changed: 85 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
using WindowsRuntime.ProjectionWriter.Resolvers;
1313
using WindowsRuntime.ProjectionWriter.Writers;
1414

15+
#pragma warning disable IDE0045
16+
1517
namespace WindowsRuntime.ProjectionWriter.Factories;
1618

1719
internal static partial class ConstructorFactory
@@ -296,13 +298,14 @@ private sealed class {{callbackName}} : {{baseClass}}
296298
string raw = p.GetRawName();
297299
string callName = IdentifierEscaping.EscapeIdentifier(raw);
298300
ArrayTempNames names = new(raw);
301+
string storageT = AbiTypeHelpers.GetArrayElementStorageType(context, szArr.BaseType);
299302
writer.WriteLine();
300303
writer.WriteLine(isMultiline: true, $$"""
301-
Unsafe.SkipInit(out InlineArray16<nint> {{names.InlineArray}});
302-
nint[] {{names.ArrayFromPool}} = null;
303-
Span<nint> {{names.Span}} = {{callName}}.Length <= 16
304+
Unsafe.SkipInit(out InlineArray16<{{storageT}}> {{names.InlineArray}});
305+
{{storageT}}[] {{names.ArrayFromPool}} = null;
306+
Span<{{storageT}}> {{names.Span}} = {{callName}}.Length <= 16
304307
? {{names.InlineArray}}[..{{callName}}.Length]
305-
: ({{names.ArrayFromPool}} = global::System.Buffers.ArrayPool<nint>.Shared.Rent({{callName}}.Length));
308+
: ({{names.ArrayFromPool}} = global::System.Buffers.ArrayPool<{{storageT}}>.Shared.Rent({{callName}}.Length));
306309
""");
307310

308311
if (szArr.BaseType.IsString())
@@ -467,7 +470,7 @@ private sealed class {{callbackName}} : {{baseClass}}
467470
}
468471
}
469472

470-
// Emit CopyToUnmanaged for non-blittable PassArray params.
473+
// Emit 'CopyToUnmanaged' for non-blittable PassArray params
471474
for (int i = 0; i < paramCount; i++)
472475
{
473476
ParameterInfo p = sig.Parameters[i];
@@ -505,14 +508,35 @@ private sealed class {{callbackName}} : {{baseClass}}
505508
else
506509
{
507510
IndentedTextWriterCallback elementProjected = TypedefNameWriter.WriteProjectionType(context, TypeSemanticsFactory.Get(szArr.BaseType));
511+
512+
// Data pointer type must match the array marshaller's 'CopyToUnmanaged' signature
513+
string dataParamType;
514+
515+
if (context.AbiTypeKindResolver.IsMappedAbiValueType(szArr.BaseType))
516+
{
517+
dataParamType = AbiTypeHelpers.GetMappedAbiTypeName(szArr.BaseType) + "*";
518+
}
519+
else if (szArr.BaseType.IsHResultException())
520+
{
521+
dataParamType = "global::ABI.System.Exception*";
522+
}
523+
else if (context.AbiTypeKindResolver.IsNonBlittableStruct(szArr.BaseType))
524+
{
525+
dataParamType = AbiTypeHelpers.GetAbiStructTypeName(context, szArr.BaseType) + "*";
526+
}
527+
else
528+
{
529+
dataParamType = "void**";
530+
}
531+
508532
UnsafeAccessorFactory.EmitStaticMethod(
509533
writer,
510534
accessName: "CopyToUnmanaged",
511535
returnType: "void",
512536
functionName: $"CopyToUnmanaged_{raw}",
513537
interopType: ArrayElementEncoder.GetArrayMarshallerInteropPath(szArr.BaseType),
514-
parameterList: $"ReadOnlySpan<{elementProjected.Format()}> span, uint length, void** data");
515-
writer.WriteLine($"CopyToUnmanaged_{raw}(null, {pname}, (uint){pname}.Length, (void**)_{raw});");
538+
parameterList: $"ReadOnlySpan<{elementProjected.Format()}> span, uint length, {dataParamType} data");
539+
writer.WriteLine($"CopyToUnmanaged_{raw}(null, {pname}, (uint){pname}.Length, ({dataParamType})_{raw});");
516540
}
517541
}
518542

@@ -534,7 +558,7 @@ private sealed class {{callbackName}} : {{baseClass}}
534558

535559
if (isComposable)
536560
{
537-
// Composable extras: baseInterface (void*), out innerInterface (void**)
561+
// Composable extras: 'baseInterface (void*), out innerInterface (void**)'
538562
writer.Write("void*, void**, ");
539563
}
540564

@@ -555,12 +579,12 @@ private sealed class {{callbackName}} : {{baseClass}}
555579
continue;
556580
}
557581

558-
// For enums, cast to underlying type. For bool, cast to byte. For char, cast to ushort.
559-
// For string params, use the marshalled HString from the fixed block.
560-
// For runtime class / object / generic instance params, use __<name>.GetThisPtrUnsafe().
582+
// For enums, cast to underlying type. For 'bool', cast to 'byte'. For 'char', cast to 'ushort'.
583+
// For 'string' params, use the marshalled 'HSTRING' from the fixed block.
584+
// For runtime class / 'object' / generic instance params, use '__<name>.GetThisPtrUnsafe()'.
561585
if (context.AbiTypeKindResolver.IsEnumType(p.Type))
562586
{
563-
// No cast needed: function pointer signature uses the projected enum type.
587+
// No cast needed: function pointer signature uses the projected enum type
564588
writer.Write(pname);
565589
}
566590
else if (p.Type is CorLibTypeSignature corlibBool &&
@@ -605,7 +629,7 @@ private sealed class {{callbackName}} : {{baseClass}}
605629

606630
if (isComposable)
607631
{
608-
// Pass __baseInterface.GetThisPtrUnsafe() and &__innerInterface.
632+
// Pass '__baseInterface.GetThisPtrUnsafe()' and '&__innerInterface'
609633
writer.Write(isMultiline: true, """
610634
,
611635
__baseInterface.GetThisPtrUnsafe(),
@@ -623,7 +647,7 @@ private sealed class {{callbackName}} : {{baseClass}}
623647

624648
writer.WriteLine("retval = __retval;");
625649

626-
// Close fixed blocks (innermost first).
650+
// Close fixed blocks (innermost first)
627651
for (int i = 0; i < fixedNesting; i++)
628652
{
629653
writer.DecreaseIndent();
@@ -642,8 +666,8 @@ private sealed class {{callbackName}} : {{baseClass}}
642666
""");
643667
writer.IncreaseIndent();
644668

645-
// Dispose pre-marshalled ABI struct input locals (frees any HSTRING / boxed
646-
// reference fields the per-field ConvertToUnmanaged may have allocated).
669+
// Dispose pre-marshalled ABI struct input locals (frees any 'HSTRING' / boxed
670+
// reference fields the per-field 'ConvertToUnmanaged' may have allocated).
647671
for (int i = 0; i < paramCount; i++)
648672
{
649673
ParameterInfo p = sig.Parameters[i];
@@ -677,9 +701,28 @@ private sealed class {{callbackName}} : {{baseClass}}
677701
continue;
678702
}
679703

704+
// Mapped value types ('DateTime'/'TimeSpan') need no disposal or pool return
705+
if (context.AbiTypeKindResolver.IsMappedAbiValueType(szArr.BaseType))
706+
{
707+
continue;
708+
}
709+
680710
string raw = p.GetRawName();
681711
ArrayTempNames names = new(raw);
682712

713+
if (szArr.BaseType.IsHResultException())
714+
{
715+
// 'HResult' ABI is just an 'int': no per-element 'Dispose', only the pool return
716+
writer.WriteLine();
717+
writer.WriteLine(isMultiline: true, $$"""
718+
if ({{names.ArrayFromPool}} is not null)
719+
{
720+
global::System.Buffers.ArrayPool<global::ABI.System.Exception>.Shared.Return({{names.ArrayFromPool}});
721+
}
722+
""");
723+
continue;
724+
}
725+
683726
if (szArr.BaseType.IsString())
684727
{
685728
writer.WriteLine();
@@ -699,27 +742,49 @@ private sealed class {{callbackName}} : {{baseClass}}
699742
}
700743
else
701744
{
745+
// Complex structs use a typed <ABI struct>* (no cast), ref types use 'void**'
746+
string disposeDataParamType;
747+
string fixedPtrType;
748+
string disposeCastType;
749+
750+
if (context.AbiTypeKindResolver.IsNonBlittableStruct(szArr.BaseType))
751+
{
752+
string abiStructName = AbiTypeHelpers.GetAbiStructTypeName(context, szArr.BaseType);
753+
disposeDataParamType = abiStructName + "* data";
754+
fixedPtrType = abiStructName + "*";
755+
disposeCastType = string.Empty;
756+
}
757+
else
758+
{
759+
disposeDataParamType = "void** data";
760+
fixedPtrType = "void*";
761+
disposeCastType = "(void**)";
762+
}
763+
702764
writer.WriteLine();
703765
UnsafeAccessorFactory.EmitStaticMethod(
704766
writer,
705767
accessName: "Dispose",
706768
returnType: "void",
707769
functionName: $"Dispose_{raw}",
708770
interopType: ArrayElementEncoder.GetArrayMarshallerInteropPath(szArr.BaseType),
709-
parameterList: "uint length, void** data");
771+
parameterList: $"uint length, {disposeDataParamType}");
710772
writer.WriteLine();
711773
writer.WriteLine(isMultiline: true, $$"""
712-
fixed(void* _{{raw}} = {{names.Span}})
774+
fixed({{fixedPtrType}} _{{raw}} = {{names.Span}})
713775
{
714-
Dispose_{{raw}}(null, (uint) {{names.Span}}.Length, (void**)_{{raw}});
776+
Dispose_{{raw}}(null, (uint) {{names.Span}}.Length, {{disposeCastType}}_{{raw}});
715777
}
716778
""");
717779
}
780+
781+
// Pool storage type matches the 'InlineArray16<storageT>' setup
782+
string poolStorageT = AbiTypeHelpers.GetArrayElementStorageType(context, szArr.BaseType);
718783
writer.WriteLine();
719784
writer.WriteLine(isMultiline: true, $$"""
720785
if ({{names.ArrayFromPool}} is not null)
721786
{
722-
global::System.Buffers.ArrayPool<nint>.Shared.Return({{names.ArrayFromPool}});
787+
global::System.Buffers.ArrayPool<{{poolStorageT}}>.Shared.Return({{names.ArrayFromPool}});
723788
}
724789
""");
725790
}

0 commit comments

Comments
 (0)