From cc280d5acce7096381d29f1b52882beb242e618b Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 20 May 2022 12:32:54 -0700 Subject: [PATCH 1/8] Convert buffer calculations to checked statements. --- .../Marshalling/AnsiStringMarshaller.cs | 4 +++- .../InteropServices/Marshalling/ArrayMarshaller.cs | 10 ++++++++-- .../Marshalling/PointerArrayMarshaller.cs | 6 ++++-- .../Marshalling/Utf8StringMarshaller.cs | 3 ++- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/AnsiStringMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/AnsiStringMarshaller.cs index 5522fa8bb55ed..bbe84977870ea 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/AnsiStringMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/AnsiStringMarshaller.cs @@ -44,8 +44,10 @@ public AnsiStringMarshaller(string? str, Span buffer) return; } + int maxBytesNeeded = checked(Marshal.SystemMaxDBCSCharSize * str.Length); + // >= for null terminator - if ((long)Marshal.SystemMaxDBCSCharSize * str.Length >= buffer.Length) + if (maxBytesNeeded >= buffer.Length) { // Calculate accurate byte count when the provided stack-allocated buffer is not sufficient int exactByteCount = Marshal.GetAnsiStringByteCount(str); // Includes null terminator diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/ArrayMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/ArrayMarshaller.cs index 0a98f2194a8d0..ac65ab279cc1e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/ArrayMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/ArrayMarshaller.cs @@ -66,7 +66,8 @@ public ArrayMarshaller(T[]? array, Span buffer, int sizeOfNativeElement) _managedArray = array; // Always allocate at least one byte when the array is zero-length. - int spaceToAllocate = Math.Max(array.Length * _sizeOfNativeElement, 1); + int bufferSize = checked(array.Length * _sizeOfNativeElement); + int spaceToAllocate = Math.Max(bufferSize, 1); if (spaceToAllocate <= buffer.Length) { _span = buffer[0..spaceToAllocate]; @@ -107,7 +108,12 @@ public ArrayMarshaller(T[]? array, Span buffer, int sizeOfNativeElement) /// public ReadOnlySpan GetNativeValuesSource(int length) { - return _allocatedMemory == IntPtr.Zero ? default : _span = new Span((void*)_allocatedMemory, length * _sizeOfNativeElement); + if (_allocatedMemory == IntPtr.Zero) + return default; + + int allocatedSize = checked(length * _sizeOfNativeElement); + _span = new Span((void*)_allocatedMemory, allocatedSize); + return _span; } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/PointerArrayMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/PointerArrayMarshaller.cs index 7399748bd1a1a..18e1bd162847e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/PointerArrayMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/PointerArrayMarshaller.cs @@ -66,7 +66,8 @@ public PointerArrayMarshaller(T*[]? array, Span buffer, int sizeOfNativeEl _managedArray = array; // Always allocate at least one byte when the array is zero-length. - int spaceToAllocate = Math.Max(array.Length * _sizeOfNativeElement, 1); + int bufferSize = checked(array.Length * _sizeOfNativeElement); + int spaceToAllocate = Math.Max(bufferSize, 1); if (spaceToAllocate <= buffer.Length) { _span = buffer[0..spaceToAllocate]; @@ -117,7 +118,8 @@ public ReadOnlySpan GetNativeValuesSource(int length) if (_allocatedMemory == IntPtr.Zero) return default; - _span = new Span((void*)_allocatedMemory, length * _sizeOfNativeElement); + int allocatedSize = checked(length * _sizeOfNativeElement); + _span = new Span((void*)_allocatedMemory, allocatedSize); return _span; } diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/Utf8StringMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/Utf8StringMarshaller.cs index be37a40586f92..31daa608be9a1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/Utf8StringMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/Utf8StringMarshaller.cs @@ -46,9 +46,10 @@ public Utf8StringMarshaller(string? str, Span buffer) } const int MaxUtf8BytesPerChar = 3; + int maxBytesNeeded = checked(MaxUtf8BytesPerChar * str.Length); // >= for null terminator - if ((long)MaxUtf8BytesPerChar * str.Length >= buffer.Length) + if (maxBytesNeeded >= buffer.Length) { // Calculate accurate byte count when the provided stack-allocated buffer is not sufficient int exactByteCount = checked(Encoding.UTF8.GetByteCount(str) + 1); // + 1 for null terminator From ff20439cfeccabe3611f07bdc318a0548a42a985 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 20 May 2022 12:35:09 -0700 Subject: [PATCH 2/8] Add comments to stages. Fix error when setting SetLastError - was ExactSpelling. Remove __ suffix from inner P/Invoke declaration. --- .../LibraryImportGenerator.cs | 9 +++---- .../GeneratedStatements.cs | 24 +++++++++++++++---- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs index 059dbf40c815a..c11cbc02bda4e 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs @@ -412,7 +412,7 @@ private static (MemberDeclarationSyntax, ImmutableArray) GenerateSou ImmutableArray forwardedAttributes = pinvokeStub.ForwardedAttributes; - const string innerPInvokeName = "__PInvoke__"; + const string innerPInvokeName = "__PInvoke"; BlockSyntax code = stubGenerator.GeneratePInvokeBody(innerPInvokeName); @@ -428,10 +428,7 @@ private static (MemberDeclarationSyntax, ImmutableArray) GenerateSou dllImport = dllImport.AddAttributeLists(AttributeList(SeparatedList(forwardedAttributes))); } - dllImport = dllImport.WithLeadingTrivia( - Comment("//"), - Comment("// Local P/Invoke"), - Comment("//")); + dllImport = dllImport.WithLeadingTrivia(Comment("// Local P/Invoke")); code = code.AddStatements(dllImport); return (pinvokeStub.ContainingSyntaxContext.WrapMemberInContainingSyntaxWithUnsafeModifier(PrintGeneratedSource(pinvokeStub.StubMethodSyntaxTemplate, pinvokeStub.SignatureContext, code)), pinvokeStub.Diagnostics.AddRange(diagnostics.Diagnostics)); @@ -514,7 +511,7 @@ private static LocalFunctionStatementSyntax CreateTargetFunctionAsLocalStatement SyntaxKind.StringLiteralExpression, Literal(libraryImportData.EntryPoint ?? stubMethodName))), AttributeArgument( - NameEquals(nameof(DllImportAttribute.ExactSpelling)), + NameEquals(nameof(DllImportAttribute.SetLastError)), null, LiteralExpression( libraryImportData.SetLastError ? SyntaxKind.TrueLiteralExpression : SyntaxKind.FalseLiteralExpression)) diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/GeneratedStatements.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/GeneratedStatements.cs index fd822b6092948..50572f3c6da4d 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/GeneratedStatements.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/GeneratedStatements.cs @@ -71,10 +71,7 @@ private static ImmutableArray GenerateStatementsForStubContext( if (statementsToUpdate.Count > 0) { // Comment separating each stage - SyntaxTriviaList newLeadingTrivia = TriviaList( - Comment($"//"), - Comment($"// {context.CurrentStage}"), - Comment($"//")); + SyntaxTriviaList newLeadingTrivia = GenerateStageTrivia(context.CurrentStage); StatementSyntax firstStatementInStage = statementsToUpdate[0]; newLeadingTrivia = newLeadingTrivia.AddRange(firstStatementInStage.GetLeadingTrivia()); statementsToUpdate[0] = firstStatementInStage.WithLeadingTrivia(newLeadingTrivia); @@ -108,5 +105,24 @@ private static StatementSyntax GenerateStatementForNativeInvoke(BoundGenerators IdentifierName(context.GetIdentifiers(marshallers.NativeReturnMarshaller.TypeInfo).native), invoke)); } + + private static SyntaxTriviaList GenerateStageTrivia(StubCodeContext.Stage stage) + { + string comment = stage switch + { + StubCodeContext.Stage.Setup => "Perform required setup.", + StubCodeContext.Stage.Marshal => "Convert managed data to native data.", + StubCodeContext.Stage.Pin => "Pin data in preparation for calling the P/Invoke.", + StubCodeContext.Stage.Invoke => "Call the P/Invoke.", + StubCodeContext.Stage.Unmarshal => "Convert native data to managed data.", + StubCodeContext.Stage.Cleanup => "Perform required cleanup.", + StubCodeContext.Stage.KeepAlive => "Keep alive any managed objects that need to stay alive across the call.", + StubCodeContext.Stage.GuaranteedUnmarshal => "Convert native data to managed data even in the case of an exception during the non-cleanup phases.", + _ => throw new ArgumentOutOfRangeException(nameof(stage)) + }; + + // Comment separating each stage + return TriviaList(Comment($"// {stage} - {comment}")); + } } } From 949140698356957b608f9c06a594982ba808f5a2 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 20 May 2022 16:42:42 -0700 Subject: [PATCH 3/8] Revert ANSI change and add comment. --- .../InteropServices/Marshalling/AnsiStringMarshaller.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/AnsiStringMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/AnsiStringMarshaller.cs index bbe84977870ea..6d729263bda5e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/AnsiStringMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/AnsiStringMarshaller.cs @@ -44,10 +44,9 @@ public AnsiStringMarshaller(string? str, Span buffer) return; } - int maxBytesNeeded = checked(Marshal.SystemMaxDBCSCharSize * str.Length); - // >= for null terminator - if (maxBytesNeeded >= buffer.Length) + // Use the cast to long to avoid the checked operation + if ((long)Marshal.SystemMaxDBCSCharSize * str.Length >= buffer.Length) { // Calculate accurate byte count when the provided stack-allocated buffer is not sufficient int exactByteCount = Marshal.GetAnsiStringByteCount(str); // Includes null terminator From 6cc0d0342d668540d51d2a0acb3ffd0bef6ea8b6 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 20 May 2022 17:43:01 -0700 Subject: [PATCH 4/8] Address confusion around DllImport forward vs local declaration. --- .../LibraryImportGenerator.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs index c11cbc02bda4e..7fcba4b85d4ef 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs @@ -416,7 +416,7 @@ private static (MemberDeclarationSyntax, ImmutableArray) GenerateSou BlockSyntax code = stubGenerator.GeneratePInvokeBody(innerPInvokeName); - LocalFunctionStatementSyntax dllImport = CreateTargetFunctionAsLocalStatement( + LocalFunctionStatementSyntax dllImport = CreateTargetDllImportAsLocalStatement( stubGenerator, options, pinvokeStub.LibraryImportData, @@ -469,14 +469,14 @@ private static MemberDeclarationSyntax PrintForwarderStub(ContainingSyntax userD .AddAttributeLists( AttributeList( SingletonSeparatedList( - CreateDllImportAttributeFromLibraryImportAttributeData(pinvokeData)))); + CreateForwarderDllImport(pinvokeData)))); MemberDeclarationSyntax toPrint = stub.ContainingSyntaxContext.WrapMemberInContainingSyntaxWithUnsafeModifier(stubMethod); return toPrint; } - private static LocalFunctionStatementSyntax CreateTargetFunctionAsLocalStatement( + private static LocalFunctionStatementSyntax CreateTargetDllImportAsLocalStatement( PInvokeStubCodeGenerator stubGenerator, LibraryImportGeneratorOptions options, LibraryImportData libraryImportData, @@ -488,8 +488,8 @@ private static LocalFunctionStatementSyntax CreateTargetFunctionAsLocalStatement (ParameterListSyntax parameterList, TypeSyntax returnType, AttributeListSyntax returnTypeAttributes) = stubGenerator.GenerateTargetMethodSignatureData(); LocalFunctionStatementSyntax localDllImport = LocalFunctionStatement(returnType, stubTargetName) .AddModifiers( - Token(SyntaxKind.ExternKeyword), Token(SyntaxKind.StaticKeyword), + Token(SyntaxKind.ExternKeyword), Token(SyntaxKind.UnsafeKeyword)) .WithSemicolonToken(Token(SyntaxKind.SemicolonToken)) .WithAttributeLists( @@ -511,10 +511,9 @@ private static LocalFunctionStatementSyntax CreateTargetFunctionAsLocalStatement SyntaxKind.StringLiteralExpression, Literal(libraryImportData.EntryPoint ?? stubMethodName))), AttributeArgument( - NameEquals(nameof(DllImportAttribute.SetLastError)), + NameEquals(nameof(DllImportAttribute.ExactSpelling)), null, - LiteralExpression( - libraryImportData.SetLastError ? SyntaxKind.TrueLiteralExpression : SyntaxKind.FalseLiteralExpression)) + LiteralExpression(SyntaxKind.TrueLiteralExpression)) } ))))))) .WithParameterList(parameterList); @@ -525,7 +524,7 @@ private static LocalFunctionStatementSyntax CreateTargetFunctionAsLocalStatement return localDllImport; } - private static AttributeSyntax CreateDllImportAttributeFromLibraryImportAttributeData(LibraryImportData target) + private static AttributeSyntax CreateForwarderDllImport(LibraryImportData target) { var newAttributeArgs = new List { @@ -539,7 +538,7 @@ private static AttributeSyntax CreateDllImportAttributeFromLibraryImportAttribut AttributeArgument( NameEquals(nameof(DllImportAttribute.ExactSpelling)), null, - CreateBoolExpressionSyntax(true)) + LiteralExpression(SyntaxKind.TrueLiteralExpression)) }; if (target.IsUserDefined.HasFlag(InteropAttributeMember.StringMarshalling)) @@ -549,6 +548,7 @@ private static AttributeSyntax CreateDllImportAttributeFromLibraryImportAttribut ExpressionSyntax value = CreateEnumExpressionSyntax(CharSet.Unicode); newAttributeArgs.Add(AttributeArgument(name, null, value)); } + if (target.IsUserDefined.HasFlag(InteropAttributeMember.SetLastError)) { NameEqualsSyntax name = NameEquals(nameof(DllImportAttribute.SetLastError)); From f4f95a83d107dbf3283ab2bbb2a361fdcb38993a Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 20 May 2022 18:35:14 -0700 Subject: [PATCH 5/8] When possible, use Unsafe.SkipInit over default on out params. --- .../LibraryImportGenerator.cs | 18 ---------- .../PInvokeStubCodeGenerator.cs | 5 ++- ...CollectionElementMarshallingCodeContext.cs | 5 +++ .../ManagedToNativeStubCodeContext.cs | 15 +++++--- .../ICustomNativeTypeMarshallingStrategy.cs | 3 ++ .../StubCodeContext.cs | 18 ++++++++++ .../VariableDeclarations.cs | 35 ++++++++++++++----- 7 files changed, 66 insertions(+), 33 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs index 7fcba4b85d4ef..1c2995cf3f43c 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/LibraryImportGenerator.cs @@ -190,24 +190,6 @@ private static MemberDeclarationSyntax PrintGeneratedSource( .WithBody(stubCode); } - private static TargetFramework DetermineTargetFramework(Compilation compilation, out Version version) - { - IAssemblySymbol systemAssembly = compilation.GetSpecialType(SpecialType.System_Object).ContainingAssembly; - version = systemAssembly.Identity.Version; - - return systemAssembly.Identity.Name switch - { - // .NET Framework - "mscorlib" => TargetFramework.Framework, - // .NET Standard - "netstandard" => TargetFramework.Standard, - // .NET Core (when version < 5.0) or .NET - "System.Runtime" or "System.Private.CoreLib" => - (version.Major < 5) ? TargetFramework.Core : TargetFramework.Net, - _ => TargetFramework.Unknown, - }; - } - private static LibraryImportData? ProcessLibraryImportAttribute(AttributeData attrData) { // Found the LibraryImport, but it has an error so report the error. diff --git a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/PInvokeStubCodeGenerator.cs b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/PInvokeStubCodeGenerator.cs index 8cd0b512a2a34..856a7b4d1c214 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/PInvokeStubCodeGenerator.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/LibraryImportGenerator/PInvokeStubCodeGenerator.cs @@ -69,14 +69,13 @@ public PInvokeStubCodeGenerator( SupportsTargetFramework = true; } - - _context = new ManagedToNativeStubCodeContext(ReturnIdentifier, ReturnIdentifier); + _context = new ManagedToNativeStubCodeContext(environment, ReturnIdentifier, ReturnIdentifier); _marshallers = new BoundGenerators(argTypes, CreateGenerator); if (_marshallers.ManagedReturnMarshaller.Generator.UsesNativeIdentifier(_marshallers.ManagedReturnMarshaller.TypeInfo, _context)) { // If we need a different native return identifier, then recreate the context with the correct identifier before we generate any code. - _context = new ManagedToNativeStubCodeContext(ReturnIdentifier, $"{ReturnIdentifier}{StubCodeContext.GeneratedNativeIdentifierSuffix}"); + _context = new ManagedToNativeStubCodeContext(environment, ReturnIdentifier, $"{ReturnIdentifier}{StubCodeContext.GeneratedNativeIdentifierSuffix}"); } bool noMarshallingNeeded = true; diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/LinearCollectionElementMarshallingCodeContext.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/LinearCollectionElementMarshallingCodeContext.cs index 88dbf737369ee..afa27b08e0e6d 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/LinearCollectionElementMarshallingCodeContext.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/LinearCollectionElementMarshallingCodeContext.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; + namespace Microsoft.Interop { internal sealed record LinearCollectionElementMarshallingCodeContext : StubCodeContext @@ -34,6 +36,9 @@ public LinearCollectionElementMarshallingCodeContext( ParentContext = parentContext; } + public override (TargetFramework framework, Version version) GetTargetFramework() + => ParentContext!.GetTargetFramework(); + /// /// Get managed and native instance identifiers for the /// diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/ManagedToNativeStubCodeContext.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/ManagedToNativeStubCodeContext.cs index 9fd0991d7abc9..36b59f4600eac 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/ManagedToNativeStubCodeContext.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/ManagedToNativeStubCodeContext.cs @@ -14,20 +14,27 @@ public sealed record ManagedToNativeStubCodeContext : StubCodeContext public override bool AdditionalTemporaryStateLivesAcrossStages => true; - public bool SupportsTargetFramework { get; init; } - - public bool StubIsBasicForwarder { get; init; } + private readonly TargetFramework _framework; + private readonly Version _frameworkVersion; private const string InvokeReturnIdentifier = "__invokeRetVal"; private readonly string _returnIdentifier; private readonly string _nativeReturnIdentifier; - public ManagedToNativeStubCodeContext(string returnIdentifier, string nativeReturnIdentifier) + public ManagedToNativeStubCodeContext( + StubEnvironment environment, + string returnIdentifier, + string nativeReturnIdentifier) { + _framework = environment.TargetFramework; + _frameworkVersion = environment.TargetFrameworkVersion; _returnIdentifier = returnIdentifier; _nativeReturnIdentifier = nativeReturnIdentifier; } + public override (TargetFramework framework, Version version) GetTargetFramework() + => (_framework, _frameworkVersion); + public override (string managed, string native) GetIdentifiers(TypePositionInfo info) { // If the info is in the managed return position, then we need to generate a name to use diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ICustomNativeTypeMarshallingStrategy.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ICustomNativeTypeMarshallingStrategy.cs index 1ae581e42729f..e7b15ffe2fe2f 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ICustomNativeTypeMarshallingStrategy.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ICustomNativeTypeMarshallingStrategy.cs @@ -119,6 +119,9 @@ public CustomNativeTypeWithToFromNativeValueContext(StubCodeContext parentContex CurrentStage = parentContext.CurrentStage; } + public override (TargetFramework framework, Version version) GetTargetFramework() + => ParentContext!.GetTargetFramework(); + public override bool SingleFrameSpansNativeContext => ParentContext!.SingleFrameSpansNativeContext; public override bool AdditionalTemporaryStateLivesAcrossStages => ParentContext!.AdditionalTemporaryStateLivesAcrossStages; diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/StubCodeContext.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/StubCodeContext.cs index bea24aa1b62cd..0e9f35be32214 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/StubCodeContext.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/StubCodeContext.cs @@ -64,8 +64,17 @@ public enum Stage GuaranteedUnmarshal } + /// + /// The current stage being generated. + /// public Stage CurrentStage { get; init; } = Stage.Invalid; + /// + /// Gets the currently targeted framework and version for stub code generation. + /// + /// A framework value and version. + public abstract (TargetFramework framework, Version version) GetTargetFramework(); + /// /// The stub emits code that runs in a single stack frame and the frame spans over the native context. /// @@ -91,6 +100,9 @@ public enum Stage /// public StubCodeContext? ParentContext { get; protected init; } + /// + /// Suffix for all generated native identifiers. + /// public const string GeneratedNativeIdentifierSuffix = "_gen_native"; /// @@ -103,6 +115,12 @@ public virtual (string managed, string native) GetIdentifiers(TypePositionInfo i return (info.InstanceIdentifier, $"__{info.InstanceIdentifier.TrimStart('@')}{GeneratedNativeIdentifierSuffix}"); } + /// + /// Compute identifiers that are unique for this generator + /// + /// TypePositionInfo the new identifier is used in service of. + /// Name of variable. + /// New identifier name for use. public virtual string GetAdditionalIdentifier(TypePositionInfo info, string name) { return $"{GetIdentifiers(info).native}__{name}"; diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/VariableDeclarations.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/VariableDeclarations.cs index cc4c0a7a42e37..abd91743b22f1 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/VariableDeclarations.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/VariableDeclarations.cs @@ -29,14 +29,33 @@ public static VariableDeclarations GenerateDeclarationsForManagedToNative(BoundG if (info.RefKind == RefKind.Out) { - // Assign out params to default - initializations.Add(ExpressionStatement( - AssignmentExpression( - SyntaxKind.SimpleAssignmentExpression, - IdentifierName(info.InstanceIdentifier), - LiteralExpression( - SyntaxKind.DefaultLiteralExpression, - Token(SyntaxKind.DefaultKeyword))))); + (TargetFramework fmk, _) = context.GetTargetFramework(); + if (info.ManagedType is not PointerTypeInfo + && fmk is TargetFramework.Net) + { + // Use the Unsafe.SkipInit API when available and + // the managed type is not a pointer. + initializations.Add(ExpressionStatement( + InvocationExpression( + MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, + ParseName(TypeNames.System_Runtime_CompilerServices_Unsafe), + IdentifierName("SkipInit"))) + .WithArgumentList( + ArgumentList(SingletonSeparatedList( + Argument(IdentifierName(info.InstanceIdentifier)) + .WithRefOrOutKeyword(Token(SyntaxKind.OutKeyword))))))); + } + else + { + // Assign out params to default when Unsafe.SkipInit API is unavailable. + initializations.Add(ExpressionStatement( + AssignmentExpression( + SyntaxKind.SimpleAssignmentExpression, + IdentifierName(info.InstanceIdentifier), + LiteralExpression( + SyntaxKind.DefaultLiteralExpression, + Token(SyntaxKind.DefaultKeyword))))); + } } // Declare variables for parameters From e569e2379f54ec37bafafcca02f57eff2a9ef6eb Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 20 May 2022 18:40:58 -0700 Subject: [PATCH 6/8] Apply upcast treatment to Utf8 marshaller as well. --- .../InteropServices/Marshalling/Utf8StringMarshaller.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/Utf8StringMarshaller.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/Utf8StringMarshaller.cs index 31daa608be9a1..e60db5416dfa6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/Utf8StringMarshaller.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshalling/Utf8StringMarshaller.cs @@ -46,10 +46,10 @@ public Utf8StringMarshaller(string? str, Span buffer) } const int MaxUtf8BytesPerChar = 3; - int maxBytesNeeded = checked(MaxUtf8BytesPerChar * str.Length); // >= for null terminator - if (maxBytesNeeded >= buffer.Length) + // Use the cast to long to avoid the checked operation + if ((long)MaxUtf8BytesPerChar * str.Length >= buffer.Length) { // Calculate accurate byte count when the provided stack-allocated buffer is not sufficient int exactByteCount = checked(Encoding.UTF8.GetByteCount(str) + 1); // + 1 for null terminator From 8cabf589960a5358b87be3cfa80d500836e41ceb Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Fri, 20 May 2022 20:28:48 -0700 Subject: [PATCH 7/8] Block byreflike types in SkipInit. --- .../ManagedTypeInfo.cs | 10 ++++++++-- .../VariableDeclarations.cs | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/ManagedTypeInfo.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/ManagedTypeInfo.cs index dfb5278daacc0..b997083799757 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/ManagedTypeInfo.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/ManagedTypeInfo.cs @@ -45,7 +45,11 @@ public static ManagedTypeInfo CreateTypeInfoForTypeSymbol(ITypeSymbol type) { return new DelegateTypeInfo(typeName, diagonsticFormattedName); } - return new SimpleManagedTypeInfo(typeName, diagonsticFormattedName); + if (type.IsValueType) + { + return new ValueTypeInfo(typeName, diagonsticFormattedName, type.IsRefLikeType); + } + return new ReferenceTypeInfo(typeName, diagonsticFormattedName); } } @@ -74,5 +78,7 @@ public sealed record SzArrayType(ManagedTypeInfo ElementTypeInfo) : ManagedTypeI public sealed record DelegateTypeInfo(string FullTypeName, string DiagnosticFormattedName) : ManagedTypeInfo(FullTypeName, DiagnosticFormattedName); - public sealed record SimpleManagedTypeInfo(string FullTypeName, string DiagnosticFormattedName) : ManagedTypeInfo(FullTypeName, DiagnosticFormattedName); + public sealed record ValueTypeInfo(string FullTypeName, string DiagnosticFormattedName, bool IsByRefLike) : ManagedTypeInfo(FullTypeName, DiagnosticFormattedName); + + public sealed record ReferenceTypeInfo(string FullTypeName, string DiagnosticFormattedName) : ManagedTypeInfo(FullTypeName, DiagnosticFormattedName); } diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/VariableDeclarations.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/VariableDeclarations.cs index abd91743b22f1..16176ad4d0310 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/VariableDeclarations.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/VariableDeclarations.cs @@ -31,10 +31,11 @@ public static VariableDeclarations GenerateDeclarationsForManagedToNative(BoundG { (TargetFramework fmk, _) = context.GetTargetFramework(); if (info.ManagedType is not PointerTypeInfo + && info.ManagedType is not ValueTypeInfo { IsByRefLike: true } && fmk is TargetFramework.Net) { - // Use the Unsafe.SkipInit API when available and - // the managed type is not a pointer. + // Use the Unsafe.SkipInit API when available and + // managed type is usable as a generic parameter. initializations.Add(ExpressionStatement( InvocationExpression( MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, @@ -47,7 +48,6 @@ public static VariableDeclarations GenerateDeclarationsForManagedToNative(BoundG } else { - // Assign out params to default when Unsafe.SkipInit API is unavailable. initializations.Add(ExpressionStatement( AssignmentExpression( SyntaxKind.SimpleAssignmentExpression, From f082ffd17ef07b1df8f744835932df3405c62c3a Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 24 May 2022 13:14:42 -0700 Subject: [PATCH 8/8] Update src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/VariableDeclarations.cs --- .../Microsoft.Interop.SourceGeneration/VariableDeclarations.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/VariableDeclarations.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/VariableDeclarations.cs index 16176ad4d0310..2efe913522514 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/VariableDeclarations.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/VariableDeclarations.cs @@ -48,6 +48,7 @@ public static VariableDeclarations GenerateDeclarationsForManagedToNative(BoundG } else { + // Assign out params to default initializations.Add(ExpressionStatement( AssignmentExpression( SyntaxKind.SimpleAssignmentExpression,