Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NativeAOT implement warning as errors as a compiler feature #96567

Merged
merged 4 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ The .NET Foundation licenses this file to you under the MIT license.
<PublishTrimmed Condition="'$(PublishTrimmed)' == ''">true</PublishTrimmed>
<IlcPgoOptimize Condition="'$(IlcPgoOptimize)' == '' and '$(OptimizationPreference)' == 'Speed'">true</IlcPgoOptimize>
<RunILLink>false</RunILLink>
<IlcTreatWarningsAsErrors Condition="'$(IlcTreatWarningsAsErrors)' == ''">$(TreatWarningsAsErrors)</IlcTreatWarningsAsErrors>
<_IsiOSLikePlatform Condition="'$(_targetOS)' == 'maccatalyst' or $(_targetOS.StartsWith('ios')) or $(_targetOS.StartsWith('tvos'))">true</_IsiOSLikePlatform>
<_IsApplePlatform Condition="'$(_targetOS)' == 'osx' or '$(_IsiOSLikePlatform)' == 'true'">true</_IsApplePlatform>
</PropertyGroup>
Expand Down Expand Up @@ -268,6 +269,9 @@ The .NET Foundation licenses this file to you under the MIT license.
<IlcArg Condition="'$(_targetOS)' == 'win' and $(IlcMultiModule) != 'true' and '$(IlcGenerateWin32Resources)' != 'false' and '$(NativeLib)' != 'Static'" Include="--win32resourcemodule:%(ManagedBinary.Filename)" />
<IlcArg Condition="$(IlcDumpIL) == 'true'" Include="--ildump:$(NativeIntermediateOutputPath)%(ManagedBinary.Filename).il" />
<IlcArg Condition="$(NoWarn) != ''" Include='--nowarn:"$([MSBuild]::Escape($(NoWarn)).Replace(`%0A`, ``).Replace(`%0D`, ``))"' />
<IlcArg Condition="$(IlcTreatWarningsAsErrors) == 'true'" Include="--warnaserror" />
<IlcArg Condition="$(WarningsAsErrors) != ''" Include='--warnaserr:"$([MSBuild]::Escape($(WarningsAsErrors)).Replace(`%0A`, ``).Replace(`%0D`, ``))"' />
<IlcArg Condition="$(WarningsNotAsErrors) != ''" Include='--nowarnaserr:"$([MSBuild]::Escape($(WarningsNotAsErrors)).Replace(`%0A`, ``).Replace(`%0D`, ``))"' />
<IlcArg Condition="$(TrimmerSingleWarn) == 'true'" Include="--singlewarn" />
<IlcArg Condition="$(SuppressTrimAnalysisWarnings) == 'true'" Include="--notrimwarn" />
<IlcArg Condition="$(SuppressAotAnalysisWarnings) == 'true'" Include="--noaotwarn" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,15 @@ protected override void ProcessMethod(TypeDesc type, XPathNavigator methodNav, o
_methodSubstitutions.Add(method, BodySubstitution.ThrowingBody);
break;
case "stub":
BodySubstitution stubBody;
BodySubstitution stubBody = null;
if (method.Signature.ReturnType.IsVoid)
stubBody = BodySubstitution.EmptyBody;
else
stubBody = BodySubstitution.Create(TryCreateSubstitution(method.Signature.ReturnType, GetAttribute(methodNav, "value")));
{
object substitution = TryCreateSubstitution(method.Signature.ReturnType, GetAttribute(methodNav, "value"));
if (substitution != null)
stubBody = BodySubstitution.Create(substitution);
}

if (stubBody != null)
{
Expand Down
24 changes: 17 additions & 7 deletions src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Logger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ public class Logger
private readonly HashSet<string> _trimWarnedAssemblies = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
private readonly HashSet<string> _aotWarnedAssemblies = new HashSet<string>(StringComparer.OrdinalIgnoreCase);

private readonly bool _treatWarningsAsErrors;
private readonly Dictionary<int, bool> _warningsAsErrors;

public static Logger Null = new Logger(new TextLogWriter(TextWriter.Null), null, false);

public bool IsVerbose { get; }
Expand All @@ -46,7 +49,9 @@ public Logger(
bool singleWarn,
IEnumerable<string> singleWarnEnabledModules,
IEnumerable<string> singleWarnDisabledModules,
IEnumerable<string> suppressedCategories)
IEnumerable<string> suppressedCategories,
bool treatWarningsAsErrors,
IDictionary<int, bool> warningsAsErrors)
{
_logWriter = writer;
IsVerbose = isVerbose;
Expand All @@ -55,17 +60,19 @@ public Logger(
_singleWarnEnabledAssemblies = new HashSet<string>(singleWarnEnabledModules, StringComparer.OrdinalIgnoreCase);
_singleWarnDisabledAssemblies = new HashSet<string>(singleWarnDisabledModules, StringComparer.OrdinalIgnoreCase);
_suppressedCategories = new HashSet<string>(suppressedCategories, StringComparer.Ordinal);
_treatWarningsAsErrors = treatWarningsAsErrors;
_warningsAsErrors = new Dictionary<int, bool>(warningsAsErrors);
_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)
: this(new TextLogWriter(writer), ilProvider, isVerbose, suppressedWarnings, singleWarn, singleWarnEnabledModules, singleWarnDisabledModules, suppressedCategories)
public Logger(TextWriter writer, ILProvider ilProvider, bool isVerbose, IEnumerable<int> suppressedWarnings, bool singleWarn, IEnumerable<string> singleWarnEnabledModules, IEnumerable<string> singleWarnDisabledModules, IEnumerable<string> suppressedCategories, bool treatWarningsAsErrors, IDictionary<int, bool> warningsAsErrors)
: this(new TextLogWriter(writer), ilProvider, isVerbose, suppressedWarnings, singleWarn, singleWarnEnabledModules, singleWarnDisabledModules, suppressedCategories, treatWarningsAsErrors, warningsAsErrors)
{
}

public Logger(ILogWriter writer, ILProvider ilProvider, bool isVerbose)
: this(writer, ilProvider, isVerbose, Array.Empty<int>(), singleWarn: false, Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>())
: this(writer, ilProvider, isVerbose, Array.Empty<int>(), singleWarn: false, Array.Empty<string>(), Array.Empty<string>(), Array.Empty<string>(), false, new Dictionary<int, bool>())
{
}

Expand Down Expand Up @@ -155,10 +162,13 @@ internal bool IsWarningSuppressed(int code, MessageOrigin origin)
return _unconditionalSuppressMessageAttributeState.IsSuppressed(code, origin);
}

internal static bool IsWarningAsError(int _/*code*/)
internal bool IsWarningAsError(int warningCode)
{
// TODO: warnaserror
return false;
bool value;
if (_treatWarningsAsErrors)
return !_warningsAsErrors.TryGetValue(warningCode, out value) || value;

return _warningsAsErrors.TryGetValue(warningCode, out value) && value;
}

internal bool IsSingleWarn(ModuleDesc owningModule, string messageSubcategory)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ internal static MessageContainer CreateErrorMessage(MessageOrigin? origin, Diagn
if (TryLogSingleWarning(context, code, origin, subcategory))
return null;

if (Logger.IsWarningAsError(code))
if (context.IsWarningAsError(code))
return new MessageContainer(MessageCategory.WarningAsError, text, code, subcategory, origin);

return new MessageContainer(MessageCategory.Warning, text, code, subcategory, origin);
Expand All @@ -148,7 +148,7 @@ internal static MessageContainer CreateErrorMessage(MessageOrigin? origin, Diagn
if (TryLogSingleWarning(context, (int)id, origin, subcategory))
return null;

if (Logger.IsWarningAsError((int)id))
if (context.IsWarningAsError((int)id))
return new MessageContainer(MessageCategory.WarningAsError, id, subcategory, origin, args);

return new MessageContainer(MessageCategory.Warning, id, subcategory, origin, args);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,8 @@ public class ILCompilerOptions
public List<string> Descriptors = new List<string> ();
public bool FrameworkCompilation;
public bool SingleWarn;
public List<string> SubstitutionFiles = new List<string> ();
public bool TreatWarningsAsErrors;
public Dictionary<int, bool> WarningsAsErrors = new Dictionary<int, bool> ();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ public virtual void AddStripLinkAttributes (bool stripLinkAttributes)

public virtual void AddSubstitutions (string file)
{
Options.SubstitutionFiles.Add (file);
MichalStrehovsky marked this conversation as resolved.
Show resolved Hide resolved
}

public virtual void AddLinkAttributes (string file)
Expand All @@ -161,6 +162,30 @@ public virtual void AddAdditionalArgument (string flag, string[] values)
else if (flag == "--singlewarn") {
Options.SingleWarn = true;
}
else if (flag.StartsWith("--warnaserror"))
{
if (flag == "--warnaserror" || flag == "--warnaserror+")
{
if (values.Length == 0)
Options.TreatWarningsAsErrors = true;
else
{
foreach (int warning in ProcessWarningCodes(values))
Options.WarningsAsErrors[warning] = true;
}

}
else if (flag == "--warnaserror-")
{
if (values.Length == 0)
Options.TreatWarningsAsErrors = false;
else
{
foreach (int warning in ProcessWarningCodes(values))
Options.WarningsAsErrors[warning] = false;
}
}
}
}

public virtual void ProcessTestInputAssembly (NPath inputAssemblyPath)
Expand Down Expand Up @@ -261,6 +286,23 @@ private static void AppendExpandedPaths (Dictionary<string, string> dictionary,
}
}

private static readonly char[] s_separator = new char[] { ',', ';', ' ' };

private static IEnumerable<int> ProcessWarningCodes(IEnumerable<string> warningCodes)
{
foreach (string value in warningCodes)
{
string[] values = value.Split(s_separator, StringSplitOptions.RemoveEmptyEntries);
foreach (string id in values)
{
if (!id.StartsWith("IL", StringComparison.Ordinal) || !ushort.TryParse(id.AsSpan(2), out ushort code))
continue;

yield return code;
}
}
}

public ILCompilerOptions Build ()
{
var options = Options;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Linq;
using System.Reflection;
using System.Runtime.InteropServices;
using System.Xml;
using ILCompiler;
using ILCompiler.Dataflow;
using ILLink.Shared.TrimAnalysis;
Expand Down Expand Up @@ -84,15 +85,33 @@ public ILScanResults Trim (ILCompilerOptions options, TrimmingCustomizations? cu
options.SingleWarn,
singleWarnEnabledModules: Enumerable.Empty<string> (),
singleWarnDisabledModules: Enumerable.Empty<string> (),
suppressedCategories: Enumerable.Empty<string> ());
suppressedCategories: Enumerable.Empty<string> (),
treatWarningsAsErrors: options.TreatWarningsAsErrors,
warningsAsErrors: options.WarningsAsErrors);

foreach (var descriptor in options.Descriptors) {
if (!File.Exists (descriptor))
throw new FileNotFoundException ($"'{descriptor}' doesn't exist");
compilationRoots.Add (new ILCompiler.DependencyAnalysis.TrimmingDescriptorNode (descriptor));
}

ilProvider = new FeatureSwitchManager (ilProvider, logger, options.FeatureSwitches, default);

var featureSwitches = options.FeatureSwitches;
BodyAndFieldSubstitutions substitutions = default;
IReadOnlyDictionary<ModuleDesc, IReadOnlySet<string>>? resourceBlocks = default;
foreach (string substitutionFilePath in options.SubstitutionFiles)
{
using FileStream fs = File.OpenRead(substitutionFilePath);
substitutions.AppendFrom(BodySubstitutionsParser.GetSubstitutions(
logger, typeSystemContext, XmlReader.Create(fs), substitutionFilePath, featureSwitches));

fs.Seek(0, SeekOrigin.Begin);

resourceBlocks = ManifestResourceBlockingPolicy.UnionBlockings(resourceBlocks,
ManifestResourceBlockingPolicy.SubstitutionsReader.GetSubstitutions(
logger, typeSystemContext, XmlReader.Create(fs), substitutionFilePath, featureSwitches));
}

ilProvider = new FeatureSwitchManager(ilProvider, logger, featureSwitches, substitutions);

CompilerGeneratedState compilerGeneratedState = new CompilerGeneratedState (ilProvider, logger);

Expand Down
9 changes: 9 additions & 0 deletions src/coreclr/tools/aot/ILCompiler/ILCompilerRootCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@ internal sealed class ILCompilerRootCommand : CliRootCommand
new("--singlewarnassembly") { DefaultValueFactory = _ => Array.Empty<string>(), Description = "Generate single AOT/trimming warning for given assembly" };
public CliOption<string[]> SingleWarnDisabledAssemblies { get; } =
new("--nosinglewarnassembly") { DefaultValueFactory = _ => Array.Empty<string>(), Description = "Expand AOT/trimming warnings for given assembly" };
public CliOption<bool> TreatWarningsAsErrors { get; } =
new("--warnaserror") { Description = "Treat warnings as errors" };
public CliOption<string[]> WarningsAsErrorsEnable { get; } =
new("--warnaserr") { Description = "Enable treating specific warnings as errors" };
public CliOption<string[]> WarningsAsErrorsDisable { get; } =
new("--nowarnaserr") { Description = "Disable treating specific warnings as errors" };
public CliOption<string[]> DirectPInvokes { get; } =
new("--directpinvoke") { DefaultValueFactory = _ => Array.Empty<string>(), Description = "PInvoke to call directly" };
public CliOption<string[]> DirectPInvokeLists { get; } =
Expand Down Expand Up @@ -225,6 +231,9 @@ public ILCompilerRootCommand(string[] args) : base(".NET Native IL Compiler")
Options.Add(NoAotWarn);
Options.Add(SingleWarnEnabledAssemblies);
Options.Add(SingleWarnDisabledAssemblies);
Options.Add(TreatWarningsAsErrors);
Options.Add(WarningsAsErrorsEnable);
Options.Add(WarningsAsErrorsDisable);
Options.Add(DirectPInvokes);
Options.Add(DirectPInvokeLists);
Options.Add(MaxGenericCycleDepth);
Expand Down
12 changes: 11 additions & 1 deletion src/coreclr/tools/aot/ILCompiler/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,18 @@ public int Run()

ILProvider ilProvider = new NativeAotILProvider();

Dictionary<int, bool> warningsAsErrors = new Dictionary<int, bool>();
foreach (int warning in ProcessWarningCodes(Get(_command.WarningsAsErrorsEnable)))
{
warningsAsErrors[warning] = true;
}
foreach (int warning in ProcessWarningCodes(Get(_command.WarningsAsErrorsDisable)))
{
warningsAsErrors[warning] = false;
}
var logger = new Logger(Console.Out, ilProvider, Get(_command.IsVerbose), ProcessWarningCodes(Get(_command.SuppressedWarnings)),
Get(_command.SingleWarn), Get(_command.SingleWarnEnabledAssemblies), Get(_command.SingleWarnDisabledAssemblies), suppressedWarningCategories);
Get(_command.SingleWarn), Get(_command.SingleWarnEnabledAssemblies), Get(_command.SingleWarnDisabledAssemblies), suppressedWarningCategories,
Get(_command.TreatWarningsAsErrors), warningsAsErrors);

// NativeAOT is full AOT and its pre-compiled methods can not be
// thrown away at runtime if they mismatch in required ISAs or
Expand Down
2 changes: 2 additions & 0 deletions src/tests/Directory.Build.targets
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,8 @@
so the default for the test tree is partial. -->
<TrimMode Condition="'$(EnableAggressiveTrimming)' != 'true'">partial</TrimMode>

<IlcTreatWarningsAsErrors>false</IlcTreatWarningsAsErrors>

<IlcToolsPath>$(CoreCLRILCompilerDir)</IlcToolsPath>
<IlcToolsPath Condition="'$(TargetArchitecture)' != '$(BuildArchitecture)' or '$(TargetOS)' != '$(HostOS)' or '$(EnableNativeSanitizers)' != ''">$(CoreCLRCrossILCompilerDir)</IlcToolsPath>
<IlcBuildTasksPath>$(CoreCLRILCompilerDir)netstandard/ILCompiler.Build.Tasks.dll</IlcBuildTasksPath>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,8 @@ private static void ThrowIfPresent(Type testType, string typeName)
}
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2072:UnrecognizedReflectionPattern",
Justification = "That's the point")]
private static void ThrowIfPresentWithUsableMethodTable(Type testType, string typeName)
{
Type t = GetTypeSecretly(testType, typeName);
Expand Down
2 changes: 2 additions & 0 deletions src/tests/nativeaot/SmokeTests/UnitTests/Devirtualization.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Diagnostics.CodeAnalysis;

class Devirtualization
{
Expand Down Expand Up @@ -103,6 +104,7 @@ static void AssertEqual<T>(T expected, T actual)
[MethodImpl(MethodImplOptions.NoInlining)]
static void TestIntf2(IIntf2 o, int expected) => AssertEqual(expected, o.GetValue());

[UnconditionalSuppressMessage("AOT", "IL3050", Justification = "MakeGenericType - Intentional")]
public static void Run()
{
TestIntf1(new Intf1Impl(), 123);
Expand Down
5 changes: 5 additions & 0 deletions src/tests/nativeaot/SmokeTests/UnitTests/Generics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
using System.Reflection;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
using System.Diagnostics.CodeAnalysis;

[UnconditionalSuppressMessage("Trimming", "IL2055", Justification = "MakeGenericType - Intentional")]
[UnconditionalSuppressMessage("Trimming", "IL2060", Justification = "MakeGenericMethod - Intentional")]
[UnconditionalSuppressMessage("AOT", "IL3050", Justification = "MakeGenericType/MakeGenericMethod - Intentional")]
[UnconditionalSuppressMessage("AOT", "IL3054", Justification = "Generic expansion aborted - Intentional")]
class Generics
{
internal static int Run()
Expand Down
6 changes: 6 additions & 0 deletions src/tests/nativeaot/SmokeTests/UnitTests/Interfaces.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Collections.Generic;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Diagnostics.CodeAnalysis;

public class Interfaces
{
Expand Down Expand Up @@ -1225,6 +1226,7 @@ public static int CallIndirect()

static Type s_fooType = typeof(Foo);

[UnconditionalSuppressMessage("AOT", "IL3050", Justification = "MakeGenericType - Intentional")]
public static void Run()
{
Type t = typeof(FrobCaller<>).MakeGenericType(s_fooType);
Expand Down Expand Up @@ -1289,6 +1291,7 @@ class Atom { }

static Type s_atomType = typeof(Atom);

[UnconditionalSuppressMessage("AOT", "IL3050", Justification = "MakeGenericType - Intentional")]
public static void Run()
{
Type t = typeof(Wrapper<>).MakeGenericType(s_atomType);
Expand Down Expand Up @@ -1354,6 +1357,7 @@ class Atom : AtomBase { }
static Type s_atomType = typeof(Atom);
static Type s_atomBaseType = typeof(AtomBase);

[UnconditionalSuppressMessage("AOT", "IL3050", Justification = "MakeGenericType - Intentional")]
public static void Run()
{
Type t = typeof(FrobCaller<,>).MakeGenericType(typeof(AbjectFail<AtomBase>), s_atomType);
Expand Down Expand Up @@ -1576,6 +1580,7 @@ class Gen<T> where T : IFoo
public static string GrabCookie() => T.ImHungryGiveMeCookie();
}

[UnconditionalSuppressMessage("AOT", "IL3050", Justification = "MakeGenericType - Intentional")]
public static void Run()
{
var r = (string)typeof(Gen<>).MakeGenericType(typeof(Baz)).GetMethod("GrabCookie").Invoke(null, Array.Empty<object>());
Expand Down Expand Up @@ -1612,6 +1617,7 @@ class Gen<T> where T : IFoo
public static string GrabCookie() => T.ImHungryGiveMeCookie();
}

[UnconditionalSuppressMessage("AOT", "IL3050", Justification = "MakeGenericType - Intentional")]
public static void Run()
{
Activator.CreateInstance(typeof(Baz<>).MakeGenericType(typeof(Atom1)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ public Task CanWarnAsError ()
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task CanWarnAsErrorGlobal ()
{
return RunTest (allowMissingWarnings: true);
}

[Fact]
public Task InvalidWarningVersion ()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
namespace Mono.Linker.Tests.Cases.Warnings
{
[ExpectNonZeroExitCode (1)]
[IgnoreTestCase ("Ignore in NativeAOT, see https://github.com/dotnet/runtime/issues/82447", IgnoredBy = Tool.NativeAot)]
[SkipKeptItemsValidation]
[SetupLinkerSubstitutionFile ("CanWarnAsErrorSubstitutions.xml")]
[SetupLinkerArgument ("--verbose")]
Expand Down
Loading
Loading