From a745c27498567ad2e8fffe73945d56ff2273ea87 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Wed, 2 Aug 2023 10:02:44 +0200 Subject: [PATCH 1/6] Verify synthesized `RequiresLocationAttribute` cannot be accessed by fnptrs --- .../Semantics/RefReadonlyParameterTests.cs | 148 +++++++++++++++++- 1 file changed, 146 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs b/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs index d6b6e7a235ee8..8d732a9dfb834 100644 --- a/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs @@ -1191,13 +1191,158 @@ public unsafe void M(delegate* p) { } } """; var comp = CreateCompilation(source, options: TestOptions.UnsafeDebugDll); - comp.MakeTypeMissing(WellKnownType.System_Runtime_CompilerServices_RequiresLocationAttribute); comp.VerifyDiagnostics( // (3,36): error CS0518: Predefined type 'System.Runtime.CompilerServices.RequiresLocationAttribute' is not defined or imported // public unsafe void M(delegate* p) { } Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "ref readonly int").WithArguments("System.Runtime.CompilerServices.RequiresLocationAttribute").WithLocation(3, 36)); } + [Fact] + public void FunctionPointer_NoAttribute_PlusNormalMethod() + { + var source = """ + class C + { + public unsafe void M1(delegate* p) { } + public void M2(ref readonly int p) { } + } + """; + var comp = CreateCompilation(source, options: TestOptions.UnsafeDebugDll); + comp.VerifyDiagnostics( + // (3,37): error CS0518: Predefined type 'System.Runtime.CompilerServices.RequiresLocationAttribute' is not defined or imported + // public unsafe void M1(delegate* p) { } + Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "ref readonly int").WithArguments("System.Runtime.CompilerServices.RequiresLocationAttribute").WithLocation(3, 37)); + } + + [Fact] + public void FunctionPointer_InternalAttribute() + { + // Attribute is synthesized for Assembly1, but it's not visible to Assembly2. + var source1 = """ + [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Assembly2")] + internal class C + { + public void M(ref readonly int p) { } + } + """; + var comp1 = CreateCompilation(source1, assemblyName: "Assembly1").VerifyDiagnostics(); + var comp1Ref = comp1.EmitToImageReference(); + var source2 = """ + class D + { + public unsafe object M(delegate* p) + { + var c = new C(); + int x = 5; + c.M(in x); + var attr = new System.Runtime.CompilerServices.RequiresLocationAttribute(); + return attr; + } + } + """; + var comp2 = CreateCompilation(source2, new[] { comp1Ref }, assemblyName: "Assembly2", options: TestOptions.UnsafeDebugDll); + comp2.VerifyDiagnostics( + // (3,38): error CS0518: Predefined type 'System.Runtime.CompilerServices.RequiresLocationAttribute' is not defined or imported + // public unsafe object M(delegate* p) + Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "ref readonly int").WithArguments("System.Runtime.CompilerServices.RequiresLocationAttribute").WithLocation(3, 38), + // (8,56): error CS0234: The type or namespace name 'RequiresLocationAttribute' does not exist in the namespace 'System.Runtime.CompilerServices' (are you missing an assembly reference?) + // var attr = new System.Runtime.CompilerServices.RequiresLocationAttribute(); + Diagnostic(ErrorCode.ERR_DottedTypeNameNotFoundInNS, "RequiresLocationAttribute").WithArguments("RequiresLocationAttribute", "System.Runtime.CompilerServices").WithLocation(8, 56)); + + // Assembly1 defines the attribute in source and has IVT to Assembly2, so the attribute is visible to Assembly2. + var comp1b = CreateCompilation(new[] { source1, RequiresLocationAttributeDefinition }, assemblyName: "Assembly1").VerifyDiagnostics(); + var comp1bRef = comp1b.EmitToImageReference(); + var comp2b = CreateCompilation(source2, new[] { comp1bRef }, assemblyName: "Assembly2", options: TestOptions.UnsafeDebugDll); + comp2b.VerifyDiagnostics(); + + // Assembly1 defines the attribute in source but doesn't have IVT to Assembly3, so the attribute isn't visible to Assembly3. + var source3 = """ + class D + { + public unsafe object M(delegate* p) + { + var attr = new System.Runtime.CompilerServices.RequiresLocationAttribute(); + return attr; + } + } + """; + var comp3 = CreateCompilation(source3, new[] { comp1bRef }, assemblyName: "Assembly3", options: TestOptions.UnsafeDebugDll); + comp3.VerifyDiagnostics( + // (3,38): error CS0518: Predefined type 'System.Runtime.CompilerServices.RequiresLocationAttribute' is not defined or imported + // public unsafe object M(delegate* p) + Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound, "ref readonly int").WithArguments("System.Runtime.CompilerServices.RequiresLocationAttribute").WithLocation(3, 38), + // (5,56): error CS0122: 'RequiresLocationAttribute' is inaccessible due to its protection level + // var attr = new System.Runtime.CompilerServices.RequiresLocationAttribute(); + Diagnostic(ErrorCode.ERR_BadAccess, "RequiresLocationAttribute").WithArguments("System.Runtime.CompilerServices.RequiresLocationAttribute").WithLocation(5, 56)); + } + + [Fact] + public void FunctionPointer_DefineManually_LaterDefinedInRuntime() + { + // Library defines an API with function pointer, manually declaring the attribute. + var source1 = """ + public class C + { + public unsafe void M(delegate* f) + { + int x = 123; + f(in x); + } + } + """; + var comp1v1 = CreateCompilation(new[] { source1, RequiresLocationAttributeDefinition }, assemblyName: "Assembly1", options: TestOptions.UnsafeReleaseDll); + comp1v1.VerifyDiagnostics(); + var comp1v1Ref = comp1v1.EmitToImageReference(); + + // Consumer can use the API. + var source2 = """ + public class D + { + public unsafe void M() + { + new C().M(&F); + } + static void F(ref readonly int x) => System.Console.Write("F" + x); + } + """; + var comp2 = CreateCompilation(source2, new[] { comp1v1Ref }, options: TestOptions.UnsafeReleaseDll).VerifyDiagnostics(); + var comp2Ref = comp2.EmitToImageReference(); + + var source3 = """ + try + { + new D().M(); + } + catch (System.MissingMethodException e) + { + System.Console.Write(e.Message); + } + """; + CompileAndVerify(source3, new[] { comp1v1Ref, comp2Ref }, expectedOutput: "F123").VerifyDiagnostics(); + + // .NET runtime declares the attribute. + var source4 = """ + namespace System.Runtime.CompilerServices + { + [AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false, Inherited = false)] + public sealed class RequiresLocationAttribute : Attribute + { + } + } + """; + var comp4 = CreateCompilation(source4).VerifyDiagnostics(); + var comp4Ref = comp4.EmitToImageReference(); + + // Library is recompiled against the newest runtime. + var comp1v2 = CreateCompilation(source1, new[] { comp4Ref }, assemblyName: "Assembly1", options: TestOptions.UnsafeReleaseDll); + comp1v2.VerifyDiagnostics(); + var comp1v2Ref = comp1v2.EmitToImageReference(); + + // That breaks the consumer. + CompileAndVerify(source3, new[] { comp1v2Ref, comp2Ref, comp4Ref }, + expectedOutput: "Method not found: 'Void C.M(Void (Int32 ByRef))'.").VerifyDiagnostics(); + } + [Fact] public void FunctionPointer_Local() { @@ -1231,7 +1376,6 @@ unsafe void M() } """; var comp = CreateCompilation(source, options: TestOptions.UnsafeDebugDll); - comp.MakeTypeMissing(WellKnownType.System_Runtime_CompilerServices_RequiresLocationAttribute); comp.VerifyDiagnostics( // (5,19): error CS0518: Predefined type 'System.Runtime.CompilerServices.RequiresLocationAttribute' is not defined or imported // delegate* p = null; From 036a91b7e0ca1cc8b9e1a1d9d7516b1b03717a4b Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Wed, 2 Aug 2023 12:52:24 +0200 Subject: [PATCH 2/6] Verify which attribute is emitted as modopt --- .../Semantics/RefReadonlyParameterTests.cs | 62 ++++++++++++++++++- 1 file changed, 59 insertions(+), 3 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs b/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs index 8d732a9dfb834..c5c6430540f6d 100644 --- a/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs @@ -4,6 +4,7 @@ using System; using System.Linq; +using ICSharpCode.Decompiler.Metadata; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Symbols.Retargeting; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -1315,7 +1316,7 @@ public unsafe void M() } catch (System.MissingMethodException e) { - System.Console.Write(e.Message); + System.Console.Write(e.Message.Contains("Void C.M(Void (Int32 ByRef))")); } """; CompileAndVerify(source3, new[] { comp1v1Ref, comp2Ref }, expectedOutput: "F123").VerifyDiagnostics(); @@ -1339,8 +1340,63 @@ public sealed class RequiresLocationAttribute : Attribute var comp1v2Ref = comp1v2.EmitToImageReference(); // That breaks the consumer. - CompileAndVerify(source3, new[] { comp1v2Ref, comp2Ref, comp4Ref }, - expectedOutput: "Method not found: 'Void C.M(Void (Int32 ByRef))'.").VerifyDiagnostics(); + CompileAndVerify(source3, new[] { comp1v2Ref, comp2Ref, comp4Ref }, expectedOutput: "True").VerifyDiagnostics(); + } + + [Fact] + public void FunctionPointer_DefineManually_AndInReference() + { + var source1 = """ + namespace System.Runtime.CompilerServices + { + [AttributeUsage(AttributeTargets.Parameter, AllowMultiple = false, Inherited = false)] + public sealed class RequiresLocationAttribute : Attribute + { + } + } + """; + var comp1 = CreateCompilation(source1, assemblyName: "Assembly1").VerifyDiagnostics(); + var comp1Ref = comp1.EmitToImageReference(); + + // Attribute declared both in the same assembly and in a reference. + var source2 = """ + public class C + { + public unsafe void M(delegate* f) { } + } + """; + var comp2 = CreateCompilation(new[] { source2, RequiresLocationAttributeDefinition }, new[] { comp1Ref }, + assemblyName: "Assembly2", options: TestOptions.UnsafeReleaseDll); + CompileAndVerify(comp2, sourceSymbolValidator: verify2, symbolValidator: verify2).VerifyDiagnostics(); + + static void verify2(ModuleSymbol m) + { + Assert.NotNull(m.GlobalNamespace.GetMember(RequiresLocationAttributeQualifiedName)); + + var modifier = verify(m); + Assert.Equal("Assembly2", modifier.ContainingAssembly.Name); + } + + // Attribute declared only in a reference. + var comp3 = CreateCompilation(source2, new[] { comp1Ref }, assemblyName: "Assembly3", options: TestOptions.UnsafeReleaseDll); + CompileAndVerify(comp3, sourceSymbolValidator: verify3, symbolValidator: verify3).VerifyDiagnostics(); + + static void verify3(ModuleSymbol m) + { + Assert.Null(m.GlobalNamespace.GetMember(RequiresLocationAttributeQualifiedName)); + + var modifier = verify(m); + Assert.Equal("Assembly1", modifier.ContainingAssembly.Name); + } + + static INamedTypeSymbol verify(ModuleSymbol m) + { + var p = m.GlobalNamespace.GetMember("C.M").Parameters.Single(); + var ptr = (FunctionPointerTypeSymbol)p.Type; + var p2 = ptr.Signature.Parameters.Single(); + VerifyRefReadonlyParameter(p2, customModifiers: VerifyModifiers.RequiresLocation); + return p2.RefCustomModifiers.Single().Modifier; + } } [Fact] From 2bddd9ab5f87c48a55a5ee6fc32d960b04c50998 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Wed, 2 Aug 2023 16:08:08 +0200 Subject: [PATCH 3/6] Test type forwarding --- .../Emit2/Semantics/RefReadonlyParameterTests.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs b/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs index c5c6430540f6d..3534f7b045efe 100644 --- a/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs @@ -1341,6 +1341,22 @@ public sealed class RequiresLocationAttribute : Attribute // That breaks the consumer. CompileAndVerify(source3, new[] { comp1v2Ref, comp2Ref, comp4Ref }, expectedOutput: "True").VerifyDiagnostics(); + + // Unless the library adds type forwarding. + var source5 = """ + using System.Runtime.CompilerServices; + [assembly: TypeForwardedToAttribute(typeof(RequiresLocationAttribute))] + """; + var comp1v3 = CreateCompilation(new[] { source1, source5 }, new[] { comp4Ref }, assemblyName: "Assembly1", options: TestOptions.UnsafeReleaseDll); + comp1v3.VerifyDiagnostics(); + var comp1v3Ref = comp1v3.EmitToImageReference(); + CompileAndVerify(source3, new[] { comp1v3Ref, comp2Ref, comp4Ref }, expectedOutput: "F123").VerifyDiagnostics(); + + // Or keeps the manual attribute definition. + var comp1v4 = CreateCompilation(new[] { source1, RequiresLocationAttributeDefinition }, new[] { comp4Ref }, assemblyName: "Assembly1", options: TestOptions.UnsafeReleaseDll); + comp1v4.VerifyDiagnostics(); + var comp1v4Ref = comp1v4.EmitToImageReference(); + CompileAndVerify(source3, new[] { comp1v4Ref, comp2Ref, comp4Ref }, expectedOutput: "F123").VerifyDiagnostics(); } [Fact] From 6f44f7bf5bf730872f597604b9141c4444fc74b1 Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Wed, 2 Aug 2023 19:41:02 +0200 Subject: [PATCH 4/6] Remove unnecessary `using` --- .../CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs b/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs index 3534f7b045efe..ea950ec9b448f 100644 --- a/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs @@ -4,7 +4,6 @@ using System; using System.Linq; -using ICSharpCode.Decompiler.Metadata; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Symbols.Retargeting; using Microsoft.CodeAnalysis.CSharp.Syntax; From e259e721d74368d11790da0d9839ef649f97af8b Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Tue, 8 Aug 2023 12:11:00 +0200 Subject: [PATCH 5/6] Verify modopt's assembly name --- .../Semantics/RefReadonlyParameterTests.cs | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs b/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs index ea950ec9b448f..b742b5a82bdd5 100644 --- a/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs @@ -1292,6 +1292,7 @@ public unsafe void M(delegate* f) """; var comp1v1 = CreateCompilation(new[] { source1, RequiresLocationAttributeDefinition }, assemblyName: "Assembly1", options: TestOptions.UnsafeReleaseDll); comp1v1.VerifyDiagnostics(); + verifyModoptFromAssembly(comp1v1, "Assembly1"); var comp1v1Ref = comp1v1.EmitToImageReference(); // Consumer can use the API. @@ -1306,6 +1307,7 @@ public unsafe void M() } """; var comp2 = CreateCompilation(source2, new[] { comp1v1Ref }, options: TestOptions.UnsafeReleaseDll).VerifyDiagnostics(); + verifyModoptFromAssembly(comp2, "Assembly1"); var comp2Ref = comp2.EmitToImageReference(); var source3 = """ @@ -1318,7 +1320,8 @@ public unsafe void M() System.Console.Write(e.Message.Contains("Void C.M(Void (Int32 ByRef))")); } """; - CompileAndVerify(source3, new[] { comp1v1Ref, comp2Ref }, expectedOutput: "F123").VerifyDiagnostics(); + var verifier3v1 = CompileAndVerify(source3, new[] { comp1v1Ref, comp2Ref }, expectedOutput: "F123").VerifyDiagnostics(); + verifyModoptFromAssembly(verifier3v1.Compilation, "Assembly1"); // .NET runtime declares the attribute. var source4 = """ @@ -1330,16 +1333,18 @@ public sealed class RequiresLocationAttribute : Attribute } } """; - var comp4 = CreateCompilation(source4).VerifyDiagnostics(); + var comp4 = CreateCompilation(source4, assemblyName: "Assembly4").VerifyDiagnostics(); var comp4Ref = comp4.EmitToImageReference(); // Library is recompiled against the newest runtime. var comp1v2 = CreateCompilation(source1, new[] { comp4Ref }, assemblyName: "Assembly1", options: TestOptions.UnsafeReleaseDll); comp1v2.VerifyDiagnostics(); + verifyModoptFromAssembly(comp1v2, "Assembly4"); var comp1v2Ref = comp1v2.EmitToImageReference(); // That breaks the consumer. - CompileAndVerify(source3, new[] { comp1v2Ref, comp2Ref, comp4Ref }, expectedOutput: "True").VerifyDiagnostics(); + var verifier3v2 = CompileAndVerify(source3, new[] { comp1v2Ref, comp2Ref, comp4Ref }, expectedOutput: "True").VerifyDiagnostics(); + verifyModoptFromAssembly(verifier3v1.Compilation, "Assembly1"); // Unless the library adds type forwarding. var source5 = """ @@ -1348,14 +1353,26 @@ public sealed class RequiresLocationAttribute : Attribute """; var comp1v3 = CreateCompilation(new[] { source1, source5 }, new[] { comp4Ref }, assemblyName: "Assembly1", options: TestOptions.UnsafeReleaseDll); comp1v3.VerifyDiagnostics(); + verifyModoptFromAssembly(comp1v3, "Assembly4"); var comp1v3Ref = comp1v3.EmitToImageReference(); CompileAndVerify(source3, new[] { comp1v3Ref, comp2Ref, comp4Ref }, expectedOutput: "F123").VerifyDiagnostics(); // Or keeps the manual attribute definition. var comp1v4 = CreateCompilation(new[] { source1, RequiresLocationAttributeDefinition }, new[] { comp4Ref }, assemblyName: "Assembly1", options: TestOptions.UnsafeReleaseDll); comp1v4.VerifyDiagnostics(); + verifyModoptFromAssembly(comp1v4, "Assembly1"); var comp1v4Ref = comp1v4.EmitToImageReference(); CompileAndVerify(source3, new[] { comp1v4Ref, comp2Ref, comp4Ref }, expectedOutput: "F123").VerifyDiagnostics(); + + static void verifyModoptFromAssembly(Compilation comp, string assemblyName) + { + var f = ((CSharpCompilation)comp).GetMember("C.M").Parameters.Single(); + var p = ((FunctionPointerTypeSymbol)f.Type).Signature.Parameters.Single(); + var mod = p.RefCustomModifiers.Single(); + Assert.True(mod.IsOptional); + Assert.Equal(RequiresLocationAttributeQualifiedName, mod.Modifier.ToTestDisplayString()); + Assert.Equal(assemblyName, mod.Modifier.ContainingAssembly.Name); + } } [Fact] From 35f669e9bee7250218206e42f2f61aada0bbd03f Mon Sep 17 00:00:00 2001 From: Jan Jones Date: Tue, 8 Aug 2023 14:48:56 +0200 Subject: [PATCH 6/6] Fix copy-paste mistake --- .../CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs b/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs index 4f75e191fed11..4ac57b8fa31fc 100644 --- a/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs +++ b/src/Compilers/CSharp/Test/Emit2/Semantics/RefReadonlyParameterTests.cs @@ -1344,7 +1344,7 @@ public sealed class RequiresLocationAttribute : Attribute // That breaks the consumer. var verifier3v2 = CompileAndVerify(source3, new[] { comp1v2Ref, comp2Ref, comp4Ref }, expectedOutput: "True").VerifyDiagnostics(); - verifyModoptFromAssembly(verifier3v1.Compilation, "Assembly1"); + verifyModoptFromAssembly(verifier3v2.Compilation, "Assembly4"); // Unless the library adds type forwarding. var source5 = """