Skip to content

Commit c1d8091

Browse files
tamasvajktamasvajk
committed
C#: Exclude base type extraction of recursive generics
1 parent 3476437 commit c1d8091

File tree

3 files changed

+253
-1
lines changed

3 files changed

+253
-1
lines changed

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

Lines changed: 214 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Collections.Concurrent;
23
using System.Collections.Generic;
34
using System.IO;
45
using System.Linq;
@@ -82,8 +83,15 @@ protected void PopulateType(TextWriter trapFile, bool constructUnderlyingTupleTy
8283

8384
var baseTypes = GetBaseTypeDeclarations();
8485

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

@@ -347,6 +355,211 @@ public override bool Equals(object? obj)
347355
}
348356

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

352565
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/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 |

0 commit comments

Comments
 (0)