From fab54358e8c4eb91f4cb97a66de1f888609d1bbc Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Fri, 11 Nov 2022 16:43:07 -0700 Subject: [PATCH 1/2] Emit a stable reference to the interface guid This way, the reference can be converted by others into a pointer and passed around without fear that the data will move. See #752 --- src/Microsoft.Windows.CsWin32/Generator.cs | 80 ++++++++++++++++++++-- 1 file changed, 75 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Windows.CsWin32/Generator.cs b/src/Microsoft.Windows.CsWin32/Generator.cs index 5b5f5c3d..d1b8e304 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.cs @@ -2295,6 +2295,18 @@ private static AttributeSyntax StructLayout(TypeAttributes typeAttributes, TypeL return structLayoutAttribute; } + private static AttributeSyntax MethodImpl(MethodImplOptions options) + { + if (options != MethodImplOptions.AggressiveInlining) + { + throw new NotImplementedException(); + } + + AttributeSyntax attribute = Attribute(IdentifierName("MethodImpl")) + .AddArgumentListArguments(AttributeArgument(MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, IdentifierName(nameof(MethodImplOptions)), IdentifierName(nameof(MethodImplOptions.AggressiveInlining))))); + return attribute; + } + private static AttributeSyntax GUID(Guid guid) { return Attribute(IdentifierName("Guid")).AddArgumentListArguments( @@ -2469,6 +2481,25 @@ private static bool IsAnsiFunction(string methodName) return false; } + /// + /// Creates the syntax for creating a new byte array populated with the specified data. + /// e.g. new byte[] { 0x01, 0x02 }. + /// + /// The content of the array. + /// The array creation syntax. + private static ArrayCreationExpressionSyntax NewByteArray(ReadOnlySpan bytes) + { + ExpressionSyntax[] elements = new ExpressionSyntax[bytes.Length]; + for (int i = 0; i < bytes.Length; i++) + { + elements[i] = LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(ToHex(bytes[i]), bytes[i])); + } + + return ArrayCreationExpression( + ArrayType(PredefinedType(Token(SyntaxKind.ByteKeyword))).AddRankSpecifiers(ArrayRankSpecifier()), + InitializerExpression(SyntaxKind.ArrayInitializerExpression, SeparatedList(elements))); + } + private static unsafe string ToHex(T value) where T : unmanaged { @@ -3887,7 +3918,7 @@ StatementSyntax ThrowOnHRFailure(ExpressionSyntax hrExpression) => ExpressionSta return ifaceDeclaration; } - private (List Members, List BaseTypes) DeclareStaticCOMInterfaceMembers(CustomAttribute? guidAttribute) + private unsafe (List Members, List BaseTypes) DeclareStaticCOMInterfaceMembers(CustomAttribute? guidAttribute) { List members = new(); List baseTypes = new(); @@ -3910,12 +3941,50 @@ StatementSyntax ThrowOnHRFailure(ExpressionSyntax hrExpression) => ExpressionSta { baseTypes.Add(SimpleBaseType(IComIIDGuidInterfaceName)); - // static ref readonly Guid IComIID.Guid => ref IID_Guid; + IdentifierNameSyntax dataLocal = IdentifierName("data"); + + // Rather than just `return ref IID_Guid`, which returns a pointer to a 'movable' field, + // We leverage C# syntax that we know the modern C# compiler will turn into a pointer directly into the dll image, + // so that the pointer does not move. + // This does rely on at least the generated code running on a little endian machine, since we're laying raw bytes on top of integer fields. + // But at the moment, we also assume this source generator is running on little endian for convenience for the reverse operation. + if (!BitConverter.IsLittleEndian) + { + throw new NotSupportedException("Conversion from big endian to little endian is not implemented."); + } + + // ReadOnlySpan data = new byte[] { ... }; + ReadOnlySpan guidBytes = new((byte*)&guidAttributeValue, sizeof(Guid)); + LocalDeclarationStatementSyntax dataDecl = LocalDeclarationStatement( + VariableDeclaration(MakeReadOnlySpanOfT(PredefinedType(Token(SyntaxKind.ByteKeyword)))).AddVariables( + VariableDeclarator(dataLocal.Identifier).WithInitializer(EqualsValueClause(NewByteArray(guidBytes))))); + + // return ref Unsafe.As(ref MemoryMarshal.GetReference(data)); + ReturnStatementSyntax returnStatement = ReturnStatement(RefExpression( + InvocationExpression( + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + IdentifierName(nameof(Unsafe)), + GenericName(nameof(Unsafe.As)).AddTypeArgumentListArguments(PredefinedType(Token(SyntaxKind.ByteKeyword)), IdentifierName(nameof(Guid)))), + ArgumentList().AddArguments( + Argument( + InvocationExpression( + MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, IdentifierName(nameof(MemoryMarshal)), IdentifierName(nameof(MemoryMarshal.GetReference))), + ArgumentList(SingletonSeparatedList(Argument(dataLocal))))) + .WithRefOrOutKeyword(Token(SyntaxKind.RefKeyword)))))); + + // The native assembly code for this property getter is just a `mov` and a `ret. + // For our callers to also enjoy just the `mov` instruction, we have to attribute for aggressive inlining. + // [MethodImpl(MethodImplOptions.AggressiveInlining)] + AttributeListSyntax methodImplAttr = AttributeList().AddAttributes(MethodImpl(MethodImplOptions.AggressiveInlining)); + + BlockSyntax getBody = Block(dataDecl, returnStatement); + + // static ref readonly Guid IComIID.Guid { get { ... } } PropertyDeclarationSyntax guidProperty = PropertyDeclaration(IdentifierName(nameof(Guid)).WithTrailingTrivia(Space), ComIIDGuidPropertyName.Identifier) .WithExplicitInterfaceSpecifier(ExplicitInterfaceSpecifier(IComIIDGuidInterfaceName)) .AddModifiers(TokenWithSpace(SyntaxKind.StaticKeyword), TokenWithSpace(SyntaxKind.RefKeyword), TokenWithSpace(SyntaxKind.ReadOnlyKeyword)) - .WithExpressionBody(ArrowExpressionClause(RefExpression(iidGuidFieldName))) - .WithSemicolonToken(Semicolon); + .WithAccessorList(AccessorList().AddAccessors(AccessorDeclaration(SyntaxKind.GetAccessorDeclaration, getBody).AddAttributeLists(methodImplAttr))); members.Add(guidProperty); } } @@ -6202,7 +6271,8 @@ private bool TryDeclareCOMGuidInterfaceIfNecessary() TokenWithSpace(SyntaxKind.AbstractKeyword), TokenWithSpace(SyntaxKind.RefKeyword), TokenWithSpace(SyntaxKind.ReadOnlyKeyword)) - .WithAccessorList(AccessorList().AddAccessors(AccessorDeclaration(SyntaxKind.GetAccessorDeclaration).WithSemicolonToken(Semicolon))); + .WithAccessorList(AccessorList().AddAccessors(AccessorDeclaration(SyntaxKind.GetAccessorDeclaration).WithSemicolonToken(Semicolon))) + .WithLeadingTrivia(ParseLeadingTrivia($"/// The IID guid for this interface.\n/// The reference that is returned comes from a permanent memory address, and is therefore safe to convert to a pointer and pass around or hold long-term.\n")); // internal interface IComIID { ... } InterfaceDeclarationSyntax ifaceDecl = InterfaceDeclaration(IComIIDGuidInterfaceName.Identifier) From 1a572bf284b95081f59c1b17912654b19deeda2b Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Fri, 11 Nov 2022 16:45:38 -0700 Subject: [PATCH 2/2] Add non-marshaling GenerationSandbox test project --- .devcontainer/Dockerfile | 2 +- Directory.Build.props | 2 +- Microsoft.Windows.CsWin32.sln | 16 +++++++- global.json | 2 +- .../GenerationSandbox.Tests.csproj | 35 +----------------- .../COMTests.cs | 23 ++++++++++++ ...enerationSandbox.Unmarshalled.Tests.csproj | 11 ++++++ .../NativeMethods.json | 6 +++ .../NativeMethods.txt | 1 + test/GenerationSandbox.props | 37 +++++++++++++++++++ 10 files changed, 96 insertions(+), 39 deletions(-) create mode 100644 test/GenerationSandbox.Unmarshalled.Tests/COMTests.cs create mode 100644 test/GenerationSandbox.Unmarshalled.Tests/GenerationSandbox.Unmarshalled.Tests.csproj create mode 100644 test/GenerationSandbox.Unmarshalled.Tests/NativeMethods.json create mode 100644 test/GenerationSandbox.Unmarshalled.Tests/NativeMethods.txt create mode 100644 test/GenerationSandbox.props diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 5c1bdc7a..c34398f4 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -1,4 +1,4 @@ -FROM mcr.microsoft.com/dotnet/sdk:6.0.401-focal +FROM mcr.microsoft.com/dotnet/sdk:7.0.100-jammy # Installing mono makes `dotnet test` work without errors even for net472. # But installing it takes a long time, so it's excluded by default. diff --git a/Directory.Build.props b/Directory.Build.props index 671bec5c..695e5164 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -5,7 +5,7 @@ $(RepoRootPath)obj\$([MSBuild]::MakeRelative($(RepoRootPath), $(MSBuildProjectDirectory)))\ $(RepoRootPath)bin\$(MSBuildProjectName)\ $(RepoRootPath)bin\Packages\$(Configuration)\ - 10.0 + 11.0 true enable latest diff --git a/Microsoft.Windows.CsWin32.sln b/Microsoft.Windows.CsWin32.sln index 7ca7b762..9e95af7a 100644 --- a/Microsoft.Windows.CsWin32.sln +++ b/Microsoft.Windows.CsWin32.sln @@ -1,7 +1,7 @@  Microsoft Visual Studio Solution File, Format Version 12.00 -# Visual Studio Version 16 -VisualStudioVersion = 16.0.29322.22 +# Visual Studio Version 17 +VisualStudioVersion = 17.5.33110.383 MinimumVisualStudioVersion = 15.0.26124.0 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{1CE9670B-D5FF-46A7-9D00-24E70E6ED48B}" ProjectSection(SolutionItems) = preProject @@ -27,6 +27,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{36CCE840-6 test\.editorconfig = test\.editorconfig test\Directory.Build.props = test\Directory.Build.props test\Directory.Build.targets = test\Directory.Build.targets + test\GenerationSandbox.props = test\GenerationSandbox.props EndProjectSection EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.Windows.CsWin32", "src\Microsoft.Windows.CsWin32\Microsoft.Windows.CsWin32.csproj", "{E3E96466-44B6-41AF-BBC8-9D30183ED8A9}" @@ -41,6 +42,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SpellChecker", "test\SpellC EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "WinRTInteropTest", "test\WinRTInteropTest\WinRTInteropTest.csproj", "{0E067B66-C2EC-4106-87D2-5310CFCDC5B8}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "GenerationSandbox.Unmarshalled.Tests", "test\GenerationSandbox.Unmarshalled.Tests\GenerationSandbox.Unmarshalled.Tests.csproj", "{3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -95,6 +98,14 @@ Global {0E067B66-C2EC-4106-87D2-5310CFCDC5B8}.Release|Any CPU.ActiveCfg = Release|Any CPU {0E067B66-C2EC-4106-87D2-5310CFCDC5B8}.Release|Any CPU.Build.0 = Release|Any CPU {0E067B66-C2EC-4106-87D2-5310CFCDC5B8}.Release|NonWindows.ActiveCfg = Release|Any CPU + {3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Debug|Any CPU.Build.0 = Debug|Any CPU + {3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Debug|NonWindows.ActiveCfg = Debug|Any CPU + {3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Debug|NonWindows.Build.0 = Debug|Any CPU + {3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Release|Any CPU.ActiveCfg = Release|Any CPU + {3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Release|Any CPU.Build.0 = Release|Any CPU + {3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Release|NonWindows.ActiveCfg = Release|Any CPU + {3D303454-7DB0-4F9F-BD9E-07F09D3C70E3}.Release|NonWindows.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -106,6 +117,7 @@ Global {7E8A5179-F94C-410F-8BBE-FDAAA95A19C3} = {36CCE840-6FE5-4DB9-A8D5-8CF3CB6D342A} {744BE74F-8C4A-49E8-9683-52D987224285} = {36CCE840-6FE5-4DB9-A8D5-8CF3CB6D342A} {0E067B66-C2EC-4106-87D2-5310CFCDC5B8} = {36CCE840-6FE5-4DB9-A8D5-8CF3CB6D342A} + {3D303454-7DB0-4F9F-BD9E-07F09D3C70E3} = {36CCE840-6FE5-4DB9-A8D5-8CF3CB6D342A} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {E3944F6A-384B-4B0F-B93F-3BD513DC57BD} diff --git a/global.json b/global.json index 7bf6fc99..e5187f36 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "6.0.401", + "version": "7.0.100", "rollForward": "patch", "allowPrerelease": false } diff --git a/test/GenerationSandbox.Tests/GenerationSandbox.Tests.csproj b/test/GenerationSandbox.Tests/GenerationSandbox.Tests.csproj index 81e95e21..d2c667b5 100644 --- a/test/GenerationSandbox.Tests/GenerationSandbox.Tests.csproj +++ b/test/GenerationSandbox.Tests/GenerationSandbox.Tests.csproj @@ -1,41 +1,8 @@  - - - - net6.0-windows7.0;net472 - - x64 - - - - - - - - - - - - - - - - + - - - - - - - - - - - - diff --git a/test/GenerationSandbox.Unmarshalled.Tests/COMTests.cs b/test/GenerationSandbox.Unmarshalled.Tests/COMTests.cs new file mode 100644 index 00000000..388e88c6 --- /dev/null +++ b/test/GenerationSandbox.Unmarshalled.Tests/COMTests.cs @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#pragma warning disable IDE0005 + +using Windows.Win32; +using Windows.Win32.System.Com; + +public class COMTests +{ +#if NET7_0_OR_GREATER + [Fact] + public void COMStaticGuid() + { + Assert.Equal(typeof(IPersistFile).GUID, IPersistFile.IID_Guid); + Assert.Equal(typeof(IPersistFile).GUID, GetGuid()); + } + + private static Guid GetGuid() + where T : IComIID + => T.Guid; +#endif +} diff --git a/test/GenerationSandbox.Unmarshalled.Tests/GenerationSandbox.Unmarshalled.Tests.csproj b/test/GenerationSandbox.Unmarshalled.Tests/GenerationSandbox.Unmarshalled.Tests.csproj new file mode 100644 index 00000000..7dd830bc --- /dev/null +++ b/test/GenerationSandbox.Unmarshalled.Tests/GenerationSandbox.Unmarshalled.Tests.csproj @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/test/GenerationSandbox.Unmarshalled.Tests/NativeMethods.json b/test/GenerationSandbox.Unmarshalled.Tests/NativeMethods.json new file mode 100644 index 00000000..11e28dd2 --- /dev/null +++ b/test/GenerationSandbox.Unmarshalled.Tests/NativeMethods.json @@ -0,0 +1,6 @@ +{ + "$schema": "..\\..\\src\\Microsoft.Windows.CsWin32\\settings.schema.json", + "emitSingleFile": true, + "multiTargetingFriendlyAPIs": true, + "allowMarshaling": false +} diff --git a/test/GenerationSandbox.Unmarshalled.Tests/NativeMethods.txt b/test/GenerationSandbox.Unmarshalled.Tests/NativeMethods.txt new file mode 100644 index 00000000..c110008a --- /dev/null +++ b/test/GenerationSandbox.Unmarshalled.Tests/NativeMethods.txt @@ -0,0 +1 @@ +IPersistFile diff --git a/test/GenerationSandbox.props b/test/GenerationSandbox.props new file mode 100644 index 00000000..871a9192 --- /dev/null +++ b/test/GenerationSandbox.props @@ -0,0 +1,37 @@ + + + + + net7.0-windows7.0;net6.0-windows7.0;net472 + + x64 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +