Skip to content

Commit 97f09e1

Browse files
tamasvajktamasvajk
authored
Merge pull request github#14101 from tamasvajk/csharp/recursive-generics
C#: Exclude base type extraction of recursive generics
2 parents cb89220 + bf96e68 commit 97f09e1

File tree

8 files changed

+289
-1
lines changed

8 files changed

+289
-1
lines changed

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs

Lines changed: 201 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
using System;
2+
using System.Collections.Concurrent;
23
using System.Collections.Generic;
34
using System.IO;
45
using System.Linq;
56
using Microsoft.CodeAnalysis;
67
using Microsoft.CodeAnalysis.CSharp.Syntax;
8+
using Semmle.Util;
79

810
namespace Semmle.Extraction.CSharp.Entities
911
{
@@ -82,8 +84,15 @@ protected void PopulateType(TextWriter trapFile, bool constructUnderlyingTupleTy
8284

8385
var baseTypes = GetBaseTypeDeclarations();
8486

87+
var hasExpandingCycle = GenericsRecursionGraph.HasExpandingCycle(Symbol);
88+
if (hasExpandingCycle)
89+
{
90+
Context.ExtractionError("Found recursive generic inheritance hierarchy. Base class of type is not extracted", Symbol.ToDisplayString(), Context.CreateLocation(ReportingLocation), severity: Util.Logging.Severity.Warning);
91+
}
92+
8593
// Visit base types
86-
if (Symbol.GetNonObjectBaseType(Context) is INamedTypeSymbol @base)
94+
if (!hasExpandingCycle
95+
&& Symbol.GetNonObjectBaseType(Context) is INamedTypeSymbol @base)
8796
{
8897
var bts = GetBaseTypeDeclarations(baseTypes, @base);
8998

@@ -347,6 +356,197 @@ public override bool Equals(object? obj)
347356
}
348357

349358
public override int GetHashCode() => SymbolEqualityComparer.Default.GetHashCode(Symbol);
359+
360+
/// <summary>
361+
/// Class to detect recursive generic inheritance hierarchies.
362+
///
363+
/// Details can be found in https://www.ecma-international.org/wp-content/uploads/ECMA-335_6th_edition_june_2012.pdf Chapter II.9.2 Generics and recursive inheritance graphs
364+
/// The dotnet runtime already implements this check as a runtime validation: https://github.com/dotnet/runtime/blob/e48e88d0fe9c2e494c0e6fd0c7c1fb54e7ddbdb1/src/coreclr/vm/generics.cpp#L748
365+
/// </summary>
366+
private class GenericsRecursionGraph
367+
{
368+
private static readonly ConcurrentDictionary<INamedTypeSymbol, bool> resultCache = new(SymbolEqualityComparer.Default);
369+
370+
/// <summary>
371+
/// Checks whether the given type has a recursive generic inheritance hierarchy. The result is cached.
372+
/// </summary>
373+
public static bool HasExpandingCycle(ITypeSymbol start)
374+
{
375+
if (start.OriginalDefinition is not INamedTypeSymbol namedTypeDefinition ||
376+
!namedTypeDefinition.IsGenericType)
377+
{
378+
return false;
379+
}
380+
381+
return resultCache.GetOrAdd(namedTypeDefinition, nt => new GenericsRecursionGraph(nt).HasExpandingCycle());
382+
}
383+
384+
private readonly INamedTypeSymbol startSymbol;
385+
private readonly HashSet<INamedTypeSymbol> instantiationClosure = new(SymbolEqualityComparer.Default);
386+
private readonly Dictionary<ITypeParameterSymbol, List<(ITypeParameterSymbol To, bool IsExpanding)>> edges = new(SymbolEqualityComparer.Default);
387+
388+
private GenericsRecursionGraph(INamedTypeSymbol startSymbol)
389+
{
390+
this.startSymbol = startSymbol;
391+
392+
ComputeInstantiationClosure();
393+
ComputeGraphEdges();
394+
}
395+
396+
private void ComputeGraphEdges()
397+
{
398+
foreach (var reference in instantiationClosure)
399+
{
400+
var definition = reference.OriginalDefinition;
401+
if (SymbolEqualityComparer.Default.Equals(reference, definition))
402+
{
403+
// It's a definition, so no edges
404+
continue;
405+
}
406+
407+
for (var i = 0; i < reference.TypeArguments.Length; i++)
408+
{
409+
var target = definition.TypeParameters[i];
410+
if (reference.TypeArguments[i] is ITypeParameterSymbol source)
411+
{
412+
// non-expanding
413+
edges.AddAnother(source, (target, false));
414+
}
415+
else if (reference.TypeArguments[i] is INamedTypeSymbol namedType)
416+
{
417+
// expanding
418+
var sources = GetAllNestedTypeParameters(namedType);
419+
foreach (var s in sources)
420+
{
421+
edges.AddAnother(s, (target, true));
422+
}
423+
}
424+
}
425+
}
426+
}
427+
428+
private static List<ITypeParameterSymbol> GetAllNestedTypeParameters(INamedTypeSymbol symbol)
429+
{
430+
var res = new List<ITypeParameterSymbol>();
431+
432+
void AddTypeParameters(INamedTypeSymbol symbol)
433+
{
434+
foreach (var typeArgument in symbol.TypeArguments)
435+
{
436+
if (typeArgument is ITypeParameterSymbol typeParameter)
437+
{
438+
res.Add(typeParameter);
439+
}
440+
else if (typeArgument is INamedTypeSymbol namedType)
441+
{
442+
AddTypeParameters(namedType);
443+
}
444+
}
445+
}
446+
447+
AddTypeParameters(symbol);
448+
449+
return res;
450+
}
451+
452+
private void ComputeInstantiationClosure()
453+
{
454+
var workQueue = new Queue<INamedTypeSymbol>();
455+
workQueue.Enqueue(startSymbol);
456+
457+
while (workQueue.Count > 0)
458+
{
459+
var current = workQueue.Dequeue();
460+
if (instantiationClosure.Contains(current) ||
461+
!current.IsGenericType)
462+
{
463+
continue;
464+
}
465+
466+
instantiationClosure.Add(current);
467+
468+
if (SymbolEqualityComparer.Default.Equals(current, current.OriginalDefinition))
469+
{
470+
// Definition, so enqueue all base types and interfaces
471+
if (current.BaseType != null)
472+
{
473+
workQueue.Enqueue(current.BaseType);
474+
}
475+
476+
foreach (var i in current.Interfaces)
477+
{
478+
workQueue.Enqueue(i);
479+
}
480+
}
481+
else
482+
{
483+
// Reference, so enqueue all type arguments and their original definitions:
484+
foreach (var namedTypeArgument in current.TypeArguments.OfType<INamedTypeSymbol>())
485+
{
486+
workQueue.Enqueue(namedTypeArgument);
487+
workQueue.Enqueue(namedTypeArgument.OriginalDefinition);
488+
}
489+
}
490+
}
491+
}
492+
493+
private bool HasExpandingCycle()
494+
{
495+
return startSymbol.TypeParameters.Any(HasExpandingCycle);
496+
}
497+
498+
private bool HasExpandingCycle(ITypeParameterSymbol start)
499+
{
500+
var visited = new HashSet<ITypeParameterSymbol>(SymbolEqualityComparer.Default);
501+
var path = new List<ITypeParameterSymbol>();
502+
var hasExpandingCycle = HasExpandingCycle(start, visited, path, hasSeenExpandingEdge: false);
503+
return hasExpandingCycle;
504+
}
505+
506+
private List<(ITypeParameterSymbol To, bool IsExpanding)> GetOutgoingEdges(ITypeParameterSymbol typeParameter)
507+
{
508+
return edges.TryGetValue(typeParameter, out var outgoingEdges)
509+
? outgoingEdges
510+
: new List<(ITypeParameterSymbol, bool)>();
511+
}
512+
513+
/// <summary>
514+
/// A modified cycle detection algorithm based on DFS.
515+
/// </summary>
516+
/// <param name="current">The current node that is being visited</param>
517+
/// <param name="visited">The nodes that have already been visited by any path.</param>
518+
/// <param name="currentPath">The nodes already visited on the current path.</param>
519+
/// <param name="hasSeenExpandingEdge">Whether an expanding edge was already seen in this path. We're looking for a cycle that has at least one expanding edge.</param>
520+
/// <returns></returns>
521+
private bool HasExpandingCycle(ITypeParameterSymbol current, HashSet<ITypeParameterSymbol> visited, List<ITypeParameterSymbol> currentPath, bool hasSeenExpandingEdge)
522+
{
523+
if (currentPath.Count > 0 && SymbolEqualityComparer.Default.Equals(current, currentPath[0]))
524+
{
525+
return hasSeenExpandingEdge;
526+
}
527+
528+
if (visited.Contains(current))
529+
{
530+
return false;
531+
}
532+
533+
visited.Add(current);
534+
currentPath.Add(current);
535+
536+
var outgoingEdges = GetOutgoingEdges(current);
537+
538+
foreach (var outgoingEdge in outgoingEdges)
539+
{
540+
if (HasExpandingCycle(outgoingEdge.To, visited, currentPath, hasSeenExpandingEdge: hasSeenExpandingEdge || outgoingEdge.IsExpanding))
541+
{
542+
return true;
543+
}
544+
}
545+
546+
currentPath.RemoveAt(currentPath.Count - 1);
547+
return false;
548+
}
549+
}
350550
}
351551

352552
internal abstract class Type<T> : Type where T : ITypeSymbol

‎csharp/ql/integration-tests/all-platforms/diag_recursive_generics/ExtractortMessages.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
| test.cs:2:14:2:20 | Found recursive generic inheritance hierarchy. Base class of type is not extracted | 4 | GenB<GenB<T>> |
2+
| test.cs:2:14:2:20 | Found recursive generic inheritance hierarchy. Base class of type is not extracted | 4 | GenB<GenB<string>> |
3+
| test.cs:2:14:2:20 | Found recursive generic inheritance hierarchy. Base class of type is not extracted | 4 | GenB<T> |
4+
| test.cs:2:14:2:20 | Found recursive generic inheritance hierarchy. Base class of type is not extracted | 4 | GenB<string> |

‎csharp/ql/integration-tests/all-platforms/diag_recursive_generics/ExtractortMessages.ql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import csharp
2+
import semmle.code.csharp.commons.Diagnostics
3+
4+
query predicate extractorMessages(ExtractorMessage msg, int severity, string elementText) {
5+
msg.getSeverity() = severity and msg.getElementText() = elementText
6+
}

‎csharp/ql/integration-tests/all-platforms/diag_recursive_generics/Types.expected

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
| test.cs:1:14:1:20 | GenA<> | System.Object |
2+
| test.cs:1:14:1:20 | GenA<GenB<GenB<>>> | System.Object |
3+
| test.cs:1:14:1:20 | GenA<GenB<GenB<String>>> | System.Object |
4+
| test.cs:2:14:2:20 | GenB<> | System.Object |
5+
| test.cs:2:14:2:20 | GenB<GenB<>> | System.Object |
6+
| test.cs:2:14:2:20 | GenB<GenB<String>> | System.Object |
7+
| test.cs:2:14:2:20 | GenB<String> | System.Object |
8+
| test.cs:4:7:4:10 | P<> | System.Object |
9+
| test.cs:4:7:4:10 | P<C<,>> | System.Object |
10+
| test.cs:4:7:4:10 | P<C<Int32,String>> | System.Object |
11+
| test.cs:4:7:4:10 | P<C<String,Int32>> | System.Object |
12+
| test.cs:4:7:4:10 | P<C<V,U>> | System.Object |
13+
| test.cs:4:7:4:10 | P<C<W,X>> | System.Object |
14+
| test.cs:4:7:4:10 | P<C<X,W>> | System.Object |
15+
| test.cs:4:7:4:10 | P<D<,>> | System.Object |
16+
| test.cs:4:7:4:10 | P<D<Int32,String>> | System.Object |
17+
| test.cs:4:7:4:10 | P<D<String,Int32>> | System.Object |
18+
| test.cs:4:7:4:10 | P<D<U,V>> | System.Object |
19+
| test.cs:4:7:4:10 | P<D<V,U>> | System.Object |
20+
| test.cs:4:7:4:10 | P<D<X,W>> | System.Object |
21+
| test.cs:5:7:5:13 | C<,> | P<D<V,U>> |
22+
| test.cs:5:7:5:13 | C<Int32,String> | P<D<System.String,System.Int32>> |
23+
| test.cs:5:7:5:13 | C<String,Int32> | P<D<System.Int32,System.String>> |
24+
| test.cs:5:7:5:13 | C<V,U> | P<D<U,V>> |
25+
| test.cs:5:7:5:13 | C<W,X> | P<D<X,W>> |
26+
| test.cs:5:7:5:13 | C<X,W> | P<D<,>> |
27+
| test.cs:6:7:6:13 | D<,> | P<C<W,X>> |
28+
| test.cs:6:7:6:13 | D<Int32,String> | P<C<System.Int32,System.String>> |
29+
| test.cs:6:7:6:13 | D<String,Int32> | P<C<System.String,System.Int32>> |
30+
| test.cs:6:7:6:13 | D<U,V> | P<C<,>> |
31+
| test.cs:6:7:6:13 | D<V,U> | P<C<V,U>> |
32+
| test.cs:6:7:6:13 | D<X,W> | P<C<X,W>> |
33+
| test.cs:8:7:8:10 | A<> | System.Object |
34+
| test.cs:8:7:8:10 | A<String> | System.Object |
35+
| test.cs:13:14:13:18 | Class | System.Object |

‎csharp/ql/integration-tests/all-platforms/diag_recursive_generics/Types.ql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import csharp
2+
3+
from Class c
4+
where c.fromSource()
5+
select c, c.getBaseClass().getQualifiedName()

‎csharp/ql/integration-tests/all-platforms/diag_recursive_generics/test.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
public class GenA<U> { };
2+
public class GenB<T> : GenA<GenB<GenB<T>>> { };
3+
4+
class P<T> { }
5+
class C<U, V> : P<D<V, U>> { }
6+
class D<W, X> : P<C<W, X>> { }
7+
8+
class A<T> : System.IEquatable<A<T>>
9+
{
10+
public bool Equals(A<T> other) { return true; }
11+
}
12+
13+
public class Class
14+
{
15+
public static int Main()
16+
{
17+
GenB<string> a = new GenB<string>();
18+
P<D<string, int>> b = new C<int, string>();
19+
A<string> c = new A<string>();
20+
21+
return 0;
22+
}
23+
}

‎csharp/ql/integration-tests/all-platforms/diag_recursive_generics/test.csproj

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
3+
<PropertyGroup>
4+
<OutputType>Exe</OutputType>
5+
<TargetFramework>net7.0</TargetFramework>
6+
</PropertyGroup>
7+
8+
<Target Name="DeleteBinObjFolders" BeforeTargets="Clean">
9+
<RemoveDir Directories=".\bin" />
10+
<RemoveDir Directories=".\obj" />
11+
</Target>
12+
</Project>

‎csharp/ql/integration-tests/all-platforms/diag_recursive_generics/test.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
from create_database_utils import *
2+
3+
run_codeql_database_create(['dotnet build'], lang="csharp", extra_args=["--extractor-option=cil=false"])

0 commit comments

Comments
 (0)