Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Use LVA in the new SE to reduce memory usage and improve performance #5475

Merged
merged 19 commits into from
Mar 23, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
First implementation and making UTs green.
  • Loading branch information
csaba-sagi-sonarsource committed Mar 23, 2022
commit 30475ee5417148f6d16dc5289cce3a20787e33e4
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public sealed record ProgramState : IEquatable<ProgramState>
private ImmutableDictionary<ISymbol, SymbolicValue> SymbolValue { get; init; }
private ImmutableDictionary<int, int> VisitCount { get; init; }
private ImmutableDictionary<CaptureId, IOperation> CaptureOperation { get; init; }
private ImmutableHashSet<ISymbol> 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;
Expand All @@ -50,6 +51,7 @@ private ProgramState()
SymbolValue = ImmutableDictionary<ISymbol, SymbolicValue>.Empty;
VisitCount = ImmutableDictionary<int, int>.Empty;
CaptureOperation = ImmutableDictionary<CaptureId, IOperation>.Empty;
PreservedSymbols = ImmutableHashSet<ISymbol>.Empty;
}

public ProgramState SetOperationValue(IOperationWrapperSonar operation, SymbolicValue value) =>
Expand Down Expand Up @@ -95,9 +97,15 @@ public IOperation ResolveCapture(IOperation operation) =>
? captured
: operation;

public ProgramState RemoveSymbols(Func<ISymbol, bool> 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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -39,15 +42,24 @@ internal class RoslynSymbolicExecution
private readonly Queue<ExplodedNode> queue = new();
private readonly SymbolicValueCounter symbolicValueCounter = new();
private readonly HashSet<ExplodedNode> visited = new();
private readonly RoslynLiveVariableAnalysis lva;
private readonly IEnumerable<IParameterSymbol> 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()
Expand Down Expand Up @@ -89,10 +101,12 @@ private IEnumerable<ExplodedNode> 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)
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ public System.Collections.Generic.IEnumerable<int> Method(bool a)
var b = a;
}";
var validator = SETestContext.CreateCSMethod(method).Validator;
validator.ValidateExitReachCount(2);
validator.ValidateExitReachCount(1);
validator.ValidateExecutionCompleted();
}

Expand All @@ -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));
Expand Down Expand Up @@ -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);
}

Expand All @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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"
Expand All @@ -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));
Expand Down Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<SymbolicCheck>().Object;
((Action)(() => new RoslynSymbolicExecution(null, new[] { check }))).Should().Throw<ArgumentNullException>().And.ParamName.Should().Be("cfg");
((Action)(() => new RoslynSymbolicExecution(cfg, null))).Should().Throw<ArgumentException>().WithMessage("At least one check is expected*");
((Action)(() => new RoslynSymbolicExecution(cfg, Array.Empty<SymbolicCheck>()))).Should().Throw<ArgumentException>().WithMessage("At least one check is expected*");
((Action)(() => new RoslynSymbolicExecution(null, new[] { check }, methodSymbol))).Should().Throw<ArgumentNullException>().And.ParamName.Should().Be("cfg");
((Action)(() => new RoslynSymbolicExecution(cfg, null, methodSymbol))).Should().Throw<ArgumentException>().WithMessage("At least one check is expected*");
((Action)(() => new RoslynSymbolicExecution(cfg, Array.Empty<SymbolicCheck>(), methodSymbol))).Should().Throw<ArgumentException>().WithMessage("At least one check is expected*");
((Action)(() => new RoslynSymbolicExecution(cfg, new[] { check }, null))).Should().Throw<ArgumentNullException>().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<InvalidOperationException>().WithMessage("Engine can be executed only once.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
17 changes: 14 additions & 3 deletions analyzers/tests/SonarAnalyzer.UnitTest/TestFramework/TestHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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) =>
Expand Down