Merge pull request #14101 from tamasvajk/csharp/recursive-generics

C#: Exclude base type extraction of recursive generics

Author: tamasvajk

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

@@ -1,9 +1,11 @@
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Semmle.Util;
 
 namespace Semmle.Extraction.CSharp.Entities
 {
@@ -82,8 +84,15 @@ protected void PopulateType(TextWriter trapFile, bool constructUnderlyingTupleTy
 
             var baseTypes = GetBaseTypeDeclarations();
 
+            var hasExpandingCycle = GenericsRecursionGraph.HasExpandingCycle(Symbol);
+            if (hasExpandingCycle)
+            {
+                Context.ExtractionError("Found recursive generic inheritance hierarchy. Base class of type is not extracted", Symbol.ToDisplayString(), Context.CreateLocation(ReportingLocation), severity: Util.Logging.Severity.Warning);
+            }
+
             // Visit base types
-            if (Symbol.GetNonObjectBaseType(Context) is INamedTypeSymbol @base)
+            if (!hasExpandingCycle
+                && Symbol.GetNonObjectBaseType(Context) is INamedTypeSymbol @base)
             {
                 var bts = GetBaseTypeDeclarations(baseTypes, @base);
 
@@ -347,6 +356,197 @@ public override bool Equals(object? obj)
         }
 
         public override int GetHashCode() => SymbolEqualityComparer.Default.GetHashCode(Symbol);
+
+        /// <summary>
+        /// Class to detect recursive generic inheritance hierarchies.
+        ///
+        /// 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
+        /// The dotnet runtime already implements this check as a runtime validation: https://github.com/dotnet/runtime/blob/e48e88d0fe9c2e494c0e6fd0c7c1fb54e7ddbdb1/src/coreclr/vm/generics.cpp#L748
+        /// </summary>
+        private class GenericsRecursionGraph
+        {
+            private static readonly ConcurrentDictionary<INamedTypeSymbol, bool> resultCache = new(SymbolEqualityComparer.Default);
+
+            /// <summary>
+            /// Checks whether the given type has a recursive generic inheritance hierarchy. The result is cached.
+            /// </summary>
+            public static bool HasExpandingCycle(ITypeSymbol start)
+            {
+                if (start.OriginalDefinition is not INamedTypeSymbol namedTypeDefinition ||
+                    !namedTypeDefinition.IsGenericType)
+                {
+                    return false;
+                }
+
+                return resultCache.GetOrAdd(namedTypeDefinition, nt => new GenericsRecursionGraph(nt).HasExpandingCycle());
+            }
+
+            private readonly INamedTypeSymbol startSymbol;
+            private readonly HashSet<INamedTypeSymbol> instantiationClosure = new(SymbolEqualityComparer.Default);
+            private readonly Dictionary<ITypeParameterSymbol, List<(ITypeParameterSymbol To, bool IsExpanding)>> edges = new(SymbolEqualityComparer.Default);
+
+            private GenericsRecursionGraph(INamedTypeSymbol startSymbol)
+            {
+                this.startSymbol = startSymbol;
+
+                ComputeInstantiationClosure();
+                ComputeGraphEdges();
+            }
+
+            private void ComputeGraphEdges()
+            {
+                foreach (var reference in instantiationClosure)
+                {
+                    var definition = reference.OriginalDefinition;
+                    if (SymbolEqualityComparer.Default.Equals(reference, definition))
+                    {
+                        // It's a definition, so no edges
+                        continue;
+                    }
+
+                    for (var i = 0; i < reference.TypeArguments.Length; i++)
+                    {
+                        var target = definition.TypeParameters[i];
+                        if (reference.TypeArguments[i] is ITypeParameterSymbol source)
+                        {
+                            // non-expanding
+                            edges.AddAnother(source, (target, false));
+                        }
+                        else if (reference.TypeArguments[i] is INamedTypeSymbol namedType)
+                        {
+                            // expanding
+                            var sources = GetAllNestedTypeParameters(namedType);
+                            foreach (var s in sources)
+                            {
+                                edges.AddAnother(s, (target, true));
+                            }
+                        }
+                    }
+                }
+            }
+
+            private static List<ITypeParameterSymbol> GetAllNestedTypeParameters(INamedTypeSymbol symbol)
+            {
+                var res = new List<ITypeParameterSymbol>();
+
+                void AddTypeParameters(INamedTypeSymbol symbol)
+                {
+                    foreach (var typeArgument in symbol.TypeArguments)
+                    {
+                        if (typeArgument is ITypeParameterSymbol typeParameter)
+                        {
+                            res.Add(typeParameter);
+                        }
+                        else if (typeArgument is INamedTypeSymbol namedType)
+                        {
+                            AddTypeParameters(namedType);
+                        }
+                    }
+                }
+
+                AddTypeParameters(symbol);
+
+                return res;
+            }
+
+            private void ComputeInstantiationClosure()
+            {
+                var workQueue = new Queue<INamedTypeSymbol>();
+                workQueue.Enqueue(startSymbol);
+
+                while (workQueue.Count > 0)
+                {
+                    var current = workQueue.Dequeue();
+                    if (instantiationClosure.Contains(current) ||
+                        !current.IsGenericType)
+                    {
+                        continue;
+                    }
+
+                    instantiationClosure.Add(current);
+
+                    if (SymbolEqualityComparer.Default.Equals(current, current.OriginalDefinition))
+                    {
+                        // Definition, so enqueue all base types and interfaces
+                        if (current.BaseType != null)
+                        {
+                            workQueue.Enqueue(current.BaseType);
+                        }
+
+                        foreach (var i in current.Interfaces)
+                        {
+                            workQueue.Enqueue(i);
+                        }
+                    }
+                    else
+                    {
+                        // Reference, so enqueue all type arguments and their original definitions:
+                        foreach (var namedTypeArgument in current.TypeArguments.OfType<INamedTypeSymbol>())
+                        {
+                            workQueue.Enqueue(namedTypeArgument);
+                            workQueue.Enqueue(namedTypeArgument.OriginalDefinition);
+                        }
+                    }
+                }
+            }
+
+            private bool HasExpandingCycle()
+            {
+                return startSymbol.TypeParameters.Any(HasExpandingCycle);
+            }
+
+            private bool HasExpandingCycle(ITypeParameterSymbol start)
+            {
+                var visited = new HashSet<ITypeParameterSymbol>(SymbolEqualityComparer.Default);
+                var path = new List<ITypeParameterSymbol>();
+                var hasExpandingCycle = HasExpandingCycle(start, visited, path, hasSeenExpandingEdge: false);
+                return hasExpandingCycle;
+            }
+
+            private List<(ITypeParameterSymbol To, bool IsExpanding)> GetOutgoingEdges(ITypeParameterSymbol typeParameter)
+            {
+                return edges.TryGetValue(typeParameter, out var outgoingEdges)
+                    ? outgoingEdges
+                    : new List<(ITypeParameterSymbol, bool)>();
+            }
+
+            /// <summary>
+            /// A modified cycle detection algorithm based on DFS.
+            /// </summary>
+            /// <param name="current">The current node that is being visited</param>
+            /// <param name="visited">The nodes that have already been visited by any path.</param>
+            /// <param name="currentPath">The nodes already visited on the current path.</param>
+            /// <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>
+            /// <returns></returns>
+            private bool HasExpandingCycle(ITypeParameterSymbol current, HashSet<ITypeParameterSymbol> visited, List<ITypeParameterSymbol> currentPath, bool hasSeenExpandingEdge)
+            {
+                if (currentPath.Count > 0 && SymbolEqualityComparer.Default.Equals(current, currentPath[0]))
+                {
+                    return hasSeenExpandingEdge;
+                }
+
+                if (visited.Contains(current))
+                {
+                    return false;
+                }
+
+                visited.Add(current);
+                currentPath.Add(current);
+
+                var outgoingEdges = GetOutgoingEdges(current);
+
+                foreach (var outgoingEdge in outgoingEdges)
+                {
+                    if (HasExpandingCycle(outgoingEdge.To, visited, currentPath, hasSeenExpandingEdge: hasSeenExpandingEdge || outgoingEdge.IsExpanding))
+                    {
+                        return true;
+                    }
+                }
+
+                currentPath.RemoveAt(currentPath.Count - 1);
+                return false;
+            }
+        }
     }
 
     internal abstract class Type<T> : Type where T : ITypeSymbol

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

@@ -0,0 +1,4 @@
+| test.cs:2:14:2:20 | Found recursive generic inheritance hierarchy. Base class of type is not extracted | 4 | GenB<GenB<T>> |
+| test.cs:2:14:2:20 | Found recursive generic inheritance hierarchy. Base class of type is not extracted | 4 | GenB<GenB<string>> |
+| test.cs:2:14:2:20 | Found recursive generic inheritance hierarchy. Base class of type is not extracted | 4 | GenB<T> |
+| 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

@@ -0,0 +1,6 @@
+import csharp
+import semmle.code.csharp.commons.Diagnostics
+
+query predicate extractorMessages(ExtractorMessage msg, int severity, string elementText) {
+  msg.getSeverity() = severity and msg.getElementText() = elementText
+}

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

@@ -0,0 +1,35 @@
+| test.cs:1:14:1:20 | GenA<> | System.Object |
+| test.cs:1:14:1:20 | GenA<GenB<GenB<>>> | System.Object |
+| test.cs:1:14:1:20 | GenA<GenB<GenB<String>>> | System.Object |
+| test.cs:2:14:2:20 | GenB<> | System.Object |
+| test.cs:2:14:2:20 | GenB<GenB<>> | System.Object |
+| test.cs:2:14:2:20 | GenB<GenB<String>> | System.Object |
+| test.cs:2:14:2:20 | GenB<String> | System.Object |
+| test.cs:4:7:4:10 | P<> | System.Object |
+| test.cs:4:7:4:10 | P<C<,>> | System.Object |
+| test.cs:4:7:4:10 | P<C<Int32,String>> | System.Object |
+| test.cs:4:7:4:10 | P<C<String,Int32>> | System.Object |
+| test.cs:4:7:4:10 | P<C<V,U>> | System.Object |
+| test.cs:4:7:4:10 | P<C<W,X>> | System.Object |
+| test.cs:4:7:4:10 | P<C<X,W>> | System.Object |
+| test.cs:4:7:4:10 | P<D<,>> | System.Object |
+| test.cs:4:7:4:10 | P<D<Int32,String>> | System.Object |
+| test.cs:4:7:4:10 | P<D<String,Int32>> | System.Object |
+| test.cs:4:7:4:10 | P<D<U,V>> | System.Object |
+| test.cs:4:7:4:10 | P<D<V,U>> | System.Object |
+| test.cs:4:7:4:10 | P<D<X,W>> | System.Object |
+| test.cs:5:7:5:13 | C<,> | P<D<V,U>> |
+| test.cs:5:7:5:13 | C<Int32,String> | P<D<System.String,System.Int32>> |
+| test.cs:5:7:5:13 | C<String,Int32> | P<D<System.Int32,System.String>> |
+| test.cs:5:7:5:13 | C<V,U> | P<D<U,V>> |
+| test.cs:5:7:5:13 | C<W,X> | P<D<X,W>> |
+| test.cs:5:7:5:13 | C<X,W> | P<D<,>> |
+| test.cs:6:7:6:13 | D<,> | P<C<W,X>> |
+| test.cs:6:7:6:13 | D<Int32,String> | P<C<System.Int32,System.String>> |
+| test.cs:6:7:6:13 | D<String,Int32> | P<C<System.String,System.Int32>> |
+| test.cs:6:7:6:13 | D<U,V> | P<C<,>> |
+| test.cs:6:7:6:13 | D<V,U> | P<C<V,U>> |
+| test.cs:6:7:6:13 | D<X,W> | P<C<X,W>> |
+| test.cs:8:7:8:10 | A<> | System.Object |
+| test.cs:8:7:8:10 | A<String> | System.Object |
+| test.cs:13:14:13:18 | Class | System.Object |

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

@@ -0,0 +1,5 @@
+import csharp
+
+from Class c
+where c.fromSource()
+select c, c.getBaseClass().getQualifiedName()

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

@@ -0,0 +1,23 @@
+public class GenA<U> { };
+public class GenB<T> : GenA<GenB<GenB<T>>> { };
+
+class P<T> { }
+class C<U, V> : P<D<V, U>> { }
+class D<W, X> : P<C<W, X>> { }
+
+class A<T> : System.IEquatable<A<T>>
+{
+    public bool Equals(A<T> other) { return true; }
+}
+
+public class Class
+{
+    public static int Main()
+    {
+        GenB<string> a = new GenB<string>();
+        P<D<string, int>> b = new C<int, string>();
+        A<string> c = new A<string>();
+
+        return 0;
+    }
+}

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

@@ -0,0 +1,12 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFramework>net7.0</TargetFramework>
+  </PropertyGroup>
+
+ <Target Name="DeleteBinObjFolders" BeforeTargets="Clean">
+   <RemoveDir Directories=".\bin" />
+   <RemoveDir Directories=".\obj" />
+ </Target>
+</Project>

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

@@ -0,0 +1,3 @@
+from create_database_utils import *
+
+run_codeql_database_create(['dotnet build'], lang="csharp", extra_args=["--extractor-option=cil=false"])