From c95fcc6183d6e85b9a1007abe208fb4447a03fb3 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Wed, 23 Sep 2020 16:43:19 -0700 Subject: [PATCH] PR Feedback: * Make UnmanagedCallersOnly decoding consistent with Obsolete * Remove IsValid tracking. This does mean that there are some otherwise bad UnmanagedCallersOnly attributes that we will no longer interpret as "bad" from metadata, but the effort in tracking this very niche scenario for slightly better errors is not worth the additional complexity. * More testing. --- .../Symbols/Metadata/PE/PEMethodSymbol.cs | 4 +- .../CSharp/Portable/Symbols/MethodSymbol.cs | 33 +-- .../Retargeting/RetargetingMethodSymbol.cs | 2 +- .../SourceMethodSymbolWithAttributes.cs | 9 +- .../CodeGen/CodeGenFunctionPointersTests.cs | 273 +++++++++++++++++- .../Core/Portable/MetadataReader/PEModule.cs | 13 +- .../UnmanagedCallersOnlyAttributeData.cs | 23 +- 7 files changed, 301 insertions(+), 56 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs index f73592ba318..09ba1c6d439 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs @@ -1338,7 +1338,7 @@ internal override DiagnosticInfo GetUseSiteDiagnostic() { Debug.Assert(!ReferenceEquals(data, UnmanagedCallersOnlyAttributeData.Uninitialized)); Debug.Assert(!ReferenceEquals(data, UnmanagedCallersOnlyAttributeData.AttributePresentDataNotBound)); - if (!data.IsValid || CheckAndReportValidUnmanagedCallersOnlyTarget(location: null, diagnostics: null)) + if (CheckAndReportValidUnmanagedCallersOnlyTarget(location: null, diagnostics: null)) { result = new CSDiagnosticInfo(ErrorCode.ERR_BindToBogus, this); } @@ -1439,7 +1439,7 @@ internal override ObsoleteAttributeData ObsoleteAttributeData { var containingModule = (PEModuleSymbol)ContainingModule; var unmanagedCallersOnlyData = containingModule.Module.TryGetUnmanagedCallersOnlyAttribute(_handle, new MetadataDecoder(containingModule), - static (name, value) => MethodSymbol.TryDecodeUnmanagedCallersOnlyCallConvsProperty(name, value, location: null, diagnostics: null)); + static (name, value, isProperty) => MethodSymbol.TryDecodeUnmanagedCallersOnlyCallConvsField(name, value, isProperty, location: null, diagnostics: null)); Debug.Assert(!ReferenceEquals(unmanagedCallersOnlyData, UnmanagedCallersOnlyAttributeData.Uninitialized) && !ReferenceEquals(unmanagedCallersOnlyData, UnmanagedCallersOnlyAttributeData.AttributePresentDataNotBound)); diff --git a/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs index 1a2e074cc45..8a7a51500b1 100644 --- a/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs @@ -959,51 +959,49 @@ internal bool CalculateUseSiteDiagnostic(ref DiagnosticInfo result) } #nullable enable - protected static UnmanagedCallersOnlyAttributeData DecodeUnmanagedCallersOnlyAttributeData(CSharpAttributeData attribute, Location? location, DiagnosticBag? diagnostics) + protected static UnmanagedCallersOnlyAttributeData DecodeUnmanagedCallersOnlyAttributeData(CSharpAttributeData attribute, Location location, DiagnosticBag diagnostics) { - Debug.Assert((location is null) == (diagnostics is null)); + Debug.Assert(attribute.AttributeClass is not null); ImmutableHashSet? callingConventionTypes = null; - bool isValid = true; if (attribute.CommonNamedArguments is { IsDefaultOrEmpty: false } namedArgs) { foreach (var (key, value) in attribute.CommonNamedArguments) { - var namedArgumentDecoded = TryDecodeUnmanagedCallersOnlyCallConvsProperty(key, value, location, diagnostics); + // Technically, CIL can define a field and a property with the same name. However, such a + // member results in an Ambiguous Member error, and we never get to piece of code at all. + // See UnmanagedCallersOnly_PropertyAndFieldNamedCallConvs for an example + bool isField = attribute.AttributeClass.GetMembers(key).Any(m => m is FieldSymbol); + + var namedArgumentDecoded = TryDecodeUnmanagedCallersOnlyCallConvsField(key, value, isField, location, diagnostics); if (namedArgumentDecoded.IsCallConvs) { - isValid = isValid && namedArgumentDecoded.IsValid; callingConventionTypes = namedArgumentDecoded.CallConvs; } } } - return UnmanagedCallersOnlyAttributeData.Create(callingConventionTypes, isValid); + return UnmanagedCallersOnlyAttributeData.Create(callingConventionTypes); } - internal static (bool IsCallConvs, ImmutableHashSet? CallConvs, bool IsValid) TryDecodeUnmanagedCallersOnlyCallConvsProperty( + internal static (bool IsCallConvs, ImmutableHashSet? CallConvs) TryDecodeUnmanagedCallersOnlyCallConvsField( string key, TypedConstant value, + bool isField, Location? location, DiagnosticBag? diagnostics) { ImmutableHashSet? callingConventionTypes = null; - bool isValid = true; - - if (!UnmanagedCallersOnlyAttributeData.IsCallConvsTypedConstant(key, in value)) - { - return (false, callingConventionTypes, isValid); - } - if (callingConventionTypes != null) + if (!UnmanagedCallersOnlyAttributeData.IsCallConvsTypedConstant(key, isField, in value)) { - isValid = false; + return (false, callingConventionTypes); } if (value.Values.IsDefaultOrEmpty) { callingConventionTypes = ImmutableHashSet.Empty; - return (true, callingConventionTypes, isValid); + return (true, callingConventionTypes); } var builder = PooledHashSet.GetInstance(); @@ -1015,7 +1013,6 @@ protected static UnmanagedCallersOnlyAttributeData DecodeUnmanagedCallersOnlyAtt { // `{0}` is not a valid calling convention type for 'UnmanagedCallersOnly'. diagnostics?.Add(ErrorCode.ERR_InvalidUnmanagedCallersOnlyCallConv, location!, callConvTypedConstant.ValueInternal ?? "null"); - isValid = false; } else { @@ -1026,7 +1023,7 @@ protected static UnmanagedCallersOnlyAttributeData DecodeUnmanagedCallersOnlyAtt callingConventionTypes = builder.ToImmutableHashSet(); builder.Free(); - return (true, callingConventionTypes, isValid); + return (true, callingConventionTypes); } /// diff --git a/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs index 981c97c731b..6ce69973953 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Retargeting/RetargetingMethodSymbol.cs @@ -244,7 +244,7 @@ public override ImmutableArray GetReturnTypeAttributes() builder.Add((INamedTypeSymbolInternal)RetargetingTranslator.Retarget((NamedTypeSymbol)identifier)); } - data = UnmanagedCallersOnlyAttributeData.Create(builder.ToImmutableHashSet(), data.IsValid); + data = UnmanagedCallersOnlyAttributeData.Create(builder.ToImmutableHashSet()); builder.Free(); } diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbolWithAttributes.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbolWithAttributes.cs index 2dcfcc5ad7e..feca0aaecc3 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbolWithAttributes.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbolWithAttributes.cs @@ -890,14 +890,9 @@ private void DecodeUnmanagedCallersOnlyAttribute(ref DecodeWellKnownAttributeArg bool reportedError = CheckAndReportValidUnmanagedCallersOnlyTarget(arguments.AttributeSyntaxOpt.Name.Location, arguments.Diagnostics); - var returnTypeSyntax = SyntaxNode switch - { - MethodDeclarationSyntax m => m.ReturnType, - LocalFunctionStatementSyntax l => l.ReturnType, - _ => null - }; + var returnTypeSyntax = this.ExtractReturnTypeSyntax(); - if (returnTypeSyntax == null) + if (returnTypeSyntax is CompilationUnitSyntax && this is not SynthesizedSimpleProgramEntryPointSymbol) { // If there's no syntax for the return type, then we already errored because this isn't a valid // unmanagedcallersonly target (it's a property getter/setter or some other non-regular-method). diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs index c5698e059d1..fb659ec1bbe 100644 --- a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs +++ b/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs @@ -8098,6 +8098,7 @@ class D { unsafe static void M1() { + C.M(); delegate* unmanaged ptr = &C.M; } } @@ -8105,10 +8106,17 @@ unsafe static void M1() comp.Assembly.SetOverrideRuntimeSupportsUnmanagedSignatureCallingConvention(); comp.VerifyDiagnostics( - // (6,42): error CS0570: 'C.M()' is not supported by the language - // delegate* unmanaged ptr = &C.M; - Diagnostic(ErrorCode.ERR_BindToBogus, "C.M", isSuppressed: false).WithArguments("C.M()").WithLocation(6, 42) + // (6,9): error CS8901: 'C.M()' is attributed with 'UnmanagedCallersOnly' and cannot be called directly. Obtain a function pointer to this method. + // C.M(); + Diagnostic(ErrorCode.ERR_UnmanagedCallersOnlyMethodsCannotBeCalledDirectly, "C.M()").WithArguments("C.M()").WithLocation(6, 9) ); + + var c = comp.GetTypeByMetadataName("C"); + var m1 = c.GetMethod("M"); + var unmanagedData = m1.GetUnmanagedCallersOnlyAttributeData(forceComplete: true); + Assert.NotSame(unmanagedData, UnmanagedCallersOnlyAttributeData.Uninitialized); + Assert.NotSame(unmanagedData, UnmanagedCallersOnlyAttributeData.AttributePresentDataNotBound); + Assert.Empty(unmanagedData!.CallingConventionTypes); } [Fact] @@ -8722,20 +8730,22 @@ public void UnmanagedCallersOnlyReferencedFromMetadata_BadTypeInList() } }"; - var comp = CreateCompilationWithIL(@" + var comp = CreateCompilationWithFunctionPointersAndIl(@" class D { - public static void M2() + public unsafe static void M2() { C.M1(); + delegate* unmanaged ptr = &C.M1; } } ", il); + comp.Assembly.SetOverrideRuntimeSupportsUnmanagedSignatureCallingConvention(); comp.VerifyDiagnostics( - // (6,11): error CS0570: 'C.M1()' is not supported by the language + // (6,9): error CS8901: 'C.M1()' is attributed with 'UnmanagedCallersOnly' and cannot be called directly. Obtain a function pointer to this method. // C.M1(); - Diagnostic(ErrorCode.ERR_BindToBogus, "M1").WithArguments("C.M1()").WithLocation(6, 11) + Diagnostic(ErrorCode.ERR_UnmanagedCallersOnlyMethodsCannotBeCalledDirectly, "C.M1()").WithArguments("C.M1()").WithLocation(6, 9) ); var c = comp.GetTypeByMetadataName("C"); @@ -8743,8 +8753,7 @@ public static void M2() var unmanagedData = m1.GetUnmanagedCallersOnlyAttributeData(forceComplete: true); Assert.NotSame(unmanagedData, UnmanagedCallersOnlyAttributeData.Uninitialized); Assert.NotSame(unmanagedData, UnmanagedCallersOnlyAttributeData.AttributePresentDataNotBound); - Assert.False(unmanagedData!.IsValid); - Assert.Empty(unmanagedData.CallingConventionTypes); + Assert.Empty(unmanagedData!.CallingConventionTypes); } [Fact] @@ -10146,6 +10155,252 @@ public UnmanagedCallersOnlyAttribute() ); } + [Fact] + public void UnmanagedCallersOnly_CallConvsAsProperty() + { + string source1 = @" +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +namespace System.Runtime.InteropServices +{ + [AttributeUsage(AttributeTargets.Method, Inherited = false)] + public sealed class UnmanagedCallersOnlyAttribute : Attribute + { + public UnmanagedCallersOnlyAttribute() + { + } + + public Type[] CallConvs { get; set; } + public string EntryPoint; + } +} + +public class C +{ + [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvCdecl) })] + public static void M() {} +}"; + string source2 = @" +class D +{ + unsafe void M2() + { + delegate* unmanaged ptr1 = &C.M; + delegate* unmanaged[Cdecl] ptr2 = &C.M; + } +}"; + var sameComp = CreateCompilationWithFunctionPointers(source1 + source2); + sameComp.Assembly.SetOverrideRuntimeSupportsUnmanagedSignatureCallingConvention(); + sameComp.VerifyDiagnostics( + // (28,50): error CS8786: Calling convention of 'C.M()' is not compatible with 'CDecl'. + // delegate* unmanaged[Cdecl] ptr2 = &C.M; + Diagnostic(ErrorCode.ERR_WrongFuncPtrCallingConvention, "C.M").WithArguments("C.M()", "CDecl").WithLocation(28, 50) + ); + + verifyUnmanagedData(sameComp); + + var refComp = CreateCompilation(source1); + + var differentComp = CreateCompilationWithFunctionPointers(source2, new[] { refComp.EmitToImageReference() }); + differentComp.Assembly.SetOverrideRuntimeSupportsUnmanagedSignatureCallingConvention(); + differentComp.VerifyDiagnostics( + // (7,50): error CS8786: Calling convention of 'C.M()' is not compatible with 'CDecl'. + // delegate* unmanaged[Cdecl] ptr2 = &C.M; + Diagnostic(ErrorCode.ERR_WrongFuncPtrCallingConvention, "C.M").WithArguments("C.M()", "CDecl").WithLocation(7, 50) + ); + + verifyUnmanagedData(differentComp); + + static void verifyUnmanagedData(CSharpCompilation compilation) + { + var c = compilation.GetTypeByMetadataName("C"); + var m = c.GetMethod("M"); + Assert.Empty(m.GetUnmanagedCallersOnlyAttributeData(forceComplete: true)!.CallingConventionTypes); + } + } + + [Fact] + public void UnmanagedCallersOnly_UnrecognizedSignature() + { + string source1 = @" +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +namespace System.Runtime.InteropServices +{ + [AttributeUsage(AttributeTargets.Method, Inherited = false)] + public sealed class UnmanagedCallersOnlyAttribute : Attribute + { + public UnmanagedCallersOnlyAttribute(Type[] CallConvs) + { + } + + public string EntryPoint; + } +} + +public class C +{ + [UnmanagedCallersOnly(CallConvs: new[] { typeof(CallConvCdecl) })] + public static void M() {} +}"; + string source2 = @" +class D +{ + unsafe void M2() + { + delegate* unmanaged ptr1 = &C.M; + delegate* unmanaged[Cdecl] ptr2 = &C.M; + delegate* ptr3 = &C.M; + } +}"; + var sameComp = CreateCompilationWithFunctionPointers(source1 + source2); + sameComp.Assembly.SetOverrideRuntimeSupportsUnmanagedSignatureCallingConvention(); + sameComp.VerifyDiagnostics( + // (26,43): error CS8786: Calling convention of 'C.M()' is not compatible with 'Unmanaged'. + // delegate* unmanaged ptr1 = &C.M; + Diagnostic(ErrorCode.ERR_WrongFuncPtrCallingConvention, "C.M").WithArguments("C.M()", "Unmanaged").WithLocation(26, 43), + // (27,50): error CS8786: Calling convention of 'C.M()' is not compatible with 'CDecl'. + // delegate* unmanaged[Cdecl] ptr2 = &C.M; + Diagnostic(ErrorCode.ERR_WrongFuncPtrCallingConvention, "C.M").WithArguments("C.M()", "CDecl").WithLocation(27, 50) + ); + + verifyUnmanagedData(sameComp); + + var refComp = CreateCompilation(source1); + + var differentComp = CreateCompilationWithFunctionPointers(source2, new[] { refComp.EmitToImageReference() }); + differentComp.Assembly.SetOverrideRuntimeSupportsUnmanagedSignatureCallingConvention(); + differentComp.VerifyDiagnostics( + // (6,43): error CS8786: Calling convention of 'C.M()' is not compatible with 'Unmanaged'. + // delegate* unmanaged ptr1 = &C.M; + Diagnostic(ErrorCode.ERR_WrongFuncPtrCallingConvention, "C.M").WithArguments("C.M()", "Unmanaged").WithLocation(6, 43), + // (7,50): error CS8786: Calling convention of 'C.M()' is not compatible with 'CDecl'. + // delegate* unmanaged[Cdecl] ptr2 = &C.M; + Diagnostic(ErrorCode.ERR_WrongFuncPtrCallingConvention, "C.M").WithArguments("C.M()", "CDecl").WithLocation(7, 50) + ); + + verifyUnmanagedData(differentComp); + + static void verifyUnmanagedData(CSharpCompilation compilation) + { + var c = compilation.GetTypeByMetadataName("C"); + var m = c.GetMethod("M"); + Assert.Null(m.GetUnmanagedCallersOnlyAttributeData(forceComplete: true)); + } + } + + [Fact] + public void UnmanagedCallersOnly_PropertyAndFieldNamedCallConvs() + { + var il = @" +.class public auto ansi sealed beforefieldinit System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute + extends [mscorlib]System.Attribute +{ + .custom instance void [mscorlib]System.AttributeUsageAttribute::.ctor(valuetype [mscorlib]System.AttributeTargets) = ( + 01 00 40 00 00 00 01 00 54 02 09 49 6e 68 65 72 + 69 74 65 64 00 + ) + // Fields + .field public class [mscorlib]System.Type[] CallConvs + .field public string EntryPoint + + // Methods + .method public hidebysig specialname rtspecialname instance void .ctor () cil managed + { + ldarg.0 + call instance void [mscorlib]System.Attribute::.ctor() + ret + } // end of method UnmanagedCallersOnlyAttribute::.ctor + + .method public hidebysig specialname instance class [mscorlib]System.Type[] get_CallConvs () cil managed + { + ldnull + ret + } // end of method UnmanagedCallersOnlyAttribute::get_CallConvs + + .method public hidebysig specialname instance void set_CallConvs ( + class [mscorlib]System.Type[] 'value' + ) cil managed + { + ret + } // end of method UnmanagedCallersOnlyAttribute::set_CallConvs + + // Properties + .property instance class [mscorlib]System.Type[] CallConvs() + { + .get instance class [mscorlib]System.Type[] System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute::get_CallConvs() + .set instance void System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute::set_CallConvs(class [mscorlib]System.Type[]) + } + +} +.class public auto ansi beforefieldinit C + extends [mscorlib]System.Object +{ + // Methods + .method public hidebysig static void M () cil managed + { + // As separate field/property assignments. Property is first. + // [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) }, CallConvs = new[] { typeof(CallConvCdecl) })] + .custom instance void System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute::.ctor() = ( + 01 00 02 00 54 1d 50 09 43 61 6c 6c 43 6f 6e 76 + 73 01 00 00 00 7c 53 79 73 74 65 6d 2e 52 75 6e + 74 69 6d 65 2e 43 6f 6d 70 69 6c 65 72 53 65 72 + 76 69 63 65 73 2e 43 61 6c 6c 43 6f 6e 76 53 74 + 64 63 61 6c 6c 2c 20 6d 73 63 6f 72 6c 69 62 2c + 20 56 65 72 73 69 6f 6e 3d 34 2e 30 2e 30 2e 30 + 2c 20 43 75 6c 74 75 72 65 3d 6e 65 75 74 72 61 + 6c 2c 20 50 75 62 6c 69 63 4b 65 79 54 6f 6b 65 + 6e 3d 62 37 37 61 35 63 35 36 31 39 33 34 65 30 + 38 39 53 1d 50 09 43 61 6c 6c 43 6f 6e 76 73 01 + 00 00 00 7a 53 79 73 74 65 6d 2e 52 75 6e 74 69 + 6d 65 2e 43 6f 6d 70 69 6c 65 72 53 65 72 76 69 + 63 65 73 2e 43 61 6c 6c 43 6f 6e 76 43 64 65 63 + 6c 2c 20 6d 73 63 6f 72 6c 69 62 2c 20 56 65 72 + 73 69 6f 6e 3d 34 2e 30 2e 30 2e 30 2c 20 43 75 + 6c 74 75 72 65 3d 6e 65 75 74 72 61 6c 2c 20 50 + 75 62 6c 69 63 4b 65 79 54 6f 6b 65 6e 3d 62 37 + 37 61 35 63 35 36 31 39 33 34 65 30 38 39 + ) + + ret + } // end of method C::M +} +"; + + var comp = CreateCompilationWithFunctionPointersAndIl(@" +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +unsafe class D +{ + static void M1() + { + delegate* unmanaged[Cdecl] ptr1 = &C.M; + delegate* unmanaged[Stdcall] ptr2 = &C.M; // Error + M2(); + } + + [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvCdecl) })] + static void M2() {} +} +", il); + + comp.Assembly.SetOverrideRuntimeSupportsUnmanagedSignatureCallingConvention(); + comp.VerifyDiagnostics( + // (9,52): error CS8786: Calling convention of 'C.M()' is not compatible with 'Standard'. + // delegate* unmanaged[Stdcall] ptr2 = &C.M; // Error + Diagnostic(ErrorCode.ERR_WrongFuncPtrCallingConvention, "C.M").WithArguments("C.M()", "Standard").WithLocation(9, 52), + // (13,27): error CS0229: Ambiguity between 'UnmanagedCallersOnlyAttribute.CallConvs' and 'UnmanagedCallersOnlyAttribute.CallConvs' + // [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvCdecl) })] + Diagnostic(ErrorCode.ERR_AmbigMember, "CallConvs").WithArguments("System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute.CallConvs", "System.Runtime.InteropServices.UnmanagedCallersOnlyAttribute.CallConvs").WithLocation(13, 27) + ); + + var c = comp.GetTypeByMetadataName("C"); + var m = c.GetMethod("M"); + var callConvCdecl = comp.GetTypeByMetadataName("System.Runtime.CompilerServices.CallConvCdecl"); + + Assert.True(callConvCdecl!.Equals((NamedTypeSymbol)m.GetUnmanagedCallersOnlyAttributeData(forceComplete: true)!.CallingConventionTypes.Single(), TypeCompareKind.ConsiderEverything)); + } + [Fact] public void UnmanagedCallersOnly_BadExpressionInArguments() { diff --git a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs index 686b443eef0..f35417c9248 100644 --- a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs +++ b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs @@ -1138,7 +1138,7 @@ internal bool HasIsByRefLikeAttribute(EntityHandle token) internal UnmanagedCallersOnlyAttributeData? TryGetUnmanagedCallersOnlyAttribute( EntityHandle token, IAttributeNamedArgumentDecoder attributeArgumentDecoder, - Func? CallConvs, bool IsValid)> unmanagedCallersOnlyDecoder) + Func? CallConvs)> unmanagedCallersOnlyDecoder) { AttributeInfo info = FindTargetAttribute(token, AttributeDescription.UnmanagedCallersOnlyAttribute); if (!info.HasValue || info.SignatureIndex != 0 || !TryGetAttributeReader(info.Handle, out BlobReader sigReader)) @@ -1147,7 +1147,6 @@ internal bool HasIsByRefLikeAttribute(EntityHandle token) } var unmanagedConventionTypes = ImmutableHashSet.Empty; - bool isValid = true; if (sigReader.RemainingBytes > 0) { @@ -1156,22 +1155,22 @@ internal bool HasIsByRefLikeAttribute(EntityHandle token) var numNamed = sigReader.ReadUInt16(); for (int i = 0; i < numNamed; i++) { - var ((name, value), _, _) = attributeArgumentDecoder.DecodeCustomAttributeNamedArgumentOrThrow(ref sigReader); - var namedArgumentDecoded = unmanagedCallersOnlyDecoder(name, value); + // typeCode of the value is checked by the decoder itself + var ((name, value), isProperty, /* typeCode */ _) = attributeArgumentDecoder.DecodeCustomAttributeNamedArgumentOrThrow(ref sigReader); + var namedArgumentDecoded = unmanagedCallersOnlyDecoder(name, value, !isProperty); if (namedArgumentDecoded.IsCallConvs) { - isValid = isValid && namedArgumentDecoded.IsValid; unmanagedConventionTypes = namedArgumentDecoded.CallConvs; + break; } } } catch (Exception ex) when (ex is BadImageFormatException or UnsupportedSignatureContent) { - return UnmanagedCallersOnlyAttributeData.Create(ImmutableHashSet.Empty, isValid: false); } } - return UnmanagedCallersOnlyAttributeData.Create(unmanagedConventionTypes, isValid); + return UnmanagedCallersOnlyAttributeData.Create(unmanagedConventionTypes); } #nullable restore diff --git a/src/Compilers/Core/Portable/Symbols/Attributes/UnmanagedCallersOnlyAttributeData.cs b/src/Compilers/Core/Portable/Symbols/Attributes/UnmanagedCallersOnlyAttributeData.cs index efef3ad4a76..4c248de97b6 100644 --- a/src/Compilers/Core/Portable/Symbols/Attributes/UnmanagedCallersOnlyAttributeData.cs +++ b/src/Compilers/Core/Portable/Symbols/Attributes/UnmanagedCallersOnlyAttributeData.cs @@ -13,31 +13,30 @@ namespace Microsoft.CodeAnalysis { internal sealed class UnmanagedCallersOnlyAttributeData { - internal static readonly UnmanagedCallersOnlyAttributeData Uninitialized = new UnmanagedCallersOnlyAttributeData(callingConventionTypes: ImmutableHashSet.Empty, isValid: false); - internal static readonly UnmanagedCallersOnlyAttributeData AttributePresentDataNotBound = new UnmanagedCallersOnlyAttributeData(callingConventionTypes: ImmutableHashSet.Empty, isValid: false); - private static readonly UnmanagedCallersOnlyAttributeData PlatformDefault = new UnmanagedCallersOnlyAttributeData(callingConventionTypes: ImmutableHashSet.Empty, isValid: true); + internal static readonly UnmanagedCallersOnlyAttributeData Uninitialized = new UnmanagedCallersOnlyAttributeData(callingConventionTypes: ImmutableHashSet.Empty); + internal static readonly UnmanagedCallersOnlyAttributeData AttributePresentDataNotBound = new UnmanagedCallersOnlyAttributeData(callingConventionTypes: ImmutableHashSet.Empty); + private static readonly UnmanagedCallersOnlyAttributeData PlatformDefault = new UnmanagedCallersOnlyAttributeData(callingConventionTypes: ImmutableHashSet.Empty); public const string CallConvsPropertyName = "CallConvs"; - internal static UnmanagedCallersOnlyAttributeData Create(ImmutableHashSet? callingConventionTypes, bool isValid) - => (callingConventionTypes, isValid) switch + internal static UnmanagedCallersOnlyAttributeData Create(ImmutableHashSet? callingConventionTypes) + => callingConventionTypes switch { - (null, true) or ({ IsEmpty: true }, true) => PlatformDefault, - _ => new UnmanagedCallersOnlyAttributeData(callingConventionTypes ?? ImmutableHashSet.Empty, isValid) + null or { IsEmpty: true } => PlatformDefault, + _ => new UnmanagedCallersOnlyAttributeData(callingConventionTypes ?? ImmutableHashSet.Empty) }; public readonly ImmutableHashSet CallingConventionTypes; - public readonly bool IsValid; - private UnmanagedCallersOnlyAttributeData(ImmutableHashSet callingConventionTypes, bool isValid) + private UnmanagedCallersOnlyAttributeData(ImmutableHashSet callingConventionTypes) { CallingConventionTypes = callingConventionTypes; - IsValid = isValid; } - internal static bool IsCallConvsTypedConstant(string key, in TypedConstant value) + internal static bool IsCallConvsTypedConstant(string key, bool isField, in TypedConstant value) { - return key == CallConvsPropertyName + return isField + && key == CallConvsPropertyName && value.Kind == TypedConstantKind.Array && (value.Values.IsDefaultOrEmpty || value.Values.All(v => v.Kind == TypedConstantKind.Type)); } -- GitLab