From 6500e7480b464d83c06da0818ddfd2417636ac8c Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Sun, 18 Oct 2026 13:13:31 -0700 Subject: [PATCH] Support BOOLEAN-returning cleanup functions for SafeHandle owners IsSupportedCleanupReturn rejected cleanup functions that return BOOLEAN, such as HidD_FreePreparsedData, so their owners were generated as abstract and producers kept raw outputs. BOOLEAN is now handled like BOOL: the generated ReleaseHandle returns the result through BOOLEAN's implicit bool conversion, where non-zero is success. Related: microsoft/win32metadata#2375 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0aa7080c-77a0-44e7-8869-029671991a9a --- .../Generator.Handle.cs | 4 +- .../Generator.SafeHandleLease.cs | 2 +- .../ExternalMetadata/NOTICE.md | 2 +- .../ExternalMetadata/SafeHandleOwnership.il | 35 +++++++++++++++- .../SafeHandleOwnership.winmd | Bin 4096 -> 4608 bytes .../SafeHandleLeaseTests.cs | 38 ++++++++++++++++++ 6 files changed, 76 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Windows.CsWin32/Generator.Handle.cs b/src/Microsoft.Windows.CsWin32/Generator.Handle.cs index aee1316d..bb8c0d5e 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.Handle.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.Handle.cs @@ -209,7 +209,7 @@ public partial class Generator { // protected override [unsafe] bool ReleaseHandle() => ReleaseMethod((struct)this.handle); // Special case release functions based on their return type as follows: (https://github.com/microsoft/win32metadata/issues/25) - // * bool => true is success + // * bool, BOOL, BOOLEAN => true (non-zero) is success // * int => zero is success // * uint => zero is success // * byte => non-zero is success @@ -226,7 +226,7 @@ public partial class Generator // As an example of that see WlanCloseHandle function. var releaseMethodIsUnsafe = releaseMethodHasReservedParameters; if (!(releaseMethodReturnType.Type is PredefinedTypeSyntax { Keyword: { RawKind: (int)SyntaxKind.BoolKeyword } } || - releaseMethodReturnType.Type is QualifiedNameSyntax { Right: { Identifier: { ValueText: "BOOL" } } })) + releaseMethodReturnType.Type is QualifiedNameSyntax { Right: { Identifier: { ValueText: "BOOL" or "BOOLEAN" } } })) { switch (releaseMethodReturnType.Type) { diff --git a/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs b/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs index 68b8c404..8d2b6a14 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs @@ -40,7 +40,7 @@ internal bool TryGetResourceReleaseMethod(TypeHandleInfo valueType, CustomAttrib private static bool IsSupportedCleanupReturn(TypeSyntax type, string method) => type switch { PredefinedTypeSyntax predefined => predefined.Keyword.Kind() is SyntaxKind.BoolKeyword or SyntaxKind.IntKeyword or SyntaxKind.UIntKeyword or SyntaxKind.ByteKeyword or SyntaxKind.VoidKeyword, - QualifiedNameSyntax { Right: IdentifierNameSyntax name } => name.Identifier.ValueText is "BOOL" or "NTSTATUS" or "HRESULT" or "WIN32_ERROR" or "CONFIGRET" or "HGLOBAL" or "HLOCAL" + QualifiedNameSyntax { Right: IdentifierNameSyntax name } => name.Identifier.ValueText is "BOOL" or "BOOLEAN" or "NTSTATUS" or "HRESULT" or "WIN32_ERROR" or "CONFIGRET" or "HGLOBAL" or "HLOCAL" || (name.Identifier.ValueText == "LRESULT" && method == "ICClose"), PointerTypeSyntax { ElementType: PredefinedTypeSyntax predefined } => predefined.Keyword.IsKind(SyntaxKind.VoidKeyword) && method == "FreeSid", _ => false, diff --git a/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/NOTICE.md b/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/NOTICE.md index 0987f19a..c81301d3 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/NOTICE.md +++ b/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/NOTICE.md @@ -6,6 +6,6 @@ ServiceFabric.winmd | [youyuanwu/fabric-metadata](https://github.com/youyuanwu/f CustomIInspectable.winmd | Generated using WinMDGenerator toolchain. Project in the subdirectory [CustomIInspectable](../../CustomIInspectable) with build instructions in [readme.md](../../CustomIInspectable/readme.md). WnfWithoutStatusSuccess.winmd | Authored fixture for issue #1677 regression coverage. Its source is checked in as [WnfWithoutStatusSuccess.il](WnfWithoutStatusSuccess.il). We keep this one as IL because that is much easier to maintain than reproducing the full end-to-end WinmdGenerator workflow for a focused test asset. SelfContained.winmd | Authored fixture reproducing the OSClient single-`Generator` build configuration (a non-`Windows.Win32` metadata assembly with the Windows.Win32 projection provided by a reference), where special string typedefs (`PCWSTR`) must be referenced under `Windows.Win32.Foundation`. Its source is checked in as [SelfContained.il](SelfContained.il). Kept as IL for the same reason as WnfWithoutStatusSuccess.winmd. -SafeHandleOwnership.winmd | Authored fixture for contextual `RAIIFree` and `FreeWith` ownership, HANDLE input inference without type-level ownership, exact native value types, and caller-defined cleanup. Its source is checked in as [SafeHandleOwnership.il](SafeHandleOwnership.il), including the IL assembler command. Tests load this asset rather than emitting metadata at runtime. +SafeHandleOwnership.winmd | Authored fixture for contextual `RAIIFree` and `FreeWith` ownership, HANDLE input inference without type-level ownership, exact native value types, caller-defined cleanup, and cleanup functions that return `BOOLEAN`. Its source is checked in as [SafeHandleOwnership.il](SafeHandleOwnership.il), including the IL assembler command. Tests load this asset rather than emitting metadata at runtime. FlatNamespace.winmd | Authored fixture for issue #1848 regression coverage, with native typedefs and associated enums at the common namespace root and in a nested namespace. Its source and regeneration command are checked in as [FlatNamespace.il](FlatNamespace.il). TypeDefConstant.winmd | Authored fixture verifying that struct-typed constants defined in the same metadata module using TypeDefinition handles (rather than TypeReference handles) correctly nest into the struct and attach to extension blocks. Its source is checked in as [TypeDefConstant.il](TypeDefConstant.il). Kept as IL for the same reason as WnfWithoutStatusSuccess.winmd. diff --git a/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/SafeHandleOwnership.il b/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/SafeHandleOwnership.il index 13797318..3d6ae3a4 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/SafeHandleOwnership.il +++ b/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/SafeHandleOwnership.il @@ -2,7 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. // // Contextual ownership without type-level RAIIFree, including exact native types, -// incompatible cleanup, and cleanup requiring caller-supplied context. +// incompatible cleanup, cleanup requiring caller-supplied context, and cleanup returning BOOLEAN. // Assemble with: ilasm /dll /output=SafeHandleOwnership.winmd SafeHandleOwnership.il .assembly extern netstandard { .publickeytoken = (CC 7B 13 FF CD 2D DD 51) .ver 2:1:0:0 } @@ -29,6 +29,13 @@ .field public void* Value } +.class public sequential ansi sealed beforefieldinit Windows.Win32.Foundation.BOOLEAN + extends [netstandard]System.ValueType +{ + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.NativeTypedefAttribute::.ctor() = (01 00 00 00) + .field public uint8 Value +} + .class public sequential ansi sealed beforefieldinit Windows.Win32.Test.RESOURCE_A extends [netstandard]System.ValueType { @@ -90,6 +97,8 @@ void FreeUnsigned(valuetype Windows.Win32.Test.UNSIGNED_RESOURCE resource) cil managed { } .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) void FreeUnsigned32(valuetype Windows.Win32.Test.UNSIGNED32_RESOURCE resource) cil managed { } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Foundation.BOOLEAN FreeBoolean(valuetype Windows.Win32.Test.RESOURCE_A resource) cil managed { } .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) void ConsumeResource([in] valuetype Windows.Win32.Test.RESOURCE_A resource) cil managed { } .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) @@ -171,6 +180,18 @@ .param [0] .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.RAIIFreeAttribute::.ctor(string) = (01 00 0A 46 72 65 65 53 68 61 72 65 64 00 00) } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Test.RESOURCE_A CreateBooleanResource() cil managed + { + .param [0] + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.RAIIFreeAttribute::.ctor(string) = (01 00 0B 46 72 65 65 42 6F 6F 6C 65 61 6E 00 00) + } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Foundation.BOOLEAN AcquireBooleanResource([out] valuetype Windows.Win32.Test.RESOURCE_A* resource) cil managed + { + .param [1] + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.RAIIFreeAttribute::.ctor(string) = (01 00 0B 46 72 65 65 42 6F 6F 6C 65 61 6E 00 00) + } } .class public abstract auto autochar sealed beforefieldinit Windows.Win32.Test.FreeWith.Apis @@ -242,4 +263,16 @@ .param [0] .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.FreeWithAttribute::.ctor(string) = (01 00 0A 46 72 65 65 53 68 61 72 65 64 00 00) } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Test.RESOURCE_A CreateBooleanResource() cil managed + { + .param [0] + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.FreeWithAttribute::.ctor(string) = (01 00 0B 46 72 65 65 42 6F 6F 6C 65 61 6E 00 00) + } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Foundation.BOOLEAN AcquireBooleanResource([out] valuetype Windows.Win32.Test.RESOURCE_A* resource) cil managed + { + .param [1] + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.FreeWithAttribute::.ctor(string) = (01 00 0B 46 72 65 65 42 6F 6F 6C 65 61 6E 00 00) + } } diff --git a/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/SafeHandleOwnership.winmd b/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/SafeHandleOwnership.winmd index 5b2604292f3bdad551e8f028c224eeed4b701b9a..afbc386ea171cdf9cc3ffe4305c0835eea70563b 100644 GIT binary patch literal 4608 zcmeHKU2GIp6h3#hUAEhn?a~%n`J1}sFTnCs44_4KyIW|bwAgKn5);_&PU(>COr4#D zqLCnAqHiRa4L+z3Hey0Te1J%NGSb8sNJvD&i;+f13{iP9B!&dfIdf-rTY*G*GU2B4 zo%@}0@A!P{)5IDJf0DwjcfObh?km7(mUVEl<{} z0D-a*0pwj}JZftbHu4MP9Bt*bBIBlFdQnTunHiZw4AoQ{mQ>zi03GH0-}Fz>x?-c; z6sUyVD!Rv<1S$VR>qx!YA>?c-?a-#^`e@twwhdd!StaBUfAa%Gjsmko^)aKFHzN3GUHQQ$BxmYNwH$)8myX>ym&WDkBQGFnK z*!^k$L=7j z_t<^q2Waq^-**=pJ+{E-6L{>n71b5KfLP$McHdEG@>subiD>p1W&NDTC~N0jQJ$3b zBF{!yFZLK^y~Jab^-_;f)~d%S>lTku*3Wy4vW|F+vTpSlWlcAPT2ZB}>HY|q#_cah z1p;&dXX)tt*g=X6r8!X|7L@)rY@oEfA4NngsU~Y@HMf;RH-E-e#;&NYl!g zL_7ys#P8rF;!T)Dya!W=C*U??g>NC$idM#?FM|3k<4MLz#wo_zi07c9B7)vp5pO~g z@g8ImPrymWNyJ()#rQ1<;e#8rtN7Nr%W@7pU|9uq3hAq1G0R$5!!iV0ScajCWjzeB zYy_R<0+?XgjP=m`g>ahXVmQlkDO_aPf;Eyo0@qnCgF7r&zEv#U< z4%%66fPR*3GR_M!&Sn{BtBmuKjI&L~c}2#F%Qy)cr&Gq+A>(w*I6FBG9a;A+=^S4j ze`M%Xx-A4(xa)nhAZx-M_-C6gsoHm$*+Ncx8B#~`wl)Ss>0Ch@I5Mum9^J^Ahx1XC zZ5yN8&4Q6l+q!8)cWHJSyJ_e}euxKWwUM}uzqZ2#TZ3rEHZABj4yAK?wkvI9a~gXs zjp!RbsAX)}QP8u{AMftoZfRP{N^FmAkCrs4^oZs->N{*`R(@0;j~>>Iu`G<}6L!JU zqS;&y49(8l7%pvPVJx39t(-nQs|sgQSG+gb(>cpjDsYye&mPq*=;-U~>5TV6e`l(1 zus_kcFFtG7fR?vQ)(#l#O?B_+?M&`-XLy>njU`=jaCc94B0kVrQpLyh{3Cov-GzFT z&0R8Q`+#U_G;L{Fu(Z5cureC!xRm~KO~N$zi!mc5%^WoH`T+yymXo%POgl_7r=<;by9Kn>q%x+!1^N5|);>%Um&?nW{6yogB}+RnRSM7WH!JHgNAP zJYWe*wJ;9OY&ueIxsJvwbSA}89YwM*p3^g!o8+9+Y^7WgFS<-_wtB{G!t3W#`40XZ z>kt01?#r&@pG|!H`>OBHlU`J)LIers2ClqQ|M2SCpI!@m{q~zf&6{q0SV8x_rgZ7i zlRqRBf2HDYR{T}?y(dEWCKV#wU(I=1xJKfND~^OU?5ag$2}3MuB z1zqX7at|GT-(mWpFn%z7u!fh9;Zy3XC0^ch{|^D!moTklF1IVK8_wH7bKV*>`r$HA bW98GH^`CYHROf%ouiBs1(X;vg#})V+%XDaJ literal 4096 zcmeHJO>9(E6h8OOv`l}eLp#t4$ZMvc1Q{t)g9z15XG$5r2nkH*J?JCTH{+W( zrPT;1xF9Bk1Tr!18i^|xEKF=%5E58m0*&loVS*GA7cLCJg+{ICocA-+3PfBG7v9YM z&iT%{_uO;;-rT|c=fDF1UaX6Y0COm*^e{dhPNKVU{Wp#9Rn5)zIWcgveQZK2$ay1o z#7IxdnY6CwOnF?DjiN4Vx*Q))%9A-o4L3B@cG#gu`Tz!m1p1ciN1f5`g4A9o>HzA| zB3ah6n^DSGd$1D7D9}fFQ*k65LSpyvK+nejP5diMn^Gq%yD;vsr9K{etTZ0)e2P^D z@Kv~d$oEw6u&GX)$S>eDc`g-mb(+Zlhr))M&1LKydeB32Ys6YndH{MW=rgH#T30NT zn*>!bA_M$X3n0k&FG_Fn?OtK`rYTKzg|~(yTO*x2C|DI_5nrhW2ptFb6y@u9_(Dn3 z(DWk(YVlo-wQiwQ5^kf8y9c5ER3eV@N90~iEYzFLjZ=n zft1FlJ&>`sW2G~)5i8A2wk4f?fG_NfYasx&xV+u425~=PElgpph7Z{O5!){yQlU76 z0%}AP`~tt@{K_mSsxUjtw@Gv%^VqDPnb&5c%%~?tQJ7WRY?|2$mr+a~vJb^d_ym0c zsBzgjW_7mjOJ=Ju^Xnq=1l-Q{HGyLMK?xv*gX;BgGan1KN1Q4W?0N8jo`ek zQj&wxUX16k4(R~UPEaYyUDCLFCrWa!G=bS$r6i}MZd_r9DUuF!F{qT}ykzoTTI_^$ z1{&ByzVp&q_d1m1S?N6ZUEd|?Gd@37zbhCe;QDSMTj{a|>04-a*+c0rw7AUU`4xgL zYxev}M~sV=*=m8%BHj_YCa#^%=pFGSv;$TMI>mSJ8*N3XaQtKSStA5s3`c82tb$I&TG+uD z!&A`?G2E+-kg<_0r&vG5-p_5MHdk4{%H9Rm7g+ZQ@_O(;rX518hgd%?*1!y0WW2_B zpV2Fky#sL;;)rvgAkM>S#0PK@aR#n2-j_nCdp(35jB!SV@igKrTtu9MYl!o3AMpWr zz1*Jh3JBqWdDLvV}b23TPE5R&BmyVV5?Xiy7%lMh5GvpLM++fxH*GJY0w;gwd{K3(84d#?$ zR(%taGX+zfguUr(Q5~DftFTYgmE5sH7-eKzcz3R-D``{9>ES`uOrx5HA>>osSWyp0 zP5ieSFPbWZGiJ_!gg%wdYRaCpu4Gk?T5d5sepJnv&|lOP7>ydx`DS%!#rk znLeyqfrgLis!^EG^5J8eKB>TAZQ3jvYFNo;L08R!iNmE01ttrboRQVWm(9ZJv?n?g zALv`=Do=2kW7wQf4H)f94yQ(AeFvk<++%9NEPHz)HIz*B5B0?lIz3!VWLw!5PmK&D zV$rd_vMHL^3Xh2$b{6PyK4+<{Z3CjoiL{|AV5o&$(a5N5<0*_*T4Fh!Us@SkQqPoL z(2nTXw~|D*0k$m^Csnt<&8-b_ygbBG(@0LkK@6t_?{>^k(`H2|rc2YanmC=KnRknt zp)R9tE(sm?*}%gT%c^O;m+Hs_qm#wC}5?v6+1 z8x)jIrUKIC;mg_S#Lt~`e~?{xWg%(_8htn3Z(h8)<@+NkAQ$okipRtpNe`m4|`fih;Gw0vapLxQN_dra?PEvERNqTC|mF3b332cfd7y4Us{2` E0Jftx(f|Me diff --git a/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs index 88ca0b60..417f0923 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs @@ -232,6 +232,44 @@ public void IncompatibleCleanupDoesNotProduceOwner(string attribute) Assert.Single(this.FindGeneratedMethod("FreeOtherResource")); } + /// + /// Verifies that a cleanup function returning BOOLEAN produces a constructible owner for returned and output resources. + /// + /// The contextual cleanup attribute. + /// Whether runtime marshaling is enabled. + [Theory, CombinatorialData] + public void BooleanCleanupProducesOwner( + [CombinatorialValues("RAIIFree", "FreeWith")] string attribute, bool allowMarshaling) + { + this.GenerateFixture(attribute, allowMarshaling, "CreateBooleanResource", "AcquireBooleanResource"); + this.compilation = this.AddCode(""" + using Windows.Win32; + using Windows.Win32.Foundation; + using Windows.Win32.Test; + static class Usage + { + static void Use() + { + using FreeBooleanSafeHandle created = PInvoke.CreateBooleanResource_SafeHandle(); + BOOLEAN acquiredResult = PInvoke.AcquireBooleanResource(out FreeBooleanSafeHandle acquired); + using (acquired) + { + using var lease = acquired.Lease(); + RESOURCE_A value = lease.Value; + } + } + } + """); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + INamedTypeSymbol? owner = this.compilation.GetTypeByMetadataName("Windows.Win32.FreeBooleanSafeHandle"); + Assert.NotNull(owner); + Assert.False(owner.IsAbstract); + TypeDeclarationSyntax ownerDeclaration = Assert.IsAssignableFrom(Assert.Single(this.FindGeneratedType("FreeBooleanSafeHandle"))); + MethodDeclarationSyntax releaseHandle = Assert.Single(ownerDeclaration.Members.OfType(), method => method.Identifier.ValueText == "ReleaseHandle"); + InvocationExpressionSyntax release = Assert.IsType(releaseHandle.ExpressionBody?.Expression); + Assert.Equal("FreeBoolean", Assert.IsType(release.Expression).Name.Identifier.ValueText); + } + /// /// Verifies that annotated cleanup functions remain callable when ownership projection is unavailable. ///