From 6098216a76dcb04970e0e9f5921cc0045e469be4 Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 24 Feb 2022 14:11:27 -0800 Subject: [PATCH 1/3] Emit a GeneratedCodeAttibute on our generated stubs for better auditing purposes. Fixes #64562 --- .../DllImportStubContext.cs | 20 ++++++-- .../TypeNames.cs | 2 + .../AdditionalAttributesOnStub.cs | 51 +++++++++++++++++++ 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/DllImportStubContext.cs b/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/DllImportStubContext.cs index 3bbcbbc51d927b..07870ee1e06815 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/DllImportStubContext.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/DllImportStubContext.cs @@ -42,6 +42,8 @@ public static bool AreCompilationSettingsEqual(StubEnvironment env1, StubEnviron internal sealed class DllImportStubContext : IEquatable { + private static readonly string GeneratorVersion = typeof(DllImportGenerator).Assembly.GetName().Version.ToString(); + // We don't need the warnings around not setting the various // non-nullable fields/properties on this type in the constructor // since we always use a property initializer. @@ -124,17 +126,27 @@ public static DllImportStubContext Create( ImmutableArray.Builder additionalAttrs = ImmutableArray.CreateBuilder(); // Define additional attributes for the stub definition. + additionalAttrs.Add( + AttributeList( + SingletonSeparatedList( + Attribute(ParseName(TypeNames.System_CodeDom_Compiler_GeneratedCodeAttribute), + AttributeArgumentList( + SeparatedList( + new[] + { + AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal("Microsoft.Interop.DllImportGenerator"))), + AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(GeneratorVersion))) + })))))); + if (env.TargetFrameworkVersion >= new Version(5, 0) && !MethodIsSkipLocalsInit(env, method)) { additionalAttrs.Add( AttributeList( - SeparatedList(new[] - { + SingletonSeparatedList( // Adding the skip locals init indiscriminately since the source generator is // targeted at non-blittable method signatures which typically will contain locals // in the generated code. - Attribute(ParseName(TypeNames.System_Runtime_CompilerServices_SkipLocalsInitAttribute)) - }))); + Attribute(ParseName(TypeNames.System_Runtime_CompilerServices_SkipLocalsInitAttribute))))); } return new DllImportStubContext() diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypeNames.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypeNames.cs index 2b7fa363edba4b..6cffa1f9daef97 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypeNames.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/TypeNames.cs @@ -67,5 +67,7 @@ public static string MarshalEx(InteropGenerationOptions options) public const string DefaultDllImportSearchPathsAttribute = "System.Runtime.InteropServices.DefaultDllImportSearchPathsAttribute"; public const string DllImportSearchPath = "System.Runtime.InteropServices.DllImportSearchPath"; + + public const string System_CodeDom_Compiler_GeneratedCodeAttribute = "System.CodeDom.Compiler.GeneratedCodeAttribute"; } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/AdditionalAttributesOnStub.cs b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/AdditionalAttributesOnStub.cs index 640ee220f088bf..720a0abaa83223 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/AdditionalAttributesOnStub.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/DllImportGenerator.UnitTests/AdditionalAttributesOnStub.cs @@ -64,6 +64,57 @@ partial class C Assert.DoesNotContain(stubMethod.GetAttributes(), attr => attr.AttributeClass!.ToDisplayString() == typeof(SkipLocalsInitAttribute).FullName); } + [ConditionalFact] + public async Task GeneratedCodeAdded() + { + string source = @" +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +[assembly:DisableRuntimeMarshalling] +partial class C +{ + [GeneratedDllImportAttribute(""DoesNotExist"")] + public static partial S Method(); +} + +[NativeMarshalling(typeof(Native))] +struct S +{ +} + +struct Native +{ + public Native(S s) { } + public S ToManaged() { return default; } +}"; + Compilation comp = await TestUtils.CreateCompilation(source); + + Compilation newComp = TestUtils.RunGenerators(comp, out _, new Microsoft.Interop.DllImportGenerator()); + + ITypeSymbol c = newComp.GetTypeByMetadataName("C")!; + IMethodSymbol stubMethod = c.GetMembers().OfType().Single(m => m.Name == "Method"); + Assert.Contains(stubMethod.GetAttributes(), attr => attr.AttributeClass!.ToDisplayString() == typeof(System.CodeDom.Compiler.GeneratedCodeAttribute).FullName); + } + + [ConditionalFact] + public async Task GeneratedCodeNotAddedOnForwardingStub() + { + string source = @" +using System.Runtime.InteropServices; +partial class C +{ + [GeneratedDllImportAttribute(""DoesNotExist"")] + public static partial void Method(); +}"; + Compilation comp = await TestUtils.CreateCompilation(source); + + Compilation newComp = TestUtils.RunGenerators(comp, out _, new Microsoft.Interop.DllImportGenerator()); + + ITypeSymbol c = newComp.GetTypeByMetadataName("C")!; + IMethodSymbol stubMethod = c.GetMembers().OfType().Single(m => m.Name == "Method"); + Assert.DoesNotContain(stubMethod.GetAttributes(), attr => attr.AttributeClass!.ToDisplayString() == typeof(System.CodeDom.Compiler.GeneratedCodeAttribute).FullName); + } + public static IEnumerable GetDownlevelTargetFrameworks() { yield return new object[] { TestTargetFramework.Net, true }; From 16c2a0896d6b26438f023bdc768e7cbe4b03360b Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Thu, 24 Feb 2022 16:43:08 -0800 Subject: [PATCH 2/3] Only emit GeneratedCodeAttribute for known target frameworks --- .../DllImportStubContext.cs | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/DllImportStubContext.cs b/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/DllImportStubContext.cs index 07870ee1e06815..efa75aaca011b2 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/DllImportStubContext.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/DllImportStubContext.cs @@ -125,18 +125,21 @@ public static DllImportStubContext Create( ImmutableArray.Builder additionalAttrs = ImmutableArray.CreateBuilder(); - // Define additional attributes for the stub definition. - additionalAttrs.Add( - AttributeList( - SingletonSeparatedList( - Attribute(ParseName(TypeNames.System_CodeDom_Compiler_GeneratedCodeAttribute), - AttributeArgumentList( - SeparatedList( - new[] - { - AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal("Microsoft.Interop.DllImportGenerator"))), - AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(GeneratorVersion))) - })))))); + if (env.TargetFramework != TargetFramework.Unknown) + { + // Define additional attributes for the stub definition. + additionalAttrs.Add( + AttributeList( + SingletonSeparatedList( + Attribute(ParseName(TypeNames.System_CodeDom_Compiler_GeneratedCodeAttribute), + AttributeArgumentList( + SeparatedList( + new[] + { + AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal("Microsoft.Interop.DllImportGenerator"))), + AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(GeneratorVersion))) + })))))); + } if (env.TargetFrameworkVersion >= new Version(5, 0) && !MethodIsSkipLocalsInit(env, method)) { From 44ebd6d179bcdc5e3beb9f6ac70a34e79023f6ce Mon Sep 17 00:00:00 2001 From: Jeremy Koritzinsky Date: Mon, 28 Feb 2022 09:49:28 -0800 Subject: [PATCH 3/3] PR feedback. --- .../src/ILLink/ILLink.LinkAttributes.Shared.xml | 3 +++ .../gen/DllImportGenerator/DllImportStubContext.cs | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.LinkAttributes.Shared.xml b/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.LinkAttributes.Shared.xml index 38f8ffdbae7f02..efd2492becf342 100644 --- a/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.LinkAttributes.Shared.xml +++ b/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.LinkAttributes.Shared.xml @@ -268,6 +268,9 @@ + + + diff --git a/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/DllImportStubContext.cs b/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/DllImportStubContext.cs index efa75aaca011b2..bdea2c6fb47018 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/DllImportStubContext.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/DllImportGenerator/DllImportStubContext.cs @@ -42,6 +42,8 @@ public static bool AreCompilationSettingsEqual(StubEnvironment env1, StubEnviron internal sealed class DllImportStubContext : IEquatable { + private static readonly string GeneratorName = typeof(DllImportGenerator).Assembly.GetName().Name; + private static readonly string GeneratorVersion = typeof(DllImportGenerator).Assembly.GetName().Version.ToString(); // We don't need the warnings around not setting the various @@ -136,7 +138,7 @@ public static DllImportStubContext Create( SeparatedList( new[] { - AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal("Microsoft.Interop.DllImportGenerator"))), + AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(GeneratorName))), AttributeArgument(LiteralExpression(SyntaxKind.StringLiteralExpression, Literal(GeneratorVersion))) })))))); }