From 30475ee5417148f6d16dc5289cce3a20787e33e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Thu, 10 Mar 2022 16:43:55 +0100 Subject: [PATCH 01/19] First implementation and making UTs green. --- .../Rules/SymbolicExecutionRunnerBase.cs | 2 +- .../SymbolicExecution/Roslyn/ProgramState.cs | 8 ++++++ .../Roslyn/RoslynSymbolicExecution.cs | 25 ++++++++++++++++++- .../RuleChecks/LocksReleasedAllPathsBase.cs | 6 ++--- .../RoslynSymbolicExecutionTest.Branching.cs | 8 +++--- .../RoslynSymbolicExecutionTest.Loops.cs | 9 ++++--- .../Roslyn/RoslynSymbolicExecutionTest.cs | 13 +++++----- .../SymbolicExecution/SETestContext.cs | 4 +-- .../TestFramework/TestHelper.cs | 17 ++++++++++--- 9 files changed, 68 insertions(+), 24 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/SymbolicExecutionRunnerBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/SymbolicExecutionRunnerBase.cs index 086e857e566..416367d14b6 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/SymbolicExecutionRunnerBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/SymbolicExecutionRunnerBase.cs @@ -83,7 +83,7 @@ private void AnalyzeRoslyn(SonarAnalysisContext sonarContext, SyntaxNodeAnalysis { if (CreateCfg(nodeContext.SemanticModel, body) is { } cfg) { - var engine = new RoslynSymbolicExecution(cfg, checks); + var engine = new RoslynSymbolicExecution(cfg, checks, symbol); engine.Execute(); } } diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs index a373baaaa71..67d6e57663c 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs @@ -38,6 +38,7 @@ public sealed record ProgramState : IEquatable private ImmutableDictionary SymbolValue { get; init; } private ImmutableDictionary VisitCount { get; init; } private ImmutableDictionary CaptureOperation { get; init; } + private ImmutableHashSet PreservedSymbols { get; init; } public SymbolicValue this[IOperationWrapperSonar operation] => this[operation.Instance]; public SymbolicValue this[IOperation operation] => OperationValue.TryGetValue(ResolveCapture(operation), out var value) ? value : null; @@ -50,6 +51,7 @@ private ProgramState() SymbolValue = ImmutableDictionary.Empty; VisitCount = ImmutableDictionary.Empty; CaptureOperation = ImmutableDictionary.Empty; + PreservedSymbols = ImmutableHashSet.Empty; } public ProgramState SetOperationValue(IOperationWrapperSonar operation, SymbolicValue value) => @@ -95,9 +97,15 @@ public IOperation ResolveCapture(IOperation operation) => ? captured : operation; + public ProgramState RemoveSymbols(Func predicate) => + this with { SymbolValue = SymbolValue.Where(kv => !predicate(kv.Key) || PreservedSymbols.Contains(kv.Key)).ToImmutableDictionary() }; + public ProgramState AddVisit(int programPointHash) => this with { VisitCount = VisitCount.SetItem(programPointHash, GetVisitCount(programPointHash) + 1) }; + public ProgramState Preserve(ISymbol symbol) => + this with { PreservedSymbols = PreservedSymbols.Add(symbol) }; + public int GetVisitCount(int programPointHash) => VisitCount.TryGetValue(programPointHash, out var count) ? count : 0; diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index 5aca0717b31..dc545b6d35e 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -21,8 +21,11 @@ using System; using System.Collections.Generic; using System.Linq; +using Microsoft.CodeAnalysis; +using SonarAnalyzer.CFG.LiveVariableAnalysis; using SonarAnalyzer.CFG.Roslyn; using SonarAnalyzer.SymbolicExecution.Constraints; +using SonarAnalyzer.Helpers; using SonarAnalyzer.SymbolicExecution.Roslyn.Checks; using SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors; using StyleCop.Analyzers.Lightup; @@ -39,15 +42,24 @@ internal class RoslynSymbolicExecution private readonly Queue queue = new(); private readonly SymbolicValueCounter symbolicValueCounter = new(); private readonly HashSet visited = new(); + private readonly RoslynLiveVariableAnalysis lva; + private readonly IEnumerable nonInDeclarationParameters; - public RoslynSymbolicExecution(ControlFlowGraph cfg, SymbolicCheck[] checks) + public RoslynSymbolicExecution(ControlFlowGraph cfg, SymbolicCheck[] checks, ISymbol declaration) { this.cfg = cfg ?? throw new ArgumentNullException(nameof(cfg)); + if (declaration == null) + { + throw new ArgumentNullException(nameof(declaration)); + } if (checks == null || checks.Length == 0) { throw new ArgumentException("At least one check is expected", nameof(checks)); } this.checks = new(new[] { new ConstantCheck() }.Concat(checks).ToArray()); + lva = new RoslynLiveVariableAnalysis(cfg, declaration); + var declarationParameters = declaration.GetParameters(); + nonInDeclarationParameters = declarationParameters.Where(p => p.RefKind != RefKind.None); } public void Execute() @@ -89,10 +101,12 @@ private IEnumerable ProcessBranching(ExplodedNode node) } else if (node.Block.ContainsThrow()) { + node = new ExplodedNode(cfg.ExitBlock, CleanStateAfterBlock(node.State, node.Block), null); yield return new(cfg.ExitBlock, node.State, null); } else { + node = new ExplodedNode(node.Block, CleanStateAfterBlock(node.State, node.Block), node.FinallyPoint); foreach (var successor in node.Block.Successors.Where(x => IsReachable(node, x))) { if (ProcessBranch(node, successor) is { } newNode) @@ -188,6 +202,15 @@ private static ProgramState ProcessOperation(SymbolicContext context) => _ => context.State }; + private ProgramState CleanStateAfterBlock(ProgramState programState, BasicBlock block) + { + var liveVariables = lva.LiveOut(block).Union(nonInDeclarationParameters); // LVA excludes out and ref parameters + + // ToDo: Remove the IFieldSymbol check when SLVS-1136 is fixed + return programState.RemoveSymbols( + symbol => symbol is not IFieldSymbol && !liveVariables.Contains(symbol)); + } + private static bool IsReachable(ExplodedNode node, ControlFlowBranch branch) => node.Block.ConditionKind != ControlFlowConditionKind.None && node.State[node.Block.BranchValue] is { } sv diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs index 4d5af866031..7bb067c0fd2 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RuleChecks/LocksReleasedAllPathsBase.cs @@ -84,7 +84,7 @@ protected override ProgramState PostProcessSimple(SymbolicContext context) && assignment.Target.TrackedSymbol() is { } symbol) { lastSymbolLock[symbol] = new IOperationWrapperSonar(objectCreation.WrappedOperation); - return AddLock(context, objectCreation.WrappedOperation); + return AddLock(context, objectCreation.WrappedOperation).Preserve(symbol); } } else if (context.Operation.Instance.AsInvocation() is { } invocation) @@ -136,7 +136,7 @@ private ProgramState AddLock(SymbolicContext context, ISymbol symbol) } lastSymbolLock[symbol] = context.Operation; - return context.SetSymbolConstraint(symbol, LockConstraint.Held); + return context.SetSymbolConstraint(symbol, LockConstraint.Held).Preserve(symbol); } private ProgramState RemoveLock(SymbolicContext context, ISymbol symbol) @@ -147,7 +147,7 @@ private ProgramState RemoveLock(SymbolicContext context, ISymbol symbol) } releasedSymbols.Add(symbol); - return context.SetSymbolConstraint(symbol, LockConstraint.Released); + return context.SetSymbolConstraint(symbol, LockConstraint.Released).Preserve(symbol); } // This method should be removed once the engine has support for `True/False` boolean constraints. diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs index 6f794939e3e..0da920a8b16 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs @@ -201,7 +201,7 @@ public System.Collections.Generic.IEnumerable Method(bool a) var b = a; }"; var validator = SETestContext.CreateCSMethod(method).Validator; - validator.ValidateExitReachCount(2); + validator.ValidateExitReachCount(1); validator.ValidateExecutionCompleted(); } @@ -220,7 +220,7 @@ public void Branching_ConstraintTrackedSeparatelyInBranches() } Tag(""End"", value);"; var validator = SETestContext.CreateCS(code).Validator; - validator.ValidateExitReachCount(2); // Once with True constraint, once with False constraint on "value" + validator.ValidateExitReachCount(1); // The exit is only reached once as the symbolicValue is removed when value is not used anymore. validator.TagValues("End").Should().HaveCount(2) .And.ContainSingle(x => x.HasConstraint(BoolConstraint.True)) .And.ContainSingle(x => x.HasConstraint(BoolConstraint.False)); @@ -269,7 +269,7 @@ public void Branching_VisitedProgramState_IsImmutable() return x.State; }); var validator = SETestContext.CreateCS(code, postProcess).Validator; - validator.ValidateExitReachCount(2); + validator.ValidateExitReachCount(1); captured.Should().OnlyContain(x => x.Value.HasConstraint(BoolConstraint.True) == x.ExpectedHasTrueConstraint); } @@ -295,7 +295,7 @@ public void Branching_VisitedSymbolicValue_IsImmutable() var validator = SETestContext.CreateCS(code, postProcess).Validator; validator.ValidateTag("ToString", x => x.HasConstraint(TestConstraint.First).Should().BeTrue()); validator.ValidateTag("GetHashCode", x => x.HasConstraint(TestConstraint.First).Should().BeFalse()); // Nobody set the constraint on that path - validator.ValidateExitReachCount(2); // Once for each state + validator.ValidateExitReachCount(1); // Once as the states are cleaned after the variables are not used anymore. } [TestMethod] diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs index 00cfa6ecc6b..7bda92e3541 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs @@ -44,7 +44,7 @@ public void Loops_InstructionVisitedMaxTwice(string loop) }} Tag(""End"", value);"; var validator = SETestContext.CreateCS(code, ", int[] items", new AddConstraintOnInvocationCheck()).Validator; - validator.ValidateExitReachCount(2); + validator.ValidateExitReachCount(1); validator.TagValues("End").Should().HaveCount(2) .And.ContainSingle(x => x == null) .And.ContainSingle(x => x != null && x.HasConstraint(TestConstraint.First) && !x.HasConstraint(BoolConstraint.True)); @@ -64,9 +64,10 @@ public void Loops_InstructionVisitedMaxTwice_ForEachOriginalState() { value.ToString(); // Add another constraint to 'value' } while (Condition); +Tag(""HoldRef"", condition); Tag(""End"", value);"; var validator = SETestContext.CreateCS(code, ", int[] items", new AddConstraintOnInvocationCheck()).Validator; - validator.ValidateExitReachCount(4); + validator.ValidateExitReachCount(1); var states = validator.TagStates("End"); var condition = states.SelectMany(x => x.SymbolsWith(BoolConstraint.False)).First(); // "False" is never set for "value" var value = states.SelectMany(x => x.SymbolsWith(TestConstraint.First)).First(); // "First" is never set for "condition" @@ -89,7 +90,7 @@ public void DoLoop_InstructionVisitedMaxTwice() }} while (value > 0); Tag(""End"", value);"; var validator = SETestContext.CreateCS(code, new AddConstraintOnInvocationCheck()).Validator; - validator.ValidateExitReachCount(2); + validator.ValidateExitReachCount(1); validator.TagValues("End").Should().HaveCount(2) .And.ContainSingle(x => x.HasConstraint(TestConstraint.First) && !x.HasConstraint(BoolConstraint.True)) .And.ContainSingle(x => x.HasConstraint(TestConstraint.First) && x.HasConstraint(BoolConstraint.True) && !x.HasConstraint(DummyConstraint.Dummy)); @@ -144,7 +145,7 @@ public void GoTo_InstructionVisitedMaxTwice() } Tag(""End"", value);"; var validator = SETestContext.CreateCS(code, new AddConstraintOnInvocationCheck()).Validator; - validator.ValidateExitReachCount(2); + validator.ValidateExitReachCount(1); validator.TagValues("End").Should().HaveCount(2) .And.ContainSingle(x => x.HasConstraint(TestConstraint.First) && !x.HasConstraint(BoolConstraint.True)) .And.ContainSingle(x => x.HasConstraint(TestConstraint.First) && x.HasConstraint(BoolConstraint.True) && !x.HasConstraint(DummyConstraint.Dummy)); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs index 70e2e6ebec5..3a5a2004f0b 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs @@ -41,18 +41,19 @@ public partial class RoslynSymbolicExecutionTest [TestMethod] public void Constructor_Throws() { - var cfg = TestHelper.CompileCfgCS("public class Sample { public void Main() { } }"); + var cfg = TestHelper.CompileCfgCS(out var methodSymbol, "public class Sample { public void Main() { } }"); var check = new Mock().Object; - ((Action)(() => new RoslynSymbolicExecution(null, new[] { check }))).Should().Throw().And.ParamName.Should().Be("cfg"); - ((Action)(() => new RoslynSymbolicExecution(cfg, null))).Should().Throw().WithMessage("At least one check is expected*"); - ((Action)(() => new RoslynSymbolicExecution(cfg, Array.Empty()))).Should().Throw().WithMessage("At least one check is expected*"); + ((Action)(() => new RoslynSymbolicExecution(null, new[] { check }, methodSymbol))).Should().Throw().And.ParamName.Should().Be("cfg"); + ((Action)(() => new RoslynSymbolicExecution(cfg, null, methodSymbol))).Should().Throw().WithMessage("At least one check is expected*"); + ((Action)(() => new RoslynSymbolicExecution(cfg, Array.Empty(), methodSymbol))).Should().Throw().WithMessage("At least one check is expected*"); + ((Action)(() => new RoslynSymbolicExecution(cfg, new[] { check }, null))).Should().Throw().And.ParamName.Should().Be("declaration"); } [TestMethod] public void Execute_SecondRun_Throws() { - var cfg = TestHelper.CompileCfgBodyCS(); - var se = new RoslynSymbolicExecution(cfg, new[] { new ValidatorTestCheck() }); + var cfg = TestHelper.CompileCfgBodyCS(out var methodSymbol); + var se = new RoslynSymbolicExecution(cfg, new[] { new ValidatorTestCheck() }, methodSymbol); se.Execute(); se.Invoking(x => x.Execute()).Should().Throw().WithMessage("Engine can be executed only once."); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs index feed1e75f6b..039cd8e37ab 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs @@ -33,8 +33,8 @@ internal class SETestContext public SETestContext(string code, AnalyzerLanguage language, SymbolicCheck[] additionalChecks) { const string Separator = "----------"; - var cfg = TestHelper.CompileCfg(code, language); - var se = new RoslynSymbolicExecution(cfg, additionalChecks.Concat(new[] { Validator }).ToArray()); + var cfg = TestHelper.CompileCfg(code, language, out var methodSymbol); + var se = new RoslynSymbolicExecution(cfg, additionalChecks.Concat(new[] { Validator }).ToArray(), methodSymbol); Console.WriteLine(Separator); Console.Write(CfgSerializer.Serialize(cfg)); Console.WriteLine(Separator); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs index 4920ccd3a5c..b610da8b9a6 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs @@ -69,7 +69,10 @@ public static (SyntaxTree Tree, SemanticModel Model) Compile(string snippet, boo } public static ControlFlowGraph CompileCfgBodyCS(string body = null, string additionalParameters = null) => - CompileCfg($"public class Sample {{ public void Main({additionalParameters}) {{ {body} }} }}", AnalyzerLanguage.CSharp); + CompileCfgBodyCS(out _, body, additionalParameters); + + public static ControlFlowGraph CompileCfgBodyCS(out ISymbol methodSymbol, string body = null, string additionalParameters = null) => + CompileCfg($"public class Sample {{ public void Main({additionalParameters}) {{ {body} }} }}", AnalyzerLanguage.CSharp, out methodSymbol); public static ControlFlowGraph CompileCfgBodyVB(string body = null) => CompileCfg( @@ -79,13 +82,21 @@ Public Sub Main() End Sub End Class", AnalyzerLanguage.VisualBasic); + public static ControlFlowGraph CompileCfgCS(out ISymbol methodSymbol, string snippet, bool ignoreErrors = false) => + CompileCfg(snippet, AnalyzerLanguage.CSharp, out methodSymbol, ignoreErrors); + public static ControlFlowGraph CompileCfgCS(string snippet, bool ignoreErrors = false) => - CompileCfg(snippet, AnalyzerLanguage.CSharp, ignoreErrors); + CompileCfgCS(out _, snippet, ignoreErrors); - public static ControlFlowGraph CompileCfg(string snippet, AnalyzerLanguage language, bool ignoreErrors = false) + public static ControlFlowGraph CompileCfg(string snippet, AnalyzerLanguage language, bool ignoreErrors = false) => + CompileCfg(snippet, language, out _, ignoreErrors); + + public static ControlFlowGraph CompileCfg(string snippet, AnalyzerLanguage language, out ISymbol methodsymbol, bool ignoreErrors = false) { var (tree, semanticModel) = Compile(snippet, ignoreErrors, language); var method = tree.GetRoot().DescendantNodes().First(IsMethod); + methodsymbol = semanticModel.GetDeclaredSymbol(method); + return ControlFlowGraph.Create(method, semanticModel); bool IsMethod(SyntaxNode node) => From 6104786193fe4fdd4eb6fe9fdeb11e7df9ea1100 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Fri, 11 Mar 2022 12:13:28 +0100 Subject: [PATCH 02/19] Fix ProgramState and add more UTs --- .../SymbolicExecution/Roslyn/ProgramState.cs | 23 +++++++- .../Roslyn/RoslynSymbolicExecution.cs | 1 - .../Roslyn/ProgramStateTest.SymbolValue.cs | 24 +++++++++ .../Roslyn/ProgramStateTest.cs | 52 +++++++++++++++---- .../RoslynSymbolicExecutionTest.Branching.cs | 42 +++++++++++++++ .../RoslynSymbolicExecutionTest.Loops.cs | 6 +-- .../SymbolicExecution/SETestContext.cs | 2 + .../SymbolicExecution/ValidatorTestCheck.cs | 14 +++++ 8 files changed, 149 insertions(+), 15 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs index 67d6e57663c..2fd5283e670 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs @@ -114,6 +114,7 @@ public override int GetHashCode() => HashCode.Combine( HashCode.DictionaryContentHash(OperationValue), HashCode.DictionaryContentHash(SymbolValue), + HashCode.DictionaryContentHash(SymbolValue)) + PreservedSymbols.Aggregate(0, (seed, x) => seed + x.GetHashCode()); HashCode.DictionaryContentHash(CaptureOperation)); public bool Equals(ProgramState other) => @@ -121,7 +122,9 @@ public bool Equals(ProgramState other) => other is not null && other.OperationValue.DictionaryEquals(OperationValue) && other.SymbolValue.DictionaryEquals(SymbolValue) - && other.CaptureOperation.DictionaryEquals(CaptureOperation); + && other.CaptureOperation.DictionaryEquals(CaptureOperation) + && other.PreservedSymbols.Count == PreservedSymbols.Count + && other.PreservedSymbols.All(x => PreservedSymbols.Contains(x)); public override string ToString() => Equals(Empty) ? "Empty" : SerializeSymbols() + SerializeOperations() + SerializeCaptures(); @@ -132,6 +135,24 @@ private string SerializeSymbols() => private string SerializeOperations() => Serialize(OperationValue, "Operations", x => x.Serialize(), x => x.ToString()); + private string SerializePreservedSymbols() + { + if (PreservedSymbols.IsEmpty) + { + return null; + } + else + { + var sb = new StringBuilder(); + sb.Append("PreservedSymbols").AppendLine(":"); + foreach (var symbol in PreservedSymbols) + { + sb.AppendLine(symbol.ToString()); + } + return sb.ToString(); + } + } + private string SerializeCaptures() => Serialize(CaptureOperation, "Captures", x => "#" + x.GetHashCode(), x => x.Serialize()); diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index dc545b6d35e..fb4557212f8 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -206,7 +206,6 @@ private ProgramState CleanStateAfterBlock(ProgramState programState, BasicBlock { var liveVariables = lva.LiveOut(block).Union(nonInDeclarationParameters); // LVA excludes out and ref parameters - // ToDo: Remove the IFieldSymbol check when SLVS-1136 is fixed return programState.RemoveSymbols( symbol => symbol is not IFieldSymbol && !liveVariables.Contains(symbol)); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs index afd367cbedb..991abd3c5fc 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs @@ -21,6 +21,7 @@ using System; using System.Linq; using FluentAssertions; +using Microsoft.CodeAnalysis; using Microsoft.VisualStudio.TestTools.UnitTesting; using SonarAnalyzer.SymbolicExecution.Roslyn; using SonarAnalyzer.UnitTest.TestFramework.SymbolicExecution; @@ -119,6 +120,29 @@ public void SymbolsWith_IgnoreNullValue() sut.SymbolsWith(DummyConstraint.Dummy).Should().BeEmpty(); } + [TestMethod] + public void Preserve_PreservedSymbolCannotBeDeleted() + { + var counter = new SymbolicValueCounter(); + var symbolicValue = new SymbolicValue(counter).WithConstraint(DummyConstraint.Dummy); + var symbol = CreateSymbols().First(); + var sut = ProgramState.Empty.SetSymbolValue(symbol, symbolicValue); + sut = sut.Preserve(symbol); + sut = sut.RemoveSymbols(x => SymbolEqualityComparer.Default.Equals(x, symbol)); + sut.SymbolsWith(DummyConstraint.Dummy).Should().Contain(symbol); + } + + [TestMethod] + public void RemoveSymbols_RemovesTheSymbol() + { + var counter = new SymbolicValueCounter(); + var symbolicValue = new SymbolicValue(counter).WithConstraint(DummyConstraint.Dummy); + var symbol = CreateSymbols().First(); + var sut = ProgramState.Empty.SetSymbolValue(symbol, symbolicValue); + sut = sut.RemoveSymbols(x => SymbolEqualityComparer.Default.Equals(x, symbol)); + sut.SymbolsWith(DummyConstraint.Dummy).Should().BeEmpty(); + } + [TestMethod] public void SetSymbolConstraint_NoValue_CreatesNewValue() { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.cs index ac4d068fdb6..708e7638704 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.cs @@ -39,8 +39,9 @@ public void Equals_ReturnsTrueForEquivalent() var counter = new SymbolicValueCounter(); var reusedValue = new SymbolicValue(counter).WithConstraint(TestConstraint.First); var anotherValue = new SymbolicValue(counter).WithConstraint(TestConstraint.Second); - var operations = TestHelper.CompileCfgBodyCS("var x = 42;").Blocks[1].Operations.ToExecutionOrder().ToArray(); + var operations = TestHelper.CompileCfgBodyCS("var x = 42; var y = 42;").Blocks[1].Operations.ToExecutionOrder().ToArray(); var symbol = operations.Select(x => x.Instance.TrackedSymbol()).First(x => x is not null); + var othersymbol = operations.Select(x => x.Instance.TrackedSymbol()).Where(x => x is not null).ToList()[1]; var empty = ProgramState.Empty; var withOperationOrig = empty.SetOperationValue(operations[0], reusedValue); var withOperationSame = empty.SetOperationValue(operations[0], reusedValue); @@ -51,9 +52,12 @@ public void Equals_ReturnsTrueForEquivalent() var withCaptureOrig = empty.SetCapture(new CaptureId(0), operations[0].Instance); var withCaptureSame = empty.SetCapture(new CaptureId(0), operations[0].Instance); var withCaptureDiff = empty.SetCapture(new CaptureId(0), operations[1].Instance); - var mixedOrig = withOperationOrig.SetSymbolValue(symbol, reusedValue); - var mixedSame = withOperationSame.SetSymbolValue(symbol, reusedValue); - var mixedDiff = withOperationDiff.SetSymbolValue(symbol, anotherValue); + var withPreservedSymbolOrig = empty.Preserve(symbol); + var withPreservedSymbolSame = empty.Preserve(symbol); + var withPreservedSymbolDiff = empty.Preserve(othersymbol); + var mixedOrig = withOperationOrig.SetSymbolValue(symbol, reusedValue).Preserve(symbol); + var mixedSame = withOperationSame.SetSymbolValue(symbol, reusedValue).Preserve(symbol); + var mixedDiff = withOperationDiff.SetSymbolValue(symbol, anotherValue).Preserve(othersymbol); empty.Equals((object)empty).Should().BeTrue(); empty.Equals((object)withOperationOrig).Should().BeFalse(); @@ -67,6 +71,7 @@ public void Equals_ReturnsTrueForEquivalent() withOperationOrig.Equals(empty).Should().BeFalse(); withOperationOrig.Equals(withSymbolDiff).Should().BeFalse(); withOperationOrig.Equals(withCaptureDiff).Should().BeFalse(); + withOperationOrig.Equals(withPreservedSymbolDiff).Should().BeFalse(); withSymbolOrig.Equals(withSymbolOrig).Should().BeTrue(); withSymbolOrig.Equals(withSymbolSame).Should().BeTrue(); @@ -74,6 +79,7 @@ public void Equals_ReturnsTrueForEquivalent() withSymbolOrig.Equals(empty).Should().BeFalse(); withSymbolOrig.Equals(withOperationDiff).Should().BeFalse(); withSymbolOrig.Equals(withCaptureDiff).Should().BeFalse(); + withSymbolOrig.Equals(withPreservedSymbolDiff).Should().BeFalse(); withCaptureOrig.Equals(withCaptureOrig).Should().BeTrue(); withCaptureOrig.Equals(withCaptureSame).Should().BeTrue(); @@ -81,6 +87,12 @@ public void Equals_ReturnsTrueForEquivalent() withCaptureOrig.Equals(empty).Should().BeFalse(); withCaptureOrig.Equals(withOperationDiff).Should().BeFalse(); withCaptureOrig.Equals(withSymbolDiff).Should().BeFalse(); + withPreservedSymbolOrig.Equals(withPreservedSymbolOrig).Should().BeTrue(); + withPreservedSymbolOrig.Equals(withPreservedSymbolSame).Should().BeTrue(); + withPreservedSymbolOrig.Equals(withPreservedSymbolDiff).Should().BeFalse(); + withPreservedSymbolOrig.Equals(empty).Should().BeFalse(); + withPreservedSymbolOrig.Equals(withOperationDiff).Should().BeFalse(); + withPreservedSymbolOrig.Equals(withSymbolDiff).Should().BeFalse(); mixedOrig.Equals(mixedOrig).Should().BeTrue(); mixedOrig.Equals(mixedSame).Should().BeTrue(); @@ -94,8 +106,9 @@ public void GetHashCode_ReturnsSameForEquivalent() var counter = new SymbolicValueCounter(); var reusedValue = new SymbolicValue(counter).WithConstraint(TestConstraint.First); var anotherValue = new SymbolicValue(counter).WithConstraint(TestConstraint.Second); - var operations = TestHelper.CompileCfgBodyCS("var x = 42;").Blocks[1].Operations.ToExecutionOrder().ToArray(); + var operations = TestHelper.CompileCfgBodyCS("var x = 42; var y = 42;").Blocks[1].Operations.ToExecutionOrder().ToArray(); var symbol = operations.Select(x => x.Instance.TrackedSymbol()).First(x => x is not null); + var othersymbol = operations.Select(x => x.Instance.TrackedSymbol()).Where(x => x is not null).ToList()[1]; var empty = ProgramState.Empty; var withOperationOrig = empty.SetOperationValue(operations[0], reusedValue); var withOperationSame = empty.SetOperationValue(operations[0], reusedValue); @@ -106,9 +119,12 @@ public void GetHashCode_ReturnsSameForEquivalent() var withCaptureOrig = empty.SetCapture(new CaptureId(0), operations[0].Instance); var withCaptureSame = empty.SetCapture(new CaptureId(0), operations[0].Instance); var withCaptureDiff = empty.SetCapture(new CaptureId(0), operations[1].Instance); - var mixedOrig = withOperationOrig.SetSymbolValue(symbol, reusedValue); - var mixedSame = withOperationSame.SetSymbolValue(symbol, reusedValue); - var mixedDiff = withOperationDiff.SetSymbolValue(symbol, anotherValue); + var withPreservedSymbolOrig = empty.Preserve(symbol); + var withPreservedSymbolSame = empty.Preserve(symbol); + var withPreservedSymbolDiff = empty.Preserve(othersymbol); + var mixedOrig = withOperationOrig.SetSymbolValue(symbol, reusedValue).Preserve(symbol); + var mixedSame = withOperationSame.SetSymbolValue(symbol, reusedValue).Preserve(symbol); + var mixedDiff = withOperationDiff.SetSymbolValue(symbol, anotherValue).Preserve(othersymbol); empty.GetHashCode().Should().Be(empty.GetHashCode()); @@ -124,6 +140,10 @@ public void GetHashCode_ReturnsSameForEquivalent() withCaptureOrig.GetHashCode().Should().Be(withCaptureSame.GetHashCode()); withCaptureOrig.GetHashCode().Should().NotBe(withCaptureDiff.GetHashCode()); + withPreservedSymbolOrig.GetHashCode().Should().Be(withPreservedSymbolSame.GetHashCode()); + withPreservedSymbolOrig.GetHashCode().Should().NotBe(withPreservedSymbolDiff.GetHashCode()); + withPreservedSymbolOrig.GetHashCode().Should().NotBe(mixedSame.GetHashCode()); + mixedOrig.GetHashCode().Should().Be(mixedSame.GetHashCode()); mixedOrig.GetHashCode().Should().NotBe(mixedDiff.GetHashCode()); } @@ -178,6 +198,18 @@ public void ToString_WithOperations() "); } + [TestMethod] + public void ToString_WithPreservedSymbols() + { + var assignment = TestHelper.CompileCfgBodyCS("var a = true;").Blocks[1].Operations[0]; + var variableSymbol = assignment.Children.First().TrackedSymbol(); + var sut = ProgramState.Empty.Preserve(variableSymbol); + sut.ToString().Should().BeIgnoringLineEndings( +@"PreservedSymbols: +a +"); + } + [TestMethod] public void ToString_WithCaptures() { @@ -193,7 +225,7 @@ public void ToString_WithCaptures() #42: SimpleAssignmentOperation / VariableDeclaratorSyntax: a = true "); } - + [TestMethod] public void ToString_WithAll() { @@ -205,7 +237,7 @@ public void ToString_WithAll() .SetSymbolValue(variableSymbol, new SymbolicValue(counter)) .SetSymbolValue(variableSymbol.ContainingSymbol, valueWithConstraint) .SetOperationValue(assignment, new SymbolicValue(counter)) - .SetOperationValue(assignment.Children.First(), valueWithConstraint) + .SetOperationValue(assignment.Children.First(), valueWithConstraint).Preserve(variableSymbol); .SetCapture(new CaptureId(0), assignment) .SetCapture(new CaptureId(1), assignment.Children.First()); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs index 0da920a8b16..0534991a08c 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs @@ -226,6 +226,48 @@ public void Branching_ConstraintTrackedSeparatelyInBranches() .And.ContainSingle(x => x.HasConstraint(BoolConstraint.False)); } + [TestMethod] + public void Branching_MultipleExistsIfSymbolIsPreserved() + { + const string code = @" +bool value; +if (boolParameter) +{ + value = true; +} +else +{ + value = false; +} +Preserve(value); +Tag(""End"", value);"; + var validator = SETestContext.CreateCS(code, new EmptyTestCheck()).Validator; + validator.ValidateExitReachCount(2); // Once with True constraint, once with False constraint on "value" + validator.TagValues("End").Should().HaveCount(2) + .And.ContainSingle(x => x.HasConstraint(BoolConstraint.True)) + .And.ContainSingle(x => x.HasConstraint(BoolConstraint.False)); + } + + [TestMethod] + public void Branching_FieldSymbolsAreNotCleaned() + { + const string code = @" +if (boolParameter) +{ + boolField = true; +} +else +{ + boolField = false; +} +Tag(""End"", boolField);"; + var validator = SETestContext.CreateCS(code, new EmptyTestCheck()).Validator; + validator.ValidateExitReachCount(2); // Once with True constraint, once with False constraint on "value" + validator.TagValues("End").Should().HaveCount(2) + .And.ContainSingle(x => x.HasConstraint(BoolConstraint.True)) + .And.ContainSingle(x => x.HasConstraint(BoolConstraint.False)); + } + [TestMethod] public void Branching_VisitedProgramState_IsSkipped() { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs index 7bda92e3541..df6dd1c9e1a 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs @@ -33,7 +33,7 @@ public partial class RoslynSymbolicExecutionTest [DataRow("foreach (var i in items)")] [DataRow("while (value > 0)")] [DataRow("while (Condition)")] - public void Loops_InstructionVisitedMaxTwice(string loop) + public void Loops_InstructionVisitedOnlyOnce(string loop) { var code = $@" var value = 42; @@ -51,7 +51,7 @@ public void Loops_InstructionVisitedMaxTwice(string loop) } [TestMethod] - public void Loops_InstructionVisitedMaxTwice_ForEachOriginalState() + public void Loops_InstructionVisitedOnlyOnce_EvenWithMultipleStates() { const string code = @" var value = 42; @@ -132,7 +132,7 @@ public void GoTo_InfiniteWithNoExitBranch_InstructionVisitedMaxTwice() } [TestMethod] - public void GoTo_InstructionVisitedMaxTwice() + public void GoTo_InstructionVisitedOnlyOnce() { const string code = @" var value = 42; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs index 039cd8e37ab..2d3560f3bed 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs @@ -56,6 +56,7 @@ public class Sample public static int StaticProperty {{get; set;}} public int Property {{get; set;}} private int field; + private bool boolField; private bool Condition => Environment.ProcessorCount == 42; // Something that cannot have constraint @@ -65,6 +66,7 @@ public void Main(bool boolParameter{additionalParameters}) }} private void Tag(string name, object arg = null) {{ }} + private void Preserve(object arg) {{ }} }}"; return new(code, AnalyzerLanguage.CSharp, additionalChecks); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/ValidatorTestCheck.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/ValidatorTestCheck.cs index 2873cacd26c..c34fbb48cea 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/ValidatorTestCheck.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/ValidatorTestCheck.cs @@ -38,6 +38,20 @@ internal class ValidatorTestCheck : SymbolicCheck private int executionCompletedCount; public override void ExitReached(SymbolicContext context) => + if (context.Operation.Instance is IInvocationOperation invocation) + { + if (invocation.TargetMethod.Name == "Tag") + { + var tagName = invocation.Arguments.First().Value.ConstantValue; + tagName.HasValue.Should().BeTrue("tag should have literal name"); + tags.Add(((string)tagName.Value, context)); + } + else if (invocation.TargetMethod.Name == "Preserve") + { + var converstion = IConversionOperationWrapper.FromOperation(invocation.Arguments.First().Value); + var symbol = converstion.Operand.TrackedSymbol(); + return context.State.Preserve(symbol); + } exitReachedCount++; public override void ExecutionCompleted() => From 6aa2436318fb41d0006d4a71f0776e33dc447375 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Fri, 11 Mar 2022 14:19:03 +0100 Subject: [PATCH 03/19] Rebase fix --- .../SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index fb4557212f8..96df7d6a67d 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -24,8 +24,8 @@ using Microsoft.CodeAnalysis; using SonarAnalyzer.CFG.LiveVariableAnalysis; using SonarAnalyzer.CFG.Roslyn; -using SonarAnalyzer.SymbolicExecution.Constraints; using SonarAnalyzer.Helpers; +using SonarAnalyzer.SymbolicExecution.Constraints; using SonarAnalyzer.SymbolicExecution.Roslyn.Checks; using SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors; using StyleCop.Analyzers.Lightup; @@ -106,8 +106,9 @@ private IEnumerable ProcessBranching(ExplodedNode node) } else { + var reachableSuccessors = node.Block.Successors.Where(x => IsReachable(node, x)).ToList(); node = new ExplodedNode(node.Block, CleanStateAfterBlock(node.State, node.Block), node.FinallyPoint); - foreach (var successor in node.Block.Successors.Where(x => IsReachable(node, x))) + foreach (var successor in reachableSuccessors) { if (ProcessBranch(node, successor) is { } newNode) { From e4d52eba694913a727f67e3caaa565bb9c0ca57a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Mon, 14 Mar 2022 18:08:31 +0100 Subject: [PATCH 04/19] S1E1 --- .../SonarAnalyzer.Common/Helpers/HashCode.cs | 3 ++ .../SymbolicExecution/Roslyn/ProgramState.cs | 24 ++------- .../Roslyn/RoslynSymbolicExecution.cs | 18 +++---- .../Roslyn/ProgramStateTest.SymbolValue.cs | 24 ++++----- .../Roslyn/ProgramStateTest.cs | 54 +++++++------------ .../RoslynSymbolicExecutionTest.Branching.cs | 33 ++---------- .../RoslynSymbolicExecutionTest.Loops.cs | 10 ++-- .../SymbolicExecution/PreserveTestCheck.cs | 44 +++++++++++++++ .../SymbolicExecution/SETestContext.cs | 1 - .../SymbolicExecution/ValidatorTestCheck.cs | 6 --- 10 files changed, 98 insertions(+), 119 deletions(-) create mode 100644 analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/HashCode.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/HashCode.cs index e7e8be4558e..59679aafe4f 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/HashCode.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/HashCode.cs @@ -34,6 +34,9 @@ namespace SonarAnalyzer.Helpers public static int DictionaryContentHash(IDictionary dictionary) => dictionary.Aggregate(0, (seed, kvp) => Combine(seed, kvp.Key, kvp.Value)); + public static int EnumerableContentHash(IEnumerable enumerable) => + enumerable.Aggregate(0, (seed, x) => seed + x.GetHashCode()); + public static int Combine(T1 a, T2 b) => (int)Seed .AddHash((uint)(a?.GetHashCode() ?? 0)) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs index 2fd5283e670..3b528346e8a 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs @@ -97,8 +97,8 @@ public IOperation ResolveCapture(IOperation operation) => ? captured : operation; - public ProgramState RemoveSymbols(Func predicate) => - this with { SymbolValue = SymbolValue.Where(kv => !predicate(kv.Key) || PreservedSymbols.Contains(kv.Key)).ToImmutableDictionary() }; + public ProgramState RemoveSymbols(Func remove) => + this with { SymbolValue = SymbolValue.Where(kv => PreservedSymbols.Contains(kv.Key) || !remove(kv.Key)).ToImmutableDictionary() }; public ProgramState AddVisit(int programPointHash) => this with { VisitCount = VisitCount.SetItem(programPointHash, GetVisitCount(programPointHash) + 1) }; @@ -114,7 +114,7 @@ public override int GetHashCode() => HashCode.Combine( HashCode.DictionaryContentHash(OperationValue), HashCode.DictionaryContentHash(SymbolValue), - HashCode.DictionaryContentHash(SymbolValue)) + PreservedSymbols.Aggregate(0, (seed, x) => seed + x.GetHashCode()); + HashCode.EnumerableContentHash(PreservedSymbols)); HashCode.DictionaryContentHash(CaptureOperation)); public bool Equals(ProgramState other) => @@ -135,24 +135,6 @@ private string SerializeSymbols() => private string SerializeOperations() => Serialize(OperationValue, "Operations", x => x.Serialize(), x => x.ToString()); - private string SerializePreservedSymbols() - { - if (PreservedSymbols.IsEmpty) - { - return null; - } - else - { - var sb = new StringBuilder(); - sb.Append("PreservedSymbols").AppendLine(":"); - foreach (var symbol in PreservedSymbols) - { - sb.AppendLine(symbol.ToString()); - } - return sb.ToString(); - } - } - private string SerializeCaptures() => Serialize(CaptureOperation, "Captures", x => "#" + x.GetHashCode(), x => x.Serialize()); diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index 96df7d6a67d..06a12e3987e 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -48,18 +48,14 @@ internal class RoslynSymbolicExecution public RoslynSymbolicExecution(ControlFlowGraph cfg, SymbolicCheck[] checks, ISymbol declaration) { this.cfg = cfg ?? throw new ArgumentNullException(nameof(cfg)); - if (declaration == null) - { - throw new ArgumentNullException(nameof(declaration)); - } if (checks == null || checks.Length == 0) { throw new ArgumentException("At least one check is expected", nameof(checks)); } + _ = declaration ?? throw new ArgumentNullException(nameof(declaration)); this.checks = new(new[] { new ConstantCheck() }.Concat(checks).ToArray()); lva = new RoslynLiveVariableAnalysis(cfg, declaration); - var declarationParameters = declaration.GetParameters(); - nonInDeclarationParameters = declarationParameters.Where(p => p.RefKind != RefKind.None); + nonInDeclarationParameters = declaration.GetParameters().Where(p => p.RefKind != RefKind.None); } public void Execute() @@ -101,13 +97,13 @@ private IEnumerable ProcessBranching(ExplodedNode node) } else if (node.Block.ContainsThrow()) { - node = new ExplodedNode(cfg.ExitBlock, CleanStateAfterBlock(node.State, node.Block), null); + node = new ExplodedNode(cfg.ExitBlock, CleanUnusedState(node.State, node.Block), null); yield return new(cfg.ExitBlock, node.State, null); } else { var reachableSuccessors = node.Block.Successors.Where(x => IsReachable(node, x)).ToList(); - node = new ExplodedNode(node.Block, CleanStateAfterBlock(node.State, node.Block), node.FinallyPoint); + node = new ExplodedNode(node.Block, CleanUnusedState(node.State, node.Block), node.FinallyPoint); foreach (var successor in reachableSuccessors) { if (ProcessBranch(node, successor) is { } newNode) @@ -203,12 +199,10 @@ private static ProgramState ProcessOperation(SymbolicContext context) => _ => context.State }; - private ProgramState CleanStateAfterBlock(ProgramState programState, BasicBlock block) + private ProgramState CleanUnusedState(ProgramState programState, BasicBlock block) { var liveVariables = lva.LiveOut(block).Union(nonInDeclarationParameters); // LVA excludes out and ref parameters - - return programState.RemoveSymbols( - symbol => symbol is not IFieldSymbol && !liveVariables.Contains(symbol)); + return programState.RemoveSymbols(x => x is not IFieldSymbol && !liveVariables.Contains(x)); } private static bool IsReachable(ExplodedNode node, ControlFlowBranch branch) => diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs index 991abd3c5fc..ac9f2f90d6e 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs @@ -123,24 +123,24 @@ public void SymbolsWith_IgnoreNullValue() [TestMethod] public void Preserve_PreservedSymbolCannotBeDeleted() { - var counter = new SymbolicValueCounter(); - var symbolicValue = new SymbolicValue(counter).WithConstraint(DummyConstraint.Dummy); + var symbolicValue = new SymbolicValue(new()).WithConstraint(DummyConstraint.Dummy); var symbol = CreateSymbols().First(); - var sut = ProgramState.Empty.SetSymbolValue(symbol, symbolicValue); - sut = sut.Preserve(symbol); - sut = sut.RemoveSymbols(x => SymbolEqualityComparer.Default.Equals(x, symbol)); + var sut = ProgramState.Empty.SetSymbolValue(symbol, symbolicValue) + .Preserve(symbol) + .RemoveSymbols(x => true); sut.SymbolsWith(DummyConstraint.Dummy).Should().Contain(symbol); } [TestMethod] - public void RemoveSymbols_RemovesTheSymbol() + public void RemoveSymbols_RemovesTheSymbolMatchingThePredicate() { - var counter = new SymbolicValueCounter(); - var symbolicValue = new SymbolicValue(counter).WithConstraint(DummyConstraint.Dummy); - var symbol = CreateSymbols().First(); - var sut = ProgramState.Empty.SetSymbolValue(symbol, symbolicValue); - sut = sut.RemoveSymbols(x => SymbolEqualityComparer.Default.Equals(x, symbol)); - sut.SymbolsWith(DummyConstraint.Dummy).Should().BeEmpty(); + var symbolicValue = new SymbolicValue(new()).WithConstraint(DummyConstraint.Dummy); + var symbols = CreateSymbols().ToArray(); + var sut = ProgramState.Empty.SetSymbolValue(symbols[0], symbolicValue) + .SetSymbolValue(symbols[1], symbolicValue) + .SetSymbolValue(symbols[2], symbolicValue) + .RemoveSymbols(x => !SymbolEqualityComparer.Default.Equals(x, symbols[1])); + sut.SymbolsWith(DummyConstraint.Dummy).Should().Contain(symbols[1]); } [TestMethod] diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.cs index 708e7638704..6f2afd445d4 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.cs @@ -40,24 +40,23 @@ public void Equals_ReturnsTrueForEquivalent() var reusedValue = new SymbolicValue(counter).WithConstraint(TestConstraint.First); var anotherValue = new SymbolicValue(counter).WithConstraint(TestConstraint.Second); var operations = TestHelper.CompileCfgBodyCS("var x = 42; var y = 42;").Blocks[1].Operations.ToExecutionOrder().ToArray(); - var symbol = operations.Select(x => x.Instance.TrackedSymbol()).First(x => x is not null); - var othersymbol = operations.Select(x => x.Instance.TrackedSymbol()).Where(x => x is not null).ToList()[1]; + var symbols = operations.Select(x => x.Instance.TrackedSymbol()).Where(x => x is not null).ToArray(); var empty = ProgramState.Empty; var withOperationOrig = empty.SetOperationValue(operations[0], reusedValue); var withOperationSame = empty.SetOperationValue(operations[0], reusedValue); var withOperationDiff = empty.SetOperationValue(operations[0], anotherValue); - var withSymbolOrig = empty.SetSymbolValue(symbol, reusedValue); - var withSymbolSame = empty.SetSymbolValue(symbol, reusedValue); - var withSymbolDiff = empty.SetSymbolValue(symbol, anotherValue); + var withSymbolOrig = empty.SetSymbolValue(symbols[0], reusedValue); + var withSymbolSame = empty.SetSymbolValue(symbols[0], reusedValue); + var withSymbolDiff = empty.SetSymbolValue(symbols[0], anotherValue); var withCaptureOrig = empty.SetCapture(new CaptureId(0), operations[0].Instance); var withCaptureSame = empty.SetCapture(new CaptureId(0), operations[0].Instance); var withCaptureDiff = empty.SetCapture(new CaptureId(0), operations[1].Instance); - var withPreservedSymbolOrig = empty.Preserve(symbol); - var withPreservedSymbolSame = empty.Preserve(symbol); - var withPreservedSymbolDiff = empty.Preserve(othersymbol); - var mixedOrig = withOperationOrig.SetSymbolValue(symbol, reusedValue).Preserve(symbol); - var mixedSame = withOperationSame.SetSymbolValue(symbol, reusedValue).Preserve(symbol); - var mixedDiff = withOperationDiff.SetSymbolValue(symbol, anotherValue).Preserve(othersymbol); + var withPreservedSymbolOrig = empty.Preserve(symbols[0]); + var withPreservedSymbolSame = empty.Preserve(symbols[0]); + var withPreservedSymbolDiff = empty.Preserve(symbols[1]); + var mixedOrig = withOperationOrig.SetSymbolValue(symbols[0], reusedValue); + var mixedSame = withOperationSame.SetSymbolValue(symbols[0], reusedValue); + var mixedDiff = withOperationDiff.SetSymbolValue(symbols[0], anotherValue); empty.Equals((object)empty).Should().BeTrue(); empty.Equals((object)withOperationOrig).Should().BeFalse(); @@ -107,24 +106,23 @@ public void GetHashCode_ReturnsSameForEquivalent() var reusedValue = new SymbolicValue(counter).WithConstraint(TestConstraint.First); var anotherValue = new SymbolicValue(counter).WithConstraint(TestConstraint.Second); var operations = TestHelper.CompileCfgBodyCS("var x = 42; var y = 42;").Blocks[1].Operations.ToExecutionOrder().ToArray(); - var symbol = operations.Select(x => x.Instance.TrackedSymbol()).First(x => x is not null); - var othersymbol = operations.Select(x => x.Instance.TrackedSymbol()).Where(x => x is not null).ToList()[1]; + var symbols = operations.Select(x => x.Instance.TrackedSymbol()).Where(x => x is not null).ToArray(); var empty = ProgramState.Empty; var withOperationOrig = empty.SetOperationValue(operations[0], reusedValue); var withOperationSame = empty.SetOperationValue(operations[0], reusedValue); var withOperationDiff = empty.SetOperationValue(operations[0], anotherValue); - var withSymbolOrig = empty.SetSymbolValue(symbol, reusedValue); - var withSymbolSame = empty.SetSymbolValue(symbol, reusedValue); - var withSymbolDiff = empty.SetSymbolValue(symbol, anotherValue); + var withSymbolOrig = empty.SetSymbolValue(symbols[0], reusedValue); + var withSymbolSame = empty.SetSymbolValue(symbols[0], reusedValue); + var withSymbolDiff = empty.SetSymbolValue(symbols[0], anotherValue); var withCaptureOrig = empty.SetCapture(new CaptureId(0), operations[0].Instance); var withCaptureSame = empty.SetCapture(new CaptureId(0), operations[0].Instance); var withCaptureDiff = empty.SetCapture(new CaptureId(0), operations[1].Instance); - var withPreservedSymbolOrig = empty.Preserve(symbol); - var withPreservedSymbolSame = empty.Preserve(symbol); - var withPreservedSymbolDiff = empty.Preserve(othersymbol); - var mixedOrig = withOperationOrig.SetSymbolValue(symbol, reusedValue).Preserve(symbol); - var mixedSame = withOperationSame.SetSymbolValue(symbol, reusedValue).Preserve(symbol); - var mixedDiff = withOperationDiff.SetSymbolValue(symbol, anotherValue).Preserve(othersymbol); + var withPreservedSymbolOrig = empty.Preserve(symbols[0]); + var withPreservedSymbolSame = empty.Preserve(symbols[0]); + var withPreservedSymbolDiff = empty.Preserve(symbols[1]); + var mixedOrig = withOperationOrig.SetSymbolValue(symbols[0], reusedValue); + var mixedSame = withOperationSame.SetSymbolValue(symbols[0], reusedValue); + var mixedDiff = withOperationDiff.SetSymbolValue(symbols[0], anotherValue); empty.GetHashCode().Should().Be(empty.GetHashCode()); @@ -198,18 +196,6 @@ public void ToString_WithOperations() "); } - [TestMethod] - public void ToString_WithPreservedSymbols() - { - var assignment = TestHelper.CompileCfgBodyCS("var a = true;").Blocks[1].Operations[0]; - var variableSymbol = assignment.Children.First().TrackedSymbol(); - var sut = ProgramState.Empty.Preserve(variableSymbol); - sut.ToString().Should().BeIgnoringLineEndings( -@"PreservedSymbols: -a -"); - } - [TestMethod] public void ToString_WithCaptures() { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs index 0534991a08c..cd324183f0f 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs @@ -200,8 +200,8 @@ public System.Collections.Generic.IEnumerable Method(bool a) var b = a; }"; - var validator = SETestContext.CreateCSMethod(method).Validator; - validator.ValidateExitReachCount(1); + var validator = SETestContext.CreateCSMethod(method, new PreserveTestCheck("b")).Validator; + validator.ValidateExitReachCount(2); validator.ValidateExecutionCompleted(); } @@ -219,30 +219,8 @@ public void Branching_ConstraintTrackedSeparatelyInBranches() value = false; } Tag(""End"", value);"; - var validator = SETestContext.CreateCS(code).Validator; - validator.ValidateExitReachCount(1); // The exit is only reached once as the symbolicValue is removed when value is not used anymore. - validator.TagValues("End").Should().HaveCount(2) - .And.ContainSingle(x => x.HasConstraint(BoolConstraint.True)) - .And.ContainSingle(x => x.HasConstraint(BoolConstraint.False)); - } - - [TestMethod] - public void Branching_MultipleExistsIfSymbolIsPreserved() - { - const string code = @" -bool value; -if (boolParameter) -{ - value = true; -} -else -{ - value = false; -} -Preserve(value); -Tag(""End"", value);"; - var validator = SETestContext.CreateCS(code, new EmptyTestCheck()).Validator; - validator.ValidateExitReachCount(2); // Once with True constraint, once with False constraint on "value" + var validator = SETestContext.CreateCS(code, new PreserveTestCheck("value")).Validator; + validator.ValidateExitReachCount(2); validator.TagValues("End").Should().HaveCount(2) .And.ContainSingle(x => x.HasConstraint(BoolConstraint.True)) .And.ContainSingle(x => x.HasConstraint(BoolConstraint.False)); @@ -311,7 +289,6 @@ public void Branching_VisitedProgramState_IsImmutable() return x.State; }); var validator = SETestContext.CreateCS(code, postProcess).Validator; - validator.ValidateExitReachCount(1); captured.Should().OnlyContain(x => x.Value.HasConstraint(BoolConstraint.True) == x.ExpectedHasTrueConstraint); } @@ -337,7 +314,7 @@ public void Branching_VisitedSymbolicValue_IsImmutable() var validator = SETestContext.CreateCS(code, postProcess).Validator; validator.ValidateTag("ToString", x => x.HasConstraint(TestConstraint.First).Should().BeTrue()); validator.ValidateTag("GetHashCode", x => x.HasConstraint(TestConstraint.First).Should().BeFalse()); // Nobody set the constraint on that path - validator.ValidateExitReachCount(1); // Once as the states are cleaned after the variables are not used anymore. + validator.ValidateExitReachCount(1); // Once as the states are cleaned by LVA. } [TestMethod] diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs index df6dd1c9e1a..00750496045 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs @@ -33,7 +33,7 @@ public partial class RoslynSymbolicExecutionTest [DataRow("foreach (var i in items)")] [DataRow("while (value > 0)")] [DataRow("while (Condition)")] - public void Loops_InstructionVisitedOnlyOnce(string loop) + public void Loops_InstructionVisitedMaxTwice(string loop) { var code = $@" var value = 42; @@ -43,15 +43,15 @@ public void Loops_InstructionVisitedOnlyOnce(string loop) value--; }} Tag(""End"", value);"; - var validator = SETestContext.CreateCS(code, ", int[] items", new AddConstraintOnInvocationCheck()).Validator; - validator.ValidateExitReachCount(1); + var validator = SETestContext.CreateCS(code, ", int[] items", new AddConstraintOnInvocationCheck(), new PreserveTestCheck("value")).Validator; + validator.ValidateExitReachCount(2); validator.TagValues("End").Should().HaveCount(2) .And.ContainSingle(x => x == null) .And.ContainSingle(x => x != null && x.HasConstraint(TestConstraint.First) && !x.HasConstraint(BoolConstraint.True)); } [TestMethod] - public void Loops_InstructionVisitedOnlyOnce_EvenWithMultipleStates() + public void Loops_InstructionVisitedMaxTwice_EvenWithMultipleStates() { const string code = @" var value = 42; @@ -132,7 +132,7 @@ public void GoTo_InfiniteWithNoExitBranch_InstructionVisitedMaxTwice() } [TestMethod] - public void GoTo_InstructionVisitedOnlyOnce() + public void GoTo_InstructionVisitedMaxTwice() { const string code = @" var value = 42; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs new file mode 100644 index 00000000000..9da8eddbe78 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs @@ -0,0 +1,44 @@ +/* + * SonarAnalyzer for .NET + * Copyright (C) 2015-2022 SonarSource SA + * mailto: contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using Microsoft.CodeAnalysis.Operations; +using SonarAnalyzer.SymbolicExecution.Roslyn; + +namespace SonarAnalyzer.UnitTest.TestFramework.SymbolicExecution +{ + /// + /// Empty check implementation used to start the engine (the engine needs at least one check to start). + /// + public class PreserveTestCheck : SymbolicCheck + { + private readonly string symbolName; + + public PreserveTestCheck(string symbolName) + { + this.symbolName = symbolName; + } + + public override ProgramState PreProcess(SymbolicContext context) => + context.Operation.Instance.TrackedSymbol() is { } symbol + && symbol.Name.Equals(symbolName) + ? context.State.Preserve(symbol) + : base.PreProcess(context); + } +} diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs index 2d3560f3bed..15c930f13a2 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs @@ -66,7 +66,6 @@ public void Main(bool boolParameter{additionalParameters}) }} private void Tag(string name, object arg = null) {{ }} - private void Preserve(object arg) {{ }} }}"; return new(code, AnalyzerLanguage.CSharp, additionalChecks); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/ValidatorTestCheck.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/ValidatorTestCheck.cs index c34fbb48cea..e31d3a5ed56 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/ValidatorTestCheck.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/ValidatorTestCheck.cs @@ -46,12 +46,6 @@ public override void ExitReached(SymbolicContext context) => tagName.HasValue.Should().BeTrue("tag should have literal name"); tags.Add(((string)tagName.Value, context)); } - else if (invocation.TargetMethod.Name == "Preserve") - { - var converstion = IConversionOperationWrapper.FromOperation(invocation.Arguments.First().Value); - var symbol = converstion.Operand.TrackedSymbol(); - return context.State.Preserve(symbol); - } exitReachedCount++; public override void ExecutionCompleted() => From 4d0efcba2ccec5dcd3c5374f46428640dff04fe1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Tue, 15 Mar 2022 12:14:28 +0100 Subject: [PATCH 05/19] S1E2 --- .../Roslyn/RoslynSymbolicExecution.cs | 2 +- .../RoslynSymbolicExecutionTest.Branching.cs | 31 ++++++++--- .../Roslyn/RoslynSymbolicExecutionTest.cs | 51 +++++++++++++++++++ .../SymbolicExecution/SETestContext.cs | 1 - 4 files changed, 76 insertions(+), 9 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index 06a12e3987e..56e0bc0946a 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -201,7 +201,7 @@ private static ProgramState ProcessOperation(SymbolicContext context) => private ProgramState CleanUnusedState(ProgramState programState, BasicBlock block) { - var liveVariables = lva.LiveOut(block).Union(nonInDeclarationParameters); // LVA excludes out and ref parameters + var liveVariables = lva.LiveOut(block).Concat(nonInDeclarationParameters).ToHashSet(); // LVA excludes out and ref parameters return programState.RemoveSymbols(x => x is not IFieldSymbol && !liveVariables.Contains(x)); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs index cd324183f0f..bdd7d89eab3 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs @@ -232,18 +232,35 @@ public void Branching_FieldSymbolsAreNotCleaned() const string code = @" if (boolParameter) { - boolField = true; + field = 1; } else { - boolField = false; + field = 2; } -Tag(""End"", boolField);"; - var validator = SETestContext.CreateCS(code, new EmptyTestCheck()).Validator; - validator.ValidateExitReachCount(2); // Once with True constraint, once with False constraint on "value" +Tag(""End"", field);"; + + var postProcess = new PostProcessTestCheck(x => + { + if (x.Operation.Instance.Kind == OperationKind.Literal && x.Operation.Instance.ConstantValue.Value is int value) + { + if (value == 1) + { + return x.SetOperationConstraint(TestConstraint.First); + } + else if (value == 2) + { + return x.SetOperationConstraint(TestConstraint.Second); + } + } + return x.State; + + }); + var validator = SETestContext.CreateCS(code, postProcess).Validator; + validator.ValidateExitReachCount(2); // Once with First constraint, once with Second constraint on "value" validator.TagValues("End").Should().HaveCount(2) - .And.ContainSingle(x => x.HasConstraint(BoolConstraint.True)) - .And.ContainSingle(x => x.HasConstraint(BoolConstraint.False)); + .And.ContainSingle(x => x.HasConstraint(TestConstraint.First)) + .And.ContainSingle(x => x.HasConstraint(TestConstraint.Second)); } [TestMethod] diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs index 3a5a2004f0b..bc09b50dd6c 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs @@ -177,6 +177,57 @@ static ISymbol LocalReferenceOperationSymbol(IOperationWrapperSonar operation) = ((ILocalReferenceOperation)operation.Instance).Local; } + [TestMethod] + public void Execute_UnusedVariable_ClearedAfterBlock() + { + const string code = @" +bool first; +if(boolParameter) + first = true; +else + first = false; +Tag(""BeforeLastUse""); +bool second = first; +if(boolParameter) +boolParameter.ToString(); +Tag(""AfterLastUse""); +bool third = second; +if(boolParameter) +boolParameter.ToString(); +Tag(""END""); +"; + ISymbol firstSymbol = null; + ISymbol secondSymbol = null; + var postProcess = new PostProcessTestCheck(x => + { + if (x.Operation.Instance.TrackedSymbol() is { } symbol) + { + if (symbol.Name.Equals("first")) + { + firstSymbol = symbol; + } + else if (symbol.Name.Equals("second")) + { + secondSymbol = symbol; + } + } + return x.State; + }); + var validator = SETestContext.CreateCS(code, postProcess).Validator; + validator.ValidateTagOrder( + "BeforeLastUse", + "BeforeLastUse", + "AfterLastUse", + "AfterLastUse", + "END"); + var beforeLastUseStates = validator.TagStates("BeforeLastUse"); + beforeLastUseStates.Should().HaveCount(2); + beforeLastUseStates.All(x => x[firstSymbol] != null).Should().BeTrue(); + var afterLastUseStates = validator.TagStates("AfterLastUse"); + afterLastUseStates.Should().HaveCount(2); + afterLastUseStates.All(x => x[firstSymbol] == null && x[secondSymbol] != null).Should().BeTrue(); + } + [TestMethod] public void Execute_TooManyBlocks_NotSupported() { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs index 15c930f13a2..039cd8e37ab 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs @@ -56,7 +56,6 @@ public class Sample public static int StaticProperty {{get; set;}} public int Property {{get; set;}} private int field; - private bool boolField; private bool Condition => Environment.ProcessorCount == 42; // Something that cannot have constraint From 50a1692043c38cbc381e9e0eeb952f07c788c28f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Tue, 15 Mar 2022 16:05:35 +0100 Subject: [PATCH 06/19] S1E3 --- .../Roslyn/RoslynSymbolicExecution.cs | 6 +-- .../RoslynSymbolicExecutionTest.Branching.cs | 39 ++++++++++++++++++- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index 56e0bc0946a..e328b44ba14 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -43,7 +43,6 @@ internal class RoslynSymbolicExecution private readonly SymbolicValueCounter symbolicValueCounter = new(); private readonly HashSet visited = new(); private readonly RoslynLiveVariableAnalysis lva; - private readonly IEnumerable nonInDeclarationParameters; public RoslynSymbolicExecution(ControlFlowGraph cfg, SymbolicCheck[] checks, ISymbol declaration) { @@ -55,7 +54,6 @@ public RoslynSymbolicExecution(ControlFlowGraph cfg, SymbolicCheck[] checks, ISy _ = declaration ?? throw new ArgumentNullException(nameof(declaration)); this.checks = new(new[] { new ConstantCheck() }.Concat(checks).ToArray()); lva = new RoslynLiveVariableAnalysis(cfg, declaration); - nonInDeclarationParameters = declaration.GetParameters().Where(p => p.RefKind != RefKind.None); } public void Execute() @@ -201,8 +199,8 @@ private static ProgramState ProcessOperation(SymbolicContext context) => private ProgramState CleanUnusedState(ProgramState programState, BasicBlock block) { - var liveVariables = lva.LiveOut(block).Concat(nonInDeclarationParameters).ToHashSet(); // LVA excludes out and ref parameters - return programState.RemoveSymbols(x => x is not IFieldSymbol && !liveVariables.Contains(x)); + var liveVariables = lva.LiveOut(block); + return programState.RemoveSymbols(x => (x is ILocalSymbol or IParameterSymbol { RefKind:RefKind.None }) && !liveVariables.Contains(x)); } private static bool IsReachable(ExplodedNode node, ControlFlowBranch branch) => diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs index bdd7d89eab3..352dccb6617 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs @@ -254,7 +254,6 @@ public void Branching_FieldSymbolsAreNotCleaned() } } return x.State; - }); var validator = SETestContext.CreateCS(code, postProcess).Validator; validator.ValidateExitReachCount(2); // Once with First constraint, once with Second constraint on "value" @@ -263,6 +262,44 @@ public void Branching_FieldSymbolsAreNotCleaned() .And.ContainSingle(x => x.HasConstraint(TestConstraint.Second)); } + [DataTestMethod] + [DataRow("out", "outParam")] + [DataRow("ref", "refParam")] + public void Branching_RefAndOutParameters_NotCleared(string paramType, string paramName) + { + string code = $@" +if (boolParameter) +{{ + {paramName} = 1; +}} +else +{{ + {paramName} = 2; +}} +Tag(""End"", {paramName});"; + + var postProcess = new PostProcessTestCheck(x => + { + if (x.Operation.Instance.Kind == OperationKind.Literal && x.Operation.Instance.ConstantValue.Value is int value) + { + if (value == 1) + { + return x.SetOperationConstraint(TestConstraint.First); + } + else if (value == 2) + { + return x.SetOperationConstraint(TestConstraint.Second); + } + } + return x.State; + }); + var validator = SETestContext.CreateCS(code, $", {paramType} int {paramName}", postProcess).Validator; + validator.ValidateExitReachCount(2); // Once with First constraint, once with Second constraint on "value" + validator.TagValues("End").Should().HaveCount(2) + .And.ContainSingle(x => x.HasConstraint(TestConstraint.First)) + .And.ContainSingle(x => x.HasConstraint(TestConstraint.Second)); + } + [TestMethod] public void Branching_VisitedProgramState_IsSkipped() { From e19de12d582356237614e10da3679e7c0c52edf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Thu, 17 Mar 2022 17:06:26 +0100 Subject: [PATCH 07/19] Rebase fix --- .../SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs | 2 +- .../Roslyn/RoslynSymbolicExecutionTest.Branching.cs | 4 ++-- .../TestFramework/SymbolicExecution/PreserveTestCheck.cs | 4 ++-- .../TestFramework/SymbolicExecution/ValidatorTestCheck.cs | 8 -------- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index e328b44ba14..267fb8bccbc 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -200,7 +200,7 @@ private static ProgramState ProcessOperation(SymbolicContext context) => private ProgramState CleanUnusedState(ProgramState programState, BasicBlock block) { var liveVariables = lva.LiveOut(block); - return programState.RemoveSymbols(x => (x is ILocalSymbol or IParameterSymbol { RefKind:RefKind.None }) && !liveVariables.Contains(x)); + return programState.RemoveSymbols(x => (x is ILocalSymbol or IParameterSymbol { RefKind: RefKind.None }) && !liveVariables.Contains(x)); } private static bool IsReachable(ExplodedNode node, ControlFlowBranch branch) => diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs index 352dccb6617..982e125a38e 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs @@ -267,7 +267,7 @@ public void Branching_FieldSymbolsAreNotCleaned() [DataRow("ref", "refParam")] public void Branching_RefAndOutParameters_NotCleared(string paramType, string paramName) { - string code = $@" + var code = $@" if (boolParameter) {{ {paramName} = 1; @@ -342,7 +342,7 @@ public void Branching_VisitedProgramState_IsImmutable() } return x.State; }); - var validator = SETestContext.CreateCS(code, postProcess).Validator; + SETestContext.CreateCS(code, postProcess); captured.Should().OnlyContain(x => x.Value.HasConstraint(BoolConstraint.True) == x.ExpectedHasTrueConstraint); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs index 9da8eddbe78..0122d74967c 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs @@ -35,10 +35,10 @@ public PreserveTestCheck(string symbolName) this.symbolName = symbolName; } - public override ProgramState PreProcess(SymbolicContext context) => + protected override ProgramState PreProcessSimple(SymbolicContext context) => context.Operation.Instance.TrackedSymbol() is { } symbol && symbol.Name.Equals(symbolName) ? context.State.Preserve(symbol) - : base.PreProcess(context); + : base.PreProcessSimple(context); } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/ValidatorTestCheck.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/ValidatorTestCheck.cs index e31d3a5ed56..2873cacd26c 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/ValidatorTestCheck.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/ValidatorTestCheck.cs @@ -38,14 +38,6 @@ internal class ValidatorTestCheck : SymbolicCheck private int executionCompletedCount; public override void ExitReached(SymbolicContext context) => - if (context.Operation.Instance is IInvocationOperation invocation) - { - if (invocation.TargetMethod.Name == "Tag") - { - var tagName = invocation.Arguments.First().Value.ConstantValue; - tagName.HasValue.Should().BeTrue("tag should have literal name"); - tags.Add(((string)tagName.Value, context)); - } exitReachedCount++; public override void ExecutionCompleted() => From 337719551a7f5626de152435874407c429f41230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Thu, 17 Mar 2022 17:23:20 +0100 Subject: [PATCH 08/19] S2E1 --- .../src/SonarAnalyzer.Common/Helpers/HashCode.cs | 2 +- .../Roslyn/ProgramStateTest.SymbolValue.cs | 2 +- .../Roslyn/RoslynSymbolicExecutionTest.Branching.cs | 2 +- .../SymbolicExecution/PreserveTestCheck.cs | 12 +++--------- .../TestFramework/TestHelper.cs | 1 - 5 files changed, 6 insertions(+), 13 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/HashCode.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/HashCode.cs index 59679aafe4f..cc4f24a876d 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/HashCode.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/HashCode.cs @@ -35,7 +35,7 @@ public static int DictionaryContentHash(IDictionary dictionary.Aggregate(0, (seed, kvp) => Combine(seed, kvp.Key, kvp.Value)); public static int EnumerableContentHash(IEnumerable enumerable) => - enumerable.Aggregate(0, (seed, x) => seed + x.GetHashCode()); + enumerable.Aggregate(0, (seed, x) => Combine(seed, x)); public static int Combine(T1 a, T2 b) => (int)Seed diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs index ac9f2f90d6e..53c84bc1ea2 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs @@ -140,7 +140,7 @@ public void RemoveSymbols_RemovesTheSymbolMatchingThePredicate() .SetSymbolValue(symbols[1], symbolicValue) .SetSymbolValue(symbols[2], symbolicValue) .RemoveSymbols(x => !SymbolEqualityComparer.Default.Equals(x, symbols[1])); - sut.SymbolsWith(DummyConstraint.Dummy).Should().Contain(symbols[1]); + sut.SymbolsWith(DummyConstraint.Dummy).Should().Contain(symbols[1]).And.HaveCount(1); } [TestMethod] diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs index 982e125a38e..25534de5641 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs @@ -220,7 +220,7 @@ public void Branching_ConstraintTrackedSeparatelyInBranches() } Tag(""End"", value);"; var validator = SETestContext.CreateCS(code, new PreserveTestCheck("value")).Validator; - validator.ValidateExitReachCount(2); + validator.ValidateExitReachCount(2); // Once with True constraint, once with False constraint on "value" validator.TagValues("End").Should().HaveCount(2) .And.ContainSingle(x => x.HasConstraint(BoolConstraint.True)) .And.ContainSingle(x => x.HasConstraint(BoolConstraint.False)); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs index 0122d74967c..584744de335 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs @@ -18,27 +18,21 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using Microsoft.CodeAnalysis.Operations; using SonarAnalyzer.SymbolicExecution.Roslyn; namespace SonarAnalyzer.UnitTest.TestFramework.SymbolicExecution { - /// - /// Empty check implementation used to start the engine (the engine needs at least one check to start). - /// public class PreserveTestCheck : SymbolicCheck { private readonly string symbolName; - public PreserveTestCheck(string symbolName) - { + public PreserveTestCheck(string symbolName) => this.symbolName = symbolName; - } protected override ProgramState PreProcessSimple(SymbolicContext context) => context.Operation.Instance.TrackedSymbol() is { } symbol - && symbol.Name.Equals(symbolName) + && symbol.Name == symbolName ? context.State.Preserve(symbol) - : base.PreProcessSimple(context); + : context.State; } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs index b610da8b9a6..d0d54497ca5 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs @@ -96,7 +96,6 @@ public static ControlFlowGraph CompileCfg(string snippet, AnalyzerLanguage langu var (tree, semanticModel) = Compile(snippet, ignoreErrors, language); var method = tree.GetRoot().DescendantNodes().First(IsMethod); methodsymbol = semanticModel.GetDeclaredSymbol(method); - return ControlFlowGraph.Create(method, semanticModel); bool IsMethod(SyntaxNode node) => From 5e3117027ff632fd46906129daef6c3fbe14db88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Fri, 18 Mar 2022 16:32:36 +0100 Subject: [PATCH 09/19] Rebase fix --- analyzers/src/SonarAnalyzer.Common/Helpers/HashCode.cs | 7 +++++++ .../SymbolicExecution/Roslyn/ProgramState.cs | 2 +- .../SymbolicExecution/Roslyn/ProgramStateTest.cs | 4 ++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/HashCode.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/HashCode.cs index cc4f24a876d..f748a06830a 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/HashCode.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/HashCode.cs @@ -48,6 +48,13 @@ public static int Combine(T1 a, T2 b, T3 c) => .AddHash((uint)(b?.GetHashCode() ?? 0)) .AddHash((uint)(c?.GetHashCode() ?? 0)); + public static int Combine(T1 a, T2 b, T3 c, T4 d) => + (int)Seed + .AddHash((uint)(a?.GetHashCode() ?? 0)) + .AddHash((uint)(b?.GetHashCode() ?? 0)) + .AddHash((uint)(c?.GetHashCode() ?? 0)) + .AddHash((uint)(d?.GetHashCode() ?? 0)); + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static uint AddHash(this uint hash, uint value) => RotateLeft(hash + value * PreMultiplier) * PostMultiplier; diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs index 3b528346e8a..f6089c1249a 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs @@ -114,7 +114,7 @@ public override int GetHashCode() => HashCode.Combine( HashCode.DictionaryContentHash(OperationValue), HashCode.DictionaryContentHash(SymbolValue), - HashCode.EnumerableContentHash(PreservedSymbols)); + HashCode.EnumerableContentHash(PreservedSymbols), HashCode.DictionaryContentHash(CaptureOperation)); public bool Equals(ProgramState other) => diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.cs index 6f2afd445d4..c17169fefab 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.cs @@ -211,7 +211,7 @@ public void ToString_WithCaptures() #42: SimpleAssignmentOperation / VariableDeclaratorSyntax: a = true "); } - + [TestMethod] public void ToString_WithAll() { @@ -223,7 +223,7 @@ public void ToString_WithAll() .SetSymbolValue(variableSymbol, new SymbolicValue(counter)) .SetSymbolValue(variableSymbol.ContainingSymbol, valueWithConstraint) .SetOperationValue(assignment, new SymbolicValue(counter)) - .SetOperationValue(assignment.Children.First(), valueWithConstraint).Preserve(variableSymbol); + .SetOperationValue(assignment.Children.First(), valueWithConstraint).Preserve(variableSymbol) .SetCapture(new CaptureId(0), assignment) .SetCapture(new CaptureId(1), assignment.Children.First()); From 6677a5ff78f3d58775d847a0f921d2f5a7896257 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Fri, 18 Mar 2022 16:54:11 +0100 Subject: [PATCH 10/19] Bump coverage, and fix code smell --- .../SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs | 1 - analyzers/tests/SonarAnalyzer.UnitTest/Helpers/HashCodeTest.cs | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index 267fb8bccbc..73eacfccf54 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -24,7 +24,6 @@ using Microsoft.CodeAnalysis; using SonarAnalyzer.CFG.LiveVariableAnalysis; using SonarAnalyzer.CFG.Roslyn; -using SonarAnalyzer.Helpers; using SonarAnalyzer.SymbolicExecution.Constraints; using SonarAnalyzer.SymbolicExecution.Roslyn.Checks; using SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/HashCodeTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/HashCodeTest.cs index 0bd43ed9a5c..7e484e82d4a 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/HashCodeTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Helpers/HashCodeTest.cs @@ -32,9 +32,11 @@ public void Combine_Null() { var two = HashCode.Combine(null, null); var three = HashCode.Combine(null, null, null); + var four = HashCode.Combine(null, null, null, null); two.Should().NotBe(0); three.Should().NotBe(0).And.NotBe(two); + four.Should().NotBe(0).And.NotBe(three).And.NotBe(two); } } } From 1214fdd0028f2254f0cd4439c599c6fb3c70b87f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Mon, 21 Mar 2022 08:45:04 +0100 Subject: [PATCH 11/19] Rebase fix --- .../Rules/SymbolicExecutionRunnerBase.cs | 2 +- .../Roslyn/RoslynSymbolicExecution.cs | 5 ++--- .../Roslyn/RoslynSymbolicExecutionTest.cs | 12 +++++------- .../SymbolicExecution/SETestContext.cs | 4 ++-- .../TestFramework/TestHelper.cs | 16 +++------------- 5 files changed, 13 insertions(+), 26 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/SymbolicExecutionRunnerBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/SymbolicExecutionRunnerBase.cs index 416367d14b6..086e857e566 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/SymbolicExecutionRunnerBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/SymbolicExecutionRunnerBase.cs @@ -83,7 +83,7 @@ private void AnalyzeRoslyn(SonarAnalysisContext sonarContext, SyntaxNodeAnalysis { if (CreateCfg(nodeContext.SemanticModel, body) is { } cfg) { - var engine = new RoslynSymbolicExecution(cfg, checks, symbol); + var engine = new RoslynSymbolicExecution(cfg, checks); engine.Execute(); } } diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index 73eacfccf54..062706012ff 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -43,16 +43,15 @@ internal class RoslynSymbolicExecution private readonly HashSet visited = new(); private readonly RoslynLiveVariableAnalysis lva; - public RoslynSymbolicExecution(ControlFlowGraph cfg, SymbolicCheck[] checks, ISymbol declaration) + public RoslynSymbolicExecution(ControlFlowGraph cfg, SymbolicCheck[] checks) { this.cfg = cfg ?? throw new ArgumentNullException(nameof(cfg)); if (checks == null || checks.Length == 0) { throw new ArgumentException("At least one check is expected", nameof(checks)); } - _ = declaration ?? throw new ArgumentNullException(nameof(declaration)); this.checks = new(new[] { new ConstantCheck() }.Concat(checks).ToArray()); - lva = new RoslynLiveVariableAnalysis(cfg, declaration); + lva = new RoslynLiveVariableAnalysis(cfg); } public void Execute() diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs index bc09b50dd6c..9eb09f2359f 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs @@ -41,19 +41,17 @@ public partial class RoslynSymbolicExecutionTest [TestMethod] public void Constructor_Throws() { - var cfg = TestHelper.CompileCfgCS(out var methodSymbol, "public class Sample { public void Main() { } }"); + var cfg = TestHelper.CompileCfgCS("public class Sample { public void Main() { } }"); var check = new Mock().Object; - ((Action)(() => new RoslynSymbolicExecution(null, new[] { check }, methodSymbol))).Should().Throw().And.ParamName.Should().Be("cfg"); - ((Action)(() => new RoslynSymbolicExecution(cfg, null, methodSymbol))).Should().Throw().WithMessage("At least one check is expected*"); - ((Action)(() => new RoslynSymbolicExecution(cfg, Array.Empty(), methodSymbol))).Should().Throw().WithMessage("At least one check is expected*"); - ((Action)(() => new RoslynSymbolicExecution(cfg, new[] { check }, null))).Should().Throw().And.ParamName.Should().Be("declaration"); + ((Action)(() => new RoslynSymbolicExecution(null, new[] { check }))).Should().Throw().And.ParamName.Should().Be("cfg"); + ((Action)(() => new RoslynSymbolicExecution(cfg, null))).Should().Throw().WithMessage("At least one check is expected*"); + ((Action)(() => new RoslynSymbolicExecution(cfg, Array.Empty()))).Should().Throw().WithMessage("At least one check is expected*"); } [TestMethod] public void Execute_SecondRun_Throws() { - var cfg = TestHelper.CompileCfgBodyCS(out var methodSymbol); - var se = new RoslynSymbolicExecution(cfg, new[] { new ValidatorTestCheck() }, methodSymbol); + var se = new RoslynSymbolicExecution(TestHelper.CompileCfgBodyCS(), new[] { new ValidatorTestCheck() }); se.Execute(); se.Invoking(x => x.Execute()).Should().Throw().WithMessage("Engine can be executed only once."); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs index 039cd8e37ab..feed1e75f6b 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs @@ -33,8 +33,8 @@ internal class SETestContext public SETestContext(string code, AnalyzerLanguage language, SymbolicCheck[] additionalChecks) { const string Separator = "----------"; - var cfg = TestHelper.CompileCfg(code, language, out var methodSymbol); - var se = new RoslynSymbolicExecution(cfg, additionalChecks.Concat(new[] { Validator }).ToArray(), methodSymbol); + var cfg = TestHelper.CompileCfg(code, language); + var se = new RoslynSymbolicExecution(cfg, additionalChecks.Concat(new[] { Validator }).ToArray()); Console.WriteLine(Separator); Console.Write(CfgSerializer.Serialize(cfg)); Console.WriteLine(Separator); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs index d0d54497ca5..4920ccd3a5c 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs @@ -69,10 +69,7 @@ public static (SyntaxTree Tree, SemanticModel Model) Compile(string snippet, boo } public static ControlFlowGraph CompileCfgBodyCS(string body = null, string additionalParameters = null) => - CompileCfgBodyCS(out _, body, additionalParameters); - - public static ControlFlowGraph CompileCfgBodyCS(out ISymbol methodSymbol, string body = null, string additionalParameters = null) => - CompileCfg($"public class Sample {{ public void Main({additionalParameters}) {{ {body} }} }}", AnalyzerLanguage.CSharp, out methodSymbol); + CompileCfg($"public class Sample {{ public void Main({additionalParameters}) {{ {body} }} }}", AnalyzerLanguage.CSharp); public static ControlFlowGraph CompileCfgBodyVB(string body = null) => CompileCfg( @@ -82,20 +79,13 @@ Public Sub Main() End Sub End Class", AnalyzerLanguage.VisualBasic); - public static ControlFlowGraph CompileCfgCS(out ISymbol methodSymbol, string snippet, bool ignoreErrors = false) => - CompileCfg(snippet, AnalyzerLanguage.CSharp, out methodSymbol, ignoreErrors); - public static ControlFlowGraph CompileCfgCS(string snippet, bool ignoreErrors = false) => - CompileCfgCS(out _, snippet, ignoreErrors); - - public static ControlFlowGraph CompileCfg(string snippet, AnalyzerLanguage language, bool ignoreErrors = false) => - CompileCfg(snippet, language, out _, ignoreErrors); + CompileCfg(snippet, AnalyzerLanguage.CSharp, ignoreErrors); - public static ControlFlowGraph CompileCfg(string snippet, AnalyzerLanguage language, out ISymbol methodsymbol, bool ignoreErrors = false) + public static ControlFlowGraph CompileCfg(string snippet, AnalyzerLanguage language, bool ignoreErrors = false) { var (tree, semanticModel) = Compile(snippet, ignoreErrors, language); var method = tree.GetRoot().DescendantNodes().First(IsMethod); - methodsymbol = semanticModel.GetDeclaredSymbol(method); return ControlFlowGraph.Create(method, semanticModel); bool IsMethod(SyntaxNode node) => From 3e302eec75cc038bb685c68e6ce8bc2dea48af42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Mon, 21 Mar 2022 15:50:16 +0100 Subject: [PATCH 12/19] S2E1 --- .../SymbolicExecution/Roslyn/ProgramState.cs | 3 +- .../Roslyn/RoslynSymbolicExecution.cs | 4 +-- .../Roslyn/ProgramStateTest.SymbolValue.cs | 4 +-- .../RoslynSymbolicExecutionTest.Branching.cs | 4 +-- .../RoslynSymbolicExecutionTest.Loops.cs | 7 ++-- .../Roslyn/RoslynSymbolicExecutionTest.cs | 33 +++---------------- .../SymbolicExecution/PreserveTestCheck.cs | 3 +- 7 files changed, 16 insertions(+), 42 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs index f6089c1249a..0d5de002aeb 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/ProgramState.cs @@ -123,8 +123,7 @@ other is not null && other.OperationValue.DictionaryEquals(OperationValue) && other.SymbolValue.DictionaryEquals(SymbolValue) && other.CaptureOperation.DictionaryEquals(CaptureOperation) - && other.PreservedSymbols.Count == PreservedSymbols.Count - && other.PreservedSymbols.All(x => PreservedSymbols.Contains(x)); + && other.PreservedSymbols.SetEquals(PreservedSymbols); public override string ToString() => Equals(Empty) ? "Empty" : SerializeSymbols() + SerializeOperations() + SerializeCaptures(); diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index 062706012ff..8cde0cc6233 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -24,6 +24,7 @@ using Microsoft.CodeAnalysis; using SonarAnalyzer.CFG.LiveVariableAnalysis; using SonarAnalyzer.CFG.Roslyn; +using SonarAnalyzer.Helpers; using SonarAnalyzer.SymbolicExecution.Constraints; using SonarAnalyzer.SymbolicExecution.Roslyn.Checks; using SonarAnalyzer.SymbolicExecution.Roslyn.OperationProcessors; @@ -93,7 +94,6 @@ private IEnumerable ProcessBranching(ExplodedNode node) } else if (node.Block.ContainsThrow()) { - node = new ExplodedNode(cfg.ExitBlock, CleanUnusedState(node.State, node.Block), null); yield return new(cfg.ExitBlock, node.State, null); } else @@ -197,7 +197,7 @@ private static ProgramState ProcessOperation(SymbolicContext context) => private ProgramState CleanUnusedState(ProgramState programState, BasicBlock block) { - var liveVariables = lva.LiveOut(block); + var liveVariables = lva.LiveOut(block).ToHashSet(); return programState.RemoveSymbols(x => (x is ILocalSymbol or IParameterSymbol { RefKind: RefKind.None }) && !liveVariables.Contains(x)); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs index 53c84bc1ea2..5af7a4ef565 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/ProgramStateTest.SymbolValue.cs @@ -121,7 +121,7 @@ public void SymbolsWith_IgnoreNullValue() } [TestMethod] - public void Preserve_PreservedSymbolCannotBeDeleted() + public void Preserve_PreservedSymbolCannotBeRemoved() { var symbolicValue = new SymbolicValue(new()).WithConstraint(DummyConstraint.Dummy); var symbol = CreateSymbols().First(); @@ -132,7 +132,7 @@ public void Preserve_PreservedSymbolCannotBeDeleted() } [TestMethod] - public void RemoveSymbols_RemovesTheSymbolMatchingThePredicate() + public void RemoveSymbols_RemovesSymbolsMatchingThePredicate() { var symbolicValue = new SymbolicValue(new()).WithConstraint(DummyConstraint.Dummy); var symbols = CreateSymbols().ToArray(); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs index 25534de5641..d0a05a0089f 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs @@ -265,7 +265,7 @@ public void Branching_FieldSymbolsAreNotCleaned() [DataTestMethod] [DataRow("out", "outParam")] [DataRow("ref", "refParam")] - public void Branching_RefAndOutParameters_NotCleared(string paramType, string paramName) + public void Branching_RefAndOutParameters_NotCleared(string refKind, string paramName) { var code = $@" if (boolParameter) @@ -293,7 +293,7 @@ public void Branching_RefAndOutParameters_NotCleared(string paramType, string pa } return x.State; }); - var validator = SETestContext.CreateCS(code, $", {paramType} int {paramName}", postProcess).Validator; + var validator = SETestContext.CreateCS(code, $", {refKind} int {paramName}", postProcess).Validator; validator.ValidateExitReachCount(2); // Once with First constraint, once with Second constraint on "value" validator.TagValues("End").Should().HaveCount(2) .And.ContainSingle(x => x.HasConstraint(TestConstraint.First)) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs index 00750496045..920969d1354 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs @@ -55,7 +55,7 @@ public void Loops_InstructionVisitedMaxTwice_EvenWithMultipleStates() { const string code = @" var value = 42; -bool condition; +bool condition = false; if (Condition) // This generates two different ProgramStates, each tracks its own visits condition = true; else @@ -64,10 +64,9 @@ public void Loops_InstructionVisitedMaxTwice_EvenWithMultipleStates() { value.ToString(); // Add another constraint to 'value' } while (Condition); -Tag(""HoldRef"", condition); Tag(""End"", value);"; - var validator = SETestContext.CreateCS(code, ", int[] items", new AddConstraintOnInvocationCheck()).Validator; - validator.ValidateExitReachCount(1); + var validator = SETestContext.CreateCS(code, ", int[] items", new AddConstraintOnInvocationCheck(), new PreserveTestCheck("condition")).Validator; + validator.ValidateExitReachCount(2); var states = validator.TagStates("End"); var condition = states.SelectMany(x => x.SymbolsWith(BoolConstraint.False)).First(); // "False" is never set for "value" var value = states.SelectMany(x => x.SymbolsWith(TestConstraint.First)).First(); // "First" is never set for "condition" diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs index 9eb09f2359f..2d90ece4241 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs @@ -179,51 +179,28 @@ static ISymbol LocalReferenceOperationSymbol(IOperationWrapperSonar operation) = public void Execute_UnusedVariable_ClearedAfterBlock() { const string code = @" -bool first; -if(boolParameter) - first = true; -else - first = false; +var first = boolParameter ? true : false; Tag(""BeforeLastUse""); bool second = first; if(boolParameter) -boolParameter.ToString(); + boolParameter.ToString(); Tag(""AfterLastUse""); -bool third = second; -if(boolParameter) -boolParameter.ToString(); -Tag(""END""); "; ISymbol firstSymbol = null; - ISymbol secondSymbol = null; var postProcess = new PostProcessTestCheck(x => { if (x.Operation.Instance.TrackedSymbol() is { } symbol) { - if (symbol.Name.Equals("first")) + if (symbol.Name == "first") { firstSymbol = symbol; } - else if (symbol.Name.Equals("second")) - { - secondSymbol = symbol; - } } return x.State; }); var validator = SETestContext.CreateCS(code, postProcess).Validator; - validator.ValidateTagOrder( - "BeforeLastUse", - "BeforeLastUse", - "AfterLastUse", - "AfterLastUse", - "END"); - var beforeLastUseStates = validator.TagStates("BeforeLastUse"); - beforeLastUseStates.Should().HaveCount(2); - beforeLastUseStates.All(x => x[firstSymbol] != null).Should().BeTrue(); - var afterLastUseStates = validator.TagStates("AfterLastUse"); - afterLastUseStates.Should().HaveCount(2); - afterLastUseStates.All(x => x[firstSymbol] == null && x[secondSymbol] != null).Should().BeTrue(); + validator.TagStates("BeforeLastUse").Should().HaveCount(2).And.OnlyContain(x => x[firstSymbol] != null); + validator.TagStates("AfterLastUse").Should().HaveCount(1).And.OnlyContain(x => x[firstSymbol] == null); } [TestMethod] diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs index 584744de335..e54ed0eddee 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/PreserveTestCheck.cs @@ -30,8 +30,7 @@ public PreserveTestCheck(string symbolName) => this.symbolName = symbolName; protected override ProgramState PreProcessSimple(SymbolicContext context) => - context.Operation.Instance.TrackedSymbol() is { } symbol - && symbol.Name == symbolName + context.Operation.Instance.TrackedSymbol() is { } symbol && symbol.Name == symbolName ? context.State.Preserve(symbol) : context.State; } From e0c946a5497ff829c0df72605d61f49f3b570d63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Mon, 21 Mar 2022 23:24:43 +0100 Subject: [PATCH 13/19] S2E2 --- .../RoslynSymbolicExecutionTest.Branching.cs | 62 ++++--------------- 1 file changed, 13 insertions(+), 49 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs index d0a05a0089f..9028eeb4602 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs @@ -232,34 +232,12 @@ public void Branching_FieldSymbolsAreNotCleaned() const string code = @" if (boolParameter) { - field = 1; -} -else -{ - field = 2; -} -Tag(""End"", field);"; + field = 42; +}"; - var postProcess = new PostProcessTestCheck(x => - { - if (x.Operation.Instance.Kind == OperationKind.Literal && x.Operation.Instance.ConstantValue.Value is int value) - { - if (value == 1) - { - return x.SetOperationConstraint(TestConstraint.First); - } - else if (value == 2) - { - return x.SetOperationConstraint(TestConstraint.Second); - } - } - return x.State; - }); + var postProcess = new PostProcessTestCheck(OperationKind.Literal, x => x.SetOperationConstraint(DummyConstraint.Dummy)); var validator = SETestContext.CreateCS(code, postProcess).Validator; - validator.ValidateExitReachCount(2); // Once with First constraint, once with Second constraint on "value" - validator.TagValues("End").Should().HaveCount(2) - .And.ContainSingle(x => x.HasConstraint(TestConstraint.First)) - .And.ContainSingle(x => x.HasConstraint(TestConstraint.Second)); + validator.ValidateExitReachCount(2); // Once with the constraint and once without it. } [DataTestMethod] @@ -270,34 +248,20 @@ public void Branching_RefAndOutParameters_NotCleared(string refKind, string para var code = $@" if (boolParameter) {{ - {paramName} = 1; + {paramName} = 42; }} else {{ - {paramName} = 2; -}} -Tag(""End"", {paramName});"; + {paramName} = 24; +}}"; - var postProcess = new PostProcessTestCheck(x => - { - if (x.Operation.Instance.Kind == OperationKind.Literal && x.Operation.Instance.ConstantValue.Value is int value) - { - if (value == 1) - { - return x.SetOperationConstraint(TestConstraint.First); - } - else if (value == 2) - { - return x.SetOperationConstraint(TestConstraint.Second); - } - } - return x.State; - }); + var postProcess = new PostProcessTestCheck( + OperationKind.Literal, + x => x.Operation.Instance.Kind == OperationKind.Literal && x.Operation.Instance.ConstantValue.Value is int value && value == 42 + ? x.SetOperationConstraint(DummyConstraint.Dummy) + : x.State); var validator = SETestContext.CreateCS(code, $", {refKind} int {paramName}", postProcess).Validator; - validator.ValidateExitReachCount(2); // Once with First constraint, once with Second constraint on "value" - validator.TagValues("End").Should().HaveCount(2) - .And.ContainSingle(x => x.HasConstraint(TestConstraint.First)) - .And.ContainSingle(x => x.HasConstraint(TestConstraint.Second)); + validator.ValidateExitReachCount(2); // Once with the constraint and once without it. } [TestMethod] From 6e1fc90f4b96e292a1c5bc9b7edb1c606aa00471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Mon, 21 Mar 2022 23:55:25 +0100 Subject: [PATCH 14/19] S2E3 --- .../Roslyn/RoslynSymbolicExecution.cs | 2 +- .../Roslyn/RoslynSymbolicExecutionTest.cs | 17 +++++++++++ .../SymbolicExecution/SETestContext.cs | 28 ++++++++++++++++++- 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index 8cde0cc6233..3aa4409031e 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -198,7 +198,7 @@ private static ProgramState ProcessOperation(SymbolicContext context) => private ProgramState CleanUnusedState(ProgramState programState, BasicBlock block) { var liveVariables = lva.LiveOut(block).ToHashSet(); - return programState.RemoveSymbols(x => (x is ILocalSymbol or IParameterSymbol { RefKind: RefKind.None }) && !liveVariables.Contains(x)); + return programState.RemoveSymbols(x => lva.IsLocal(x) && (x is ILocalSymbol or IParameterSymbol { RefKind: RefKind.None }) && !liveVariables.Contains(x)); } private static bool IsReachable(ExplodedNode node, ControlFlowBranch branch) => diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs index 2d90ece4241..289380bd2df 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs @@ -203,6 +203,23 @@ public void Execute_UnusedVariable_ClearedAfterBlock() validator.TagStates("AfterLastUse").Should().HaveCount(1).And.OnlyContain(x => x[firstSymbol] == null); } + [TestMethod] + public void Execute_CapturedVariable_NotCleared() + { + const string code = @" +void LocalFunction() +{ +boolParameter = false; +var misc = true; + if(misc) + misc.ToString(); +Tag(""LocalFunctionEnd"", boolParameter); +}"; + var validator = SETestContext.CreateCSForLocalFunction(code, null, "LocalFunction").Validator; + validator.TagValues("LocalFunctionEnd").Should().HaveCount(1) + .And.ContainSingle(x => x.HasConstraint(BoolConstraint.False)); + } + [TestMethod] public void Execute_TooManyBlocks_NotSupported() { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs index feed1e75f6b..e8cedf47727 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs @@ -30,10 +30,14 @@ internal class SETestContext { public readonly ValidatorTestCheck Validator = new(); - public SETestContext(string code, AnalyzerLanguage language, SymbolicCheck[] additionalChecks) + public SETestContext(string code, AnalyzerLanguage language, SymbolicCheck[] additionalChecks, string localFunctionName = null) { const string Separator = "----------"; var cfg = TestHelper.CompileCfg(code, language); + if (localFunctionName != null) + { + cfg = cfg.GetLocalFunctionControlFlowGraph(cfg.LocalFunctions.Single(x => x.Name == localFunctionName)); + } var se = new RoslynSymbolicExecution(cfg, additionalChecks.Concat(new[] { Validator }).ToArray()); Console.WriteLine(Separator); Console.Write(CfgSerializer.Serialize(cfg)); @@ -69,6 +73,28 @@ private void Tag(string name, object arg = null) {{ }} return new(code, AnalyzerLanguage.CSharp, additionalChecks); } + public static SETestContext CreateCSForLocalFunction(string methodBody, string additionalParameters, string localFunctionName, params SymbolicCheck[] additionalChecks) + { + var code = $@" +using System.Collections.Generic; + +public class Sample +{{ + public static int StaticField; + public static int StaticProperty {{get; set;}} + public int Property {{get; set;}} + private int field; + + public void Main(bool boolParameter{additionalParameters}) + {{ + {methodBody} + }} + + private void Tag(string name, object arg = null) {{ }} +}}"; + return new(code, AnalyzerLanguage.CSharp, additionalChecks, localFunctionName); + } + public static SETestContext CreateCSMethod(string method, params SymbolicCheck[] additionalChecks) => new($@" public class Sample From 068ba759cd529bd863775ca0874a1535533d51e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Tue, 22 Mar 2022 10:05:17 +0100 Subject: [PATCH 15/19] S2E4 --- .../SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs | 6 ++---- .../Extensions/SafeVisualBasicSyntaxWalkerTest.cs | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index 3aa4409031e..e199807c95a 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -98,9 +98,7 @@ private IEnumerable ProcessBranching(ExplodedNode node) } else { - var reachableSuccessors = node.Block.Successors.Where(x => IsReachable(node, x)).ToList(); - node = new ExplodedNode(node.Block, CleanUnusedState(node.State, node.Block), node.FinallyPoint); - foreach (var successor in reachableSuccessors) + foreach (var successor in node.Block.Successors.Where(x => IsReachable(node, x))) { if (ProcessBranch(node, successor) is { } newNode) { @@ -161,7 +159,7 @@ private ProgramState ProcessBranchState(ControlFlowBranch branch, ProgramState s { state = state.RemoveCapture(capture); } - return state.ResetOperations(); + return CleanUnusedState(state, branch.Source).ResetOperations(); } private IEnumerable ProcessOperation(ExplodedNode node) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SafeVisualBasicSyntaxWalkerTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SafeVisualBasicSyntaxWalkerTest.cs index 051ca103248..5175b9c9287 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SafeVisualBasicSyntaxWalkerTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SafeVisualBasicSyntaxWalkerTest.cs @@ -39,7 +39,7 @@ public void GivenSyntaxNodeWithHighDepth_SafeVisit_ReturnsFalse() var code = $@" Public Class Sample Public Function Main(Arg as Boolean) As Boolean - Return Arg {Enumerable.Repeat("AndAlso Arg", 5000).JoinStr(" ")} + Return Arg {Enumerable.Repeat("AndAlso Arg", 7000).JoinStr(" ")} End Function End Class"; From 789b026ead7793b7435958f0be15d3d71d96182f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Tue, 22 Mar 2022 14:59:03 +0100 Subject: [PATCH 16/19] S3E1 --- .../Roslyn/RoslynSymbolicExecution.cs | 11 ++-- .../RoslynLiveVariableAnalysisTest.cs | 6 +-- .../RoslynSymbolicExecutionTest.Branching.cs | 38 -------------- .../RoslynSymbolicExecutionTest.Loops.cs | 6 +-- .../Roslyn/RoslynSymbolicExecutionTest.cs | 52 +++++++++++++------ .../SymbolicExecution/SETestContext.cs | 33 ++---------- .../TestFramework/TestHelper.cs | 9 +++- 7 files changed, 57 insertions(+), 98 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index e199807c95a..af32620c811 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -159,7 +159,10 @@ private ProgramState ProcessBranchState(ControlFlowBranch branch, ProgramState s { state = state.RemoveCapture(capture); } - return CleanUnusedState(state, branch.Source).ResetOperations(); + + var liveVariables = lva.LiveOut(branch.Source).ToHashSet(); + return state.RemoveSymbols(x => lva.IsLocal(x) && (x is ILocalSymbol or IParameterSymbol { RefKind: RefKind.None }) && !liveVariables.Contains(x)) + .ResetOperations(); } private IEnumerable ProcessOperation(ExplodedNode node) @@ -193,12 +196,6 @@ private static ProgramState ProcessOperation(SymbolicContext context) => _ => context.State }; - private ProgramState CleanUnusedState(ProgramState programState, BasicBlock block) - { - var liveVariables = lva.LiveOut(block).ToHashSet(); - return programState.RemoveSymbols(x => lva.IsLocal(x) && (x is ILocalSymbol or IParameterSymbol { RefKind: RefKind.None }) && !liveVariables.Contains(x)); - } - private static bool IsReachable(ExplodedNode node, ControlFlowBranch branch) => node.Block.ConditionKind != ControlFlowConditionKind.None && node.State[node.Block.BranchValue] is { } sv diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.cs index ca621587fd4..4d263608eac 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/LiveVariableAnalysis/RoslynLiveVariableAnalysisTest.cs @@ -973,11 +973,7 @@ private class Context public Context(string code, AnalyzerLanguage language, string localFunctionName = null) { - Cfg = TestHelper.CompileCfg(code, language, code.Contains("// Error CS")); - if (localFunctionName != null) - { - Cfg = Cfg.GetLocalFunctionControlFlowGraph(Cfg.LocalFunctions.Single(x => x.Name == localFunctionName)); - } + Cfg = TestHelper.CompileCfg(code, language, code.Contains("// Error CS"), localFunctionName); Console.WriteLine(CfgSerializer.Serialize(Cfg)); Lva = new RoslynLiveVariableAnalysis(Cfg); } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs index 9028eeb4602..370d22f4359 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs @@ -226,44 +226,6 @@ public void Branching_ConstraintTrackedSeparatelyInBranches() .And.ContainSingle(x => x.HasConstraint(BoolConstraint.False)); } - [TestMethod] - public void Branching_FieldSymbolsAreNotCleaned() - { - const string code = @" -if (boolParameter) -{ - field = 42; -}"; - - var postProcess = new PostProcessTestCheck(OperationKind.Literal, x => x.SetOperationConstraint(DummyConstraint.Dummy)); - var validator = SETestContext.CreateCS(code, postProcess).Validator; - validator.ValidateExitReachCount(2); // Once with the constraint and once without it. - } - - [DataTestMethod] - [DataRow("out", "outParam")] - [DataRow("ref", "refParam")] - public void Branching_RefAndOutParameters_NotCleared(string refKind, string paramName) - { - var code = $@" -if (boolParameter) -{{ - {paramName} = 42; -}} -else -{{ - {paramName} = 24; -}}"; - - var postProcess = new PostProcessTestCheck( - OperationKind.Literal, - x => x.Operation.Instance.Kind == OperationKind.Literal && x.Operation.Instance.ConstantValue.Value is int value && value == 42 - ? x.SetOperationConstraint(DummyConstraint.Dummy) - : x.State); - var validator = SETestContext.CreateCS(code, $", {refKind} int {paramName}", postProcess).Validator; - validator.ValidateExitReachCount(2); // Once with the constraint and once without it. - } - [TestMethod] public void Branching_VisitedProgramState_IsSkipped() { diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs index 920969d1354..8acba81a338 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Loops.cs @@ -55,7 +55,7 @@ public void Loops_InstructionVisitedMaxTwice_EvenWithMultipleStates() { const string code = @" var value = 42; -bool condition = false; +bool condition; if (Condition) // This generates two different ProgramStates, each tracks its own visits condition = true; else @@ -65,8 +65,8 @@ public void Loops_InstructionVisitedMaxTwice_EvenWithMultipleStates() value.ToString(); // Add another constraint to 'value' } while (Condition); Tag(""End"", value);"; - var validator = SETestContext.CreateCS(code, ", int[] items", new AddConstraintOnInvocationCheck(), new PreserveTestCheck("condition")).Validator; - validator.ValidateExitReachCount(2); + var validator = SETestContext.CreateCS(code, ", int[] items", new AddConstraintOnInvocationCheck(), new PreserveTestCheck("condition"), new PreserveTestCheck("value")).Validator; + validator.ValidateExitReachCount(4); var states = validator.TagStates("End"); var condition = states.SelectMany(x => x.SymbolsWith(BoolConstraint.False)).First(); // "False" is never set for "value" var value = states.SelectMany(x => x.SymbolsWith(TestConstraint.First)).First(); // "First" is never set for "condition" diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs index 289380bd2df..0458bccd620 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs @@ -184,18 +184,11 @@ public void Execute_UnusedVariable_ClearedAfterBlock() bool second = first; if(boolParameter) boolParameter.ToString(); -Tag(""AfterLastUse""); -"; +Tag(""AfterLastUse"");"; ISymbol firstSymbol = null; var postProcess = new PostProcessTestCheck(x => { - if (x.Operation.Instance.TrackedSymbol() is { } symbol) - { - if (symbol.Name == "first") - { - firstSymbol = symbol; - } - } + firstSymbol ??= x.Operation.Instance.TrackedSymbol() is { Name: "first" } symbol ? symbol : null; return x.State; }); var validator = SETestContext.CreateCS(code, postProcess).Validator; @@ -204,20 +197,18 @@ public void Execute_UnusedVariable_ClearedAfterBlock() } [TestMethod] - public void Execute_CapturedVariable_NotCleared() + public void Execute_OuterMethodParameter_NotCleared() { const string code = @" void LocalFunction() { boolParameter = false; var misc = true; - if(misc) +if(misc) misc.ToString(); -Tag(""LocalFunctionEnd"", boolParameter); +Tag(""LocalFunctionEnd""); }"; - var validator = SETestContext.CreateCSForLocalFunction(code, null, "LocalFunction").Validator; - validator.TagValues("LocalFunctionEnd").Should().HaveCount(1) - .And.ContainSingle(x => x.HasConstraint(BoolConstraint.False)); + SETestContext.CreateCS(code, null, "LocalFunction").Validator.TagStates("LocalFunctionEnd").Should().HaveCount(1); } [TestMethod] @@ -294,5 +285,36 @@ private static ProgramState[] DecorateIntLiteral(SymbolicContext context, Symbol [TestMethod] public void Execute_LocalScopeRegion_AssignDefaultBoolConstraint() => SETestContext.CreateVB(@"Dim B As Boolean : Tag(""B"", B)").Validator.ValidateTag("B", x => x.HasConstraint(BoolConstraint.False).Should().BeTrue()); + + [TestMethod] + public void Execute_FieldSymbolsAreNotCleaned() + { + const string code = @" +if (boolParameter) +{ + field = 42; +}"; + + var postProcess = new PostProcessTestCheck(OperationKind.Literal, x => x.SetOperationConstraint(DummyConstraint.Dummy)); + var validator = SETestContext.CreateCS(code, postProcess).Validator; + validator.ValidateExitReachCount(2); // Once with the constraint and once without it. + } + + [DataTestMethod] + [DataRow("out", "outParam")] + [DataRow("ref", "refParam")] + public void Execute_RefAndOutParameters_NotCleared(string refKind, string paramName) + { + var code = $@" +{paramName} = int.MinValue; +if (boolParameter) +{{ + {paramName} = 42; +}}"; + + var postProcess = new PostProcessTestCheck(OperationKind.Literal, x => x.SetOperationConstraint(DummyConstraint.Dummy)); + var validator = SETestContext.CreateCS(code, $", {refKind} int {paramName}", postProcess).Validator; + validator.ValidateExitReachCount(2); // Once with the constraint and once without it. + } } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs index e8cedf47727..0a7cffce820 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs @@ -33,11 +33,7 @@ internal class SETestContext public SETestContext(string code, AnalyzerLanguage language, SymbolicCheck[] additionalChecks, string localFunctionName = null) { const string Separator = "----------"; - var cfg = TestHelper.CompileCfg(code, language); - if (localFunctionName != null) - { - cfg = cfg.GetLocalFunctionControlFlowGraph(cfg.LocalFunctions.Single(x => x.Name == localFunctionName)); - } + var cfg = TestHelper.CompileCfg(code, language, localFunctionName: localFunctionName); var se = new RoslynSymbolicExecution(cfg, additionalChecks.Concat(new[] { Validator }).ToArray()); Console.WriteLine(Separator); Console.Write(CfgSerializer.Serialize(cfg)); @@ -46,34 +42,15 @@ public SETestContext(string code, AnalyzerLanguage language, SymbolicCheck[] add } public static SETestContext CreateCS(string methodBody, params SymbolicCheck[] additionalChecks) => - CreateCS(methodBody, null, additionalChecks); + CreateCS(methodBody, null, null, additionalChecks); - public static SETestContext CreateCS(string methodBody, string additionalParameters, params SymbolicCheck[] additionalChecks) - { - var code = $@" + public static SETestContext CreateCS(string methodBody, string additionalParameters, params SymbolicCheck[] additionalChecks) => + CreateCS(methodBody, additionalParameters, null, additionalChecks); using System; -using System.Collections.Generic; - -public class Sample -{{ - public static int StaticField; - public static int StaticProperty {{get; set;}} - public int Property {{get; set;}} - private int field; - private bool Condition => Environment.ProcessorCount == 42; // Something that cannot have constraint - public void Main(bool boolParameter{additionalParameters}) - {{ - {methodBody} - }} - - private void Tag(string name, object arg = null) {{ }} -}}"; - return new(code, AnalyzerLanguage.CSharp, additionalChecks); - } - public static SETestContext CreateCSForLocalFunction(string methodBody, string additionalParameters, string localFunctionName, params SymbolicCheck[] additionalChecks) + public static SETestContext CreateCS(string methodBody, string additionalParameters, string localFunctionName, params SymbolicCheck[] additionalChecks) { var code = $@" using System.Collections.Generic; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs index 4920ccd3a5c..a448a8e1ad7 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs @@ -82,11 +82,16 @@ End Sub public static ControlFlowGraph CompileCfgCS(string snippet, bool ignoreErrors = false) => CompileCfg(snippet, AnalyzerLanguage.CSharp, ignoreErrors); - public static ControlFlowGraph CompileCfg(string snippet, AnalyzerLanguage language, bool ignoreErrors = false) + public static ControlFlowGraph CompileCfg(string snippet, AnalyzerLanguage language, bool ignoreErrors = false, string localFunctionName = null) { var (tree, semanticModel) = Compile(snippet, ignoreErrors, language); var method = tree.GetRoot().DescendantNodes().First(IsMethod); - return ControlFlowGraph.Create(method, semanticModel); + var cfg = ControlFlowGraph.Create(method, semanticModel); + if (localFunctionName != null) + { + cfg = cfg.GetLocalFunctionControlFlowGraph(cfg.LocalFunctions.Single(x => x.Name == localFunctionName)); + } + return cfg; bool IsMethod(SyntaxNode node) => language == AnalyzerLanguage.CSharp From 6c43c1124876df2f28f1224896c87d0d1eb3d21a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Tue, 22 Mar 2022 16:21:22 +0100 Subject: [PATCH 17/19] S4E1 --- .../Roslyn/RoslynSymbolicExecution.cs | 1 - .../Roslyn/RoslynSymbolicExecutionTest.cs | 17 +++++++++-------- .../SymbolicExecution/SETestContext.cs | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs index af32620c811..54815552392 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Roslyn/RoslynSymbolicExecution.cs @@ -159,7 +159,6 @@ private ProgramState ProcessBranchState(ControlFlowBranch branch, ProgramState s { state = state.RemoveCapture(capture); } - var liveVariables = lva.LiveOut(branch.Source).ToHashSet(); return state.RemoveSymbols(x => lva.IsLocal(x) && (x is ILocalSymbol or IParameterSymbol { RefKind: RefKind.None }) && !liveVariables.Contains(x)) .ResetOperations(); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs index 0458bccd620..313480cdd4c 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs @@ -202,13 +202,14 @@ public void Execute_OuterMethodParameter_NotCleared() const string code = @" void LocalFunction() { -boolParameter = false; -var misc = true; -if(misc) - misc.ToString(); -Tag(""LocalFunctionEnd""); + boolParameter = false; + var misc = true; + if(misc) + misc.ToString(); + Tag(""LocalFunctionEnd""); }"; - SETestContext.CreateCS(code, null, "LocalFunction").Validator.TagStates("LocalFunctionEnd").Should().HaveCount(1); + SETestContext.CreateCS(code, null, "LocalFunction").Validator.TagStates("LocalFunctionEnd").Should().HaveCount(1) + .And.OnlyContain(x => x.SymbolsWith(BoolConstraint.False).Count() == 1); } [TestMethod] @@ -287,7 +288,7 @@ public void Execute_LocalScopeRegion_AssignDefaultBoolConstraint() => SETestContext.CreateVB(@"Dim B As Boolean : Tag(""B"", B)").Validator.ValidateTag("B", x => x.HasConstraint(BoolConstraint.False).Should().BeTrue()); [TestMethod] - public void Execute_FieldSymbolsAreNotCleaned() + public void Execute_FieldSymbolsAreNotRemovedByLva() { const string code = @" if (boolParameter) @@ -303,7 +304,7 @@ public void Execute_FieldSymbolsAreNotCleaned() [DataTestMethod] [DataRow("out", "outParam")] [DataRow("ref", "refParam")] - public void Execute_RefAndOutParameters_NotCleared(string refKind, string paramName) + public void Execute_RefAndOutParameters_NotRemovedByLva(string refKind, string paramName) { var code = $@" {paramName} = int.MinValue; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs index 0a7cffce820..cbfbb6376a5 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs @@ -33,7 +33,7 @@ internal class SETestContext public SETestContext(string code, AnalyzerLanguage language, SymbolicCheck[] additionalChecks, string localFunctionName = null) { const string Separator = "----------"; - var cfg = TestHelper.CompileCfg(code, language, localFunctionName: localFunctionName); + var cfg = TestHelper.CompileCfg(code, language, false, localFunctionName); var se = new RoslynSymbolicExecution(cfg, additionalChecks.Concat(new[] { Validator }).ToArray()); Console.WriteLine(Separator); Console.Write(CfgSerializer.Serialize(cfg)); From f6b4eab42d1553114ffb33fd62bd0b1b2a32a8a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Wed, 23 Mar 2022 09:43:37 +0100 Subject: [PATCH 18/19] Rebase fix --- .../Roslyn/RoslynSymbolicExecutionTest.Branching.cs | 9 +++------ .../Roslyn/RoslynSymbolicExecutionTest.cs | 2 +- .../TestFramework/SymbolicExecution/SETestContext.cs | 12 ++---------- 3 files changed, 6 insertions(+), 17 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs index 370d22f4359..423923e56ed 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.Branching.cs @@ -153,7 +153,7 @@ public int Method(bool a) return 2; }"; var validator = SETestContext.CreateCSMethod(method).Validator; - validator.ValidateExitReachCount(2); + validator.ValidateExitReachCount(1); validator.ValidateExecutionCompleted(); } @@ -169,7 +169,7 @@ public int Method(bool a) return 2; }"; var validator = SETestContext.CreateCSMethod(method).Validator; - validator.ValidateExitReachCount(2); + validator.ValidateExitReachCount(1); validator.ValidateExecutionCompleted(); } @@ -185,7 +185,7 @@ public System.Collections.Generic.IEnumerable Method(bool a) yield return 2; }"; var validator = SETestContext.CreateCSMethod(method).Validator; - validator.ValidateExitReachCount(2); + validator.ValidateExitReachCount(1); validator.ValidateExecutionCompleted(); } @@ -390,7 +390,6 @@ public void Branching_NoConstraint_VisitsBothBranches() SETestContext.CreateCS(code).Validator.ValidateTagOrder( "If", "Else", - "End", "End"); } @@ -411,7 +410,6 @@ public void Branching_OtherConstraint_VisitsBothBranches() SETestContext.CreateCS(code, check).Validator.ValidateTagOrder( "If", "Else", - "End", "End"); } @@ -521,7 +519,6 @@ public void Branching_BoolSymbol_LearnsBoolConstraint() "False", "TrueTrue", "FalseFalse", - "End", "End"); validator.ValidateTag("True", x => x.HasConstraint(BoolConstraint.True).Should().BeTrue()); validator.ValidateTag("False", x => x.HasConstraint(BoolConstraint.False).Should().BeTrue()); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs index 313480cdd4c..64966d5fb66 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/SymbolicExecution/Roslyn/RoslynSymbolicExecutionTest.cs @@ -193,7 +193,7 @@ public void Execute_UnusedVariable_ClearedAfterBlock() }); var validator = SETestContext.CreateCS(code, postProcess).Validator; validator.TagStates("BeforeLastUse").Should().HaveCount(2).And.OnlyContain(x => x[firstSymbol] != null); - validator.TagStates("AfterLastUse").Should().HaveCount(1).And.OnlyContain(x => x[firstSymbol] == null); + validator.TagStates("AfterLastUse").Should().HaveCount(2).And.OnlyContain(x => x[firstSymbol] == null); // Once emtpy and once with the learned boolParameter true } [TestMethod] diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs index cbfbb6376a5..74ae1f7efaf 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs @@ -46,27 +46,23 @@ public static SETestContext CreateCS(string methodBody, params SymbolicCheck[] a public static SETestContext CreateCS(string methodBody, string additionalParameters, params SymbolicCheck[] additionalChecks) => CreateCS(methodBody, additionalParameters, null, additionalChecks); -using System; - private bool Condition => Environment.ProcessorCount == 42; // Something that cannot have constraint - public static SETestContext CreateCS(string methodBody, string additionalParameters, string localFunctionName, params SymbolicCheck[] additionalChecks) { var code = $@" +using System; using System.Collections.Generic; - public class Sample {{ public static int StaticField; public static int StaticProperty {{get; set;}} public int Property {{get; set;}} private int field; - + private bool Condition => Environment.ProcessorCount == 42; // Something that cannot have constraint public void Main(bool boolParameter{additionalParameters}) {{ {methodBody} }} - private void Tag(string name, object arg = null) {{ }} }}"; return new(code, AnalyzerLanguage.CSharp, additionalChecks, localFunctionName); @@ -86,16 +82,12 @@ public static SETestContext CreateVB(string methodBody, string additionalParamet { var code = $@" Public Class Sample - Private Readonly Property Condition As Boolean = Environment.ProcessorCount = 42 ' Something that cannot have constraint - Public Sub Main(BoolParameter As Boolean{additionalParameters}) {methodBody} End Sub - Private Sub Tag(Name As String, Optional Arg As Object = Nothing) End Sub - End Class"; return new(code, AnalyzerLanguage.VisualBasic, additionalChecks); } From c745a63b71f29052d8fa68cffaa14e6aec363062 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=8Caba=20=C5=A0agi?= Date: Wed, 23 Mar 2022 10:25:58 +0100 Subject: [PATCH 19/19] S5E1: Is this the real life? Is this just fantasy? --- .../TestFramework/SymbolicExecution/SETestContext.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs index 74ae1f7efaf..e77c9327239 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/SymbolicExecution/SETestContext.cs @@ -52,17 +52,21 @@ public static SETestContext CreateCS(string methodBody, string additionalParamet var code = $@" using System; using System.Collections.Generic; + public class Sample {{ public static int StaticField; public static int StaticProperty {{get; set;}} public int Property {{get; set;}} private int field; + private bool Condition => Environment.ProcessorCount == 42; // Something that cannot have constraint + public void Main(bool boolParameter{additionalParameters}) {{ {methodBody} }} + private void Tag(string name, object arg = null) {{ }} }}"; return new(code, AnalyzerLanguage.CSharp, additionalChecks, localFunctionName); @@ -82,12 +86,16 @@ public static SETestContext CreateVB(string methodBody, string additionalParamet { var code = $@" Public Class Sample + Private Readonly Property Condition As Boolean = Environment.ProcessorCount = 42 ' Something that cannot have constraint + Public Sub Main(BoolParameter As Boolean{additionalParameters}) {methodBody} End Sub + Private Sub Tag(Name As String, Optional Arg As Object = Nothing) End Sub + End Class"; return new(code, AnalyzerLanguage.VisualBasic, additionalChecks); }