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

Fix StackOverflow in CfgAllPathValidator.AreAllSuccessorsValid #8984

Merged
merged 23 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
e8419d9
Add Repro
martin-strecker-sonarsource Mar 26, 2024
43fbedc
Minimize Repro
martin-strecker-sonarsource Mar 26, 2024
1f3e7a6
Remove unused OpenXML Nuget
martin-strecker-sonarsource Mar 26, 2024
9347774
Add comment with link to issue
martin-strecker-sonarsource Mar 26, 2024
da5a367
Clean-up
martin-strecker-sonarsource Mar 26, 2024
1fa63c3
Refactor
martin-strecker-sonarsource Mar 26, 2024
dab5005
WIP: Refactor
martin-strecker-sonarsource Mar 26, 2024
5f9011a
Clean-up
martin-strecker-sonarsource Mar 26, 2024
264f09c
Re-work
martin-strecker-sonarsource Mar 27, 2024
740f171
Simplify
martin-strecker-sonarsource Mar 27, 2024
0a903c0
Clean-up
martin-strecker-sonarsource Mar 27, 2024
9e03c2d
Clean-up and code comment
martin-strecker-sonarsource Mar 27, 2024
3660053
Formatting
martin-strecker-sonarsource Mar 27, 2024
f7a4ae5
Inline visitedStatus
martin-strecker-sonarsource Mar 27, 2024
f01e2f8
Move visited check to the top.
martin-strecker-sonarsource Mar 27, 2024
d0bbdd7
Formatting in test case
martin-strecker-sonarsource Mar 27, 2024
95b38ac
Add test case by generating the code (InfiniteRecursion_RoslynCfg_897…
martin-strecker-sonarsource Mar 27, 2024
e4adb63
Fix compiler error in test case
martin-strecker-sonarsource Mar 27, 2024
f6a7237
Force CI
martin-strecker-sonarsource Mar 27, 2024
824eead
Move ExitBlock up
martin-strecker-sonarsource Mar 27, 2024
08ec7af
Move test case
martin-strecker-sonarsource Mar 28, 2024
1f04738
Swap conditions
martin-strecker-sonarsource Mar 28, 2024
be51537
NewLine
martin-strecker-sonarsource Mar 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 29 additions & 15 deletions analyzers/src/SonarAnalyzer.CFG/Roslyn/CfgAllPathValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,43 @@ namespace SonarAnalyzer.CFG.Roslyn
public abstract class CfgAllPathValidator
{
private readonly ControlFlowGraph cfg;
private readonly Dictionary<BasicBlock, bool> visitedStatus = new Dictionary<BasicBlock, bool>();

protected abstract bool IsValid(BasicBlock block);
protected abstract bool IsInvalid(BasicBlock block);

protected CfgAllPathValidator(ControlFlowGraph cfg) =>
this.cfg = cfg;

public bool CheckAllPaths() =>
IsBlockOrAllSuccessorsValid(cfg.EntryBlock);

private bool IsBlockOrAllSuccessorsValid(BasicBlock block)
{
var isValid = !IsInvalid(block) && (IsValid(block) || AreAllSuccessorsValid(block));
visitedStatus[block] = isValid;
return isValid;
}

private bool AreAllSuccessorsValid(BasicBlock block)
public bool CheckAllPaths()
{
visitedStatus[block] = true; // protects from loops, don't fail the computation if hits itself
return block.SuccessorBlocks.Any()
&& block.SuccessorBlocks.All(x => x != cfg.ExitBlock && (visitedStatus.ContainsKey(x) ? visitedStatus[x] : IsBlockOrAllSuccessorsValid(x)));
HashSet<BasicBlock> visited = [];
var blocks = new Stack<BasicBlock>();
pavel-mikula-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
blocks.Push(cfg.EntryBlock);
while (blocks.Count > 0)
{
var block = blocks.Pop();
if (!visited.Add(block))
{
continue; // We already visited this block. (This protects from endless loops)
}
if (block == cfg.ExitBlock || IsInvalid(block))
{
return false;
}
if (IsValid(block))
{
continue;
}
if (block.SuccessorBlocks.IsEmpty)
{
return false;
}
foreach (var successorBlock in block.SuccessorBlocks)
{
blocks.Push(successorBlock);
}
}
return true;
}
}
}
96 changes: 96 additions & 0 deletions analyzers/tests/SonarAnalyzer.Test/Rules/InfiniteRecursionTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System.Text;
using SonarAnalyzer.Rules.CSharp;

namespace SonarAnalyzer.Test.Rules
Expand Down Expand Up @@ -60,5 +61,100 @@ public class InfiniteRecursionTest

#endif

// https://github.com/SonarSource/sonar-dotnet/issues/8977
[TestMethod]
public void InfiniteRecursion_RoslynCfg_8977()
{
const int rows = 4_000;
var code = new StringBuilder();
code.Append("""
using UInt32Value = System.UInt32;
using StringValue = System.String;

public class WorksheetPart
{
public Worksheet Worksheet { get; set; }
}
public class Worksheet
{
public MarkupCompatibilityAttributes MCAttributes { get; set; }
public void AddNamespaceDeclaration(string alias, string xmlNamespace) { }
public void Append(SheetData sheetData1) { }
}
public class SheetData
{
public void Append(Row r) { }
}
public class MarkupCompatibilityAttributes
{
public string Ignorable { get; set; }
}
public class Row
{
public UInt32Value RowIndex { get; set; }
public ListValue<StringValue> Spans { get; set; }
public double DyDescent { get; set; }
public void Append(Cell c) { }
}
public class ListValue<T>
{
public string InnerText { get; set; }
}
public class Cell
{
public string CellReference { get; set; }
public UInt32Value StyleIndex { get; set; }
public void Append(CellValue c) { }
}
public class CellValue
{
public string Text { get; set; }
}

class Program
{
public static void Main() { }

void GenerateWorksheetPart1Content(WorksheetPart worksheetPart1)
{
Worksheet worksheet1 = new Worksheet() { MCAttributes = new MarkupCompatibilityAttributes() { Ignorable = "x14ac xr xr2 xr3" } };
worksheet1.AddNamespaceDeclaration("r", "http://schemas.openxmlformats.org/officeDocument/2006/relationships");
worksheet1.AddNamespaceDeclaration("mc", "http://schemas.openxmlformats.org/markup-compatibility/2006");
worksheet1.AddNamespaceDeclaration("x14ac", "http://schemas.microsoft.com/office/spreadsheetml/2009/9/ac");
worksheet1.AddNamespaceDeclaration("xr", "http://schemas.microsoft.com/office/spreadsheetml/2014/revision");
worksheet1.AddNamespaceDeclaration("xr2", "http://schemas.microsoft.com/office/spreadsheetml/2015/revision2");
worksheet1.AddNamespaceDeclaration("xr3", "http://schemas.microsoft.com/office/spreadsheetml/2016/revision3");

SheetData sheetData1 = new SheetData();
""");
for (var i = 1; i <= rows; i++)
{
code.Append($$"""
Row row{{i}} = new Row() { RowIndex = (UInt32Value)1U, Spans = new ListValue<StringValue>() { InnerText = "1:1" }, DyDescent = 0.25D };

Cell cell{{i}} = new Cell() { CellReference = "A{{i}}", StyleIndex = (UInt32Value)1U };
CellValue cellValue{{i}} = new CellValue();
cellValue{{i}}.Text = "{{i}}";

cell{{i}}.Append(cellValue{{i}});

row{{i}}.Append(cell{{i}});
""");
}
for (var i = 1; i <= rows; i++)
{
code.AppendLine($$""" sheetData1.Append(row{{i}});""");
}
code.Append(""""
worksheet1.Append(sheetData1);
worksheetPart1.Worksheet = worksheet1;
}
}
"""");

roslynCfg.AddSnippet(code.ToString())
.WithOptions(ParseOptionsHelper.FromCSharp8)
.Verify();
}
}
}