From 315643cc93d6b3a9f8aaa56a6ff8d37eb8650c04 Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Wed, 7 Oct 2026 17:44:14 -0700 Subject: [PATCH 1/6] Project contextual native ownership with typed SafeHandle leases Support compatible RAIIFree and FreeWith return/out annotations, preserve native resource types, and add allocation-free scoped leases with lifetime diagnostics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 06478ace-c5d7-418a-9386-f31b4d379d93 --- docfx/docs/features.md | 55 ++ .../AnalyzerReleases.Unshipped.md | 4 + .../Generator.FriendlyOverloads.cs | 83 ++- .../Generator.GeneratedCode.cs | 21 +- .../Generator.Handle.cs | 349 ++++++----- .../Generator.Invariants.cs | 6 + .../Generator.SafeHandleLease.cs | 186 ++++++ src/Microsoft.Windows.CsWin32/Generator.cs | 17 + .../MetadataIndex.cs | 17 + .../SafeHandleAnalyzer.cs | 291 +++++++++ .../templates/NativeOwnershipAttribute.cs | 18 + .../templates/SafeHandleLease.cs | 60 ++ .../CsWin32GeneratorTests.cs | 22 +- .../FriendlyOverloadTests.cs | 3 +- .../HandleTests.cs | 6 +- .../SafeHandleLeaseTests.cs | 560 ++++++++++++++++++ 16 files changed, 1504 insertions(+), 194 deletions(-) create mode 100644 src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs create mode 100644 src/Microsoft.Windows.CsWin32/SafeHandleAnalyzer.cs create mode 100644 src/Microsoft.Windows.CsWin32/templates/NativeOwnershipAttribute.cs create mode 100644 src/Microsoft.Windows.CsWin32/templates/SafeHandleLease.cs create mode 100644 test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs diff --git a/docfx/docs/features.md b/docfx/docs/features.md index f912c88d..76b7b0d7 100644 --- a/docfx/docs/features.md +++ b/docfx/docs/features.md @@ -7,3 +7,58 @@ - [Layered composition](composition.md): multiple assemblies can extend a single shared `PInvoke` static class so callers reach every native API through one symbol. ![Animation demonstrating p/invoke code generation](../images/demo.gif) + +## Owned outputs and scoped native values + +`RAIIFree` and compatible `FreeWith` annotations on return values and output parameters +tell CsWin32 which cleanup function owns that particular result. Friendly overloads +can return a SafeHandle for pointer-shaped allocations as well as native handles. +Unannotated pointers and the raw native signatures are unchanged. + +CsWin32-generated SafeHandles expose a `Value` property with the original native type. +For example, `SHGetKnownFolderPath` supplies a `CoTaskMemFreePWSTRSafeHandle` whose +`Value` is `PWSTR`, although `CoTaskMemFree` accepts `void*`. BCL wrappers such as +`SafeFileHandle` retain their existing APIs. + +For an owner received from another component or as a parameter, acquire a scoped reference: + +```csharp +using Windows.Win32; + +static bool PathExists(CoTaskMemFreePWSTRSafeHandle path) +{ + using var lease = path.Lease(); + return PInvoke.PathFileExists(lease.Value); +} +``` + +`Lease()` pairs `DangerousAddRef` and `DangerousRelease` without an additional heap +allocation. Disposal through another alias cannot release the resource until the +lease ends. The lease's `Value` has the same native type as its owner. The lease +uses the `Dispose()` pattern; it does not require `IDisposable`. +Lease helpers require C# 9 or later. + +An owner already held in a visible `using` scope can supply `owner.Value` directly. +This assumes ordinary exclusive ownership: another alias must not explicitly +dispose it during the call. A SafeHandle parameter alone is not such a lifetime +guarantee. Existing friendly input overloads accept base `SafeHandle` for recognized +native handles and already keep a reference alive throughout their calls. +Existing overloads mixing SafeHandles with native inputs remain available. + +The lifetime analyzer warns about leases that are not `using` locals, copies, +field storage, parameter passing, returns, and explicit disposal (`PInvoke015`). +It also warns about `Value` without a visible scope (`PInvoke016`), saving or +returning a raw resource (`PInvoke017`), and releasing an owned resource directly +(`PInvoke018`). Use `Value` directly in calls, or copy its contents into managed +data such as a string. C# ref structs remain copyable: keep these diagnostics +enabled to enforce the supported no-copy convention. + +Neither a lease nor this analyzer proves that an arbitrary native call does not +retain the resource, transfer ownership, or invalidate it independently. Such +APIs need their own explicit lifetime arrangement. + +For cleanup requiring additional arguments, request a helper such as +`DeleteTimerQueueTimerSafeHandle` in `NativeMethods.txt`. It is abstract: derive +from it, keep the necessary cleanup context, and override `ReleaseHandle`. +Its typed `Value` is available inside that override. CsWin32 will not construct +this helper automatically or guess the additional cleanup arguments. diff --git a/src/Microsoft.Windows.CsWin32/AnalyzerReleases.Unshipped.md b/src/Microsoft.Windows.CsWin32/AnalyzerReleases.Unshipped.md index 01505536..8a5e8ffd 100644 --- a/src/Microsoft.Windows.CsWin32/AnalyzerReleases.Unshipped.md +++ b/src/Microsoft.Windows.CsWin32/AnalyzerReleases.Unshipped.md @@ -19,3 +19,7 @@ PInvoke011 | Configuration | Error | SourceGenerator PInvoke012 | Configuration | Error | SourceGenerator PInvoke013 | Configuration | Error | SourceGenerator PInvoke014 | Configuration | Error | SourceGenerator +PInvoke015 | Lifetime | Warning | SafeHandleAnalyzer +PInvoke016 | Lifetime | Warning | SafeHandleAnalyzer +PInvoke017 | Lifetime | Warning | SafeHandleAnalyzer +PInvoke018 | Lifetime | Warning | SafeHandleAnalyzer diff --git a/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs b/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs index 33613adb..b8571d5a 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs @@ -38,13 +38,26 @@ private static ExpressionSyntax GetIsSpanEmpty(ExpressionSyntax span, bool isRef private ExpressionSyntax GetIntPtrFromTypeDef(ExpressionSyntax typedefValue, TypeHandleInfo typeDefTypeInfo) { ExpressionSyntax intPtrValue = typedefValue; + if (typeDefTypeInfo is PointerTypeHandleInfo) + { + return CastExpression(IntPtrTypeSyntax, typedefValue); + } + + if (this.TryGetTypeDefFieldType(typeDefTypeInfo, out TypeHandleInfo? backingType) && backingType is PointerTypeHandleInfo) + { + return CastExpression(IntPtrTypeSyntax, MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, typedefValue, IdentifierName("Value"))); + } + if (this.TryGetTypeDefFieldType(typeDefTypeInfo, out TypeHandleInfo? returnTypeField) && returnTypeField is PrimitiveTypeHandleInfo primitiveReturnField) { switch (primitiveReturnField.PrimitiveTypeCode) { case PrimitiveTypeCode.UInt32: - // (IntPtr)result.Value; - intPtrValue = CastExpression(IntPtrTypeSyntax, MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, typedefValue, IdentifierName("Value"))); + intPtrValue = UncheckedExpression(CastExpression( + IntPtrTypeSyntax, + CastExpression(IdentifierName("nint"), CastExpression( + IdentifierName("nuint"), + MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, typedefValue, IdentifierName("Value")))))); break; case PrimitiveTypeCode.UIntPtr: // unchecked((IntPtr)(long)(ulong)result.Value) @@ -92,18 +105,35 @@ private IEnumerable DeclareFriendlyOverloads(MethodDefi } bool improvePointersToSpansAndRefs = this.canUseSpan; - FriendlyMethodBookkeeping bookkeeping = new(); - foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs, omitOptionalParams: false, bookkeeping)) + HashSet signatures = new(StringComparer.Ordinal); + for (int pass = 0; pass < 2; pass++) { - yield return method; - } + bool legacyHandleInputs = pass == 1; + FriendlyMethodBookkeeping bookkeeping = new(); + foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs, omitOptionalParams: false, bookkeeping, legacyHandleInputs)) + { + if (signatures.Add(method.Identifier.ValueText + method.TypeParameterList + method.ParameterList)) + { + yield return method; + } + } - if (this.Options.FriendlyOverloads.IncludePointerOverloads && improvePointersToSpansAndRefs && bookkeeping.NumSpanByteParameters > 0) - { - // If we could use Span and _did_ use span Span and the pointer overloads were requested, then Generate overloads that use pointer types instead of Span/ReadOnlySpan. - foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs: false, omitOptionalParams: false)) + if (this.Options.FriendlyOverloads.IncludePointerOverloads && improvePointersToSpansAndRefs && bookkeeping.NumSpanByteParameters > 0) { - yield return method; + // Also offer pointer-based overloads for byte buffers. + foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs: false, omitOptionalParams: false, legacyHandleInputs: legacyHandleInputs)) + { + if (signatures.Add(method.Identifier.ValueText + method.TypeParameterList + method.ParameterList)) + { + yield return method; + } + } + } + + // Preserve signatures mixing existing SafeHandle inputs with native values for newly recognized inputs. + if (!bookkeeping.HasNewHandleInputs) + { + break; } } } @@ -117,7 +147,8 @@ private IEnumerable DeclareFriendlyOverload( bool avoidWinmdRootAlias, bool improvePointersToSpansAndRefs, bool omitOptionalParams, - FriendlyMethodBookkeeping? bookkeeping = null) + FriendlyMethodBookkeeping? bookkeeping = null, + bool legacyHandleInputs = false) { #pragma warning disable SA1114 // Parameter list should follow declaration bool isReleaseMethod = this.MetadataIndex.ReleaseMethods.Contains(externMethodDeclaration.Identifier.ValueText); @@ -556,8 +587,8 @@ private IEnumerable DeclareFriendlyOverload( bool hasOut = externParam.Modifiers.Any(SyntaxKind.OutKeyword); arguments[paramIndex] = arguments[paramIndex].WithRefKindKeyword(TokenWithSpace(hasOut ? SyntaxKind.OutKeyword : SyntaxKind.RefKeyword)); } - else if (isOut && !isIn && !isReleaseMethod && parameterTypeInfo is PointerTypeHandleInfo { ElementType: HandleTypeHandleInfo pointedElementInfo } && - pointedElementInfo.Generator.TryGetHandleReleaseMethod(pointedElementInfo.Handle, paramAttributes, out string? outReleaseMethod) && !this.Reader.StringComparer.Equals(methodDefinition.Name, outReleaseMethod) && + else if (isOut && !isIn && !isReleaseMethod && parameterTypeInfo is PointerTypeHandleInfo { ElementType: TypeHandleInfo pointedElementInfo } && + this.TryGetResourceReleaseMethod(pointedElementInfo, paramAttributes, out string? outReleaseMethod) && !this.Reader.StringComparer.Equals(methodDefinition.Name, outReleaseMethod) && (memorySize is null) && !isArray) { signatureChanged = true; @@ -565,7 +596,7 @@ private IEnumerable DeclareFriendlyOverload( IdentifierNameSyntax localName = IdentifierName(externParam.Identifier.ValueText + "Local"); // NOTE: We don't handle scenarios where the parameter is [MemorySize] annotated (e.g. EnumProcessModules) or [NativeArrayInfo] (e.g. ITypeInfo.GetNames) - if (this.RequestSafeHandle(outReleaseMethod) is TypeSyntax safeHandleType) + if (this.RequestSafeHandle(outReleaseMethod, pointedElementInfo) is TypeSyntax safeHandleType) { // out SafeHandle parameters[paramIndex] = externParam @@ -641,9 +672,16 @@ private IEnumerable DeclareFriendlyOverload( arguments[paramIndex] = Argument(localName); } } - else if (this.options.UseSafeHandles && isIn && !isOut && !isReleaseMethod && parameterTypeInfo is HandleTypeHandleInfo parameterHandleTypeInfo && this.TryGetHandleReleaseMethod(parameterHandleTypeInfo.Handle, paramAttributes, out string? releaseMethod) && !this.Reader.StringComparer.Equals(methodDefinition.Name, releaseMethod) + else if (this.options.UseSafeHandles && isIn && !isOut && !isReleaseMethod && parameterTypeInfo is HandleTypeHandleInfo parameterHandleTypeInfo + && (parameterHandleTypeInfo.Generator.TryGetHandleReleaseMethod(parameterHandleTypeInfo.Handle, paramAttributes, out _) + || (!legacyHandleInputs && parameterHandleTypeInfo.Generator.IsHandle(parameterHandleTypeInfo.Handle, out _))) && !(this.TryGetTypeDefFieldType(parameterHandleTypeInfo, out TypeHandleInfo? fieldType) && !this.IsSafeHandleCompatibleTypeDefFieldType(fieldType))) { + if (bookkeeping is not null && !parameterHandleTypeInfo.Generator.TryGetHandleReleaseMethod(parameterHandleTypeInfo.Handle, paramAttributes, out _)) + { + bookkeeping.HasNewHandleInputs = true; + } + IdentifierNameSyntax typeDefHandleName = IdentifierName(externParam.Identifier.ValueText + "Local"); signatureChanged = true; @@ -1385,9 +1423,8 @@ bool TryHandleCountParam(TypeSyntax elementType, bool nullableSource) signatureChanged = true; } - TypeSyntax? returnSafeHandleType = originalSignature.ReturnType is HandleTypeHandleInfo returnTypeHandleInfo - && returnTypeHandleInfo.Generator.TryGetHandleReleaseMethod(returnTypeHandleInfo.Handle, returnTypeAttributes, out string? returnReleaseMethod) - ? this.RequestSafeHandle(returnReleaseMethod) : null; + TypeSyntax? returnSafeHandleType = this.TryGetResourceReleaseMethod(originalSignature.ReturnType, returnTypeAttributes, out string? returnReleaseMethod) + ? this.RequestSafeHandle(returnReleaseMethod, originalSignature.ReturnType) : null; IdentifierNameSyntax resultLocal = IdentifierName("__result"); @@ -1591,10 +1628,10 @@ bool TryHandleCountParam(TypeSyntax elementType, bool nullableSource) } // If we're using C# 13 or later, consider adding the overload resolution attribute if it would likely resolve ambiguities. - if (this.LanguageVersion >= (LanguageVersion)1300 && parameters.Count == externMethodDeclaration.ParameterList.Parameters.Count) + if (this.LanguageVersion >= (LanguageVersion)1300 && (legacyHandleInputs || parameters.Count == externMethodDeclaration.ParameterList.Parameters.Count)) { this.volatileCode.GenerationTransaction(() => this.DeclareOverloadResolutionPriorityAttributeIfNecessary()); - friendlyDeclaration = friendlyDeclaration.AddAttributeLists(AttributeList(OverloadResolutionPriorityAttribute(1))); + friendlyDeclaration = friendlyDeclaration.AddAttributeLists(AttributeList(OverloadResolutionPriorityAttribute(legacyHandleInputs ? 2 : 1))); } friendlyDeclaration = friendlyDeclaration @@ -1623,7 +1660,7 @@ bool TryHandleCountParam(TypeSyntax elementType, bool nullableSource) if (numOptionalParams > 0 && !omitOptionalParams && improvePointersToSpansAndRefs) { // Generate overloads for optional parameters. - foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs, omitOptionalParams: true)) + foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs, omitOptionalParams: true, legacyHandleInputs: legacyHandleInputs)) { yield return method; } @@ -1795,5 +1832,7 @@ private StatementSyntax COMFreeNativePointerStatement(ExpressionSyntax nativePoi private class FriendlyMethodBookkeeping { public int NumSpanByteParameters { get; set; } = 0; + + public bool HasNewHandleInputs { get; set; } } } diff --git a/src/Microsoft.Windows.CsWin32/Generator.GeneratedCode.cs b/src/Microsoft.Windows.CsWin32/Generator.GeneratedCode.cs index 6aedffaf..7558548b 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.GeneratedCode.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.GeneratedCode.cs @@ -48,7 +48,7 @@ private class GeneratedCode private readonly Dictionary<(string Namespace, string Name), MemberDeclarationSyntax> inlineArrays = new(); - private readonly Dictionary releaseMethodsWithSafeHandleTypesGenerating = new(); + private readonly Dictionary releaseMethodsWithSafeHandleTypesGenerating = new(); private readonly List inlineArrayIndexerExtensionsMembers = new(); @@ -380,17 +380,26 @@ internal string GenerateCustomTypeMarshaller(string marshallerName, Func(Dictionary source, Dictionary? target) diff --git a/src/Microsoft.Windows.CsWin32/Generator.Handle.cs b/src/Microsoft.Windows.CsWin32/Generator.Handle.cs index 87e9b768..aee1316d 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.Handle.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.Handle.cs @@ -5,7 +5,14 @@ namespace Microsoft.Windows.CsWin32; public partial class Generator { - internal TypeSyntax? RequestSafeHandle(string releaseMethod) + /// + /// Generates an owner for a native value and its cleanup function. + /// + /// The cleanup function. + /// The original native value type, or null to use the cleanup parameter type. + /// Whether caller-defined cleanup helpers may be returned. + /// The owner type, or null when automatic ownership cannot be generated. + internal TypeSyntax? RequestSafeHandle(string releaseMethod, TypeHandleInfo? valueType = null, bool allowAbstract = false) { if (!this.options.UseSafeHandles) { @@ -14,18 +21,15 @@ public partial class Generator try { - if (this.volatileCode.TryGetSafeHandleForReleaseMethod(releaseMethod, out TypeSyntax? safeHandleType)) + if (BclInteropSafeHandles.TryGetValue(releaseMethod, out TypeSyntax? knownOwner) + && (valueType is null || (valueType is HandleTypeHandleInfo knownHandle + && knownHandle.Generator.GetQualifiedTypeDefinition(knownHandle.Handle) is QualifiedTypeDefinition definition + && definition.Reader.GetString(definition.Definition.Namespace) == (releaseMethod == "CloseHandle" ? "Windows.Win32.Foundation" : "Windows.Win32.System.Registry") + && definition.Reader.GetString(definition.Definition.Name) == (releaseMethod == "CloseHandle" ? "HANDLE" : "HKEY")))) { - return safeHandleType; + return knownOwner; } - if (BclInteropSafeHandles.TryGetValue(releaseMethod, out TypeSyntax? bclType)) - { - return bclType; - } - - string safeHandleClassName = $"{releaseMethod}SafeHandle"; - MethodDefinitionHandle? releaseMethodHandle = this.GetMethodByName(releaseMethod); if (!releaseMethodHandle.HasValue) { @@ -35,15 +39,10 @@ public partial class Generator MethodDefinition releaseMethodDef = this.Reader.GetMethodDefinition(releaseMethodHandle.Value); string releaseMethodModule = this.GetNormalizedModuleName(releaseMethodDef.GetImport()); - IdentifierNameSyntax? safeHandleTypeIdentifier = IdentifierName(safeHandleClassName); - safeHandleType = QualifiedName(ParseName($"global::{this.Namespace}"), safeHandleTypeIdentifier); - MethodSignature releaseMethodSignature = releaseMethodDef.DecodeSignature(this.SignatureHandleProvider, null); - TypeHandleInfo releaseMethodParameterTypeHandleInfo = releaseMethodSignature.ParameterTypes[0]; - TypeSyntaxAndMarshaling releaseMethodParameterType = releaseMethodParameterTypeHandleInfo.ToTypeSyntax(this.externSignatureTypeSettings, GeneratingElement.HelperClassMember, default); - var nonReservedParameterCount = 0; var handleParameterName = string.Empty; + int handleParameterIndex = -1; foreach (ParameterHandle paramHandle in releaseMethodDef.GetParameters()) { Parameter param = this.Reader.GetParameter(paramHandle); @@ -57,43 +56,73 @@ public partial class Generator if (this.FindInteropDecorativeAttribute(paramAttributes, "ReservedAttribute") is null) { nonReservedParameterCount++; - handleParameterName = this.Reader.GetString(param.Name); + if (handleParameterIndex < 0) + { + handleParameterIndex = param.SequenceNumber - 1; + handleParameterName = this.Reader.GetString(param.Name); + } } } - // If the release method takes more than one non-reserved parameter, we can't generate a SafeHandle for it. - if (nonReservedParameterCount != 1) + if (handleParameterIndex < 0) { - safeHandleType = null; + return null; } - // If the handle type is *always* 64-bits, even in 32-bit processes, SafeHandle cannot represent it, since it's based on IntPtr. - // We could theoretically do this for x64-specific compilations though if required. - if (!this.TryGetTypeDefFieldType(releaseMethodParameterTypeHandleInfo, out TypeHandleInfo? typeDefStructFieldType)) + TypeHandleInfo releaseMethodParameterTypeHandleInfo = releaseMethodSignature.ParameterTypes[handleParameterIndex]; + valueType ??= releaseMethodParameterTypeHandleInfo; + TypeSyntax nativeValueType = valueType.ToTypeSyntax(this.externSignatureTypeSettings, GeneratingElement.HelperClassMember, default).Type; + TypeSyntaxAndMarshaling releaseMethodParameterType = releaseMethodParameterTypeHandleInfo.ToTypeSyntax(this.externSignatureTypeSettings, GeneratingElement.HelperClassMember, default); + bool canonicalValueType = nativeValueType.IsEquivalentTo(releaseMethodParameterType.Type); + if (!this.IsCleanupParameterCompatible(valueType, releaseMethodParameterTypeHandleInfo, releaseMethod)) { - safeHandleType = null; + return null; } - if (!this.IsSafeHandleCompatibleTypeDefFieldType(typeDefStructFieldType)) + if (canonicalValueType && BclInteropSafeHandles.TryGetValue(releaseMethod, out TypeSyntax? bclType)) { - safeHandleType = null; + return bclType; } - this.volatileCode.GenerationTransaction(delegate + string cacheKey = releaseMethod + ":" + nativeValueType; + if (this.volatileCode.TryGetSafeHandleForReleaseMethod(cacheKey, out TypeSyntax? safeHandleType, out bool cachedCanConstruct)) { - this.volatileCode.AddSafeHandleNameForReleaseMethod(releaseMethod, safeHandleType); - }); + return allowAbstract || cachedCanConstruct ? safeHandleType : null; + } - if (safeHandleType is null) + TypeHandleInfo? typeDefStructFieldType = this.GetSafeHandleBackingType(releaseMethodParameterTypeHandleInfo); + if (!this.IsSafeHandleCompatibleTypeDefFieldType(typeDefStructFieldType) + || !this.IsSafeHandleCompatibleTypeDefFieldType(this.GetSafeHandleBackingType(valueType))) { - return safeHandleType; + return null; } + string safeHandleClassName = canonicalValueType + ? $"{releaseMethod}SafeHandle" + : $"{releaseMethod}{this.GetSafeHandleValueName(valueType, nativeValueType)}SafeHandle"; + IdentifierNameSyntax safeHandleTypeIdentifier = IdentifierName(safeHandleClassName); + safeHandleType = QualifiedName(ParseName($"global::{this.Namespace}"), safeHandleTypeIdentifier); + CustomAttributeHandleCollection? atts = this.GetReturnTypeCustomAttributes(releaseMethodDef); + TypeSyntaxAndMarshaling releaseMethodReturnType = releaseMethodSignature.ReturnType.ToTypeSyntax(this.externSignatureTypeSettings, GeneratingElement.HelperClassMember, atts?.QualifyWith(this)); + bool canConstruct = nonReservedParameterCount == 1 && IsSupportedCleanupReturn(releaseMethodReturnType.Type, releaseMethod); + this.volatileCode.GenerationTransaction(delegate + { + this.volatileCode.AddSafeHandleNameForReleaseMethod(cacheKey, safeHandleType, canConstruct); + }); + // Bail out early if someone already made the SafeHandle type string safeHandleFullyQualifiedName = $"{this.Namespace}.{safeHandleClassName}"; if (this.IsTypeAlreadyFullyDeclared(safeHandleFullyQualifiedName)) { - return safeHandleType; + return allowAbstract || canConstruct ? safeHandleType : null; + } + + TypeSyntax baseType = SafeHandleTypeSyntax; + if (!canonicalValueType && !BclInteropSafeHandles.ContainsKey(releaseMethod) + && !this.IsTypeAlreadyFullyDeclared($"{this.Namespace}.{releaseMethod}SafeHandle") + && this.RequestSafeHandle(releaseMethod, allowAbstract: true) is TypeSyntax canonicalOwner) + { + baseType = canonicalOwner; } this.volatileCode.GenerationTransaction(delegate @@ -103,11 +132,15 @@ public partial class Generator // Collect all the known invalid values for this handle. // If no invalid values are given (e.g. BSTR), we'll just assume 0 is invalid. - HashSet invalidHandleValues = this.GetInvalidHandleValues(((HandleTypeHandleInfo)releaseMethodParameterTypeHandleInfo).Handle); - IntPtr preferredInvalidValue = GetPreferredInvalidHandleValue(invalidHandleValues, new IntPtr(-1)); + HashSet invalidHandleValues = valueType is HandleTypeHandleInfo nativeHandle + ? nativeHandle.Generator.GetInvalidHandleValues(nativeHandle.Handle) + : new() { IntPtr.Zero }; + if (invalidHandleValues.Count == 0 && releaseMethodParameterTypeHandleInfo is PointerTypeHandleInfo) + { + invalidHandleValues.Add(IntPtr.Zero); + } - CustomAttributeHandleCollection? atts = this.GetReturnTypeCustomAttributes(releaseMethodDef); - TypeSyntaxAndMarshaling releaseMethodReturnType = releaseMethodSignature.ReturnType.ToTypeSyntax(this.externSignatureTypeSettings, GeneratingElement.HelperClassMember, atts?.QualifyWith(this)); + IntPtr preferredInvalidValue = GetPreferredInvalidHandleValue(invalidHandleValues, new IntPtr(-1)); this.TryGetRenamedMethod(releaseMethod, out string? renamedReleaseMethod); @@ -156,141 +189,145 @@ public partial class Generator .WithExpressionBody(ArrowExpressionClause(overallTest)) .WithSemicolonToken(SemicolonWithLineFeed)); - // If there are more than 1 parameter other parameters are reserved. - // Otherwise we would have quit earlier + this.AddTypedSafeHandleMembers(members, safeHandleTypeIdentifier, valueType, releaseMethod, !baseType.IsEquivalentTo(SafeHandleTypeSyntax)); + + // Automatic cleanup supplies defaults only for additional reserved parameters. var releaseMethodHasReservedParameters = releaseMethodSignature.RequiredParameterCount > 1; // (struct)this.handle or (struct)checked((fieldType)(nint))this.handle, as appropriate. // If we have other reserved parameters make this a named argument to be extra sure // we are passing value for correct parameter - bool implicitConversion = typeDefStructFieldType is PrimitiveTypeHandleInfo { PrimitiveTypeCode: PrimitiveTypeCode.IntPtr } or PointerTypeHandleInfo; + ExpressionSyntax releaseNativeValue = this.GetNativeValueFromHandle(thisHandle, typeDefStructFieldType!); ArgumentSyntax releaseHandleArgument = Argument( releaseMethodHasReservedParameters ? NameColon(IdentifierName(handleParameterName)) : null, default, CastExpression( releaseMethodParameterType.Type, - implicitConversion ? thisHandle : CheckedExpression(CastExpression(typeDefStructFieldType!.ToTypeSyntax(this.fieldTypeSettings, GeneratingElement.HelperClassMember, null).Type, CastExpression(IdentifierName("nint"), thisHandle))))); - - // 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 - // * int => zero is success - // * uint => zero is success - // * byte => non-zero is success - ExpressionSyntax releaseInvocation = InvocationExpression( - MemberAccessExpression( - SyntaxKind.SimpleMemberAccessExpression, - IdentifierName(this.options.ClassName), - IdentifierName(renamedReleaseMethod ?? releaseMethod)), - [releaseHandleArgument]); - BlockSyntax? releaseBlock = null; - - // Reserved parameters can be pointers. - // Thus we need unsafe modifier even though we don't pass values for reserved parameters explicitly. - // 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" } } })) + releaseNativeValue)); + + if (canConstruct && baseType.IsEquivalentTo(SafeHandleTypeSyntax)) { - switch (releaseMethodReturnType.Type) + // 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 + // * int => zero is success + // * uint => zero is success + // * byte => non-zero is success + ExpressionSyntax releaseInvocation = InvocationExpression( + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + IdentifierName(this.options.ClassName), + IdentifierName(renamedReleaseMethod ?? releaseMethod)), + [releaseHandleArgument]); + BlockSyntax? releaseBlock = null; + + // Reserved parameters can be pointers. + // Thus we need unsafe modifier even though we don't pass values for reserved parameters explicitly. + // 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" } } })) { - case PredefinedTypeSyntax predefined: - SyntaxKind returnType = predefined.Keyword.Kind(); - if (returnType == SyntaxKind.IntKeyword) - { - releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0))); - } - else if (returnType == SyntaxKind.UIntKeyword) - { - releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0))); - } - else if (returnType == SyntaxKind.ByteKeyword) - { - releaseInvocation = BinaryExpression(SyntaxKind.NotEqualsExpression, releaseInvocation, LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0))); - } - else if (returnType == SyntaxKind.VoidKeyword) - { - releaseBlock = Block( - ExpressionStatement(releaseInvocation), - ReturnStatement(LiteralExpression(SyntaxKind.TrueLiteralExpression))); - } - else - { - throw new NotSupportedException($"Return type {returnType} on release method {releaseMethod} not supported."); - } - - break; - case QualifiedNameSyntax { Right: IdentifierNameSyntax identifierName }: - switch (identifierName.Identifier.ValueText) - { - case "NTSTATUS": - this.TryGenerateConstantOrThrow("STATUS_SUCCESS"); - ExpressionSyntax statusSuccess = MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, ParseName("global::Windows.Win32.Foundation.NTSTATUS"), IdentifierName("STATUS_SUCCESS")); - releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, statusSuccess); - break; - case "HRESULT": - this.TryGenerateConstantOrThrow("S_OK"); - ExpressionSyntax ok = MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, ParseName("global::Windows.Win32.Foundation.HRESULT"), IdentifierName("S_OK")); - releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, ok); - break; - case "WIN32_ERROR": - ExpressionSyntax noerror = MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, ParseName("global::Windows.Win32.Foundation.WIN32_ERROR"), IdentifierName("NO_ERROR")); - releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, noerror); - break; - case "CONFIGRET": - noerror = MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, ParseName("global::Windows.Win32.Devices.DeviceAndDriverInstallation.CONFIGRET"), IdentifierName("CR_SUCCESS")); - releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, noerror); - break; - case "LRESULT" when releaseMethod == "ICClose": - this.TryGenerateConstantOrThrow("ICERR_OK"); - noerror = CastExpression(ParseName("global::Windows.Win32.Foundation.LRESULT"), MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, this.methodsAndConstantsClassName, IdentifierName("ICERR_OK"))); - releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, noerror); - break; - case "HGLOBAL": - case "HLOCAL": - releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, DefaultExpression(releaseMethodReturnType.Type)); - break; - default: - throw new NotSupportedException($"Return type {identifierName.Identifier.ValueText} on release method {releaseMethod} not supported."); - } - - break; - case PointerTypeSyntax { ElementType: PredefinedTypeSyntax elementType }: - releaseMethodIsUnsafe = true; - switch (elementType.Keyword.Kind()) - { - case SyntaxKind.VoidKeyword when releaseMethod == "FreeSid": - releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, LiteralExpression(SyntaxKind.NullLiteralExpression)); - break; - default: - throw new NotSupportedException($"Return type {elementType}* on release method {releaseMethod} not supported."); - } - - break; + switch (releaseMethodReturnType.Type) + { + case PredefinedTypeSyntax predefined: + SyntaxKind returnType = predefined.Keyword.Kind(); + if (returnType == SyntaxKind.IntKeyword) + { + releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0))); + } + else if (returnType == SyntaxKind.UIntKeyword) + { + releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0))); + } + else if (returnType == SyntaxKind.ByteKeyword) + { + releaseInvocation = BinaryExpression(SyntaxKind.NotEqualsExpression, releaseInvocation, LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0))); + } + else if (returnType == SyntaxKind.VoidKeyword) + { + releaseBlock = Block( + ExpressionStatement(releaseInvocation), + ReturnStatement(LiteralExpression(SyntaxKind.TrueLiteralExpression))); + } + else + { + throw new NotSupportedException($"Return type {returnType} on release method {releaseMethod} not supported."); + } + + break; + case QualifiedNameSyntax { Right: IdentifierNameSyntax identifierName }: + switch (identifierName.Identifier.ValueText) + { + case "NTSTATUS": + this.TryGenerateConstantOrThrow("STATUS_SUCCESS"); + ExpressionSyntax statusSuccess = MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, ParseName("global::Windows.Win32.Foundation.NTSTATUS"), IdentifierName("STATUS_SUCCESS")); + releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, statusSuccess); + break; + case "HRESULT": + this.TryGenerateConstantOrThrow("S_OK"); + ExpressionSyntax ok = MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, ParseName("global::Windows.Win32.Foundation.HRESULT"), IdentifierName("S_OK")); + releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, ok); + break; + case "WIN32_ERROR": + ExpressionSyntax noerror = MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, ParseName("global::Windows.Win32.Foundation.WIN32_ERROR"), IdentifierName("NO_ERROR")); + releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, noerror); + break; + case "CONFIGRET": + noerror = MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, ParseName("global::Windows.Win32.Devices.DeviceAndDriverInstallation.CONFIGRET"), IdentifierName("CR_SUCCESS")); + releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, noerror); + break; + case "LRESULT" when releaseMethod == "ICClose": + this.TryGenerateConstantOrThrow("ICERR_OK"); + noerror = CastExpression(ParseName("global::Windows.Win32.Foundation.LRESULT"), MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, this.methodsAndConstantsClassName, IdentifierName("ICERR_OK"))); + releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, noerror); + break; + case "HGLOBAL": + case "HLOCAL": + releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, DefaultExpression(releaseMethodReturnType.Type)); + break; + default: + throw new NotSupportedException($"Return type {identifierName.Identifier.ValueText} on release method {releaseMethod} not supported."); + } + + break; + case PointerTypeSyntax { ElementType: PredefinedTypeSyntax elementType }: + releaseMethodIsUnsafe = true; + switch (elementType.Keyword.Kind()) + { + case SyntaxKind.VoidKeyword when releaseMethod == "FreeSid": + releaseInvocation = BinaryExpression(SyntaxKind.EqualsExpression, releaseInvocation, LiteralExpression(SyntaxKind.NullLiteralExpression)); + break; + default: + throw new NotSupportedException($"Return type {elementType}* on release method {releaseMethod} not supported."); + } + + break; + } } - } - MethodDeclarationSyntax releaseHandleDeclaration = MethodDeclaration(PredefinedType(TokenWithSpace(SyntaxKind.BoolKeyword)), Identifier("ReleaseHandle")) - .AddModifiers(TokenWithSpace(SyntaxKind.ProtectedKeyword), TokenWithSpace(SyntaxKind.OverrideKeyword)); + MethodDeclarationSyntax releaseHandleDeclaration = MethodDeclaration(PredefinedType(TokenWithSpace(SyntaxKind.BoolKeyword)), Identifier("ReleaseHandle")) + .AddModifiers(TokenWithSpace(SyntaxKind.ProtectedKeyword), TokenWithSpace(SyntaxKind.OverrideKeyword)); - if (releaseMethodIsUnsafe) - { - releaseHandleDeclaration = releaseHandleDeclaration.AddModifiers(TokenWithSpace(SyntaxKind.UnsafeKeyword)); - } + if (releaseMethodIsUnsafe) + { + releaseHandleDeclaration = releaseHandleDeclaration.AddModifiers(TokenWithSpace(SyntaxKind.UnsafeKeyword)); + } - releaseHandleDeclaration = releaseBlock is null - ? releaseHandleDeclaration - .WithExpressionBody(ArrowExpressionClause(releaseInvocation)) - .WithSemicolonToken(SemicolonWithLineFeed) - : releaseHandleDeclaration - .WithBody(releaseBlock); - members.Add(releaseHandleDeclaration); + releaseHandleDeclaration = releaseBlock is null + ? releaseHandleDeclaration + .WithExpressionBody(ArrowExpressionClause(releaseInvocation)) + .WithSemicolonToken(SemicolonWithLineFeed) + : releaseHandleDeclaration + .WithBody(releaseBlock); + members.Add(releaseHandleDeclaration); + } IEnumerable xmlDocParameterTypes = releaseMethodSignature.ParameterTypes.Select(p => p.ToTypeSyntax(this.externSignatureTypeSettings, GeneratingElement.HelperClassMember, default).Type); ClassDeclarationSyntax safeHandleDeclaration = ClassDeclaration(Identifier(safeHandleClassName), [.. members]) - .AddModifiers(visibilityModifier, TokenWithSpace(SyntaxKind.PartialKeyword)) - .WithBaseList(BaseList(SimpleBaseType(SafeHandleTypeSyntax))) + .AddModifiers(visibilityModifier, TokenWithSpace(SyntaxKind.UnsafeKeyword), TokenWithSpace(SyntaxKind.PartialKeyword)) + .WithBaseList(BaseList(SimpleBaseType(baseType))) .AddAttributeLists(AttributeList(GeneratedCodeAttribute)) .WithLeadingTrivia(ParseLeadingTrivia($@" /// @@ -298,12 +335,17 @@ public partial class Generator /// ")); + if (!canConstruct) + { + safeHandleDeclaration = safeHandleDeclaration.WithModifiers(safeHandleDeclaration.Modifiers.Insert(1, TokenWithSpace(SyntaxKind.AbstractKeyword))); + } + this.volatileCode.GenerationTransaction(delegate { this.volatileCode.AddSafeHandleType(safeHandleDeclaration); }); - return safeHandleType; + return allowAbstract || canConstruct ? safeHandleType : null; } catch (Exception ex) { @@ -338,7 +380,8 @@ internal bool TryGetHandleReleaseMethod(EntityHandle handleStructDefHandle, Cust internal bool TryGetHandleReleaseMethod(TypeDefinitionHandle handleStructDefHandle, CustomAttributeHandleCollection? handleReferenceAttributes, [NotNullWhen(true)] out string? releaseMethod) { // Prefer direct attributes on the type reference over the default release method for the struct type. - if (this.FindAttribute(handleReferenceAttributes, InteropDecorationNamespace, RAIIFreeAttribute) is CustomAttribute raii) + if ((this.FindAttribute(handleReferenceAttributes, InteropDecorationNamespace, RAIIFreeAttribute) + ?? this.FindAttribute(handleReferenceAttributes, InteropDecorationNamespace, FreeWithAttribute)) is CustomAttribute raii) { CustomAttributeValue args = raii.DecodeValue(CustomAttributeTypeProvider.Instance); if (args.FixedArguments[0].Value is string localRelease) diff --git a/src/Microsoft.Windows.CsWin32/Generator.Invariants.cs b/src/Microsoft.Windows.CsWin32/Generator.Invariants.cs index 3e19080b..1bcfea19 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.Invariants.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.Invariants.cs @@ -16,6 +16,12 @@ public partial class Generator internal const string ContainsInteriorPointersAttribute = "ContainsInteriorPointersAttribute"; internal const string RAIIFreeAttribute = "RAIIFreeAttribute"; + + /// + /// Identifies contextual cleanup information equivalent to . + /// + internal const string FreeWithAttribute = "FreeWithAttribute"; + internal const string DoNotReleaseAttribute = "DoNotReleaseAttribute"; internal const string AssociatedEnumAttribute = "AssociatedEnumAttribute"; internal const string AssociatedConstantAttribute = "AssociatedConstantAttribute"; diff --git a/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs b/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs new file mode 100644 index 00000000..40bb1bb8 --- /dev/null +++ b/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs @@ -0,0 +1,186 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Microsoft.Windows.CsWin32; + +public partial class Generator +{ + /// + /// Finds contextual cleanup information, falling back to a typedef's legacy default. + /// + /// The native value type. + /// The return or parameter annotations. + /// Receives the cleanup function name. + /// Whether cleanup information is available. + internal bool TryGetResourceReleaseMethod(TypeHandleInfo valueType, CustomAttributeHandleCollection? attributes, [NotNullWhen(true)] out string? releaseMethod) + { + if ((this.FindAttribute(attributes, InteropDecorationNamespace, RAIIFreeAttribute) + ?? this.FindAttribute(attributes, InteropDecorationNamespace, FreeWithAttribute)) is CustomAttribute cleanup + && cleanup.DecodeValue(CustomAttributeTypeProvider.Instance).FixedArguments[0].Value is string name) + { + releaseMethod = name; + return true; + } + + if (valueType is HandleTypeHandleInfo nativeType) + { + return nativeType.Generator.TryGetHandleReleaseMethod(nativeType.Handle, null, out releaseMethod); + } + + releaseMethod = null; + return false; + } + + private static string GetNativeValueName(TypeSyntax type) => type switch + { + QualifiedNameSyntax qualified => GetNativeValueName(qualified.Right), + IdentifierNameSyntax identifier => identifier.Identifier.ValueText, + PointerTypeSyntax pointer => GetNativeValueName(pointer.ElementType) + "Pointer", + PredefinedTypeSyntax predefined => predefined.Keyword.ValueText, + _ => throw new GenerationFailedException($"Unsupported native resource type: {type}."), + }; + + 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" + || (name.Identifier.ValueText == "LRESULT" && method == "ICClose"), + PointerTypeSyntax { ElementType: PredefinedTypeSyntax predefined } => predefined.Keyword.IsKind(SyntaxKind.VoidKeyword) && method == "FreeSid", + _ => false, + }; + + private string GetSafeHandleValueName(TypeHandleInfo nativeType, TypeSyntax syntax) + { + string name = GetNativeValueName(syntax); + if (nativeType is HandleTypeHandleInfo handle + && handle.Generator.MetadataIndex.MetadataByNamespace.Values.Count(metadata => metadata.Types.ContainsKey(name)) > 1) + { + QualifiedTypeDefinition definition = handle.Generator.GetQualifiedTypeDefinition(handle.Handle); + return definition.Reader.GetString(definition.Definition.Namespace).Replace('.', '_') + "_" + name; + } + + return name; + } + + private TypeHandleInfo? GetSafeHandleBackingType(TypeHandleInfo type) => + this.TryGetTypeDefFieldType(type, out TypeHandleInfo? field) ? field : type; + + private ExpressionSyntax GetNativeValueFromHandle(ExpressionSyntax handle, TypeHandleInfo backingType) + { + TypeSyntax fieldType = backingType.ToTypeSyntax(this.fieldTypeSettings, GeneratingElement.HelperClassMember, null).Type; + return backingType switch + { + PointerTypeHandleInfo => CastExpression(fieldType, handle), + PrimitiveTypeHandleInfo { PrimitiveTypeCode: PrimitiveTypeCode.IntPtr } => handle, + PrimitiveTypeHandleInfo { PrimitiveTypeCode: PrimitiveTypeCode.UIntPtr } => UncheckedExpression(CastExpression(fieldType, CastExpression(IdentifierName("nint"), handle))), + PrimitiveTypeHandleInfo { PrimitiveTypeCode: PrimitiveTypeCode.UInt32 } => CheckedExpression(CastExpression(fieldType, UncheckedExpression(CastExpression(IdentifierName("nuint"), CastExpression(IdentifierName("nint"), handle))))), + _ => CheckedExpression(CastExpression(fieldType, CastExpression(IdentifierName("nint"), handle))), + }; + } + + private bool IsCleanupParameterCompatible(TypeHandleInfo value, TypeHandleInfo parameter, string releaseMethod) + { + TypeSyntax valueSyntax = value.ToTypeSyntax(this.externSignatureTypeSettings, GeneratingElement.HelperClassMember, null).Type; + TypeSyntax parameterSyntax = parameter.ToTypeSyntax(this.externSignatureTypeSettings, GeneratingElement.HelperClassMember, null).Type; + if (valueSyntax.IsEquivalentTo(parameterSyntax)) + { + return true; + } + + if (this.GetSafeHandleBackingType(value) is PointerTypeHandleInfo + && (parameter is PointerTypeHandleInfo { ElementType: PrimitiveTypeHandleInfo { PrimitiveTypeCode: PrimitiveTypeCode.Void } } + || releaseMethod is "LocalFree" or "GlobalFree")) + { + return true; + } + + if (value is HandleTypeHandleInfo valueHandle && parameter is HandleTypeHandleInfo parameterHandle) + { + QualifiedTypeDefinition valueDefinition = valueHandle.Generator.GetQualifiedTypeDefinition(valueHandle.Handle); + QualifiedTypeDefinition parameterDefinition = parameterHandle.Generator.GetQualifiedTypeDefinition(parameterHandle.Handle); + return valueDefinition.Reader.GetString(valueDefinition.Definition.Namespace) == parameterDefinition.Reader.GetString(parameterDefinition.Definition.Namespace) + && valueDefinition.Generator.GetAlsoUsableForValues(valueDefinition.Definition).Contains(parameterDefinition.Reader.GetString(parameterDefinition.Definition.Name)); + } + + return false; + } + + private void AddTypedSafeHandleMembers(List members, IdentifierNameSyntax ownerType, TypeHandleInfo nativeType, string releaseMethod, bool hidesBase) + { + const string AttributeName = "NativeOwnershipAttribute"; + this.volatileCode.GenerationTransaction(() => this.volatileCode.GenerateSpecialType(AttributeName, () => + { + FetchTemplate(AttributeName, this, out ClassDeclarationSyntax attribute); + this.volatileCode.AddSpecialType(AttributeName, attribute.AddAttributeLists(AttributeList(GeneratedCodeAttribute))); + })); + + TypeSyntax valueType = nativeType.ToTypeSyntax(this.externSignatureTypeSettings, GeneratingElement.HelperClassMember, null).Type; + ExpressionSyntax handle = InvocationExpression(MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, ThisExpression(), IdentifierName(nameof(SafeHandle.DangerousGetHandle)))); + TypeHandleInfo backingType = this.GetSafeHandleBackingType(nativeType) ?? throw new GenerationFailedException("Missing native resource representation."); + ExpressionSyntax nativeValue = this.GetNativeValueFromHandle(handle, backingType); + if (nativeType is HandleTypeHandleInfo) + { + nativeValue = CastExpression(valueType, nativeValue); + } + + AttributeSyntax ValueAttribute(bool lease = false) => Attribute(ParseName($"global::{this.Namespace}.NativeOwnership")) + .WithArgumentList(AttributeArgumentList( + AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(releaseMethod))), + AttributeArgument(LiteralExpression(lease ? SyntaxKind.TrueLiteralExpression : SyntaxKind.FalseLiteralExpression)))); + + PropertyDeclarationSyntax value = PropertyDeclaration(valueType.WithTrailingTrivia(Space), "Value") + .AddModifiers(TokenWithSpace(this.Visibility)) + .AddAttributeLists(AttributeList(ValueAttribute())) + .WithExpressionBody(ArrowExpressionClause(nativeValue)) + .WithSemicolonToken(SemicolonWithLineFeed) + .WithLeadingTrivia(ParseLeadingTrivia(""" + /// + /// Gets the native resource. Use it only within an owner's using scope or obtain a lease. + /// + /// Do not save or return this value, or use it to release the resource. + + """)); + if (hidesBase) + { + value = value.AddModifiers(TokenWithSpace(SyntaxKind.NewKeyword)); + } + + members.Add(value); + if (this.LanguageVersion < LanguageVersion.CSharp9) + { + return; + } + + FetchTemplate("SafeHandleLease", this, out StructDeclarationSyntax lease); + lease = lease.ReplaceNodes( + lease.DescendantNodes().OfType().Where(identifier => identifier.Identifier.ValueText is "__OwnerType" or "__NativeType"), + (original, rewritten) => original.Identifier.ValueText == "__OwnerType" ? ownerType.WithTriviaFrom(rewritten) : valueType.WithTriviaFrom(rewritten)); + SyntaxTriviaList leaseTrivia = lease.GetLeadingTrivia(); + lease = lease.WithoutLeadingTrivia().WithModifiers([TokenWithSpace(this.Visibility), TokenWithSpace(SyntaxKind.RefKeyword)]) + .AddAttributeLists(AttributeList(ValueAttribute(lease: true))).WithLeadingTrivia(leaseTrivia); + PropertyDeclarationSyntax leaseValue = lease.Members.OfType().Single(); + lease = lease.ReplaceNode(leaseValue, leaseValue.WithoutLeadingTrivia().AddAttributeLists(AttributeList(ValueAttribute())).WithLeadingTrivia(leaseValue.GetLeadingTrivia())); + + MethodDeclarationSyntax leaseMethod = MethodDeclaration(IdentifierName("LeaseScope").WithTrailingTrivia(Space), Identifier("Lease")) + .AddModifiers(TokenWithSpace(this.Visibility)) + .WithExpressionBody(ArrowExpressionClause(ObjectCreationExpression(IdentifierName("LeaseScope"), [Argument(ThisExpression())]))) + .WithSemicolonToken(SemicolonWithLineFeed) + .WithLeadingTrivia(ParseLeadingTrivia(""" + /// + /// Acquires a reference to this resource until the returned lease's using scope ends. + /// + /// An allocation-free lease. Declare it as a using local and do not copy it. + + """)); + if (hidesBase) + { + lease = lease.WithModifiers(lease.Modifiers.Insert(1, TokenWithSpace(SyntaxKind.NewKeyword))); + leaseMethod = leaseMethod.AddModifiers(TokenWithSpace(SyntaxKind.NewKeyword)); + } + + members.Add(leaseMethod); + + // Restore the generated file's disabled context after the struct, outside rewritten brace trivia. + members.Add(lease.WithTrailingTrivia(ParseLeadingTrivia("\n#nullable disable\n"))); + } +} diff --git a/src/Microsoft.Windows.CsWin32/Generator.cs b/src/Microsoft.Windows.CsWin32/Generator.cs index 1197c9ef..643a202d 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.cs @@ -707,6 +707,23 @@ public bool TryGenerateType(string possiblyQualifiedName, out IReadOnlyCollectio return true; } + const string SafeHandleSuffix = "SafeHandle"; + if (this.options.UseSafeHandles && typeName.EndsWith(SafeHandleSuffix, StringComparison.Ordinal) + && (typeNamespace is null || typeNamespace == this.Namespace)) + { + string releaseMethod = typeName.Substring(0, typeName.Length - SafeHandleSuffix.Length); + if (this.GetMethodByName(releaseMethod).HasValue && !BclInteropSafeHandles.ContainsKey(releaseMethod)) + { + TypeSyntax? helperType = null; + this.volatileCode.GenerationTransaction(() => helperType = this.RequestSafeHandle(releaseMethod, allowAbstract: true)); + if (helperType is not null) + { + preciseApi = ImmutableList.Create($"{this.Namespace}.{typeName}"); + return true; + } + } + } + if (foundApiWithMismatchedPlatform) { throw new PlatformIncompatibleException($"The requested API ({possiblyQualifiedName}) was found but is not available given the target platform ({this.compilation?.Options.Platform})."); diff --git a/src/Microsoft.Windows.CsWin32/MetadataIndex.cs b/src/Microsoft.Windows.CsWin32/MetadataIndex.cs index 88857ee7..bf6a039e 100644 --- a/src/Microsoft.Windows.CsWin32/MetadataIndex.cs +++ b/src/Microsoft.Windows.CsWin32/MetadataIndex.cs @@ -114,6 +114,23 @@ void PopulateNamespace(NamespaceDefinition ns, string? parentNamespace) if (MetadataUtilities.IsCompatibleWithPlatform(mr, this, platform, methodDef.GetCustomAttributes())) { nsMetadata.Methods.Add(methodName, methodDefHandle); + foreach (ParameterHandle parameterHandle in methodDef.GetParameters()) + { + Parameter parameter = mr.GetParameter(parameterHandle); + foreach (CustomAttributeHandle attributeHandle in parameter.GetCustomAttributes()) + { + CustomAttribute cleanup = mr.GetCustomAttribute(attributeHandle); + if (MetadataUtilities.IsAttribute(mr, cleanup, Generator.InteropDecorationNamespace, Generator.RAIIFreeAttribute) + || MetadataUtilities.IsAttribute(mr, cleanup, Generator.InteropDecorationNamespace, Generator.FreeWithAttribute)) + { + CustomAttributeValue arguments = cleanup.DecodeValue(CustomAttributeTypeProvider.Instance); + if (arguments.FixedArguments[0].Value is string cleanupMethod) + { + this.releaseMethods.Add(cleanupMethod); + } + } + } + } } else { diff --git a/src/Microsoft.Windows.CsWin32/SafeHandleAnalyzer.cs b/src/Microsoft.Windows.CsWin32/SafeHandleAnalyzer.cs new file mode 100644 index 00000000..1a207464 --- /dev/null +++ b/src/Microsoft.Windows.CsWin32/SafeHandleAnalyzer.cs @@ -0,0 +1,291 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.Windows.CsWin32; + +/// +/// Enforces scoped access to generated native owners and their allocation-free leases. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class SafeHandleAnalyzer : DiagnosticAnalyzer +{ + /// + /// Reports lease creation, copying, storage, or disposal outside the supported using-local pattern. + /// + public static readonly DiagnosticDescriptor LeaseMustBeUsingLocal = new( + "PInvoke015", + "Keep leases in using locals", + "Create each lease with 'using var lease = owner.Lease()'; do not copy, store, pass, return, or explicitly dispose it", + "Lifetime", + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + /// Reports native value access without a visible lifetime scope. + /// + public static readonly DiagnosticDescriptor ValueNeedsScope = new( + "PInvoke016", + "Keep the native resource alive", + "Read Value inside the owner's using scope, or acquire a lease in a using local", + "Lifetime", + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + /// Reports raw resource values saved or returned instead of consumed within their scope. + /// + public static readonly DiagnosticDescriptor ValueMustNotEscape = new( + "PInvoke017", + "Do not save or return raw resources", + "Use Value directly in a call or copy its contents; do not save or return the raw resource", + "Lifetime", + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + /// Reports attempts to release a resource that still belongs to its SafeHandle. + /// + public static readonly DiagnosticDescriptor ValueMustNotRelease = new( + "PInvoke018", + "Do not release an owned resource", + "Do not pass Value to '{0}'; dispose the SafeHandle instead", + "Lifetime", + DiagnosticSeverity.Warning, + isEnabledByDefault: true); + + /// + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create( + LeaseMustBeUsingLocal, ValueNeedsScope, ValueMustNotEscape, ValueMustNotRelease); + + /// + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + context.RegisterSyntaxNodeAction(AnalyzeLocal, SyntaxKind.VariableDeclarator); + context.RegisterSymbolAction(AnalyzeDeclaration, SymbolKind.Field, SymbolKind.Parameter, SymbolKind.Property, SymbolKind.Method); + context.RegisterOperationAction(AnalyzeLeaseCreation, OperationKind.Invocation, OperationKind.ObjectCreation, OperationKind.DefaultValue); + context.RegisterOperationAction(AnalyzeLeaseReference, OperationKind.LocalReference); + context.RegisterOperationAction(AnalyzeValue, OperationKind.PropertyReference); + } + + private static AttributeData? GetOwnership(ISymbol symbol) => symbol.GetAttributes().FirstOrDefault(attribute => + attribute.AttributeClass is { Name: "NativeOwnershipAttribute" } marker + && marker.GetAttributes().Any(generated => generated.AttributeClass?.ToDisplayString() == "System.CodeDom.Compiler.GeneratedCodeAttribute" + && generated.ConstructorArguments[0].Value is string tool && tool == "Microsoft.Windows.CsWin32")); + + private static bool IsLease(ITypeSymbol? type) => type is not null + && GetOwnership(type) is { ConstructorArguments.Length: >= 2 } attribute + && attribute.ConstructorArguments[1].Value is true; + + private static bool IsWithinNameOf(IOperation operation) + { + for (IOperation? parent = operation.Parent; parent is not null; parent = parent.Parent) + { + if (parent is INameOfOperation) + { + return true; + } + } + + return false; + } + + private static IOperation Unwrap(IOperation operation) + { + while (operation is IConversionOperation conversion) + { + operation = conversion.Operand; + } + + return operation; + } + + private static bool IsUsingLocal(ILocalSymbol local) => local.DeclaringSyntaxReferences.Any(reference => + reference.GetSyntax() is VariableDeclaratorSyntax { Parent: VariableDeclarationSyntax declaration } + && ((declaration.Parent is LocalDeclarationStatementSyntax statement && !statement.UsingKeyword.IsKind(SyntaxKind.None)) + || declaration.Parent is UsingStatementSyntax)); + + private static bool IsDirectLeaseFactory(IOperation? operation) => operation is not null + && Unwrap(operation) is IInvocationOperation { TargetMethod.Name: "Lease" } invocation + && IsLease(invocation.Type); + + private static void AnalyzeLocal(SyntaxNodeAnalysisContext context) + { + var declaration = (VariableDeclaratorSyntax)context.Node; + if (context.SemanticModel.GetDeclaredSymbol(declaration, context.CancellationToken) is ILocalSymbol local + && IsLease(local.Type) + && (!IsUsingLocal(local) || !IsDirectLeaseFactory(declaration.Initializer is { } initializer + ? context.SemanticModel.GetOperation(initializer.Value, context.CancellationToken) : null))) + { + context.ReportDiagnostic(Diagnostic.Create(LeaseMustBeUsingLocal, declaration.GetLocation())); + } + } + + private static void AnalyzeDeclaration(SymbolAnalysisContext context) + { + ITypeSymbol? type = context.Symbol switch + { + IFieldSymbol field => field.Type, + IParameterSymbol parameter => parameter.Type, + IPropertySymbol property => property.Type, + IMethodSymbol method => method.ReturnType, + _ => null, + }; + if (IsLease(type) && !context.Symbol.IsImplicitlyDeclared) + { + foreach (Location location in context.Symbol.Locations.Where(location => location.IsInSource)) + { + context.ReportDiagnostic(Diagnostic.Create(LeaseMustBeUsingLocal, location)); + } + } + } + + private static void AnalyzeLeaseCreation(OperationAnalysisContext context) + { + if (!IsLease(context.Operation.Type)) + { + return; + } + + IOperation operation = context.Operation; + while (operation.Parent is IConversionOperation conversion) + { + operation = conversion; + } + + if (operation.Parent is not IVariableInitializerOperation { Parent: IVariableDeclaratorOperation declarator } + || !IsUsingLocal(declarator.Symbol) || !IsDirectLeaseFactory(operation)) + { + context.ReportDiagnostic(Diagnostic.Create(LeaseMustBeUsingLocal, context.Operation.Syntax.GetLocation())); + } + } + + private static void AnalyzeLeaseReference(OperationAnalysisContext context) + { + var reference = (ILocalReferenceOperation)context.Operation; + if (!IsLease(reference.Type) || IsWithinNameOf(reference)) + { + return; + } + + if (reference.Parent is not IPropertyReferenceOperation { Property.Name: "Value" } property || GetOwnership(property.Property) is null) + { + context.ReportDiagnostic(Diagnostic.Create(LeaseMustBeUsingLocal, reference.Syntax.GetLocation())); + } + } + + private static bool HasScope(IOperation? instance, SyntaxNode usage, SemanticModel semanticModel, CancellationToken cancellationToken) + { + if (instance is null) + { + return false; + } + + ISymbol? owner = Unwrap(instance) switch + { + ILocalReferenceOperation local => local.Local, + IParameterReferenceOperation parameter => parameter.Parameter, + _ => null, + }; + if (owner is null) + { + return false; + } + + foreach (SyntaxNode ancestor in usage.Ancestors()) + { + if (ancestor is AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax) + { + return false; + } + + if (ancestor is UsingStatementSyntax { Expression: { } expression } + && SymbolEqualityComparer.Default.Equals(semanticModel.GetSymbolInfo(expression, cancellationToken).Symbol, owner)) + { + return true; + } + + if (owner is ILocalSymbol local && IsUsingLocal(local)) + { + foreach (SyntaxReference reference in local.DeclaringSyntaxReferences) + { + if (reference.GetSyntax(cancellationToken) is VariableDeclaratorSyntax { Parent: VariableDeclarationSyntax declaration } + && ((declaration.Parent is LocalDeclarationStatementSyntax { Parent: { } scope } && scope == ancestor) + || (declaration.Parent is UsingStatementSyntax statement && statement == ancestor))) + { + return true; + } + } + } + } + + return false; + } + + private static bool IsReleaseImplementation(IOperation? instance, ISymbol containingSymbol) => + instance is IInstanceReferenceOperation { ReferenceKind: InstanceReferenceKind.ContainingTypeInstance } + && containingSymbol is IMethodSymbol { Name: "ReleaseHandle", IsOverride: true, Parameters.Length: 0 }; + + private static void AnalyzeValue(OperationAnalysisContext context) + { + var property = (IPropertyReferenceOperation)context.Operation; + if (GetOwnership(property.Property) is not { } ownership || IsWithinNameOf(property)) + { + return; + } + + bool releasing = IsReleaseImplementation(property.Instance, context.ContainingSymbol); + if (!releasing && (property.SemanticModel is not { } semanticModel || !HasScope(property.Instance, property.Syntax, semanticModel, context.CancellationToken))) + { + context.ReportDiagnostic(Diagnostic.Create(ValueNeedsScope, property.Syntax.GetLocation())); + } + + IOperation use = property; + while (true) + { + if (use.Parent is IConversionOperation or IParenthesizedOperation + || (use.Parent is IFieldReferenceOperation field && field.Instance == use) + || (use.Parent is IPropertyReferenceOperation member && member.Instance == use)) + { + use = use.Parent; + if (use.Type?.SpecialType is SpecialType.System_Boolean or SpecialType.System_String) + { + return; + } + } + else + { + break; + } + } + + if (use.Parent is IArgumentOperation { Parent: IInvocationOperation call } argument) + { + if (argument.Parameter?.RefKind is RefKind.Ref or RefKind.Out) + { + context.ReportDiagnostic(Diagnostic.Create(ValueMustNotEscape, use.Syntax.GetLocation())); + } + + string? releaseMethod = ownership.ConstructorArguments[0].Value as string; + if (!releasing && (call.TargetMethod.Name == releaseMethod || call.TargetMethod.GetDllImportData()?.EntryPointName == releaseMethod)) + { + context.ReportDiagnostic(Diagnostic.Create(ValueMustNotRelease, use.Syntax.GetLocation(), releaseMethod)); + } + + return; + } + + if ((use.Parent is IInvocationOperation { Type.SpecialType: SpecialType.System_String } invocation && invocation.Instance == use) + || use.Parent is IBinaryOperation { Type.SpecialType: SpecialType.System_Boolean } + || use.Parent is IIsPatternOperation) + { + return; + } + + context.ReportDiagnostic(Diagnostic.Create(ValueMustNotEscape, use.Syntax.GetLocation())); + } +} diff --git a/src/Microsoft.Windows.CsWin32/templates/NativeOwnershipAttribute.cs b/src/Microsoft.Windows.CsWin32/templates/NativeOwnershipAttribute.cs new file mode 100644 index 00000000..37bcbcb1 --- /dev/null +++ b/src/Microsoft.Windows.CsWin32/templates/NativeOwnershipAttribute.cs @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +/// +/// Describes generated resource access for the CsWin32 lifetime analyzer. +/// +[global::System.AttributeUsage(global::System.AttributeTargets.Property | global::System.AttributeTargets.Struct)] +internal sealed class NativeOwnershipAttribute : global::System.Attribute +{ + /// + /// Initializes a new instance of the class. + /// + /// The native cleanup function. + /// Whether the annotated type represents a scoped reference. + public NativeOwnershipAttribute(string releaseMethod, bool isLease = false) + { + } +} diff --git a/src/Microsoft.Windows.CsWin32/templates/SafeHandleLease.cs b/src/Microsoft.Windows.CsWin32/templates/SafeHandleLease.cs new file mode 100644 index 00000000..91996341 --- /dev/null +++ b/src/Microsoft.Windows.CsWin32/templates/SafeHandleLease.cs @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +/// +/// Keeps a reference to the original resource until this lease's using scope ends. +/// +/// +/// Create this lease with using var lease = owner.Lease(). Do not copy it, +/// store it in a field, pass it to another method, return it, or dispose it explicitly. +/// +public ref struct LeaseScope +{ +#nullable enable + private __OwnerType? owner; + + /// + /// Initializes a new instance of the struct. + /// + /// The owner whose reference count is acquired. + internal LeaseScope(__OwnerType owner) + { + this.owner = null; + bool added = false; + try + { + owner.DangerousAddRef(ref added); + this.owner = owner; + } + catch + { + if (added) + { + owner.DangerousRelease(); + } + + throw; + } + } + + /// + /// Gets the native resource while this lease is active. + /// + /// The lease is not active. + public readonly __NativeType Value => this.owner is { } owner + ? owner.Value + : throw new global::System.ObjectDisposedException(nameof(LeaseScope)); + + /// + /// Releases this lease's reference without disposing the original owner. + /// + public void Dispose() + { + __OwnerType? owner = this.owner; + this.owner = null; + if (owner is not null) + { + owner.DangerousRelease(); + } + } +} diff --git a/test/CsWin32Generator.Tests/CsWin32GeneratorTests.cs b/test/CsWin32Generator.Tests/CsWin32GeneratorTests.cs index 2eef5c5a..ae1cf83d 100644 --- a/test/CsWin32Generator.Tests/CsWin32GeneratorTests.cs +++ b/test/CsWin32Generator.Tests/CsWin32GeneratorTests.cs @@ -719,17 +719,19 @@ public void UseInitHandleApiWhenPossible( this.compilation = this.starterCompilations[initHandleApiAvailable ? "net8.0" : "net472"]; this.GenerateApi(api); - MethodDeclarationSyntax friendlyOverload = Assert.Single( - this.FindGeneratedMethod(api), - m => !m.AttributeLists.Any(al => al.Attributes.Any(a => a.Name.ToString() == "DllImport"))); - - if (initHandleApiAvailable) - { - Assert.Contains(friendlyOverload.DescendantNodes(), n => n is IdentifierNameSyntax { Identifier.Text: "InitHandle" }); - } - else + var friendlyOverloads = this.FindGeneratedMethod(api) + .Where(m => !m.AttributeLists.Any(al => al.Attributes.Any(a => a.Name.ToString() == "DllImport"))).ToArray(); + Assert.NotEmpty(friendlyOverloads); + foreach (MethodDeclarationSyntax friendlyOverload in friendlyOverloads) { - Assert.DoesNotContain(friendlyOverload.DescendantNodes(), n => n is IdentifierNameSyntax { Identifier.Text: "InitHandle" }); + if (initHandleApiAvailable) + { + Assert.Contains(friendlyOverload.DescendantNodes(), n => n is IdentifierNameSyntax { Identifier.Text: "InitHandle" }); + } + else + { + Assert.DoesNotContain(friendlyOverload.DescendantNodes(), n => n is IdentifierNameSyntax { Identifier.Text: "InitHandle" }); + } } } } diff --git a/test/Microsoft.Windows.CsWin32.Tests/FriendlyOverloadTests.cs b/test/Microsoft.Windows.CsWin32.Tests/FriendlyOverloadTests.cs index 1b4e4eb8..a50c5c65 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/FriendlyOverloadTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/FriendlyOverloadTests.cs @@ -66,8 +66,9 @@ public void OutPWSTR_Parameters_AsSpan() { const string name = "GetWindowText"; this.Generate(name); - MethodDeclarationSyntax friendlyOverload = Assert.Single(this.FindGeneratedMethod(name), m => m.ParameterList.Parameters.Count == 2); + MethodDeclarationSyntax friendlyOverload = Assert.Single(this.FindGeneratedMethod(name), m => m.ParameterList.Parameters.Count == 2 && m.ParameterList.Parameters[0].Type is QualifiedNameSyntax { Right.Identifier.ValueText: "HWND" }); Assert.Equal("Span", friendlyOverload.ParameterList.Parameters[1].Type?.ToString()); + Assert.Contains(this.FindGeneratedMethod(name), m => m.ParameterList.Parameters.Count == 2 && m.ParameterList.Parameters[0].Type?.ToString() == "SafeHandle"); } [Theory] diff --git a/test/Microsoft.Windows.CsWin32.Tests/HandleTests.cs b/test/Microsoft.Windows.CsWin32.Tests/HandleTests.cs index 698007ed..0f8da880 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/HandleTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/HandleTests.cs @@ -19,10 +19,12 @@ public void BSTR_FieldsDoNotBecomeSafeHandles(bool allowMarshaling) } [Fact] - public void NamespaceHandleGetsNoSafeHandle() + public void NamespaceHandleRequiresCallerDefinedCleanup() { this.GenerateApi("CreatePrivateNamespace"); - Assert.Empty(this.FindGeneratedType("ClosePrivateNamespaceSafeHandle")); + ClassDeclarationSyntax helper = Assert.IsType(Assert.Single(this.FindGeneratedType("ClosePrivateNamespaceSafeHandle"))); + Assert.Contains(helper.Modifiers, modifier => modifier.IsKind(SyntaxKind.AbstractKeyword)); + Assert.DoesNotContain(this.FindGeneratedMethod("CreatePrivateNamespace"), method => method.ReturnType.ToString().EndsWith("SafeHandle", StringComparison.Ordinal)); } [Fact] diff --git a/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs new file mode 100644 index 00000000..c2b8fce6 --- /dev/null +++ b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs @@ -0,0 +1,560 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis.Diagnostics; +using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory; + +namespace Microsoft.Windows.CsWin32.Tests; + +/// +/// Tests user-visible ownership projections, lease behavior, and lifetime diagnostics. +/// +public class SafeHandleLeaseTests : GeneratorTestBase +{ + private const string MetadataSource = """ + using System; + using System.Runtime.InteropServices; + using Windows.Win32.Foundation.Metadata; + + namespace Windows.Win32.Foundation.Metadata + { + [AttributeUsage(AttributeTargets.Struct)] + public sealed class NativeTypedefAttribute : Attribute { } + [AttributeUsage(AttributeTargets.Struct, AllowMultiple = true)] + public sealed class InvalidHandleValueAttribute : Attribute + { + public InvalidHandleValueAttribute(long value) { } + } + [AttributeUsage(AttributeTargets.Struct | AttributeTargets.ReturnValue | AttributeTargets.Parameter)] + public sealed class RAIIFreeAttribute : Attribute + { + public RAIIFreeAttribute(string method) { } + } + [AttributeUsage(AttributeTargets.ReturnValue | AttributeTargets.Parameter)] + public sealed class FreeWithAttribute : Attribute + { + public FreeWithAttribute(string method) { } + } + } + namespace Windows.Win32.Other + { + [NativeTypedef, InvalidHandleValue(0)] + public unsafe struct DUPLICATE_RESOURCE { public void* Value; } + } + namespace Windows.Win32.Test + { + [NativeTypedef, InvalidHandleValue(0)] + public unsafe struct RESOURCE_A { public void* Value; } + [NativeTypedef, InvalidHandleValue(0)] + public unsafe struct RESOURCE_B { public void* Value; } + [NativeTypedef, InvalidHandleValue(0)] + public struct UNSIGNED_RESOURCE { public UIntPtr Value; } + [NativeTypedef, InvalidHandleValue(0)] + public struct UNSIGNED32_RESOURCE { public uint Value; } + [NativeTypedef, InvalidHandleValue(0)] + public unsafe struct DUPLICATE_RESOURCE { public void* Value; } + + public static unsafe class Apis + { + [DllImport("fixture.dll", ExactSpelling = true)] + [return: __Cleanup(nameof(FreeShared))] + public static extern RESOURCE_A CreateResource(); + [DllImport("fixture.dll", ExactSpelling = true)] + [return: __Cleanup(nameof(FreeShared))] + public static extern RESOURCE_B CreateOtherResource(); + [DllImport("fixture.dll", ExactSpelling = true)] + [return: __Cleanup(nameof(FreeShared))] + public static extern void* CreatePointer(); + [DllImport("fixture.dll", ExactSpelling = true)] + public static extern int AcquirePointer([Out, __Cleanup(nameof(FreeShared))] void** resource); + [DllImport("fixture.dll", ExactSpelling = true)] + public static extern int AcquireResource([Out, __Cleanup(nameof(FreeShared))] RESOURCE_A* resource); + [DllImport("fixture.dll", ExactSpelling = true)] + public static extern void FreeShared(void* resource); + [DllImport("fixture.dll", ExactSpelling = true)] + public static extern void ConsumeResource([In] RESOURCE_A resource); + [DllImport("fixture.dll", ExactSpelling = true)] + public static extern void ConsumePointer([In] void* resource); + [DllImport("fixture.dll", ExactSpelling = true)] + [return: __Cleanup(nameof(FreeOtherResource))] + public static extern RESOURCE_A IncorrectCleanup(); + [DllImport("fixture.dll", ExactSpelling = true)] + public static extern void FreeOtherResource(RESOURCE_B resource); + [DllImport("fixture.dll", ExactSpelling = true)] + [return: __Cleanup(nameof(FreeContext))] + public static extern RESOURCE_A CreateWithContext(); + [DllImport("fixture.dll", ExactSpelling = true)] + public static extern bool FreeContext(RESOURCE_A resource, int context); + [DllImport("fixture.dll", ExactSpelling = true)] + [return: __Cleanup(nameof(FreeUnsigned))] + public static extern UNSIGNED_RESOURCE CreateUnsigned(); + [DllImport("fixture.dll", ExactSpelling = true)] + public static extern void FreeUnsigned(UNSIGNED_RESOURCE resource); + [DllImport("fixture.dll", ExactSpelling = true)] + [return: __Cleanup(nameof(FreeUnsigned32))] + public static extern UNSIGNED32_RESOURCE CreateUnsigned32(); + [DllImport("fixture.dll", ExactSpelling = true)] + public static extern void FreeUnsigned32(UNSIGNED32_RESOURCE resource); + [DllImport("fixture.dll", ExactSpelling = true)] + [return: __Cleanup(nameof(FreeShared))] + public static extern DUPLICATE_RESOURCE CreateDuplicate(); + [DllImport("fixture.dll", ExactSpelling = true)] + [return: __Cleanup(nameof(FreeShared))] + public static extern Windows.Win32.Other.DUPLICATE_RESOURCE CreateOtherDuplicate(); + } + } + """; + + /// + /// Initializes a new instance of the class. + /// + /// The test output. + public SafeHandleLeaseTests(ITestOutputHelper logger) + : base(logger) + { + } + + /// + /// Verifies native value types and caller compatibility when several resource types share a cleanup function. + /// + /// The contextual cleanup attribute. + /// Whether runtime marshaling is enabled. + [Theory, CombinatorialData] + public void ContextualOwnersPreserveTheirNativeTypes( + [CombinatorialValues("RAIIFree", "FreeWith")] string attribute, bool allowMarshaling) + { + this.GenerateFixture(attribute, allowMarshaling, "CreateResource", "CreateOtherResource", "CreatePointer", "AcquirePointer", "AcquireResource", "ConsumeResource", "ConsumePointer"); + this.compilation = this.AddCode(""" + using Windows.Win32; + using Windows.Win32.Test; + static unsafe class Usage + { + static void Use(FreeSharedRESOURCE_ASafeHandle a, FreeSharedRESOURCE_BSafeHandle b, FreeSharedSafeHandle pointer) + { + using var leaseA = a.Lease(); + using var leaseB = b.Lease(); + using var leasePointer = pointer.Lease(); + RESOURCE_A valueA = leaseA.Value; + RESOURCE_B valueB = leaseB.Value; + void* valuePointer = leasePointer.Value; + FreeSharedSafeHandle compatibleWithExistingOwner = a; + PInvoke.ConsumeResource(a); + PInvoke.ConsumeResource(leaseA.Value); + PInvoke.ConsumePointer(leasePointer.Value); + PInvoke.AcquirePointer(out FreeSharedSafeHandle ownedPointer); + PInvoke.AcquireResource(out FreeSharedRESOURCE_ASafeHandle ownedA); + } + } + """); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + Assert.Contains(this.FindGeneratedMethod("ConsumeResource"), method => method.ParameterList.Parameters[0].Type?.ToString() == "SafeHandle"); + Assert.DoesNotContain(this.FindGeneratedMethod("ConsumePointer"), method => method.ParameterList.Parameters.Any(parameter => parameter.Type?.ToString() == "SafeHandle")); + } + + /// + /// Verifies the existing allocated-string annotation with a real native string consumer. + /// + [Fact] + public async Task KnownFolderPathHasTypedOwnerAndLease() + { + this.generator = this.CreateGenerator(); + Assert.True(this.generator.TryGenerate("SHGetKnownFolderPath", CancellationToken.None)); + Assert.True(this.generator.TryGenerate("PathFileExists", CancellationToken.None)); + this.CollectGeneratedCode(this.generator); + this.compilation = this.AddCode(""" + using Windows.Win32; + using Windows.Win32.Foundation; + static class Usage + { + static bool Exists(CoTaskMemFreePWSTRSafeHandle path) + { + using var lease = path.Lease(); + return PInvoke.PathFileExists(lease.Value); + } + static void Get(System.Guid folder) + { + PInvoke.SHGetKnownFolderPath(folder, 0, null, out CoTaskMemFreePWSTRSafeHandle path); + using (path) + { + PInvoke.PathFileExists(path.Value); + } + } + } + """); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + Assert.Empty(await this.AnalyzeAsync()); + } + + /// + /// Verifies that incompatible opaque handle cleanup retains the raw API rather than an unsafe owner. + /// + /// The cleanup attribute. + [Theory] + [InlineData("RAIIFree")] + [InlineData("FreeWith")] + public void IncompatibleCleanupDoesNotProduceOwner(string attribute) + { + this.GenerateFixture(attribute, true, "IncorrectCleanup"); + Assert.All(this.FindGeneratedMethod("IncorrectCleanup"), method => + Assert.Equal("RESOURCE_A", Assert.IsType(method.ReturnType).Right.Identifier.ValueText)); + Assert.Empty(this.FindGeneratedMethod("IncorrectCleanup_SafeHandle")); + } + + /// + /// Verifies that the real incompatible recording-DC annotations leave the raw methods available. + /// + /// The recording DC factory. + [Theory] + [InlineData("CreateMetaFile")] + [InlineData("CreateEnhMetaFile")] + public void MetafileRecordingContextsDoNotGetDeletionOwners(string api) + { + this.GenerateApi(api); + Assert.All(this.FindGeneratedMethod(api), method => + Assert.Equal("HDC", Assert.IsType(method.ReturnType).Right.Identifier.ValueText)); + Assert.Empty(this.FindGeneratedMethod(api + "_SafeHandle")); + } + + /// + /// Verifies helpers with caller-supplied cleanup context without changing the factory to an abstract return type. + /// + [Fact] + public async Task MultiParameterCleanupSupportsDerivedOwner() + { + this.GenerateFixture("RAIIFree", true, "CreateWithContext"); + this.compilation = this.AddCode(""" + using Windows.Win32; + sealed class Owner : FreeContextSafeHandle + { + private readonly int context; + internal Owner(System.IntPtr value, int context) : base(value) { this.context = context; } + protected override bool ReleaseHandle() => PInvoke.FreeContext(this.Value, this.context); + } + """); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + Assert.Empty(await this.AnalyzeAsync()); + Assert.All(this.FindGeneratedMethod("CreateWithContext"), method => + Assert.Equal("RESOURCE_A", Assert.IsType(method.ReturnType).Right.Identifier.ValueText)); + } + + /// + /// Verifies that a timer cleanup helper can be requested independently of a contextual factory annotation. + /// + [Fact] + public void TimerQueueCleanupHelperCanBeRequested() + { + this.GenerateApi("DeleteTimerQueueTimerSafeHandle"); + var type = this.compilation.GetTypeByMetadataName("Windows.Win32.DeleteTimerQueueTimerSafeHandle"); + Assert.NotNull(type); + Assert.True(type.IsAbstract); + Assert.Equal("HANDLE", Assert.IsAssignableFrom(Assert.Single(type.GetMembers("Value"))).Type.Name); + } + + /// + /// Verifies that disabling SafeHandles still exposes ordinary native outputs. + /// + [Fact] + public void SafeHandlesCanStillBeDisabled() + { + this.GenerateFixture("FreeWith", true, ["CreateResource", "AcquirePointer"], useSafeHandles: false); + Assert.Empty(this.FindGeneratedType("FreeSharedSafeHandle")); + Assert.All(this.FindGeneratedMethod("CreateResource"), method => + Assert.Equal("RESOURCE_A", Assert.IsType(method.ReturnType).Right.Identifier.ValueText)); + } + + /// + /// Verifies that same-named native types retain distinct ownership projections. + /// + [Fact] + public void NativeNamespacesKeepOwnersDistinct() + { + this.GenerateFixture("RAIIFree", true, "CreateDuplicate", "CreateOtherDuplicate"); + this.compilation = this.AddCode(""" + using Windows.Win32; + static class Usage + { + static void Use() + { + using var a = PInvoke.CreateDuplicate_SafeHandle(); + using var b = PInvoke.CreateOtherDuplicate_SafeHandle(); + Windows.Win32.Test.DUPLICATE_RESOURCE valueA = a.Value; + Windows.Win32.Other.DUPLICATE_RESOURCE valueB = b.Value; + } + } + """); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + } + + /// + /// Verifies that a predeclared canonical owner need not support inheritance or the new typed helpers. + /// + [Fact] + public void ExistingSealedOwnerRemainsUsable() + { + this.compilation = this.starterCompilations["net8.0"]; + this.compilation = this.AddCode(""" + namespace Windows.Win32 + { + sealed class FreeSharedSafeHandle : global::System.Runtime.InteropServices.SafeHandle + { + public FreeSharedSafeHandle() : base(global::System.IntPtr.Zero, true) { } + public FreeSharedSafeHandle(global::System.IntPtr value, bool ownsHandle = true) : base(global::System.IntPtr.Zero, ownsHandle) { this.SetHandle(value); } + public override bool IsInvalid => this.handle == global::System.IntPtr.Zero; + protected override bool ReleaseHandle() => true; + } + } + """); + this.GenerateFixture("RAIIFree", true, ["CreateResource"], useSafeHandles: true, preserveCompilation: true); + this.compilation = this.AddCode(""" + using Windows.Win32; + static class Usage + { + static void Use() + { + using var owner = PInvoke.CreateResource_SafeHandle(); + Windows.Win32.Test.RESOURCE_A value = owner.Value; + } + } + """); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + } + + /// + /// Verifies leases against older reference assemblies. + /// + /// The caller's target framework. + [Theory] + [InlineData("net35")] + [InlineData("net472")] + [InlineData("netstandard2.0")] + public void LeasesCompileForSupportedFrameworks(string framework) + { + this.compilation = this.starterCompilations[framework]; + this.GenerateFixture("RAIIFree", true, ["CreateResource", "ConsumeResource"], useSafeHandles: true, preserveCompilation: true); + this.compilation = this.AddCode(""" + using Windows.Win32; + static class Usage + { + static void Use(FreeSharedRESOURCE_ASafeHandle owner) + { + using var lease = owner.Lease(); + PInvoke.ConsumeResource(lease.Value); + } + } + """); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + } + + /// + /// Verifies that generated leases do not change the nullability context of other generated members. + /// + [Fact] + public void NullableEnabledProjectsCompile() + { + this.compilation = this.starterCompilations["net8.0"].WithOptions( + this.starterCompilations["net8.0"].Options.WithNullableContextOptions(NullableContextOptions.Enable)); + this.GenerateFixture("RAIIFree", true, ["CreateResource", "CreateOtherResource"], useSafeHandles: true, preserveCompilation: true); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + } + + /// + /// Verifies accepted and rejected lifetime patterns through their diagnostics. + /// + /// The caller's code. + /// The expected diagnostic, or null for a supported pattern. + [Theory] + [InlineData("using var lease = owner.Lease(); PInvoke.ConsumeResource(lease.Value);", null)] + [InlineData("using (var lease = owner.Lease()) { PInvoke.ConsumeResource(lease.Value); }", null)] + [InlineData("using (owner) { PInvoke.ConsumeResource(owner.Value); }", null)] + [InlineData("using var local = owner; PInvoke.ConsumeResource(local.Value);", null)] + [InlineData("using var lease = owner.Lease(); string text = lease.Value.ToString();", null)] + [InlineData("using var lease = owner.Lease(); bool empty = lease.Value.IsNull;", null)] + [InlineData("_ = nameof(owner.Value);", null)] + [InlineData("using var lease = owner.Lease(); _ = nameof(lease.Value.Value);", null)] + [InlineData("PInvoke.ConsumeResource(owner.Value);", "PInvoke016")] + [InlineData("var lease = owner.Lease(); PInvoke.ConsumeResource(lease.Value);", "PInvoke015")] + [InlineData("PInvoke.ConsumeResource(owner.Lease().Value);", "PInvoke015")] + [InlineData("using var lease = owner.Lease(); var copy = lease;", "PInvoke015")] + [InlineData("using var lease = owner.Lease(); lease.Dispose();", "PInvoke015")] + [InlineData("using var lease = owner.Lease(); PassLease(lease);", "PInvoke015")] + [InlineData("using var lease = owner.Lease(); var saved = lease.Value;", "PInvoke017")] + [InlineData("using (owner) { System.Action action = () => PInvoke.ConsumeResource(owner.Value); }", "PInvoke016")] + [InlineData("using var lease = owner.Lease(); PInvoke.FreeShared(lease.Value.Value);", "PInvoke018")] + public async Task LifetimeUsageDiagnostics(string body, string? expectedDiagnostic) + { + this.GenerateFixture("RAIIFree", true, "CreateResource", "ConsumeResource"); + this.compilation = this.AddCode($$""" + using Windows.Win32; + static unsafe class Usage + { + static void Use(FreeSharedRESOURCE_ASafeHandle owner) { {{body}} } + static void PassLease(FreeSharedRESOURCE_ASafeHandle.LeaseScope lease) { } + } + """); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + var diagnostics = await this.AnalyzeAsync(); + diagnostics = diagnostics.Where(diagnostic => diagnostic.Location.SourceSpan.Start < this.compilation.SyntaxTrees.Last().GetText().ToString().IndexOf("static void PassLease", StringComparison.Ordinal)).ToImmutableArray(); + if (expectedDiagnostic is null) + { + Assert.Empty(diagnostics); + } + else + { + Assert.Contains(diagnostics, diagnostic => diagnostic.Id == expectedDiagnostic); + } + } + + /// + /// Verifies forbidden lease declarations even when the compiler permits them in ref structs or method signatures. + /// + /// The declaration that stores or transfers the lease. + [Theory] + [InlineData("ref struct Storage { internal FreeSharedRESOURCE_ASafeHandle.LeaseScope Field; }")] + [InlineData("static class Storage { static void Use(FreeSharedRESOURCE_ASafeHandle.LeaseScope lease) { } }")] + [InlineData("static class Storage { static FreeSharedRESOURCE_ASafeHandle.LeaseScope Get() => default; }")] + [InlineData("ref struct Storage { internal FreeSharedRESOURCE_ASafeHandle.LeaseScope Property { get; set; } }")] + public async Task LeaseStorageAndTransfersAreDiagnosed(string declaration) + { + this.GenerateFixture("RAIIFree", true, "CreateResource"); + this.compilation = this.AddCode("using Windows.Win32;\n" + declaration); + Assert.Contains(await this.AnalyzeAsync(), diagnostic => diagnostic.Id == "PInvoke015"); + } + + /// + /// Observes reference balance, acquisition failures, failure outputs, and allocation counts. + /// + [Fact] + public void LeaseAndFailureOutputRuntimeBehavior() + { + this.GenerateFixture("RAIIFree", true, "CreateResource", "AcquirePointer", "CreateUnsigned", "CreateUnsigned32"); + foreach (SyntaxTree tree in this.compilation.SyntaxTrees.ToArray()) + { + var methods = tree.GetRoot(TestContext.Current.CancellationToken).DescendantNodes().OfType() + .Where(method => method.Modifiers.Any(SyntaxKind.ExternKeyword) && method.Identifier.ValueText is "FreeShared" or "AcquirePointer" or "FreeUnsigned" or "FreeUnsigned32").ToArray(); + if (methods.Length == 0) + { + continue; + } + + SyntaxNode rewritten = tree.GetRoot(TestContext.Current.CancellationToken).ReplaceNodes(methods, (original, current) => + current.WithAttributeLists(default) + .WithModifiers(TokenList(current.Modifiers.Where(modifier => !modifier.IsKind(SyntaxKind.ExternKeyword)))) + .WithBody(ParseStatement(original.Identifier.ValueText switch + { + "FreeShared" => "{ TestBackend.Frees++; }", + "FreeUnsigned" => "{ if (resource.Value != unchecked((nuint)(nint)(-2))) throw new global::System.Exception(\"Lost unsigned bits during cleanup.\"); }", + "FreeUnsigned32" => "{ if (resource.Value != 0xfffffffeu) throw new global::System.Exception(\"Lost unsigned 32-bit value during cleanup.\"); }", + _ => "{ *resource = (void*)1; return -1; }", + }) as BlockSyntax) + .WithSemicolonToken(default)); + this.compilation = this.compilation.ReplaceSyntaxTree(tree, CSharpSyntaxTree.ParseText(rewritten.ToFullString(), this.parseOptions, tree.FilePath, cancellationToken: TestContext.Current.CancellationToken)); + } + + this.compilation = this.AddCode(""" + using System; + using Windows.Win32; + internal static class TestBackend { internal static int Frees; } + /// Executes the generated owners and leases. + public static unsafe class RuntimeUsage + { + /// Returns the allocation count for a warmed-up lease loop. + public static long Run() + { + TestBackend.Frees = 0; + var owner = new FreeSharedRESOURCE_ASafeHandle((IntPtr)1); + try + { + using var lease = owner.Lease(); + owner.Dispose(); + if (TestBackend.Frees != 0 || (nint)lease.Value.Value != 1) throw new Exception("Released while leased."); + throw new InvalidOperationException(); + } + catch (InvalidOperationException) { } + if (TestBackend.Frees != 1) throw new Exception("Unbalanced exceptional cleanup."); + bool rejected = false; + try { using var invalidLease = owner.Lease(); } + catch (ObjectDisposedException) { rejected = true; } + if (!rejected || TestBackend.Frees != 1) throw new Exception("Acquired a closed resource."); + + int result = PInvoke.AcquirePointer(out FreeSharedSafeHandle allocation); + if (result != -1 || (nint)allocation.Value != 1) throw new Exception("Lost failure output."); + allocation.Dispose(); + if (TestBackend.Frees != 2) throw new Exception("Did not release failure output."); + + using (var unsigned = new FreeUnsignedSafeHandle((IntPtr)(-2))) + { + using var lease = unsigned.Lease(); + if (lease.Value.Value != unchecked((nuint)(nint)(-2))) throw new Exception("Lost unsigned native value."); + } + + using (var unsigned = new FreeUnsigned32SafeHandle(unchecked((IntPtr)(nint)(nuint)0xfffffffeu))) + { + using var lease = unsigned.Lease(); + if (lease.Value.Value != 0xfffffffeu) throw new Exception("Lost unsigned 32-bit native value."); + } + + using var measuredOwner = new FreeSharedRESOURCE_ASafeHandle((IntPtr)1); + for (int i = 0; i < 100; i++) { using var lease = measuredOwner.Lease(); } + long before = GC.GetAllocatedBytesForCurrentThread(); + for (int i = 0; i < 1000; i++) + { + using var lease = measuredOwner.Lease(); + if ((nint)lease.Value.Value != 1) throw new Exception("Wrong native value."); + } + if (TestBackend.Frees != 2) throw new Exception("Lease disposed the owner."); + return GC.GetAllocatedBytesForCurrentThread() - before; + } + } + """); + using var stream = new MemoryStream(); + var emitted = this.compilation.Emit(stream, cancellationToken: TestContext.Current.CancellationToken); + Assert.True(emitted.Success, string.Join(Environment.NewLine, emitted.Diagnostics)); + Assembly assembly = Assembly.Load(stream.ToArray()); + Assert.Equal(0L, assembly.GetType("RuntimeUsage")!.GetMethod("Run")!.Invoke(null, null)); + Assert.Equal(3, assembly.GetType("TestBackend")!.GetField("Frees", BindingFlags.NonPublic | BindingFlags.Static)!.GetValue(null)); + } + + private Task> AnalyzeAsync() => this.compilation.WithAnalyzers( + ImmutableArray.Create(new SafeHandleAnalyzer())).GetAnalyzerDiagnosticsAsync(); + + private void GenerateFixture(string attribute, bool allowMarshaling, params string[] apis) => + this.GenerateFixture(attribute, allowMarshaling, apis, useSafeHandles: true); + + private void GenerateFixture(string attribute, bool allowMarshaling, string[] apis, bool useSafeHandles, bool preserveCompilation = false) + { + if (!preserveCompilation) + { + this.compilation = this.starterCompilations["net8.0"]; + } + + CSharpCompilation metadata = CSharpCompilation.Create( + "Windows.Win32", + [CSharpSyntaxTree.ParseText(MetadataSource.Replace("__Cleanup", attribute, StringComparison.Ordinal), this.parseOptions)], + this.starterCompilations["net8.0"].References, + this.starterCompilations["net8.0"].Options); + string metadataPath = Path.Combine(Path.GetTempPath(), $"CsWin32-Ownership-{Guid.NewGuid():N}.winmd"); + try + { + using (FileStream stream = File.Create(metadataPath)) + { + var result = metadata.Emit(stream); + Assert.True(result.Success, string.Join(Environment.NewLine, result.Diagnostics)); + } + + this.generator = new Generator(metadataPath, null, [], DefaultTestGeneratorOptions with { AllowMarshaling = allowMarshaling, UseSafeHandles = useSafeHandles }, this.compilation, this.parseOptions); + foreach (string api in apis) + { + Assert.True(this.generator.TryGenerate(api, CancellationToken.None)); + } + + this.CollectGeneratedCode(this.generator); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + } + finally + { + this.generator?.Dispose(); + this.generator = null; + File.Delete(metadataPath); + } + } +} From 2906a58c1e4b55d7bec9d9520b586495958886df Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Thu, 8 Oct 2026 14:09:39 -0700 Subject: [PATCH 2/6] Enforce lifetime errors and allow scoped string interpolation Recognize immediate interpolated string copies while rejecting delayed formatting that retains native arguments. Make all generated SafeHandle lifetime diagnostics errors by default. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 06478ace-c5d7-418a-9386-f31b4d379d93 --- docfx/docs/features.md | 9 ++- .../AnalyzerReleases.Unshipped.md | 8 +-- .../SafeHandleAnalyzer.cs | 28 ++++++-- .../SafeHandleLeaseTests.cs | 65 +++++++++++++++++++ 4 files changed, 99 insertions(+), 11 deletions(-) diff --git a/docfx/docs/features.md b/docfx/docs/features.md index 76b7b0d7..32b0381a 100644 --- a/docfx/docs/features.md +++ b/docfx/docs/features.md @@ -45,12 +45,15 @@ guarantee. Existing friendly input overloads accept base `SafeHandle` for recogn native handles and already keep a reference alive throughout their calls. Existing overloads mixing SafeHandles with native inputs remain available. -The lifetime analyzer warns about leases that are not `using` locals, copies, +The lifetime analyzer reports errors for leases that are not `using` locals, copies, field storage, parameter passing, returns, and explicit disposal (`PInvoke015`). -It also warns about `Value` without a visible scope (`PInvoke016`), saving or +It also reports errors for `Value` without a visible scope (`PInvoke016`), saving or returning a raw resource (`PInvoke017`), and releasing an owned resource directly (`PInvoke018`). Use `Value` directly in calls, or copy its contents into managed -data such as a string. C# ref structs remain copyable: keep these diagnostics +data such as a string. String interpolation such as `$"Path: {path.Value}"` is +allowed inside the owner's or lease's scope. Interpolation into `FormattableString` +or `IFormattable` retains the raw arguments for later formatting and is not a string +copy. C# ref structs remain copyable: keep these diagnostics enabled to enforce the supported no-copy convention. Neither a lease nor this analyzer proves that an arbitrary native call does not diff --git a/src/Microsoft.Windows.CsWin32/AnalyzerReleases.Unshipped.md b/src/Microsoft.Windows.CsWin32/AnalyzerReleases.Unshipped.md index 8a5e8ffd..05e0055a 100644 --- a/src/Microsoft.Windows.CsWin32/AnalyzerReleases.Unshipped.md +++ b/src/Microsoft.Windows.CsWin32/AnalyzerReleases.Unshipped.md @@ -19,7 +19,7 @@ PInvoke011 | Configuration | Error | SourceGenerator PInvoke012 | Configuration | Error | SourceGenerator PInvoke013 | Configuration | Error | SourceGenerator PInvoke014 | Configuration | Error | SourceGenerator -PInvoke015 | Lifetime | Warning | SafeHandleAnalyzer -PInvoke016 | Lifetime | Warning | SafeHandleAnalyzer -PInvoke017 | Lifetime | Warning | SafeHandleAnalyzer -PInvoke018 | Lifetime | Warning | SafeHandleAnalyzer +PInvoke015 | Lifetime | Error | SafeHandleAnalyzer +PInvoke016 | Lifetime | Error | SafeHandleAnalyzer +PInvoke017 | Lifetime | Error | SafeHandleAnalyzer +PInvoke018 | Lifetime | Error | SafeHandleAnalyzer diff --git a/src/Microsoft.Windows.CsWin32/SafeHandleAnalyzer.cs b/src/Microsoft.Windows.CsWin32/SafeHandleAnalyzer.cs index 1a207464..70a5df81 100644 --- a/src/Microsoft.Windows.CsWin32/SafeHandleAnalyzer.cs +++ b/src/Microsoft.Windows.CsWin32/SafeHandleAnalyzer.cs @@ -20,7 +20,7 @@ public sealed class SafeHandleAnalyzer : DiagnosticAnalyzer "Keep leases in using locals", "Create each lease with 'using var lease = owner.Lease()'; do not copy, store, pass, return, or explicitly dispose it", "Lifetime", - DiagnosticSeverity.Warning, + DiagnosticSeverity.Error, isEnabledByDefault: true); /// @@ -31,7 +31,7 @@ public sealed class SafeHandleAnalyzer : DiagnosticAnalyzer "Keep the native resource alive", "Read Value inside the owner's using scope, or acquire a lease in a using local", "Lifetime", - DiagnosticSeverity.Warning, + DiagnosticSeverity.Error, isEnabledByDefault: true); /// @@ -42,7 +42,7 @@ public sealed class SafeHandleAnalyzer : DiagnosticAnalyzer "Do not save or return raw resources", "Use Value directly in a call or copy its contents; do not save or return the raw resource", "Lifetime", - DiagnosticSeverity.Warning, + DiagnosticSeverity.Error, isEnabledByDefault: true); /// @@ -53,7 +53,7 @@ public sealed class SafeHandleAnalyzer : DiagnosticAnalyzer "Do not release an owned resource", "Do not pass Value to '{0}'; dispose the SafeHandle instead", "Lifetime", - DiagnosticSeverity.Warning, + DiagnosticSeverity.Error, isEnabledByDefault: true); /// @@ -230,6 +230,25 @@ private static bool IsReleaseImplementation(IOperation? instance, ISymbol contai instance is IInstanceReferenceOperation { ReferenceKind: InstanceReferenceKind.ContainingTypeInstance } && containingSymbol is IMethodSymbol { Name: "ReleaseHandle", IsOverride: true, Parameters.Length: 0 }; + private static bool IsImmediateStringInterpolation(IOperation use) + { + if (use.Parent is not IInterpolationOperation { Parent: IInterpolatedStringOperation interpolation } + || interpolation.Type?.SpecialType != SpecialType.System_String) + { + return false; + } + + IOperation result = interpolation; + while (result.Parent is IParenthesizedOperation parenthesized) + { + result = parenthesized; + } + + // FormattableString and IFormattable keep the arguments for later formatting, not a string copy. + return result.Parent is not IConversionOperation conversion + || conversion.Type?.ToDisplayString() is not ("System.FormattableString" or "System.IFormattable"); + } + private static void AnalyzeValue(OperationAnalysisContext context) { var property = (IPropertyReferenceOperation)context.Operation; @@ -280,6 +299,7 @@ private static void AnalyzeValue(OperationAnalysisContext context) } if ((use.Parent is IInvocationOperation { Type.SpecialType: SpecialType.System_String } invocation && invocation.Instance == use) + || IsImmediateStringInterpolation(use) || use.Parent is IBinaryOperation { Type.SpecialType: SpecialType.System_Boolean } || use.Parent is IIsPatternOperation) { diff --git a/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs index c2b8fce6..cae13e1a 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs @@ -185,6 +185,70 @@ static void Get(System.Guid folder) Assert.Empty(await this.AnalyzeAsync()); } + /// + /// Verifies that interpolating an owned path into a string copies its contents within the lifetime scope. + /// + /// The caller's target framework. + /// The caller's formatting code. + /// The expected diagnostic, or null for a supported pattern. + [Theory] + [InlineData("net472", "using (path) { WriteLine($\"Path: {path.Value}\"); }", null)] + [InlineData("net8.0", "using (path) { WriteLine($\"Path: {path.Value}\"); }", null)] + [InlineData("net472", "using var lease = path.Lease(); WriteLine($\"Path: {lease.Value,20}\");", null)] + [InlineData("net8.0", "using var lease = path.Lease(); WriteLine($\"Path: {lease.Value,20}\");", null)] + [InlineData("net472", "string text; using (path) { text = $\"Path: {path.Value}\"; } WriteLine(text);", null)] + [InlineData("net8.0", "string text; using (path) { text = $\"Path: {path.Value}\"; } WriteLine(text);", null)] + [InlineData("net472", "WriteLine($\"Path: {path.Value}\");", "PInvoke016")] + [InlineData("net8.0", "WriteLine($\"Path: {path.Value}\");", "PInvoke016")] + [InlineData("net472", "System.FormattableString text; using (path) { text = $\"Path: {path.Value}\"; } WriteLine(text.ToString());", "PInvoke017")] + [InlineData("net8.0", "System.FormattableString text; using (path) { text = $\"Path: {path.Value}\"; } WriteLine(text.ToString());", "PInvoke017")] + [InlineData("net472", "System.IFormattable text; using (path) { text = $\"Path: {path.Value}\"; } WriteLine(text.ToString(null, null));", "PInvoke017")] + [InlineData("net8.0", "System.IFormattable text; using (path) { text = $\"Path: {path.Value}\"; } WriteLine(text.ToString(null, null));", "PInvoke017")] + public async Task KnownFolderPathFormattingDiagnostics(string framework, string body, string? expectedDiagnostic) + { + this.compilation = this.starterCompilations[framework]; + this.generator = this.CreateGenerator(); + Assert.True(this.generator.TryGenerate("SHGetKnownFolderPath", CancellationToken.None)); + this.CollectGeneratedCode(this.generator); + this.compilation = this.AddCode($$""" + using Windows.Win32; + static class Usage + { + static void Use() + { + PInvoke.SHGetKnownFolderPath(default, 0, null, out var path); + {{body}} + } + static void WriteLine(string text) { } + } + """); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + ImmutableArray diagnostics = await this.AnalyzeAsync(); + if (expectedDiagnostic is null) + { + Assert.Empty(diagnostics); + } + else + { + Diagnostic diagnostic = Assert.Single(diagnostics); + Assert.Equal(expectedDiagnostic, diagnostic.Id); + Assert.Equal(DiagnosticSeverity.Error, diagnostic.Severity); + } + } + + /// + /// Verifies that the lifetime contract is enforced with errors by default. + /// + [Fact] + public void LifetimeDiagnosticsAreErrorsByDefault() + { + Assert.All(new SafeHandleAnalyzer().SupportedDiagnostics, descriptor => + { + Assert.True(descriptor.IsEnabledByDefault); + Assert.Equal(DiagnosticSeverity.Error, descriptor.DefaultSeverity); + }); + } + /// /// Verifies that incompatible opaque handle cleanup retains the raw API rather than an unsafe owner. /// @@ -401,6 +465,7 @@ static void PassLease(FreeSharedRESOURCE_ASafeHandle.LeaseScope lease) { } else { Assert.Contains(diagnostics, diagnostic => diagnostic.Id == expectedDiagnostic); + Assert.All(diagnostics, diagnostic => Assert.Equal(DiagnosticSeverity.Error, diagnostic.Severity)); } } From 69a9f2c20eed1d00721f73d15ff23a77293fc2e0 Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Thu, 8 Oct 2026 14:48:39 -0700 Subject: [PATCH 3/6] Distinguish owner DangerousValue from lease Value Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 06478ace-c5d7-418a-9386-f31b4d379d93 --- docfx/docs/features.md | 20 ++-- .../Generator.SafeHandleLease.cs | 9 +- .../SafeHandleAnalyzer.cs | 14 +-- .../templates/SafeHandleLease.cs | 2 +- .../SafeHandleLeaseTests.cs | 95 ++++++++++++++----- 5 files changed, 98 insertions(+), 42 deletions(-) diff --git a/docfx/docs/features.md b/docfx/docs/features.md index 32b0381a..857757f7 100644 --- a/docfx/docs/features.md +++ b/docfx/docs/features.md @@ -15,9 +15,10 @@ tell CsWin32 which cleanup function owns that particular result. Friendly overlo can return a SafeHandle for pointer-shaped allocations as well as native handles. Unannotated pointers and the raw native signatures are unchanged. -CsWin32-generated SafeHandles expose a `Value` property with the original native type. +CsWin32-generated SafeHandles expose a `DangerousValue` property with the original native type. For example, `SHGetKnownFolderPath` supplies a `CoTaskMemFreePWSTRSafeHandle` whose -`Value` is `PWSTR`, although `CoTaskMemFree` accepts `void*`. BCL wrappers such as +`DangerousValue` is `PWSTR`, although `CoTaskMemFree` accepts `void*`. Reading this +property does not acquire a reference to keep the resource alive. BCL wrappers such as `SafeFileHandle` retain their existing APIs. For an owner received from another component or as a parameter, acquire a scoped reference: @@ -38,7 +39,7 @@ lease ends. The lease's `Value` has the same native type as its owner. The lease uses the `Dispose()` pattern; it does not require `IDisposable`. Lease helpers require C# 9 or later. -An owner already held in a visible `using` scope can supply `owner.Value` directly. +An owner already held in a visible `using` scope can supply `owner.DangerousValue` directly. This assumes ordinary exclusive ownership: another alias must not explicitly dispose it during the call. A SafeHandle parameter alone is not such a lifetime guarantee. Existing friendly input overloads accept base `SafeHandle` for recognized @@ -47,11 +48,12 @@ Existing overloads mixing SafeHandles with native inputs remain available. The lifetime analyzer reports errors for leases that are not `using` locals, copies, field storage, parameter passing, returns, and explicit disposal (`PInvoke015`). -It also reports errors for `Value` without a visible scope (`PInvoke016`), saving or -returning a raw resource (`PInvoke017`), and releasing an owned resource directly -(`PInvoke018`). Use `Value` directly in calls, or copy its contents into managed -data such as a string. String interpolation such as `$"Path: {path.Value}"` is -allowed inside the owner's or lease's scope. Interpolation into `FormattableString` +It also reports errors for `DangerousValue` or a lease's `Value` without a visible +scope (`PInvoke016`), saving or returning a raw resource (`PInvoke017`), and releasing +an owned resource directly (`PInvoke018`). Use these accessors directly in calls, +or copy their contents into managed data such as a string. String interpolation +such as `$"Path: {path.DangerousValue}"` is allowed inside the owner's or lease's +scope. Interpolation into `FormattableString` or `IFormattable` retains the raw arguments for later formatting and is not a string copy. C# ref structs remain copyable: keep these diagnostics enabled to enforce the supported no-copy convention. @@ -63,5 +65,5 @@ APIs need their own explicit lifetime arrangement. For cleanup requiring additional arguments, request a helper such as `DeleteTimerQueueTimerSafeHandle` in `NativeMethods.txt`. It is abstract: derive from it, keep the necessary cleanup context, and override `ReleaseHandle`. -Its typed `Value` is available inside that override. CsWin32 will not construct +Its typed `DangerousValue` is available inside that override. CsWin32 will not construct this helper automatically or guess the additional cleanup arguments. diff --git a/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs b/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs index 40bb1bb8..79bcc6bc 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs @@ -128,16 +128,19 @@ AttributeSyntax ValueAttribute(bool lease = false) => Attribute(ParseName($"glob AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(releaseMethod))), AttributeArgument(LiteralExpression(lease ? SyntaxKind.TrueLiteralExpression : SyntaxKind.FalseLiteralExpression)))); - PropertyDeclarationSyntax value = PropertyDeclaration(valueType.WithTrailingTrivia(Space), "Value") + PropertyDeclarationSyntax value = PropertyDeclaration(valueType.WithTrailingTrivia(Space), "DangerousValue") .AddModifiers(TokenWithSpace(this.Visibility)) .AddAttributeLists(AttributeList(ValueAttribute())) .WithExpressionBody(ArrowExpressionClause(nativeValue)) .WithSemicolonToken(SemicolonWithLineFeed) .WithLeadingTrivia(ParseLeadingTrivia(""" /// - /// Gets the native resource. Use it only within an owner's using scope or obtain a lease. + /// Gets the native resource without acquiring a reference to keep it alive. /// - /// Do not save or return this value, or use it to release the resource. + /// + /// Use this property only within the owner's using scope, or acquire a lease and use its Value instead. + /// Do not save or return the native value, or use it to release the resource. + /// """)); if (hidesBase) diff --git a/src/Microsoft.Windows.CsWin32/SafeHandleAnalyzer.cs b/src/Microsoft.Windows.CsWin32/SafeHandleAnalyzer.cs index 70a5df81..108a1beb 100644 --- a/src/Microsoft.Windows.CsWin32/SafeHandleAnalyzer.cs +++ b/src/Microsoft.Windows.CsWin32/SafeHandleAnalyzer.cs @@ -29,7 +29,7 @@ public sealed class SafeHandleAnalyzer : DiagnosticAnalyzer public static readonly DiagnosticDescriptor ValueNeedsScope = new( "PInvoke016", "Keep the native resource alive", - "Read Value inside the owner's using scope, or acquire a lease in a using local", + "Read {0} inside the owner's using scope, or acquire a lease in a using local", "Lifetime", DiagnosticSeverity.Error, isEnabledByDefault: true); @@ -40,7 +40,7 @@ public sealed class SafeHandleAnalyzer : DiagnosticAnalyzer public static readonly DiagnosticDescriptor ValueMustNotEscape = new( "PInvoke017", "Do not save or return raw resources", - "Use Value directly in a call or copy its contents; do not save or return the raw resource", + "Use {0} directly in a call or copy its contents; do not save or return the raw resource", "Lifetime", DiagnosticSeverity.Error, isEnabledByDefault: true); @@ -51,7 +51,7 @@ public sealed class SafeHandleAnalyzer : DiagnosticAnalyzer public static readonly DiagnosticDescriptor ValueMustNotRelease = new( "PInvoke018", "Do not release an owned resource", - "Do not pass Value to '{0}'; dispose the SafeHandle instead", + "Do not pass {0} to '{1}'; dispose the SafeHandle instead", "Lifetime", DiagnosticSeverity.Error, isEnabledByDefault: true); @@ -260,7 +260,7 @@ private static void AnalyzeValue(OperationAnalysisContext context) bool releasing = IsReleaseImplementation(property.Instance, context.ContainingSymbol); if (!releasing && (property.SemanticModel is not { } semanticModel || !HasScope(property.Instance, property.Syntax, semanticModel, context.CancellationToken))) { - context.ReportDiagnostic(Diagnostic.Create(ValueNeedsScope, property.Syntax.GetLocation())); + context.ReportDiagnostic(Diagnostic.Create(ValueNeedsScope, property.Syntax.GetLocation(), property.Property.Name)); } IOperation use = property; @@ -286,13 +286,13 @@ private static void AnalyzeValue(OperationAnalysisContext context) { if (argument.Parameter?.RefKind is RefKind.Ref or RefKind.Out) { - context.ReportDiagnostic(Diagnostic.Create(ValueMustNotEscape, use.Syntax.GetLocation())); + context.ReportDiagnostic(Diagnostic.Create(ValueMustNotEscape, use.Syntax.GetLocation(), property.Property.Name)); } string? releaseMethod = ownership.ConstructorArguments[0].Value as string; if (!releasing && (call.TargetMethod.Name == releaseMethod || call.TargetMethod.GetDllImportData()?.EntryPointName == releaseMethod)) { - context.ReportDiagnostic(Diagnostic.Create(ValueMustNotRelease, use.Syntax.GetLocation(), releaseMethod)); + context.ReportDiagnostic(Diagnostic.Create(ValueMustNotRelease, use.Syntax.GetLocation(), property.Property.Name, releaseMethod)); } return; @@ -306,6 +306,6 @@ private static void AnalyzeValue(OperationAnalysisContext context) return; } - context.ReportDiagnostic(Diagnostic.Create(ValueMustNotEscape, use.Syntax.GetLocation())); + context.ReportDiagnostic(Diagnostic.Create(ValueMustNotEscape, use.Syntax.GetLocation(), property.Property.Name)); } } diff --git a/src/Microsoft.Windows.CsWin32/templates/SafeHandleLease.cs b/src/Microsoft.Windows.CsWin32/templates/SafeHandleLease.cs index 91996341..dfa2460b 100644 --- a/src/Microsoft.Windows.CsWin32/templates/SafeHandleLease.cs +++ b/src/Microsoft.Windows.CsWin32/templates/SafeHandleLease.cs @@ -42,7 +42,7 @@ internal LeaseScope(__OwnerType owner) /// /// The lease is not active. public readonly __NativeType Value => this.owner is { } owner - ? owner.Value + ? owner.DangerousValue : throw new global::System.ObjectDisposedException(nameof(LeaseScope)); /// diff --git a/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs index cae13e1a..4ea79666 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs @@ -149,6 +149,17 @@ static void Use(FreeSharedRESOURCE_ASafeHandle a, FreeSharedRESOURCE_BSafeHandle this.AssertNoDiagnostics(logAllGeneratedCode: false); Assert.Contains(this.FindGeneratedMethod("ConsumeResource"), method => method.ParameterList.Parameters[0].Type?.ToString() == "SafeHandle"); Assert.DoesNotContain(this.FindGeneratedMethod("ConsumePointer"), method => method.ParameterList.Parameters.Any(parameter => parameter.Type?.ToString() == "SafeHandle")); + foreach (string name in new[] { "FreeSharedSafeHandle", "FreeSharedRESOURCE_ASafeHandle", "FreeSharedRESOURCE_BSafeHandle" }) + { + INamedTypeSymbol? owner = this.compilation.GetTypeByMetadataName("Windows.Win32." + name); + Assert.NotNull(owner); + IPropertySymbol ownerValue = Assert.IsAssignableFrom(Assert.Single(owner.GetMembers("DangerousValue"))); + INamedTypeSymbol lease = Assert.Single(owner.GetTypeMembers("LeaseScope")); + IPropertySymbol leaseValue = Assert.IsAssignableFrom(Assert.Single(lease.GetMembers("Value"))); + Assert.True(SymbolEqualityComparer.Default.Equals(ownerValue.Type, leaseValue.Type)); + Assert.Empty(owner.GetMembers("Value")); + Assert.Empty(lease.GetMembers("DangerousValue")); + } } /// @@ -176,7 +187,7 @@ static void Get(System.Guid folder) PInvoke.SHGetKnownFolderPath(folder, 0, null, out CoTaskMemFreePWSTRSafeHandle path); using (path) { - PInvoke.PathFileExists(path.Value); + PInvoke.PathFileExists(path.DangerousValue); } } } @@ -192,18 +203,18 @@ static void Get(System.Guid folder) /// The caller's formatting code. /// The expected diagnostic, or null for a supported pattern. [Theory] - [InlineData("net472", "using (path) { WriteLine($\"Path: {path.Value}\"); }", null)] - [InlineData("net8.0", "using (path) { WriteLine($\"Path: {path.Value}\"); }", null)] + [InlineData("net472", "using (path) { WriteLine($\"Path: {path.DangerousValue}\"); }", null)] + [InlineData("net8.0", "using (path) { WriteLine($\"Path: {path.DangerousValue}\"); }", null)] [InlineData("net472", "using var lease = path.Lease(); WriteLine($\"Path: {lease.Value,20}\");", null)] [InlineData("net8.0", "using var lease = path.Lease(); WriteLine($\"Path: {lease.Value,20}\");", null)] - [InlineData("net472", "string text; using (path) { text = $\"Path: {path.Value}\"; } WriteLine(text);", null)] - [InlineData("net8.0", "string text; using (path) { text = $\"Path: {path.Value}\"; } WriteLine(text);", null)] - [InlineData("net472", "WriteLine($\"Path: {path.Value}\");", "PInvoke016")] - [InlineData("net8.0", "WriteLine($\"Path: {path.Value}\");", "PInvoke016")] - [InlineData("net472", "System.FormattableString text; using (path) { text = $\"Path: {path.Value}\"; } WriteLine(text.ToString());", "PInvoke017")] - [InlineData("net8.0", "System.FormattableString text; using (path) { text = $\"Path: {path.Value}\"; } WriteLine(text.ToString());", "PInvoke017")] - [InlineData("net472", "System.IFormattable text; using (path) { text = $\"Path: {path.Value}\"; } WriteLine(text.ToString(null, null));", "PInvoke017")] - [InlineData("net8.0", "System.IFormattable text; using (path) { text = $\"Path: {path.Value}\"; } WriteLine(text.ToString(null, null));", "PInvoke017")] + [InlineData("net472", "string text; using (path) { text = $\"Path: {path.DangerousValue}\"; } WriteLine(text);", null)] + [InlineData("net8.0", "string text; using (path) { text = $\"Path: {path.DangerousValue}\"; } WriteLine(text);", null)] + [InlineData("net472", "WriteLine($\"Path: {path.DangerousValue}\");", "PInvoke016")] + [InlineData("net8.0", "WriteLine($\"Path: {path.DangerousValue}\");", "PInvoke016")] + [InlineData("net472", "System.FormattableString text; using (path) { text = $\"Path: {path.DangerousValue}\"; } WriteLine(text.ToString());", "PInvoke017")] + [InlineData("net8.0", "System.FormattableString text; using (path) { text = $\"Path: {path.DangerousValue}\"; } WriteLine(text.ToString());", "PInvoke017")] + [InlineData("net472", "System.IFormattable text; using (path) { text = $\"Path: {path.DangerousValue}\"; } WriteLine(text.ToString(null, null));", "PInvoke017")] + [InlineData("net8.0", "System.IFormattable text; using (path) { text = $\"Path: {path.DangerousValue}\"; } WriteLine(text.ToString(null, null));", "PInvoke017")] public async Task KnownFolderPathFormattingDiagnostics(string framework, string body, string? expectedDiagnostic) { this.compilation = this.starterCompilations[framework]; @@ -292,7 +303,7 @@ sealed class Owner : FreeContextSafeHandle { private readonly int context; internal Owner(System.IntPtr value, int context) : base(value) { this.context = context; } - protected override bool ReleaseHandle() => PInvoke.FreeContext(this.Value, this.context); + protected override bool ReleaseHandle() => PInvoke.FreeContext(this.DangerousValue, this.context); } """); this.AssertNoDiagnostics(logAllGeneratedCode: false); @@ -311,7 +322,7 @@ public void TimerQueueCleanupHelperCanBeRequested() var type = this.compilation.GetTypeByMetadataName("Windows.Win32.DeleteTimerQueueTimerSafeHandle"); Assert.NotNull(type); Assert.True(type.IsAbstract); - Assert.Equal("HANDLE", Assert.IsAssignableFrom(Assert.Single(type.GetMembers("Value"))).Type.Name); + Assert.Equal("HANDLE", Assert.IsAssignableFrom(Assert.Single(type.GetMembers("DangerousValue"))).Type.Name); } /// @@ -341,8 +352,8 @@ static void Use() { using var a = PInvoke.CreateDuplicate_SafeHandle(); using var b = PInvoke.CreateOtherDuplicate_SafeHandle(); - Windows.Win32.Test.DUPLICATE_RESOURCE valueA = a.Value; - Windows.Win32.Other.DUPLICATE_RESOURCE valueB = b.Value; + Windows.Win32.Test.DUPLICATE_RESOURCE valueA = a.DangerousValue; + Windows.Win32.Other.DUPLICATE_RESOURCE valueB = b.DangerousValue; } } """); @@ -376,7 +387,7 @@ static class Usage static void Use() { using var owner = PInvoke.CreateResource_SafeHandle(); - Windows.Win32.Test.RESOURCE_A value = owner.Value; + Windows.Win32.Test.RESOURCE_A value = owner.DangerousValue; } } """); @@ -429,21 +440,23 @@ public void NullableEnabledProjectsCompile() [Theory] [InlineData("using var lease = owner.Lease(); PInvoke.ConsumeResource(lease.Value);", null)] [InlineData("using (var lease = owner.Lease()) { PInvoke.ConsumeResource(lease.Value); }", null)] - [InlineData("using (owner) { PInvoke.ConsumeResource(owner.Value); }", null)] - [InlineData("using var local = owner; PInvoke.ConsumeResource(local.Value);", null)] + [InlineData("using (owner) { PInvoke.ConsumeResource(owner.DangerousValue); }", null)] + [InlineData("using var local = owner; PInvoke.ConsumeResource(local.DangerousValue);", null)] [InlineData("using var lease = owner.Lease(); string text = lease.Value.ToString();", null)] [InlineData("using var lease = owner.Lease(); bool empty = lease.Value.IsNull;", null)] - [InlineData("_ = nameof(owner.Value);", null)] + [InlineData("_ = nameof(owner.DangerousValue);", null)] [InlineData("using var lease = owner.Lease(); _ = nameof(lease.Value.Value);", null)] - [InlineData("PInvoke.ConsumeResource(owner.Value);", "PInvoke016")] + [InlineData("PInvoke.ConsumeResource(owner.DangerousValue);", "PInvoke016")] [InlineData("var lease = owner.Lease(); PInvoke.ConsumeResource(lease.Value);", "PInvoke015")] [InlineData("PInvoke.ConsumeResource(owner.Lease().Value);", "PInvoke015")] [InlineData("using var lease = owner.Lease(); var copy = lease;", "PInvoke015")] [InlineData("using var lease = owner.Lease(); lease.Dispose();", "PInvoke015")] [InlineData("using var lease = owner.Lease(); PassLease(lease);", "PInvoke015")] [InlineData("using var lease = owner.Lease(); var saved = lease.Value;", "PInvoke017")] - [InlineData("using (owner) { System.Action action = () => PInvoke.ConsumeResource(owner.Value); }", "PInvoke016")] + [InlineData("using (owner) { var saved = owner.DangerousValue; }", "PInvoke017")] + [InlineData("using (owner) { System.Action action = () => PInvoke.ConsumeResource(owner.DangerousValue); }", "PInvoke016")] [InlineData("using var lease = owner.Lease(); PInvoke.FreeShared(lease.Value.Value);", "PInvoke018")] + [InlineData("using (owner) { PInvoke.FreeShared(owner.DangerousValue.Value); }", "PInvoke018")] public async Task LifetimeUsageDiagnostics(string body, string? expectedDiagnostic) { this.GenerateFixture("RAIIFree", true, "CreateResource", "ConsumeResource"); @@ -469,6 +482,44 @@ static void PassLease(FreeSharedRESOURCE_ASafeHandle.LeaseScope lease) { } } } + /// + /// Verifies that lifetime errors identify the owner's or lease's actual accessor. + /// + [Fact] + public async Task LifetimeDiagnosticsIdentifyAccessor() + { + this.GenerateFixture("RAIIFree", true, "CreateResource", "ConsumeResource"); + this.compilation = this.AddCode(""" + using Windows.Win32; + static unsafe class Usage + { + static void Use(FreeSharedRESOURCE_ASafeHandle owner) + { + PInvoke.ConsumeResource(owner.DangerousValue); + using (owner) + { + var savedOwner = owner.DangerousValue; + PInvoke.FreeShared(owner.DangerousValue.Value); + using var lease = owner.Lease(); + var savedLease = lease.Value; + PInvoke.FreeShared(lease.Value.Value); + } + } + } + """); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + var diagnostics = await this.AnalyzeAsync(); + Assert.Equal( + [ + "Read DangerousValue inside the owner's using scope, or acquire a lease in a using local", + "Use DangerousValue directly in a call or copy its contents; do not save or return the raw resource", + "Do not pass DangerousValue to 'FreeShared'; dispose the SafeHandle instead", + "Use Value directly in a call or copy its contents; do not save or return the raw resource", + "Do not pass Value to 'FreeShared'; dispose the SafeHandle instead", + ], + diagnostics.OrderBy(diagnostic => diagnostic.Location.SourceSpan.Start).Select(diagnostic => diagnostic.GetMessage()).ToArray()); + } + /// /// Verifies forbidden lease declarations even when the compiler permits them in ref structs or method signatures. /// @@ -542,7 +593,7 @@ public static long Run() if (!rejected || TestBackend.Frees != 1) throw new Exception("Acquired a closed resource."); int result = PInvoke.AcquirePointer(out FreeSharedSafeHandle allocation); - if (result != -1 || (nint)allocation.Value != 1) throw new Exception("Lost failure output."); + if (result != -1 || (nint)allocation.DangerousValue != 1) throw new Exception("Lost failure output."); allocation.Dispose(); if (TestBackend.Frees != 2) throw new Exception("Did not release failure output."); From 6e85ab5a0cc3d37c8c77867f58e91111067912ff Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Thu, 8 Oct 2026 16:05:54 -0700 Subject: [PATCH 4/6] Limit SafeHandle input inference to HANDLE Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 06478ace-c5d7-418a-9386-f31b4d379d93 --- docfx/docs/features.md | 11 +++- .../Generator.FriendlyOverloads.cs | 55 ++++++------------- .../templates/NativeOwnershipAttribute.cs | 9 ++- .../CsWin32GeneratorTests.cs | 2 +- .../FriendlyOverloadTests.cs | 4 +- .../SafeHandleLeaseTests.cs | 47 +++++++++++++++- 6 files changed, 80 insertions(+), 48 deletions(-) diff --git a/docfx/docs/features.md b/docfx/docs/features.md index 857757f7..560ba889 100644 --- a/docfx/docs/features.md +++ b/docfx/docs/features.md @@ -42,9 +42,14 @@ Lease helpers require C# 9 or later. An owner already held in a visible `using` scope can supply `owner.DangerousValue` directly. This assumes ordinary exclusive ownership: another alias must not explicitly dispose it during the call. A SafeHandle parameter alone is not such a lifetime -guarantee. Existing friendly input overloads accept base `SafeHandle` for recognized -native handles and already keep a reference alive throughout their calls. -Existing overloads mixing SafeHandles with native inputs remain available. +guarantee. Friendly input overloads accept base `SafeHandle` for types with existing +cleanup annotations and keep a reference alive throughout their calls. They also +accept `SafeHandle` for `Windows.Win32.Foundation.HANDLE` even without a type-level +cleanup annotation, preserving this behavior as ownership annotations move to outputs. +This heuristic does not apply to other types such as `HWND`, `PWSTR`, or arbitrary +pointers. Such resources can be passed using their owner's `DangerousValue` or a +lease's `Value`. Set `useSafeHandles` to `false` in `NativeMethods.json` to disable +SafeHandle projection. The lifetime analyzer reports errors for leases that are not `using` locals, copies, field storage, parameter passing, returns, and explicit disposal (`PInvoke015`). diff --git a/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs b/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs index b8571d5a..0cbee606 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs @@ -105,35 +105,18 @@ private IEnumerable DeclareFriendlyOverloads(MethodDefi } bool improvePointersToSpansAndRefs = this.canUseSpan; - HashSet signatures = new(StringComparer.Ordinal); - for (int pass = 0; pass < 2; pass++) + FriendlyMethodBookkeeping bookkeeping = new(); + foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs, omitOptionalParams: false, bookkeeping)) { - bool legacyHandleInputs = pass == 1; - FriendlyMethodBookkeeping bookkeeping = new(); - foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs, omitOptionalParams: false, bookkeeping, legacyHandleInputs)) - { - if (signatures.Add(method.Identifier.ValueText + method.TypeParameterList + method.ParameterList)) - { - yield return method; - } - } - - if (this.Options.FriendlyOverloads.IncludePointerOverloads && improvePointersToSpansAndRefs && bookkeeping.NumSpanByteParameters > 0) - { - // Also offer pointer-based overloads for byte buffers. - foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs: false, omitOptionalParams: false, legacyHandleInputs: legacyHandleInputs)) - { - if (signatures.Add(method.Identifier.ValueText + method.TypeParameterList + method.ParameterList)) - { - yield return method; - } - } - } + yield return method; + } - // Preserve signatures mixing existing SafeHandle inputs with native values for newly recognized inputs. - if (!bookkeeping.HasNewHandleInputs) + if (this.Options.FriendlyOverloads.IncludePointerOverloads && improvePointersToSpansAndRefs && bookkeeping.NumSpanByteParameters > 0) + { + // Also offer pointer-based overloads for byte buffers. + foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs: false, omitOptionalParams: false)) { - break; + yield return method; } } } @@ -147,8 +130,7 @@ private IEnumerable DeclareFriendlyOverload( bool avoidWinmdRootAlias, bool improvePointersToSpansAndRefs, bool omitOptionalParams, - FriendlyMethodBookkeeping? bookkeeping = null, - bool legacyHandleInputs = false) + FriendlyMethodBookkeeping? bookkeeping = null) { #pragma warning disable SA1114 // Parameter list should follow declaration bool isReleaseMethod = this.MetadataIndex.ReleaseMethods.Contains(externMethodDeclaration.Identifier.ValueText); @@ -674,14 +656,11 @@ private IEnumerable DeclareFriendlyOverload( } else if (this.options.UseSafeHandles && isIn && !isOut && !isReleaseMethod && parameterTypeInfo is HandleTypeHandleInfo parameterHandleTypeInfo && (parameterHandleTypeInfo.Generator.TryGetHandleReleaseMethod(parameterHandleTypeInfo.Handle, paramAttributes, out _) - || (!legacyHandleInputs && parameterHandleTypeInfo.Generator.IsHandle(parameterHandleTypeInfo.Handle, out _))) + || (parameterHandleTypeInfo.Generator.GetQualifiedTypeDefinition(parameterHandleTypeInfo.Handle) is QualifiedTypeDefinition inputHandleDefinition + && inputHandleDefinition.Reader.StringComparer.Equals(inputHandleDefinition.Definition.Namespace, "Windows.Win32.Foundation") + && inputHandleDefinition.Reader.StringComparer.Equals(inputHandleDefinition.Definition.Name, "HANDLE"))) && !(this.TryGetTypeDefFieldType(parameterHandleTypeInfo, out TypeHandleInfo? fieldType) && !this.IsSafeHandleCompatibleTypeDefFieldType(fieldType))) { - if (bookkeeping is not null && !parameterHandleTypeInfo.Generator.TryGetHandleReleaseMethod(parameterHandleTypeInfo.Handle, paramAttributes, out _)) - { - bookkeeping.HasNewHandleInputs = true; - } - IdentifierNameSyntax typeDefHandleName = IdentifierName(externParam.Identifier.ValueText + "Local"); signatureChanged = true; @@ -1628,10 +1607,10 @@ bool TryHandleCountParam(TypeSyntax elementType, bool nullableSource) } // If we're using C# 13 or later, consider adding the overload resolution attribute if it would likely resolve ambiguities. - if (this.LanguageVersion >= (LanguageVersion)1300 && (legacyHandleInputs || parameters.Count == externMethodDeclaration.ParameterList.Parameters.Count)) + if (this.LanguageVersion >= (LanguageVersion)1300 && parameters.Count == externMethodDeclaration.ParameterList.Parameters.Count) { this.volatileCode.GenerationTransaction(() => this.DeclareOverloadResolutionPriorityAttributeIfNecessary()); - friendlyDeclaration = friendlyDeclaration.AddAttributeLists(AttributeList(OverloadResolutionPriorityAttribute(legacyHandleInputs ? 2 : 1))); + friendlyDeclaration = friendlyDeclaration.AddAttributeLists(AttributeList(OverloadResolutionPriorityAttribute(1))); } friendlyDeclaration = friendlyDeclaration @@ -1660,7 +1639,7 @@ bool TryHandleCountParam(TypeSyntax elementType, bool nullableSource) if (numOptionalParams > 0 && !omitOptionalParams && improvePointersToSpansAndRefs) { // Generate overloads for optional parameters. - foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs, omitOptionalParams: true, legacyHandleInputs: legacyHandleInputs)) + foreach (MethodDeclarationSyntax method in this.DeclareFriendlyOverload(methodDefinition, externMethodDeclaration, declaringTypeName, overloadOf, helperMethodsAdded, avoidWinmdRootAlias, improvePointersToSpansAndRefs, omitOptionalParams: true)) { yield return method; } @@ -1832,7 +1811,5 @@ private StatementSyntax COMFreeNativePointerStatement(ExpressionSyntax nativePoi private class FriendlyMethodBookkeeping { public int NumSpanByteParameters { get; set; } = 0; - - public bool HasNewHandleInputs { get; set; } } } diff --git a/src/Microsoft.Windows.CsWin32/templates/NativeOwnershipAttribute.cs b/src/Microsoft.Windows.CsWin32/templates/NativeOwnershipAttribute.cs index 37bcbcb1..3c9adf43 100644 --- a/src/Microsoft.Windows.CsWin32/templates/NativeOwnershipAttribute.cs +++ b/src/Microsoft.Windows.CsWin32/templates/NativeOwnershipAttribute.cs @@ -4,13 +4,20 @@ /// /// Describes generated resource access for the CsWin32 lifetime analyzer. /// +/// +/// A valid lifetime scope does not make manually freeing the resource safe. A lease +/// prevents its SafeHandle from releasing the resource, but cannot prevent a native +/// cleanup call from freeing it directly. The analyzer uses the cleanup function's +/// identity to reject such calls even within a valid owner or lease scope. +/// This attribute provides analysis metadata; it does not acquire references or perform cleanup. +/// [global::System.AttributeUsage(global::System.AttributeTargets.Property | global::System.AttributeTargets.Struct)] internal sealed class NativeOwnershipAttribute : global::System.Attribute { /// /// Initializes a new instance of the class. /// - /// The native cleanup function. + /// The native cleanup function callers must not invoke with the owned value. /// Whether the annotated type represents a scoped reference. public NativeOwnershipAttribute(string releaseMethod, bool isLease = false) { diff --git a/test/CsWin32Generator.Tests/CsWin32GeneratorTests.cs b/test/CsWin32Generator.Tests/CsWin32GeneratorTests.cs index ae1cf83d..eeef1a6f 100644 --- a/test/CsWin32Generator.Tests/CsWin32GeneratorTests.cs +++ b/test/CsWin32Generator.Tests/CsWin32GeneratorTests.cs @@ -270,7 +270,7 @@ public async Task DelegatesGetStructsGenerated() ["PSCreateMemoryPropertyStore", "PSCreateMemoryPropertyStore", "out T ppv"], ["DeviceIoControl", "DeviceIoControl", "SafeHandle hDevice, uint dwIoControlCode, [Optional] ReadOnlySpan lpInBuffer, [Optional] Span lpOutBuffer, out uint lpBytesReturned, [Optional] global::System.Threading.NativeOverlapped* lpOverlapped"], ["DeviceIoControl", "DeviceIoControl", "SafeHandle hDevice, uint dwIoControlCode, [Optional] ReadOnlySpan lpInBuffer, [Optional] Span lpOutBuffer, out uint lpBytesReturned, [Optional] global::System.Threading.NativeOverlapped* lpOverlapped", true, "NativeMethods.IncludePointerOverloads.json"], - ["NtQueryObject", "NtQueryObject", "[Optional] global::Windows.Win32.Foundation.HANDLE Handle, winmdroot.Foundation.OBJECT_INFORMATION_CLASS ObjectInformationClass, [Optional] Span ObjectInformation, out uint ReturnLength"], + ["NtQueryObject", "NtQueryObject", "[Optional] SafeHandle Handle, winmdroot.Foundation.OBJECT_INFORMATION_CLASS ObjectInformationClass, [Optional] Span ObjectInformation, out uint ReturnLength"], ["ITypeInfo", "GetFuncDesc", "uint index, out winmdroot.System.Com.FUNCDESC_unmanaged* ppFuncDesc"], ["ITsSbResourcePluginStore", "EnumerateTargets", "winmdroot.Foundation.BSTR FarmName, winmdroot.Foundation.BSTR EnvName, winmdroot.System.RemoteDesktop.TS_SB_SORT_BY sortByFieldId, winmdroot.Foundation.BSTR sortyByPropName, ref uint pdwCount, out winmdroot.System.RemoteDesktop.ITsSbTarget_unmanaged** pVal"], ["MFEnumDeviceSources", "MFEnumDeviceSources", "winmdroot.Media.MediaFoundation.IMFAttributes pAttributes, out winmdroot.Media.MediaFoundation.IMFActivate_unmanaged** pppSourceActivate, out uint pcSourceActivate"], diff --git a/test/Microsoft.Windows.CsWin32.Tests/FriendlyOverloadTests.cs b/test/Microsoft.Windows.CsWin32.Tests/FriendlyOverloadTests.cs index a50c5c65..f378ea61 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/FriendlyOverloadTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/FriendlyOverloadTests.cs @@ -66,9 +66,9 @@ public void OutPWSTR_Parameters_AsSpan() { const string name = "GetWindowText"; this.Generate(name); - MethodDeclarationSyntax friendlyOverload = Assert.Single(this.FindGeneratedMethod(name), m => m.ParameterList.Parameters.Count == 2 && m.ParameterList.Parameters[0].Type is QualifiedNameSyntax { Right.Identifier.ValueText: "HWND" }); + MethodDeclarationSyntax friendlyOverload = Assert.Single(this.FindGeneratedMethod(name), m => m.ParameterList.Parameters.Count == 2); + Assert.Equal("HWND", Assert.IsType(friendlyOverload.ParameterList.Parameters[0].Type).Right.Identifier.ValueText); Assert.Equal("Span", friendlyOverload.ParameterList.Parameters[1].Type?.ToString()); - Assert.Contains(this.FindGeneratedMethod(name), m => m.ParameterList.Parameters.Count == 2 && m.ParameterList.Parameters[0].Type?.ToString() == "SafeHandle"); } [Theory] diff --git a/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs index 4ea79666..7cd6e04b 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs @@ -36,10 +36,17 @@ public sealed class FreeWithAttribute : Attribute public FreeWithAttribute(string method) { } } } + namespace Windows.Win32.Foundation + { + [NativeTypedef, InvalidHandleValue(0)] + public unsafe struct HANDLE { public void* Value; } + } namespace Windows.Win32.Other { [NativeTypedef, InvalidHandleValue(0)] public unsafe struct DUPLICATE_RESOURCE { public void* Value; } + [NativeTypedef, InvalidHandleValue(0)] + public unsafe struct HANDLE { public void* Value; } } namespace Windows.Win32.Test { @@ -74,6 +81,10 @@ public static unsafe class Apis [DllImport("fixture.dll", ExactSpelling = true)] public static extern void ConsumeResource([In] RESOURCE_A resource); [DllImport("fixture.dll", ExactSpelling = true)] + public static extern void ConsumeHandle([In] Windows.Win32.Foundation.HANDLE resource); + [DllImport("fixture.dll", ExactSpelling = true)] + public static extern void ConsumeOtherHandle([In] Windows.Win32.Other.HANDLE resource); + [DllImport("fixture.dll", ExactSpelling = true)] public static extern void ConsumePointer([In] void* resource); [DllImport("fixture.dll", ExactSpelling = true)] [return: __Cleanup(nameof(FreeOtherResource))] @@ -138,7 +149,6 @@ static void Use(FreeSharedRESOURCE_ASafeHandle a, FreeSharedRESOURCE_BSafeHandle RESOURCE_B valueB = leaseB.Value; void* valuePointer = leasePointer.Value; FreeSharedSafeHandle compatibleWithExistingOwner = a; - PInvoke.ConsumeResource(a); PInvoke.ConsumeResource(leaseA.Value); PInvoke.ConsumePointer(leasePointer.Value); PInvoke.AcquirePointer(out FreeSharedSafeHandle ownedPointer); @@ -147,7 +157,7 @@ static void Use(FreeSharedRESOURCE_ASafeHandle a, FreeSharedRESOURCE_BSafeHandle } """); this.AssertNoDiagnostics(logAllGeneratedCode: false); - Assert.Contains(this.FindGeneratedMethod("ConsumeResource"), method => method.ParameterList.Parameters[0].Type?.ToString() == "SafeHandle"); + Assert.DoesNotContain(this.FindGeneratedMethod("ConsumeResource"), method => method.ParameterList.Parameters[0].Type?.ToString() == "SafeHandle"); Assert.DoesNotContain(this.FindGeneratedMethod("ConsumePointer"), method => method.ParameterList.Parameters.Any(parameter => parameter.Type?.ToString() == "SafeHandle")); foreach (string name in new[] { "FreeSharedSafeHandle", "FreeSharedRESOURCE_ASafeHandle", "FreeSharedRESOURCE_BSafeHandle" }) { @@ -162,6 +172,39 @@ static void Use(FreeSharedRESOURCE_ASafeHandle a, FreeSharedRESOURCE_BSafeHandle } } + /// + /// Verifies that only the Win32 HANDLE gains SafeHandle inputs without type-level ownership annotations. + /// + /// Whether runtime marshaling is enabled. + /// Whether SafeHandle projection is enabled. + [Theory, CombinatorialData] + public void HandleInputsWithoutTypeLevelOwnership(bool allowMarshaling, bool useSafeHandles) + { + this.GenerateFixture("RAIIFree", allowMarshaling, ["ConsumeHandle", "ConsumeOtherHandle", "ConsumeResource", "ConsumePointer"], useSafeHandles: useSafeHandles); + this.compilation = this.AddCode($$""" + using System.Runtime.InteropServices; + using Windows.Win32; + using Windows.Win32.Foundation; + using Windows.Win32.Test; + static unsafe class Usage + { + static void Use(SafeHandle owner, HANDLE handle, Windows.Win32.Other.HANDLE other, RESOURCE_A resource, void* pointer) + { + PInvoke.ConsumeHandle({{(useSafeHandles ? "owner" : "handle")}}); + PInvoke.ConsumeOtherHandle(other); + PInvoke.ConsumeResource(resource); + PInvoke.ConsumePointer(pointer); + } + } + """); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + Assert.Equal(useSafeHandles, this.FindGeneratedMethod("ConsumeHandle").Any(method => method.ParameterList.Parameters[0].Type?.ToString() == "SafeHandle")); + foreach (string name in new[] { "ConsumeOtherHandle", "ConsumeResource", "ConsumePointer" }) + { + Assert.DoesNotContain(this.FindGeneratedMethod(name), method => method.ParameterList.Parameters[0].Type?.ToString() == "SafeHandle"); + } + } + /// /// Verifies the existing allocated-string annotation with a real native string consumer. /// From 39255eb0a052234b83617a2e286c081f369ef6a3 Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Thu, 8 Oct 2026 17:08:49 -0700 Subject: [PATCH 5/6] Fix full-generation inference and expose annotated cleanup dependencies Replace runtime-emitted metadata with an authored IL fixture and move the ownership guide to Getting Started. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 06478ace-c5d7-418a-9386-f31b4d379d93 --- docfx/docs/features.md | 65 ----- docfx/docs/getting-started.md | 65 +++++ .../Generator.Extern.cs | 11 + .../Generator.FriendlyOverloads.cs | 3 +- .../Generator.SafeHandleLease.cs | 19 +- .../ExternalMetadata/NOTICE.md | 3 +- .../ExternalMetadata/SafeHandleOwnership.il | 245 ++++++++++++++++++ .../SafeHandleOwnership.winmd | Bin 0 -> 4096 bytes .../HandleTests.cs | 10 +- .../SafeHandleLeaseTests.cs | 188 ++++---------- 10 files changed, 401 insertions(+), 208 deletions(-) create mode 100644 test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/SafeHandleOwnership.il create mode 100644 test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/SafeHandleOwnership.winmd diff --git a/docfx/docs/features.md b/docfx/docs/features.md index 560ba889..f912c88d 100644 --- a/docfx/docs/features.md +++ b/docfx/docs/features.md @@ -7,68 +7,3 @@ - [Layered composition](composition.md): multiple assemblies can extend a single shared `PInvoke` static class so callers reach every native API through one symbol. ![Animation demonstrating p/invoke code generation](../images/demo.gif) - -## Owned outputs and scoped native values - -`RAIIFree` and compatible `FreeWith` annotations on return values and output parameters -tell CsWin32 which cleanup function owns that particular result. Friendly overloads -can return a SafeHandle for pointer-shaped allocations as well as native handles. -Unannotated pointers and the raw native signatures are unchanged. - -CsWin32-generated SafeHandles expose a `DangerousValue` property with the original native type. -For example, `SHGetKnownFolderPath` supplies a `CoTaskMemFreePWSTRSafeHandle` whose -`DangerousValue` is `PWSTR`, although `CoTaskMemFree` accepts `void*`. Reading this -property does not acquire a reference to keep the resource alive. BCL wrappers such as -`SafeFileHandle` retain their existing APIs. - -For an owner received from another component or as a parameter, acquire a scoped reference: - -```csharp -using Windows.Win32; - -static bool PathExists(CoTaskMemFreePWSTRSafeHandle path) -{ - using var lease = path.Lease(); - return PInvoke.PathFileExists(lease.Value); -} -``` - -`Lease()` pairs `DangerousAddRef` and `DangerousRelease` without an additional heap -allocation. Disposal through another alias cannot release the resource until the -lease ends. The lease's `Value` has the same native type as its owner. The lease -uses the `Dispose()` pattern; it does not require `IDisposable`. -Lease helpers require C# 9 or later. - -An owner already held in a visible `using` scope can supply `owner.DangerousValue` directly. -This assumes ordinary exclusive ownership: another alias must not explicitly -dispose it during the call. A SafeHandle parameter alone is not such a lifetime -guarantee. Friendly input overloads accept base `SafeHandle` for types with existing -cleanup annotations and keep a reference alive throughout their calls. They also -accept `SafeHandle` for `Windows.Win32.Foundation.HANDLE` even without a type-level -cleanup annotation, preserving this behavior as ownership annotations move to outputs. -This heuristic does not apply to other types such as `HWND`, `PWSTR`, or arbitrary -pointers. Such resources can be passed using their owner's `DangerousValue` or a -lease's `Value`. Set `useSafeHandles` to `false` in `NativeMethods.json` to disable -SafeHandle projection. - -The lifetime analyzer reports errors for leases that are not `using` locals, copies, -field storage, parameter passing, returns, and explicit disposal (`PInvoke015`). -It also reports errors for `DangerousValue` or a lease's `Value` without a visible -scope (`PInvoke016`), saving or returning a raw resource (`PInvoke017`), and releasing -an owned resource directly (`PInvoke018`). Use these accessors directly in calls, -or copy their contents into managed data such as a string. String interpolation -such as `$"Path: {path.DangerousValue}"` is allowed inside the owner's or lease's -scope. Interpolation into `FormattableString` -or `IFormattable` retains the raw arguments for later formatting and is not a string -copy. C# ref structs remain copyable: keep these diagnostics -enabled to enforce the supported no-copy convention. - -Neither a lease nor this analyzer proves that an arbitrary native call does not -retain the resource, transfer ownership, or invalidate it independently. Such -APIs need their own explicit lifetime arrangement. - -For cleanup requiring additional arguments, request a helper such as -`DeleteTimerQueueTimerSafeHandle` in `NativeMethods.txt`. It is abstract: derive -from it, keep the necessary cleanup context, and override `ReleaseHandle`. -Its typed `DangerousValue` is available inside that override. CsWin32 will not construct -this helper automatically or guess the additional cleanup arguments. diff --git a/docfx/docs/getting-started.md b/docfx/docs/getting-started.md index 8e2457ea..b2189ccc 100644 --- a/docfx/docs/getting-started.md +++ b/docfx/docs/getting-started.md @@ -120,6 +120,71 @@ CsWin32 will notice that your project already declares the type and skip generat Note that if that type is the only thing that references some other generated type, CsWin32 will stop generating that type too. To keep CsWin32 generating the referred types you need, add them explicitly to `NativeMethods.txt`. +### Owned outputs and scoped native values + +`RAIIFree` and compatible `FreeWith` annotations on return values and output parameters +tell CsWin32 which cleanup function owns that particular result. Friendly overloads +can return a SafeHandle for pointer-shaped allocations as well as native handles. +Unannotated pointers and the raw native signatures are unchanged. + +CsWin32-generated SafeHandles expose a `DangerousValue` property with the original native type. +For example, `SHGetKnownFolderPath` supplies a `CoTaskMemFreePWSTRSafeHandle` whose +`DangerousValue` is `PWSTR`, although `CoTaskMemFree` accepts `void*`. Reading this +property does not acquire a reference to keep the resource alive. BCL wrappers such as +`SafeFileHandle` retain their existing APIs. + +For an owner received from another component or as a parameter, acquire a scoped reference: + +```csharp +using Windows.Win32; + +static bool PathExists(CoTaskMemFreePWSTRSafeHandle path) +{ + using var lease = path.Lease(); + return PInvoke.PathFileExists(lease.Value); +} +``` + +`Lease()` pairs `DangerousAddRef` and `DangerousRelease` without an additional heap +allocation. Disposal through another alias cannot release the resource until the +lease ends. The lease's `Value` has the same native type as its owner. The lease +uses the `Dispose()` pattern; it does not require `IDisposable`. +Lease helpers require C# 9 or later. + +An owner already held in a visible `using` scope can supply `owner.DangerousValue` directly. +This assumes ordinary exclusive ownership: another alias must not explicitly +dispose it during the call. A SafeHandle parameter alone is not such a lifetime +guarantee. Friendly input overloads accept base `SafeHandle` for types with existing +cleanup annotations and keep a reference alive throughout their calls. They also +accept `SafeHandle` for `Windows.Win32.Foundation.HANDLE` even without a type-level +cleanup annotation, preserving this behavior as ownership annotations move to outputs. +This heuristic does not apply to other types such as `HWND`, `PWSTR`, or arbitrary +pointers. Such resources can be passed using their owner's `DangerousValue` or a +lease's `Value`. Set `useSafeHandles` to `false` in `NativeMethods.json` to disable +SafeHandle projection. + +The lifetime analyzer reports errors for leases that are not `using` locals, copies, +field storage, parameter passing, returns, and explicit disposal (`PInvoke015`). +It also reports errors for `DangerousValue` or a lease's `Value` without a visible +scope (`PInvoke016`), saving or returning a raw resource (`PInvoke017`), and releasing +an owned resource directly (`PInvoke018`). Use these accessors directly in calls, +or copy their contents into managed data such as a string. String interpolation +such as `$"Path: {path.DangerousValue}"` is allowed inside the owner's or lease's +scope. Interpolation into `FormattableString` or `IFormattable` retains the raw +arguments for later formatting and is not a string copy. C# ref structs remain +copyable: keep these diagnostics enabled to enforce the supported no-copy convention. + +Neither a lease nor this analyzer proves that an arbitrary native call does not +retain the resource, transfer ownership, or invalidate it independently. Such +APIs need their own explicit lifetime arrangement. + +If an annotated cleanup function needs additional arguments, CsWin32 generates that +function for you to call but cannot automatically construct a SafeHandle: it will +not guess the additional cleanup arguments. A compatible abstract helper, such as +`DeleteTimerQueueTimerSafeHandle`, can be used to supply your own cleanup context +in a derived `ReleaseHandle`. Its typed `DangerousValue` is available inside that +override. You can also request the helper directly in `NativeMethods.txt`. + ### Support for trimming, AOT, and/or disabling the runtime marshaler **NEW!** CsWin32 now has improved support for NativeAOT by generating code from an MSBuild task rather than a source generator so that it can leverage other source generators that support `[LibraryImport]` and others. diff --git a/src/Microsoft.Windows.CsWin32/Generator.Extern.cs b/src/Microsoft.Windows.CsWin32/Generator.Extern.cs index ff3004f9..3c1b7180 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.Extern.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.Extern.cs @@ -198,6 +198,17 @@ private void DeclareExternMethod(MethodDefinitionHandle methodDefinitionHandle) return; } + foreach (ParameterHandle parameterHandle in methodDefinition.GetParameters()) + { + Parameter parameter = this.Reader.GetParameter(parameterHandle); + if (this.TryGetContextualReleaseMethod(parameter.GetCustomAttributes(), out string? releaseMethod)) + { + MethodDefinitionHandle releaseMethodHandle = this.GetMethodByName(releaseMethod) + ?? throw new GenerationFailedException("Unable to find release method named: " + releaseMethod); + this.RequestExternMethod(releaseMethodHandle); + } + } + string? moduleName = this.GetNormalizedModuleName(import); string? entrypoint = null; diff --git a/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs b/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs index 0cbee606..7a6c3551 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs @@ -656,7 +656,8 @@ private IEnumerable DeclareFriendlyOverload( } else if (this.options.UseSafeHandles && isIn && !isOut && !isReleaseMethod && parameterTypeInfo is HandleTypeHandleInfo parameterHandleTypeInfo && (parameterHandleTypeInfo.Generator.TryGetHandleReleaseMethod(parameterHandleTypeInfo.Handle, paramAttributes, out _) - || (parameterHandleTypeInfo.Generator.GetQualifiedTypeDefinition(parameterHandleTypeInfo.Handle) is QualifiedTypeDefinition inputHandleDefinition + || (parameterHandleTypeInfo.Generator.TryGetTypeDefHandle(parameterHandleTypeInfo.Handle, out QualifiedTypeDefinitionHandle inputHandle) + && inputHandle.Resolve() is QualifiedTypeDefinition inputHandleDefinition && inputHandleDefinition.Reader.StringComparer.Equals(inputHandleDefinition.Definition.Namespace, "Windows.Win32.Foundation") && inputHandleDefinition.Reader.StringComparer.Equals(inputHandleDefinition.Definition.Name, "HANDLE"))) && !(this.TryGetTypeDefFieldType(parameterHandleTypeInfo, out TypeHandleInfo? fieldType) && !this.IsSafeHandleCompatibleTypeDefFieldType(fieldType))) diff --git a/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs b/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs index 79bcc6bc..68b8c404 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.SafeHandleLease.cs @@ -14,11 +14,8 @@ public partial class Generator /// Whether cleanup information is available. internal bool TryGetResourceReleaseMethod(TypeHandleInfo valueType, CustomAttributeHandleCollection? attributes, [NotNullWhen(true)] out string? releaseMethod) { - if ((this.FindAttribute(attributes, InteropDecorationNamespace, RAIIFreeAttribute) - ?? this.FindAttribute(attributes, InteropDecorationNamespace, FreeWithAttribute)) is CustomAttribute cleanup - && cleanup.DecodeValue(CustomAttributeTypeProvider.Instance).FixedArguments[0].Value is string name) + if (this.TryGetContextualReleaseMethod(attributes, out releaseMethod)) { - releaseMethod = name; return true; } @@ -49,6 +46,20 @@ internal bool TryGetResourceReleaseMethod(TypeHandleInfo valueType, CustomAttrib _ => false, }; + private bool TryGetContextualReleaseMethod(CustomAttributeHandleCollection? attributes, [NotNullWhen(true)] out string? releaseMethod) + { + if ((this.FindAttribute(attributes, InteropDecorationNamespace, RAIIFreeAttribute) + ?? this.FindAttribute(attributes, InteropDecorationNamespace, FreeWithAttribute)) is CustomAttribute cleanup + && cleanup.DecodeValue(CustomAttributeTypeProvider.Instance).FixedArguments[0].Value is string name) + { + releaseMethod = name; + return true; + } + + releaseMethod = null; + return false; + } + private string GetSafeHandleValueName(TypeHandleInfo nativeType, TypeSyntax syntax) { string name = GetNativeValueName(syntax); diff --git a/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/NOTICE.md b/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/NOTICE.md index 50d9a5a5..e301f04f 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/NOTICE.md +++ b/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/NOTICE.md @@ -5,4 +5,5 @@ Metadata | Source ServiceFabric.winmd | [youyuanwu/fabric-metadata](https://github.com/youyuanwu/fabric-metadata/raw/a1bcca6ad6f6a772c9e5ff4bdba80ae5e5f24cfc/.windows/winmd/ServiceFabric.winmd) 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. \ No newline at end of file +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. \ No newline at end of file diff --git a/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/SafeHandleOwnership.il b/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/SafeHandleOwnership.il new file mode 100644 index 00000000..13797318 --- /dev/null +++ b/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/SafeHandleOwnership.il @@ -0,0 +1,245 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// 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. +// 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 } +.assembly extern mscorlib { .publickeytoken = (B7 7A 5C 56 19 34 E0 89) .ver 4:0:0:0 } +.assembly extern Windows.Win32.winmd { .ver 0:0:0:0 } +.assembly Windows.Win32 { .ver 1:0:0:0 } +.module SafeHandleOwnership.winmd +.module extern fixture.dll +.corflags 0x00000001 + +.class public sequential ansi sealed beforefieldinit Windows.Win32.Foundation.HANDLE + extends [netstandard]System.ValueType +{ + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.NativeTypedefAttribute::.ctor() = (01 00 00 00) + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.InvalidHandleValueAttribute::.ctor(int64) = (01 00 00 00 00 00 00 00 00 00 00 00) + .field public void* Value +} + +.class public sequential ansi sealed beforefieldinit Windows.Win32.Other.HANDLE + extends [netstandard]System.ValueType +{ + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.NativeTypedefAttribute::.ctor() = (01 00 00 00) + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.InvalidHandleValueAttribute::.ctor(int64) = (01 00 00 00 00 00 00 00 00 00 00 00) + .field public void* Value +} + +.class public sequential ansi sealed beforefieldinit Windows.Win32.Test.RESOURCE_A + extends [netstandard]System.ValueType +{ + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.NativeTypedefAttribute::.ctor() = (01 00 00 00) + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.InvalidHandleValueAttribute::.ctor(int64) = (01 00 00 00 00 00 00 00 00 00 00 00) + .field public void* Value +} + +.class public sequential ansi sealed beforefieldinit Windows.Win32.Test.RESOURCE_B + extends [netstandard]System.ValueType +{ + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.NativeTypedefAttribute::.ctor() = (01 00 00 00) + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.InvalidHandleValueAttribute::.ctor(int64) = (01 00 00 00 00 00 00 00 00 00 00 00) + .field public void* Value +} + +.class public sequential ansi sealed beforefieldinit Windows.Win32.Test.UNSIGNED_RESOURCE + extends [netstandard]System.ValueType +{ + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.NativeTypedefAttribute::.ctor() = (01 00 00 00) + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.InvalidHandleValueAttribute::.ctor(int64) = (01 00 00 00 00 00 00 00 00 00 00 00) + .field public native unsigned int Value +} + +.class public sequential ansi sealed beforefieldinit Windows.Win32.Test.UNSIGNED32_RESOURCE + extends [netstandard]System.ValueType +{ + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.NativeTypedefAttribute::.ctor() = (01 00 00 00) + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.InvalidHandleValueAttribute::.ctor(int64) = (01 00 00 00 00 00 00 00 00 00 00 00) + .field public uint32 Value +} + +.class public sequential ansi sealed beforefieldinit Windows.Win32.Test.DUPLICATE_RESOURCE + extends [netstandard]System.ValueType +{ + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.NativeTypedefAttribute::.ctor() = (01 00 00 00) + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.InvalidHandleValueAttribute::.ctor(int64) = (01 00 00 00 00 00 00 00 00 00 00 00) + .field public void* Value +} + +.class public sequential ansi sealed beforefieldinit Windows.Win32.Other.DUPLICATE_RESOURCE + extends [netstandard]System.ValueType +{ + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.NativeTypedefAttribute::.ctor() = (01 00 00 00) + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.InvalidHandleValueAttribute::.ctor(int64) = (01 00 00 00 00 00 00 00 00 00 00 00) + .field public void* Value +} + +.class public abstract auto autochar sealed beforefieldinit Windows.Win32.Test.Apis + extends [netstandard]System.Object +{ + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + void FreeShared(void* resource) cil managed { } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + void FreeOtherResource(valuetype Windows.Win32.Test.RESOURCE_B resource) cil managed { } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + bool FreeContext(valuetype Windows.Win32.Test.RESOURCE_A resource, int32 context) cil managed { } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + 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) + void ConsumeResource([in] valuetype Windows.Win32.Test.RESOURCE_A resource) cil managed { } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + void ConsumeHandle([in] valuetype Windows.Win32.Foundation.HANDLE resource) cil managed { } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + void ConsumeOtherHandle([in] valuetype Windows.Win32.Other.HANDLE resource) cil managed { } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + void ConsumePointer([in] void* resource) cil managed { } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + void ConsumeGuid([in] valuetype [netstandard]System.Guid resource) cil managed { } +} + +.class public abstract auto autochar sealed beforefieldinit Windows.Win32.Test.RAIIFree.Apis + extends [netstandard]System.Object +{ + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Test.RESOURCE_A CreateResource() cil managed + { + .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_B CreateOtherResource() cil managed + { + .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) + void* CreatePointer() cil managed + { + .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) + int32 AcquirePointer([out] void** resource) cil managed + { + .param [1] + .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) + int32 AcquireResource([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 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 IncorrectCleanup() cil managed + { + .param [0] + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.RAIIFreeAttribute::.ctor(string) = (01 00 11 46 72 65 65 4F 74 68 65 72 52 65 73 6F 75 72 63 65 00 00) + } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Test.RESOURCE_A CreateWithContext() cil managed + { + .param [0] + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.RAIIFreeAttribute::.ctor(string) = (01 00 0B 46 72 65 65 43 6F 6E 74 65 78 74 00 00) + } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Test.UNSIGNED_RESOURCE CreateUnsigned() cil managed + { + .param [0] + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.RAIIFreeAttribute::.ctor(string) = (01 00 0C 46 72 65 65 55 6E 73 69 67 6E 65 64 00 00) + } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Test.UNSIGNED32_RESOURCE CreateUnsigned32() cil managed + { + .param [0] + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.RAIIFreeAttribute::.ctor(string) = (01 00 0E 46 72 65 65 55 6E 73 69 67 6E 65 64 33 32 00 00) + } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Test.DUPLICATE_RESOURCE CreateDuplicate() cil managed + { + .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.Other.DUPLICATE_RESOURCE CreateOtherDuplicate() cil managed + { + .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) + } +} + +.class public abstract auto autochar sealed beforefieldinit Windows.Win32.Test.FreeWith.Apis + extends [netstandard]System.Object +{ + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Test.RESOURCE_A CreateResource() cil managed + { + .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_B CreateOtherResource() cil managed + { + .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) + void* CreatePointer() cil managed + { + .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) + int32 AcquirePointer([out] void** resource) cil managed + { + .param [1] + .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) + int32 AcquireResource([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 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 IncorrectCleanup() cil managed + { + .param [0] + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.FreeWithAttribute::.ctor(string) = (01 00 11 46 72 65 65 4F 74 68 65 72 52 65 73 6F 75 72 63 65 00 00) + } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Test.RESOURCE_A CreateWithContext() cil managed + { + .param [0] + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.FreeWithAttribute::.ctor(string) = (01 00 0B 46 72 65 65 43 6F 6E 74 65 78 74 00 00) + } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Test.UNSIGNED_RESOURCE CreateUnsigned() cil managed + { + .param [0] + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.FreeWithAttribute::.ctor(string) = (01 00 0C 46 72 65 65 55 6E 73 69 67 6E 65 64 00 00) + } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Test.UNSIGNED32_RESOURCE CreateUnsigned32() cil managed + { + .param [0] + .custom instance void [Windows.Win32.winmd]Windows.Win32.Foundation.Metadata.FreeWithAttribute::.ctor(string) = (01 00 0E 46 72 65 65 55 6E 73 69 67 6E 65 64 33 32 00 00) + } + .method public hidebysig static pinvokeimpl("fixture.dll" nomangle winapi) + valuetype Windows.Win32.Test.DUPLICATE_RESOURCE CreateDuplicate() cil managed + { + .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.Other.DUPLICATE_RESOURCE CreateOtherDuplicate() cil managed + { + .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) + } +} diff --git a/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/SafeHandleOwnership.winmd b/test/Microsoft.Windows.CsWin32.Tests/ExternalMetadata/SafeHandleOwnership.winmd new file mode 100644 index 0000000000000000000000000000000000000000..5b2604292f3bdad551e8f028c224eeed4b701b9a GIT binary patch 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 literal 0 HcmV?d00001 diff --git a/test/Microsoft.Windows.CsWin32.Tests/HandleTests.cs b/test/Microsoft.Windows.CsWin32.Tests/HandleTests.cs index 0f8da880..5c0fe8f7 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/HandleTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/HandleTests.cs @@ -19,12 +19,12 @@ public void BSTR_FieldsDoNotBecomeSafeHandles(bool allowMarshaling) } [Fact] - public void NamespaceHandleRequiresCallerDefinedCleanup() + public void ProcessHeapGetsNoCleanupSafeHandle() { - this.GenerateApi("CreatePrivateNamespace"); - ClassDeclarationSyntax helper = Assert.IsType(Assert.Single(this.FindGeneratedType("ClosePrivateNamespaceSafeHandle"))); - Assert.Contains(helper.Modifiers, modifier => modifier.IsKind(SyntaxKind.AbstractKeyword)); - Assert.DoesNotContain(this.FindGeneratedMethod("CreatePrivateNamespace"), method => method.ReturnType.ToString().EndsWith("SafeHandle", StringComparison.Ordinal)); + this.GenerateApi("GetProcessHeap"); + Assert.Empty(this.FindGeneratedType("HeapDestroySafeHandle")); + Assert.All(this.FindGeneratedMethod("GetProcessHeap"), method => + Assert.Equal("HANDLE", Assert.IsType(method.ReturnType).Right.Identifier.ValueText)); } [Fact] diff --git a/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs index 7cd6e04b..628284dd 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs @@ -11,111 +11,6 @@ namespace Microsoft.Windows.CsWin32.Tests; /// public class SafeHandleLeaseTests : GeneratorTestBase { - private const string MetadataSource = """ - using System; - using System.Runtime.InteropServices; - using Windows.Win32.Foundation.Metadata; - - namespace Windows.Win32.Foundation.Metadata - { - [AttributeUsage(AttributeTargets.Struct)] - public sealed class NativeTypedefAttribute : Attribute { } - [AttributeUsage(AttributeTargets.Struct, AllowMultiple = true)] - public sealed class InvalidHandleValueAttribute : Attribute - { - public InvalidHandleValueAttribute(long value) { } - } - [AttributeUsage(AttributeTargets.Struct | AttributeTargets.ReturnValue | AttributeTargets.Parameter)] - public sealed class RAIIFreeAttribute : Attribute - { - public RAIIFreeAttribute(string method) { } - } - [AttributeUsage(AttributeTargets.ReturnValue | AttributeTargets.Parameter)] - public sealed class FreeWithAttribute : Attribute - { - public FreeWithAttribute(string method) { } - } - } - namespace Windows.Win32.Foundation - { - [NativeTypedef, InvalidHandleValue(0)] - public unsafe struct HANDLE { public void* Value; } - } - namespace Windows.Win32.Other - { - [NativeTypedef, InvalidHandleValue(0)] - public unsafe struct DUPLICATE_RESOURCE { public void* Value; } - [NativeTypedef, InvalidHandleValue(0)] - public unsafe struct HANDLE { public void* Value; } - } - namespace Windows.Win32.Test - { - [NativeTypedef, InvalidHandleValue(0)] - public unsafe struct RESOURCE_A { public void* Value; } - [NativeTypedef, InvalidHandleValue(0)] - public unsafe struct RESOURCE_B { public void* Value; } - [NativeTypedef, InvalidHandleValue(0)] - public struct UNSIGNED_RESOURCE { public UIntPtr Value; } - [NativeTypedef, InvalidHandleValue(0)] - public struct UNSIGNED32_RESOURCE { public uint Value; } - [NativeTypedef, InvalidHandleValue(0)] - public unsafe struct DUPLICATE_RESOURCE { public void* Value; } - - public static unsafe class Apis - { - [DllImport("fixture.dll", ExactSpelling = true)] - [return: __Cleanup(nameof(FreeShared))] - public static extern RESOURCE_A CreateResource(); - [DllImport("fixture.dll", ExactSpelling = true)] - [return: __Cleanup(nameof(FreeShared))] - public static extern RESOURCE_B CreateOtherResource(); - [DllImport("fixture.dll", ExactSpelling = true)] - [return: __Cleanup(nameof(FreeShared))] - public static extern void* CreatePointer(); - [DllImport("fixture.dll", ExactSpelling = true)] - public static extern int AcquirePointer([Out, __Cleanup(nameof(FreeShared))] void** resource); - [DllImport("fixture.dll", ExactSpelling = true)] - public static extern int AcquireResource([Out, __Cleanup(nameof(FreeShared))] RESOURCE_A* resource); - [DllImport("fixture.dll", ExactSpelling = true)] - public static extern void FreeShared(void* resource); - [DllImport("fixture.dll", ExactSpelling = true)] - public static extern void ConsumeResource([In] RESOURCE_A resource); - [DllImport("fixture.dll", ExactSpelling = true)] - public static extern void ConsumeHandle([In] Windows.Win32.Foundation.HANDLE resource); - [DllImport("fixture.dll", ExactSpelling = true)] - public static extern void ConsumeOtherHandle([In] Windows.Win32.Other.HANDLE resource); - [DllImport("fixture.dll", ExactSpelling = true)] - public static extern void ConsumePointer([In] void* resource); - [DllImport("fixture.dll", ExactSpelling = true)] - [return: __Cleanup(nameof(FreeOtherResource))] - public static extern RESOURCE_A IncorrectCleanup(); - [DllImport("fixture.dll", ExactSpelling = true)] - public static extern void FreeOtherResource(RESOURCE_B resource); - [DllImport("fixture.dll", ExactSpelling = true)] - [return: __Cleanup(nameof(FreeContext))] - public static extern RESOURCE_A CreateWithContext(); - [DllImport("fixture.dll", ExactSpelling = true)] - public static extern bool FreeContext(RESOURCE_A resource, int context); - [DllImport("fixture.dll", ExactSpelling = true)] - [return: __Cleanup(nameof(FreeUnsigned))] - public static extern UNSIGNED_RESOURCE CreateUnsigned(); - [DllImport("fixture.dll", ExactSpelling = true)] - public static extern void FreeUnsigned(UNSIGNED_RESOURCE resource); - [DllImport("fixture.dll", ExactSpelling = true)] - [return: __Cleanup(nameof(FreeUnsigned32))] - public static extern UNSIGNED32_RESOURCE CreateUnsigned32(); - [DllImport("fixture.dll", ExactSpelling = true)] - public static extern void FreeUnsigned32(UNSIGNED32_RESOURCE resource); - [DllImport("fixture.dll", ExactSpelling = true)] - [return: __Cleanup(nameof(FreeShared))] - public static extern DUPLICATE_RESOURCE CreateDuplicate(); - [DllImport("fixture.dll", ExactSpelling = true)] - [return: __Cleanup(nameof(FreeShared))] - public static extern Windows.Win32.Other.DUPLICATE_RESOURCE CreateOtherDuplicate(); - } - } - """; - /// /// Initializes a new instance of the class. /// @@ -205,6 +100,24 @@ static void Use(SafeHandle owner, HANDLE handle, Windows.Win32.Other.HANDLE othe } } + /// + /// Verifies that framework value types do not require native metadata definitions for input inference. + /// + [Fact] + public void FrameworkTypesRemainNativeInputs() + { + this.GenerateFixture("RAIIFree", true, "ConsumeGuid"); + this.compilation = this.AddCode(""" + using Windows.Win32; + static class Usage + { + static void Use(System.Guid value) => PInvoke.ConsumeGuid(value); + } + """); + this.AssertNoDiagnostics(logAllGeneratedCode: false); + Assert.DoesNotContain(this.FindGeneratedMethod("ConsumeGuid"), method => method.ParameterList.Parameters[0].Type?.ToString() == "SafeHandle"); + } + /// /// Verifies the existing allocated-string annotation with a real native string consumer. /// @@ -316,6 +229,34 @@ public void IncompatibleCleanupDoesNotProduceOwner(string attribute) Assert.All(this.FindGeneratedMethod("IncorrectCleanup"), method => Assert.Equal("RESOURCE_A", Assert.IsType(method.ReturnType).Right.Identifier.ValueText)); Assert.Empty(this.FindGeneratedMethod("IncorrectCleanup_SafeHandle")); + Assert.Single(this.FindGeneratedMethod("FreeOtherResource")); + } + + /// + /// Verifies that annotated cleanup functions remain callable when ownership projection is unavailable. + /// + /// The contextual cleanup attribute. + /// Whether runtime marshaling is enabled. + /// Whether SafeHandle projection is enabled. + [Theory, CombinatorialData] + public void CleanupDependenciesDoNotRequireSafeHandleProjection( + [CombinatorialValues("RAIIFree", "FreeWith")] string attribute, bool allowMarshaling, bool useSafeHandles) + { + this.GenerateFixture(attribute, allowMarshaling, ["CreateWithContext", "IncorrectCleanup", "AcquirePointer"], useSafeHandles); + this.compilation = this.AddCode(""" + using Windows.Win32; + using Windows.Win32.Test; + static unsafe class Usage + { + static void Release(RESOURCE_A resource, RESOURCE_B other, void* pointer, int context) + { + PInvoke.FreeContext(resource, context); + PInvoke.FreeOtherResource(other); + PInvoke.FreeShared(pointer); + } + } + """); + this.AssertNoDiagnostics(logAllGeneratedCode: false); } /// @@ -686,34 +627,17 @@ private void GenerateFixture(string attribute, bool allowMarshaling, string[] ap this.compilation = this.starterCompilations["net8.0"]; } - CSharpCompilation metadata = CSharpCompilation.Create( - "Windows.Win32", - [CSharpSyntaxTree.ParseText(MetadataSource.Replace("__Cleanup", attribute, StringComparison.Ordinal), this.parseOptions)], - this.starterCompilations["net8.0"].References, - this.starterCompilations["net8.0"].Options); - string metadataPath = Path.Combine(Path.GetTempPath(), $"CsWin32-Ownership-{Guid.NewGuid():N}.winmd"); - try + string metadataPath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location!)!, "ExternalMetadata", "SafeHandleOwnership.winmd"); + this.generator = new Generator(metadataPath, null, [], DefaultTestGeneratorOptions with { AllowMarshaling = allowMarshaling, UseSafeHandles = useSafeHandles }, this.compilation, this.parseOptions); + foreach (string api in apis) { - using (FileStream stream = File.Create(metadataPath)) - { - var result = metadata.Emit(stream); - Assert.True(result.Success, string.Join(Environment.NewLine, result.Diagnostics)); - } - - this.generator = new Generator(metadataPath, null, [], DefaultTestGeneratorOptions with { AllowMarshaling = allowMarshaling, UseSafeHandles = useSafeHandles }, this.compilation, this.parseOptions); - foreach (string api in apis) - { - Assert.True(this.generator.TryGenerate(api, CancellationToken.None)); - } - - this.CollectGeneratedCode(this.generator); - this.AssertNoDiagnostics(logAllGeneratedCode: false); - } - finally - { - this.generator?.Dispose(); - this.generator = null; - File.Delete(metadataPath); + string apiNamespace = api.StartsWith("Create", StringComparison.Ordinal) || api.StartsWith("Acquire", StringComparison.Ordinal) || api == "IncorrectCleanup" + ? $"Windows.Win32.Test.{attribute}" + : "Windows.Win32.Test"; + Assert.True(this.generator.TryGenerate($"{apiNamespace}.{api}", CancellationToken.None)); } + + this.CollectGeneratedCode(this.generator); + this.AssertNoDiagnostics(logAllGeneratedCode: false); } } From f3f5a69aa4823466ef7b55bbdc8e5ba41ec021e6 Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Thu, 8 Oct 2026 17:30:35 -0700 Subject: [PATCH 6/6] Expose annotated cleanup dependencies for COM outputs as well Resolve cleanup functions in shared signature generation so native and COM outputs expose cleanup even with SafeHandles disabled. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 06478ace-c5d7-418a-9386-f31b4d379d93 --- src/Microsoft.Windows.CsWin32/Generator.Extern.cs | 11 ----------- src/Microsoft.Windows.CsWin32/Generator.cs | 15 ++++++++++++++- .../SafeHandleLeaseTests.cs | 12 ++++++++++++ 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.Windows.CsWin32/Generator.Extern.cs b/src/Microsoft.Windows.CsWin32/Generator.Extern.cs index 3c1b7180..ff3004f9 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.Extern.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.Extern.cs @@ -198,17 +198,6 @@ private void DeclareExternMethod(MethodDefinitionHandle methodDefinitionHandle) return; } - foreach (ParameterHandle parameterHandle in methodDefinition.GetParameters()) - { - Parameter parameter = this.Reader.GetParameter(parameterHandle); - if (this.TryGetContextualReleaseMethod(parameter.GetCustomAttributes(), out string? releaseMethod)) - { - MethodDefinitionHandle releaseMethodHandle = this.GetMethodByName(releaseMethod) - ?? throw new GenerationFailedException("Unable to find release method named: " + releaseMethod); - this.RequestExternMethod(releaseMethodHandle); - } - } - string? moduleName = this.GetNormalizedModuleName(import); string? entrypoint = null; diff --git a/src/Microsoft.Windows.CsWin32/Generator.cs b/src/Microsoft.Windows.CsWin32/Generator.cs index 643a202d..7292b64a 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.cs @@ -2058,7 +2058,20 @@ private string GetNamespaceForPossiblyNestedType(TypeDefinition nestedTypeDef) } private ParameterListSyntax CreateParameterList(MethodDefinition methodDefinition, MethodSignature signature, TypeSyntaxSettings typeSettings, GeneratingElement forElement) - => FixTrivia(ParameterList([.. methodDefinition.GetParameters().Select(this.Reader.GetParameter).Where(p => !p.Name.IsNil).Select(p => this.CreateParameter(signature, signature.ParameterTypes[p.SequenceNumber - 1], p, typeSettings, forElement))])); + { + foreach (ParameterHandle parameterHandle in methodDefinition.GetParameters()) + { + Parameter parameter = this.Reader.GetParameter(parameterHandle); + if (this.TryGetContextualReleaseMethod(parameter.GetCustomAttributes(), out string? releaseMethod)) + { + MethodDefinitionHandle releaseMethodHandle = this.GetMethodByName(releaseMethod) + ?? throw new GenerationFailedException("Unable to find release method named: " + releaseMethod); + this.RequestExternMethod(releaseMethodHandle); + } + } + + return FixTrivia(ParameterList([.. methodDefinition.GetParameters().Select(this.Reader.GetParameter).Where(p => !p.Name.IsNil).Select(p => this.CreateParameter(signature, signature.ParameterTypes[p.SequenceNumber - 1], p, typeSettings, forElement))])); + } private ParameterSyntax CreateParameter(MethodSignature signature, TypeHandleInfo parameterInfo, Parameter parameter, TypeSyntaxSettings typeSettings, GeneratingElement forElement) { diff --git a/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs index 628284dd..88ca0b60 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/SafeHandleLeaseTests.cs @@ -259,6 +259,18 @@ static void Release(RESOURCE_A resource, RESOURCE_B other, void* pointer, int co this.AssertNoDiagnostics(logAllGeneratedCode: false); } + /// + /// Verifies that COM output annotations expose their native cleanup without SafeHandle projection. + /// + /// Whether runtime marshaling is enabled. + [Theory, CombinatorialData] + public void ComCleanupDependenciesDoNotRequireSafeHandleProjection(bool allowMarshaling) + { + this.generator = this.CreateGenerator(DefaultTestGeneratorOptions with { AllowMarshaling = allowMarshaling, UseSafeHandles = false }); + this.GenerateApi("IBackgroundCopyJob"); + Assert.True(this.IsMethodGenerated("CoTaskMemFree")); + } + /// /// Verifies that the real incompatible recording-DC annotations leave the raw methods available. ///