Skip to content

Commit

Permalink
NativeAOT Fully implements warning suppressions logic (#81266)
Browse files Browse the repository at this point in the history 
Ports the warning suppression logic from linker. The only unsupported feature is reading the suppressions from attribute XML. This also doesn't port the "redundant suppression detection" logic from linker.

Other changes:
* Extracts some helpers for attribute handling into CustomAttributeExtensions (mostly around properties and events)
* Implements equality and hashcode on property and event descriptors - so that they can be used as keys in a dictionary
* Implements passing along the "--singlewarn" command line option in the test infra.

Also ports over all of the warning suppression tests from linker:
* Modified all of the "redundant suppressions" tests to be "trimmer-only" as that feature is only implemented there.
* Removed the "suppressions from XML" tests

Per discussion with @MichalStrehovsky I will rename `PropertyPseudoDesc` and `EventPseudoDesc` to `EcmaProperty` and `EcmaEvent`. But I'll do that in a separate PR as it would add a lot of noise to this one.
  • Loading branch information
@vitek-karas
vitek-karas committed Feb 6, 2023
1 parent 09a3007 commit 2ca0229
Show file tree
Hide file tree
Showing 43 changed files with 2,680 additions and 125 deletions.
13 changes: 7 additions & 6 deletions src/coreclr/tools/Common/Compiler/EventPseudoDesc.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,12 @@ public EventPseudoDesc(EcmaType type, EventDefinitionHandle handle)

public override TypeSystemContext Context => _type.Context;

#region Do not use these
public override bool Equals(object obj) => throw new NotImplementedException();
public override int GetHashCode() => throw new NotImplementedException();
public static bool operator ==(EventPseudoDesc a, EventPseudoDesc b) => throw new NotImplementedException();
public static bool operator !=(EventPseudoDesc a, EventPseudoDesc b) => throw new NotImplementedException();
#endregion
public override bool Equals(object obj) => obj is not EventPseudoDesc @event ? false : this == @event;

public override int GetHashCode() => _type.GetHashCode() ^ _handle.GetHashCode();

public static bool operator ==(EventPseudoDesc a, EventPseudoDesc b) => a._type == b._type && a._handle == b._handle;

public static bool operator !=(EventPseudoDesc a, EventPseudoDesc b) => !(a == b);
}
}
13 changes: 7 additions & 6 deletions src/coreclr/tools/Common/Compiler/PropertyPseudoDesc.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,12 @@ public PropertyPseudoDesc(EcmaType type, PropertyDefinitionHandle handle)

public override TypeSystemContext Context => _type.Context;

#region Do not use these
public override bool Equals(object obj) => throw new NotImplementedException();
public override int GetHashCode() => throw new NotImplementedException();
public static bool operator ==(PropertyPseudoDesc a, PropertyPseudoDesc b) => throw new NotImplementedException();
public static bool operator !=(PropertyPseudoDesc a, PropertyPseudoDesc b) => throw new NotImplementedException();
#endregion
public override bool Equals(object obj) => obj is not PropertyPseudoDesc property ? false : this == property;

public override int GetHashCode() => _type.GetHashCode() ^ _handle.GetHashCode();

public static bool operator ==(PropertyPseudoDesc a, PropertyPseudoDesc b) => a._type == b._type && a._handle == b._handle;

public static bool operator !=(PropertyPseudoDesc a, PropertyPseudoDesc b) => !(a == b);
}
}
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/TypeSystem/Ecma/MetadataExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public static CustomAttributeHandle GetCustomAttributeHandle(this MetadataReader
return default(CustomAttributeHandle);
}

private static bool IsEqualCustomAttributeName(CustomAttributeHandle attributeHandle, MetadataReader metadataReader,
public static bool IsEqualCustomAttributeName(CustomAttributeHandle attributeHandle, MetadataReader metadataReader,
string attributeNamespace, string attributeName)
{
StringHandle namespaceHandle, nameHandle;
Expand Down
85 changes: 85 additions & 0 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/CustomAttributeExtensions.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Reflection.Metadata;
using Internal.TypeSystem;
using Internal.TypeSystem.Ecma;

namespace ILCompiler
{
public static class CustomAttributeExtensions
{
public static CustomAttributeValue<TypeDesc>? GetDecodedCustomAttribute(this PropertyPseudoDesc prop, string attributeNamespace, string attributeName)
{
var ecmaType = prop.OwningType as EcmaType;
var metadataReader = ecmaType.MetadataReader;

var attributeHandle = metadataReader.GetCustomAttributeHandle(prop.GetCustomAttributes,
attributeNamespace, attributeName);

if (attributeHandle.IsNil)
return null;

return metadataReader.GetCustomAttribute(attributeHandle).DecodeValue(new CustomAttributeTypeProvider(ecmaType.EcmaModule));
}

public static IEnumerable<CustomAttributeValue<TypeDesc>> GetDecodedCustomAttributes(this PropertyPseudoDesc prop, string attributeNamespace, string attributeName)
{
var ecmaType = prop.OwningType as EcmaType;
var metadataReader = ecmaType.MetadataReader;

var attributeHandles = prop.GetCustomAttributes;
foreach (var attributeHandle in attributeHandles)
{
if (MetadataExtensions.IsEqualCustomAttributeName(attributeHandle, metadataReader, attributeNamespace, attributeName))
{
yield return metadataReader.GetCustomAttribute(attributeHandle).DecodeValue(new CustomAttributeTypeProvider(ecmaType.EcmaModule));
}
}
}

public static CustomAttributeValue<TypeDesc>? GetDecodedCustomAttribute(this EventPseudoDesc @event, string attributeNamespace, string attributeName)
{
var ecmaType = @event.OwningType as EcmaType;
var metadataReader = ecmaType.MetadataReader;

var attributeHandle = metadataReader.GetCustomAttributeHandle(@event.GetCustomAttributes,
attributeNamespace, attributeName);

if (attributeHandle.IsNil)
return null;

return metadataReader.GetCustomAttribute(attributeHandle).DecodeValue(new CustomAttributeTypeProvider(ecmaType.EcmaModule));
}

public static IEnumerable<CustomAttributeValue<TypeDesc>> GetDecodedCustomAttributes(this EventPseudoDesc @event, string attributeNamespace, string attributeName)
{
var ecmaType = @event.OwningType as EcmaType;
var metadataReader = ecmaType.MetadataReader;

var attributeHandles = @event.GetCustomAttributes;
foreach (var attributeHandle in attributeHandles)
{
if (MetadataExtensions.IsEqualCustomAttributeName(attributeHandle, metadataReader, attributeNamespace, attributeName))
{
yield return metadataReader.GetCustomAttribute(attributeHandle).DecodeValue(new CustomAttributeTypeProvider(ecmaType.EcmaModule));
}
}
}

public static IEnumerable<CustomAttributeValue<TypeDesc>> GetDecodedCustomAttributesForModule(this EcmaModule module, string attributeNamespace, string attributeName)
{
var metadataReader = module.MetadataReader;

var attributeHandles = metadataReader.GetModuleDefinition().GetCustomAttributes();
foreach (var attributeHandle in attributeHandles)
{
if (MetadataExtensions.IsEqualCustomAttributeName(attributeHandle, metadataReader, attributeNamespace, attributeName))
{
yield return metadataReader.GetCustomAttribute(attributeHandle).DecodeValue(new CustomAttributeTypeProvider(module));
}
}
}
}
}
28 changes: 0 additions & 28 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/DiagnosticUtilities.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,34 +69,6 @@ internal static bool TryGetRequiresAttribute(TypeSystemEntity member, string req
return true;
}

public static CustomAttributeValue<TypeDesc>? GetDecodedCustomAttribute(this PropertyPseudoDesc prop, string attributeNamespace, string attributeName)
{
var ecmaType = prop.OwningType as EcmaType;
var metadataReader = ecmaType.MetadataReader;

var attributeHandle = metadataReader.GetCustomAttributeHandle(prop.GetCustomAttributes,
attributeNamespace, attributeName);

if (attributeHandle.IsNil)
return null;

return metadataReader.GetCustomAttribute(attributeHandle).DecodeValue(new CustomAttributeTypeProvider(ecmaType.EcmaModule));
}

public static CustomAttributeValue<TypeDesc>? GetDecodedCustomAttribute(this EventPseudoDesc @event, string attributeNamespace, string attributeName)
{
var ecmaType = @event.OwningType as EcmaType;
var metadataReader = ecmaType.MetadataReader;

var attributeHandle = metadataReader.GetCustomAttributeHandle(@event.GetCustomAttributes,
attributeNamespace, attributeName);

if (attributeHandle.IsNil)
return null;

return metadataReader.GetCustomAttribute(attributeHandle).DecodeValue(new CustomAttributeTypeProvider(ecmaType.EcmaModule));
}

internal static string GetRequiresAttributeMessage(CustomAttributeValue<TypeDesc> attribute)
{
if (attribute.FixedArguments.Length != 0)
Expand Down
87 changes: 4 additions & 83 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Logger.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,8 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Reflection.Metadata;

using Internal.TypeSystem;
using Internal.TypeSystem.Ecma;

using ILCompiler.Dataflow;
using ILCompiler.Logging;
Expand All @@ -22,6 +20,7 @@ public class Logger
{
private readonly ILogWriter _logWriter;
private readonly CompilerGeneratedState _compilerGeneratedState;
private readonly UnconditionalSuppressMessageAttributeState _unconditionalSuppressMessageAttributeState;

private readonly HashSet<int> _suppressedWarnings;
private readonly HashSet<string> _suppressedCategories;
Expand All @@ -47,13 +46,14 @@ public Logger(
IEnumerable<string> suppressedCategories)
{
_logWriter = writer;
_compilerGeneratedState = ilProvider == null ? null : new CompilerGeneratedState(ilProvider, this);
IsVerbose = isVerbose;
_suppressedWarnings = new HashSet<int>(suppressedWarnings);
_isSingleWarn = singleWarn;
_singleWarnEnabledAssemblies = new HashSet<string>(singleWarnEnabledModules, StringComparer.OrdinalIgnoreCase);
_singleWarnDisabledAssemblies = new HashSet<string>(singleWarnDisabledModules, StringComparer.OrdinalIgnoreCase);
_suppressedCategories = new HashSet<string>(suppressedCategories, StringComparer.Ordinal);
_compilerGeneratedState = ilProvider == null ? null : new CompilerGeneratedState(ilProvider, this);
_unconditionalSuppressMessageAttributeState = new UnconditionalSuppressMessageAttributeState(_compilerGeneratedState, this);
}

public Logger(TextWriter writer, ILProvider ilProvider, bool isVerbose, IEnumerable<int> suppressedWarnings, bool singleWarn, IEnumerable<string> singleWarnEnabledModules, IEnumerable<string> singleWarnDisabledModules, IEnumerable<string> suppressedCategories)
Expand Down Expand Up @@ -154,86 +154,7 @@ internal bool IsWarningSuppressed(int code, MessageOrigin origin)
if (_suppressedWarnings.Contains(code))
return true;

// TODO: Suppressions with different scopes

TypeSystemEntity member = origin.MemberDefinition;
if (IsSuppressed(code, member))
return true;

MethodDesc owningMethod;
if (_compilerGeneratedState != null)
{
while (_compilerGeneratedState?.TryGetOwningMethodForCompilerGeneratedMember(member, out owningMethod) == true)
{
Debug.Assert(owningMethod != member);
if (IsSuppressed(code, owningMethod))
return true;
member = owningMethod;
}
}

return false;
}

private static bool IsSuppressed(int id, TypeSystemEntity warningOrigin)
{
TypeSystemEntity warningOriginMember = warningOrigin;
while (warningOriginMember != null)
{
if (IsSuppressedOnElement(id, warningOriginMember))
return true;

warningOriginMember = warningOriginMember.GetOwningType();
}

// TODO: Assembly-level suppressions

return false;
}

private static bool IsSuppressedOnElement(int id, TypeSystemEntity provider)
{
if (provider == null)
return false;

// TODO: Assembly-level suppressions

IEnumerable<CustomAttributeValue<TypeDesc>> suppressions = null;

if (provider is TypeDesc type)
{
var ecmaType = type.GetTypeDefinition() as EcmaType;
suppressions = ecmaType?.GetDecodedCustomAttributes("System.Diagnostics.CodeAnalysis", "UnconditionalSuppressMessageAttribute");
}

if (provider is MethodDesc method)
{
var ecmaMethod = method.GetTypicalMethodDefinition() as EcmaMethod;
suppressions = ecmaMethod?.GetDecodedCustomAttributes("System.Diagnostics.CodeAnalysis", "UnconditionalSuppressMessageAttribute");
}

if (suppressions != null)
{
foreach (CustomAttributeValue<TypeDesc> suppression in suppressions)
{
if (suppression.FixedArguments.Length != 2
|| suppression.FixedArguments[1].Value is not string warningId
|| warningId.Length < 6
|| !warningId.StartsWith("IL")
|| (warningId.Length > 6 && warningId[6] != ':')
|| !int.TryParse(warningId.AsSpan(2, 4), out int suppressedCode))
{
continue;
}

if (id == suppressedCode)
{
return true;
}
}
}

return false;
return _unconditionalSuppressMessageAttributeState.IsSuppressed(code, origin);
}

internal static bool IsWarningAsError(int _/*code*/)
Expand Down
13 changes: 13 additions & 0 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Logging/SuppressMessageInfo.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace ILCompiler.Logging
{
public struct SuppressMessageInfo
{
public int Id;
public string Scope;
public string Target;
public string MessageId;
}
}
Loading

0 comments on commit 2ca0229

Please sign in to comment.