From 8b1d06a5170628834b27c06b751ac0e54f65c7e3 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Tue, 28 May 2024 17:34:42 -0700 Subject: [PATCH] Avoid new constraint with structs in ILLink analyzer (#102784) This fixes an issue where the trim analyzer was producing warnings inside Visual Studio, but not from a command-line build, for code similar to the following: ```csharp var typeName = Console.ReadLine() ?? string.Empty; if (RuntimeFeature.IsEnabled) { var type = Type.GetType(typeName); // warning IL2057: Unrecognized value passed to the parameter 'typeName'...'System.Type.GetType'... } else { Console.WriteLine($"Cannot lookup {typeName} because the feature id disabled."); } internal static class RuntimeFeature { #pragma warning disable IL4000 [FeatureGuard(typeof(RequiresUnreferencedCodeAttribute))] internal static bool IsEnabled => AppContext.TryGetSwitch("RuntimeFeature.IsEnabled", out bool enabled) ? enabled : true; #pragma warning restore IL4000 } ``` This was due to https://github.com/dotnet/runtime/issues/6536 which was fixed in .NET Core, but still exists in .NET Framework. It means that we can't rely the parameterless struct constructor being called for struct types used through a generic parameter with a `new()` constraint. This fixes the issue by avoiding use of the `new()` constraint for generic parameters that might be structs. With the fix, the warning is no longer reported when running in Visual Studio locally, but there are no tests because we don't run tests on full framework. Includes some unrelated debug helpers that were useful while investigating this. --- .../DataFlow/FeatureContextLattice.cs | 9 +++++++++ .../DataFlow/LocalDataFlowAnalysis.cs | 19 +++++++++++++------ .../TrimAnalysis/TrimDataFlowAnalysis.cs | 19 ++++++++++++++++++- .../DataFlow/ForwardDataFlowAnalysis.cs | 5 ++++- .../DataFlow/IControlFlowGraph.cs | 2 +- 5 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureContextLattice.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureContextLattice.cs index 319730629e90a..347f2a85afd62 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureContextLattice.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureContextLattice.cs @@ -52,6 +52,15 @@ public FeatureContext Union (FeatureContext other) { return new FeatureContext (ValueSet.Union (EnabledFeatures, other.EnabledFeatures)); } + + public override string ToString () + { + if (EnabledFeatures.IsUnknown ()) + return "All"; + if (EnabledFeatures.IsEmpty ()) + return "None"; + return string.Join (", ", EnabledFeatures.GetKnownValues ()); + } } public readonly struct FeatureContextLattice : ILattice diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs index 1a8f3cdef3635..00410c9bc45bd 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/LocalDataFlowAnalysis.cs @@ -29,8 +29,8 @@ public abstract class LocalDataFlowAnalysis where TValue : struct, IEquatable where TContext : struct, IEquatable - where TLattice : ILattice, new() - where TContextLattice : ILattice, new() + where TLattice : ILattice + where TContextLattice : ILattice where TTransfer : LocalDataFlowVisitor where TConditionValue : struct, INegate { @@ -39,18 +39,25 @@ public abstract class LocalDataFlowAnalysis GetLatticeAndEntryValue( + TLattice lattice, + TContextLattice contextLattice, TContext initialContext, out LocalStateAndContext entryValue) { - LocalStateAndContextLattice lattice = new (new (new TLattice ()), new TContextLattice ()); + LocalStateAndContextLattice localStateAndContextLattice = new (new (lattice), contextLattice); entryValue = new LocalStateAndContext (default (LocalState), initialContext); - return lattice; + return localStateAndContextLattice; } // The initial value of the local dataflow is the empty local state (no tracked assignments), // with an initial context that must be specified by the derived class. - protected LocalDataFlowAnalysis (OperationBlockAnalysisContext context, IOperation operationBlock, TContext initialContext) - : base (GetLatticeAndEntryValue (initialContext, out var entryValue), entryValue) + protected LocalDataFlowAnalysis ( + OperationBlockAnalysisContext context, + IOperation operationBlock, + TLattice lattice, + TContextLattice contextLattice, + TContext initialContext) + : base (GetLatticeAndEntryValue (lattice, contextLattice, initialContext, out var entryValue), entryValue) { Context = context; OperationBlock = operationBlock; diff --git a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimDataFlowAnalysis.cs b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimDataFlowAnalysis.cs index 60cbe85649932..cb669462f354a 100644 --- a/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimDataFlowAnalysis.cs +++ b/src/tools/illink/src/ILLink.RoslynAnalyzer/TrimAnalysis/TrimDataFlowAnalysis.cs @@ -41,7 +41,12 @@ public TrimDataFlowAnalysis ( OperationBlockAnalysisContext context, DataFlowAnalyzerContext dataFlowAnalyzerContext, IOperation operationBlock) - : base (context, operationBlock, initialContext: FeatureContext.None) + : base ( + context, + operationBlock, + default (ValueSetLattice), + new FeatureContextLattice (), + initialContext: FeatureContext.None) { TrimAnalysisPatterns = new TrimAnalysisPatternStore (lattice.LocalStateLattice.Lattice.ValueLattice, lattice.ContextLattice); _dataFlowAnalyzerContext = dataFlowAnalyzerContext; @@ -172,6 +177,18 @@ static void WriteIndented (string? s, int level) } } + public override void TraceEdgeInput ( + IControlFlowGraph.ControlFlowBranch branch, + LocalStateValue state + ) { + if (trace && showStates) { + var source = branch.Source.Block.Ordinal; + var target = branch.Destination?.Block.Ordinal; + WriteIndented ($"--- Edge from [{source}] to [{target}] ---", 1); + WriteIndented (state.ToString (), 2); + } + } + public override void TraceBlockInput ( LocalStateValue normalState, LocalStateValue? exceptionState, diff --git a/src/tools/illink/src/ILLink.Shared/DataFlow/ForwardDataFlowAnalysis.cs b/src/tools/illink/src/ILLink.Shared/DataFlow/ForwardDataFlowAnalysis.cs index 807f0c46b9b5f..181e81125d27f 100644 --- a/src/tools/illink/src/ILLink.Shared/DataFlow/ForwardDataFlowAnalysis.cs +++ b/src/tools/illink/src/ILLink.Shared/DataFlow/ForwardDataFlowAnalysis.cs @@ -163,6 +163,9 @@ public virtual void TraceStart (TControlFlowGraph cfg) { } [Conditional ("DEBUG")] public virtual void TraceVisitBlock (TBlock block) { } + [Conditional ("DEBUG")] + public virtual void TraceEdgeInput (IControlFlowGraph.ControlFlowBranch branch, TValue state) { } + [Conditional ("DEBUG")] public virtual void TraceBlockInput (TValue normalState, TValue? exceptionState, TValue? exceptionFinallyState) { } @@ -280,7 +283,7 @@ public void Fixpoint (TControlFlowGraph cfg, TTransfer transfer) TValue predecessorState = cfgState.Get (predecessor).Current; FlowStateThroughExitedFinallys (predecessor, ref predecessorState); - + TraceEdgeInput (predecessor, predecessorState); currentState = lattice.Meet (currentState, predecessorState); } // State at start of a catch also includes the exceptional state from diff --git a/src/tools/illink/src/ILLink.Shared/DataFlow/IControlFlowGraph.cs b/src/tools/illink/src/ILLink.Shared/DataFlow/IControlFlowGraph.cs index d7b6a45b9ae0c..2fb81fb5e8ac0 100644 --- a/src/tools/illink/src/ILLink.Shared/DataFlow/IControlFlowGraph.cs +++ b/src/tools/illink/src/ILLink.Shared/DataFlow/IControlFlowGraph.cs @@ -45,7 +45,7 @@ public interface IControlFlowGraph public readonly struct ControlFlowBranch : IEquatable { public readonly TBlock Source; - private readonly TBlock? Destination; + public readonly TBlock? Destination; // The finally regions exited when control flows through this edge. // For example: