C#: Avoid combinatorial explosions in GVN construction for types

csharp/ql/consistency-queries/TypeConsistency.qll

@@ -0,0 +1,13 @@
+import csharp
+import semmle.code.csharp.Unification
+
+query predicate missingGvn(Type t, string cls) {
+  not exists(Gvn::getGlobalValueNumber(t)) and
+  cls = t.getPrimaryQlClasses()
+}
+
+query predicate multipleGvn(Type t, Gvn::GvnType g, string cls) {
+  g = Gvn::getGlobalValueNumber(t) and
+  strictcount(Gvn::getGlobalValueNumber(t)) > 1 and
+  cls = t.getPrimaryQlClasses()
+}

csharp/ql/lib/semmle/code/csharp/AnnotatedType.qll

@@ -401,6 +401,8 @@ class AnnotatedArrayType extends AnnotatedType {
 class AnnotatedConstructedType extends AnnotatedType {
   override ConstructedType type;
 
+  AnnotatedConstructedType() { not type instanceof NullableType }
+
   /** Gets the `i`th type argument of this constructed type. */
   AnnotatedType getTypeArgument(int i) {
     result.getType() = type.getTypeArgument(i) and

csharp/ql/lib/semmle/code/csharp/Generics.qll

@@ -26,7 +26,8 @@ private import TypeRef
 class Generic extends DotNet::Generic, Declaration, @generic {
   Generic() {
     type_parameters(_, _, this, _) or
-    type_arguments(_, _, this)
+    type_arguments(_, _, this) or
+    nullable_underlying_type(this, _)
   }
 }
 
@@ -39,7 +40,7 @@ class Generic extends DotNet::Generic, Declaration, @generic {
 class UnboundGeneric extends DotNet::UnboundGeneric, Generic {
   UnboundGeneric() { type_parameters(_, _, this, _) }
 
-  override TypeParameter getTypeParameter(int n) { type_parameters(result, n, this, _) }
+  final override TypeParameter getTypeParameter(int n) { type_parameters(result, n, this, _) }
 
   override ConstructedGeneric getAConstructedGeneric() { result.getUnboundGeneric() = this }
 
@@ -67,7 +68,11 @@ private string getTypeParameterCommas(UnboundGeneric ug) {
  * generic method (`ConstructedMethod`).
  */
 class ConstructedGeneric extends DotNet::ConstructedGeneric, Generic {
-  ConstructedGeneric() { type_arguments(_, _, this) }
+  ConstructedGeneric() {
+    type_arguments(_, _, this)
+    or
+    nullable_underlying_type(this, _)
+  }
 
   override UnboundGeneric getUnboundGeneric() { constructed_generic(this, result) }
 
@@ -75,8 +80,6 @@ class ConstructedGeneric extends DotNet::ConstructedGeneric, Generic {
     result = this.getUnboundGeneric().getUnboundDeclaration()
   }
 
-  override int getNumberOfTypeArguments() { result = count(int i | type_arguments(_, i, this)) }
-
   override Type getTypeArgument(int i) { none() }
 
   override Type getATypeArgument() { result = this.getTypeArgument(_) }
@@ -410,13 +413,13 @@ class ConstructedType extends ValueOrRefType, ConstructedGeneric {
 
   override Location getALocation() { result = this.getUnboundDeclaration().getALocation() }
 
-  override Type getTypeArgument(int n) { type_arguments(getTypeRef(result), n, getTypeRef(this)) }
+  override Type getTypeArgument(int n) { type_arguments(getTypeRef(result), n, this) }
 
   override UnboundGenericType getUnboundGeneric() { constructed_generic(this, getTypeRef(result)) }
 
   final override Type getChild(int n) { result = this.getTypeArgument(n) }
 
-  final override string toStringWithTypes() {
+  override string toStringWithTypes() {
     result = this.getUndecoratedName() + "<" + getTypeArgumentsToString(this) + ">"
   }
 
@@ -424,7 +427,7 @@ class ConstructedType extends ValueOrRefType, ConstructedGeneric {
     result = this.getUndecoratedName() + "<" + getTypeArgumentsNames(this) + ">"
   }
 
-  final override predicate hasQualifiedName(string qualifier, string name) {
+  override predicate hasQualifiedName(string qualifier, string name) {
     exists(string name0 | name = name0 + "<" + getTypeArgumentsQualifiedNames(this) + ">" |
       exists(string enclosing |
         this.getDeclaringType().hasQualifiedName(qualifier, enclosing) and

csharp/ql/lib/semmle/code/csharp/Type.qll

@@ -974,29 +974,27 @@ class NullType extends RefType, @null_type {
 /**
  * A nullable type, for example `int?`.
  */
-class NullableType extends ValueType, DotNet::ConstructedGeneric, @nullable_type {
+class NullableType extends ValueType, ConstructedType, @nullable_type {
   /**
    * Gets the underlying value type of this nullable type.
    * For example `int` in `int?`.
    */
   Type getUnderlyingType() { nullable_underlying_type(this, getTypeRef(result)) }
 
+  override UnboundGenericStruct getUnboundGeneric() {
+    result.hasQualifiedName("System", "Nullable<>")
+  }
+
   override string toStringWithTypes() {
     result = this.getUnderlyingType().toStringWithTypes() + "?"
   }
 
-  override Type getChild(int n) { result = this.getUnderlyingType() and n = 0 }
-
   override Location getALocation() { result = this.getUnderlyingType().getALocation() }
 
   override Type getTypeArgument(int p) { p = 0 and result = this.getUnderlyingType() }
 
   override string getAPrimaryQlClass() { result = "NullableType" }
 
-  final override string getName() {
-    result = "Nullable<" + this.getUnderlyingType().getName() + ">"
-  }
-
   final override predicate hasQualifiedName(string qualifier, string name) {
     qualifier = "System" and
     name = "Nullable<" + this.getUnderlyingType().getQualifiedName() + ">"
@@ -1126,7 +1124,10 @@ class ArglistType extends Type, @arglist_type {
  * A type that could not be resolved. This could happen if an indirect reference
  * is not available at compilation time.
  */
-class UnknownType extends Type, @unknown_type { }
+class UnknownType extends Type, @unknown_type {
+  /** Holds if this is the canonical unknown type, and not a type that failed to extract properly. */
+  predicate isCanonical() { types(this, _, "<unknown type>") }
+}
 
 /**
  * A type representing a tuple. For example, `(int, bool, string)`.

csharp/ql/lib/semmle/code/csharp/TypeRef.qll

@@ -16,7 +16,7 @@ private class TypeRef extends @typeref {
     typeref_type(this, result)
     or
     not typeref_type(this, _) and
-    result instanceof UnknownType
+    result.(UnknownType).isCanonical()
   }
 }
 

csharp/ql/lib/semmle/code/csharp/Unification.qll

@@ -15,9 +15,11 @@ module Gvn {
    * but only if the enclosing type is not a `GenericType`.
    */
   string getNameNested(Type t) {
-    if not t instanceof NestedType or t.(NestedType).getDeclaringType() instanceof GenericType
-    then result = t.getName()
-    else result = getNameNested(t.(NestedType).getDeclaringType()) + "+" + t.getName()
+    exists(string name | name = t.getName() |
+      if not t instanceof NestedType or t.(NestedType).getDeclaringType() instanceof GenericType
+      then result = name
+      else result = getNameNested(t.(NestedType).getDeclaringType()) + "+" + name
+    )
   }
 
   /**
@@ -47,8 +49,22 @@ module Gvn {
       not exists(this.getGenericDeclaringType()) and result = 0
     }
 
+    /**
+     * Same as `getChild`, but safe-guards against potential extractor issues where
+     * multiple children exist at the same index, which may result in a combinatorial
+     * explosion.
+     */
+    private Type getChildUnique(int i) {
+      result = unique(Type t | t = this.getChild(i) | t)
+      or
+      strictcount(this.getChild(i)) > 1 and
+      result.(UnknownType).isCanonical()
+    }
+
     /** Gets the number of arguments of this type, not taking nested types into account. */
-    int getNumberOfArgumentsSelf() { result = count(int i | exists(this.getChild(i)) and i >= 0) }
+    int getNumberOfArgumentsSelf() {
+      result = count(int i | exists(this.getChildUnique(i)) and i >= 0)
+    }
 
     /** Gets the number of arguments of this type, taking nested types into account. */
     int getNumberOfArguments() {
@@ -61,7 +77,7 @@ module Gvn {
       or
       exists(int offset |
         offset = this.getNumberOfDeclaringArguments() and
-        result = this.getChild(i - offset) and
+        result = this.getChildUnique(i - offset) and
         i >= offset
       )
     }
@@ -91,13 +107,9 @@ module Gvn {
     int getNumberOfTypeParameters() {
       this = TPointerTypeKind() and result = 1
       or
-      this = TNullableTypeKind() and result = 1
-      or
       this = TArrayTypeKind(_, _) and result = 1
       or
-      exists(GenericType t | this = TConstructedType(t.getUnboundDeclaration()) |
-        result = t.getNumberOfArguments()
-      )
+      exists(GenericType t | this = TConstructedType(t) | result = t.getNumberOfArguments())
     }
 
     /** Gets the unbound declaration type that this kind corresponds to, if any. */
@@ -106,15 +118,12 @@ module Gvn {
     /**
      * Gets a textual representation of this kind when applied to arguments `args`.
      *
-     * This predicate is restricted to built-in generics (pointers, nullables, and
-     * arrays).
+     * This predicate is restricted to built-in generics (pointers and arrays).
      */
     bindingset[args]
     string toStringBuiltin(string args) {
       this = TPointerTypeKind() and result = args + "*"
       or
-      this = TNullableTypeKind() and result = args + "?"
-      or
       exists(int rnk | this = TArrayTypeKind(_, rnk) |
         result = args + "[" + concat(int i | i in [0 .. rnk - 2] | ",") + "]"
       )
@@ -135,8 +144,6 @@ module Gvn {
   CompoundTypeKind getTypeKind(Type t) {
     result = TPointerTypeKind() and t instanceof PointerType
     or
-    result = TNullableTypeKind() and t instanceof NullableType
-    or
     t = any(ArrayType at | result = TArrayTypeKind(at.getDimension(), at.getRank()))
     or
     result = TConstructedType(t.getUnboundDeclaration())
@@ -280,6 +287,7 @@ module Gvn {
 
     pragma[noinline]
     private predicate toStringPart(int i, int j) {
+      this.isFullyConstructed() and
       exists(int offset |
         exists(GenericType t, int children |
           t = this.getConstructedGenericDeclaringTypeAt(i) and
@@ -449,14 +457,12 @@ module Gvn {
     cached
     newtype TCompoundTypeKind =
       TPointerTypeKind() { Stages::UnificationStage::forceCachingInSameStage() } or
-      TNullableTypeKind() or
       TArrayTypeKind(int dim, int rnk) {
         exists(ArrayType at | dim = at.getDimension() and rnk = at.getRank())
       } or
       TConstructedType(GenericType unboundDecl) {
         unboundDecl = any(GenericType t).getUnboundDeclaration() and
         not unboundDecl instanceof PointerType and
-        not unboundDecl instanceof NullableType and
         not unboundDecl instanceof ArrayType and
         not unboundDecl instanceof TupleType
       }

csharp/ql/lib/semmle/code/dotnet/Generics.qll

@@ -41,7 +41,7 @@ abstract class ConstructedGeneric extends Generic {
   UnboundGeneric getUnboundGeneric() { none() }
 
   /** Gets the total number of type arguments. */
-  int getNumberOfTypeArguments() { result = count(int i | exists(this.getTypeArgument(i))) }
+  final int getNumberOfTypeArguments() { result = count(int i | exists(this.getTypeArgument(i))) }
 }
 
 /**