Address PR review comments

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

@@ -21,12 +21,10 @@ namespace Semmle.Extraction.CSharp.Entities
 
         public override void Populate(TextWriter trapFile)
         {
-            var unmanagedCallingConventionTypes = symbol.Signature.UnmanagedCallingConventionTypes.Select(nt => Create(Context, nt)).ToArray();
-
             trapFile.function_pointer_calling_conventions(this, (int)symbol.Signature.CallingConvention);
-            for (var i = 0; i < unmanagedCallingConventionTypes.Length; i++)
+            foreach (var (conv, i) in symbol.Signature.UnmanagedCallingConventionTypes.Select((nt, i) => (Create(Context, nt), i)))
             {
-                trapFile.has_unmanaged_calling_conventions(this, i, unmanagedCallingConventionTypes[i].TypeRef);
+                trapFile.has_unmanaged_calling_conventions(this, i, conv.TypeRef);
             }
 
             PopulateType(trapFile);

csharp/extractor/Semmle.Extraction.CSharp/SymbolExtensions.cs

@@ -278,49 +278,7 @@ namespace Semmle.Extraction.CSharp
 
         private static void BuildFunctionPointerTypeId(this IFunctionPointerTypeSymbol funptr, Context cx, TextWriter trapFile, ISymbol symbolBeingDefined)
         {
-            trapFile.Write("delegate* ");
-            trapFile.Write(funptr.Signature.CallingConvention.ToString().ToLowerInvariant());
-            if (funptr.Signature.UnmanagedCallingConventionTypes.Any())
-            {
-                trapFile.Write('[');
-                trapFile.BuildList(",", funptr.Signature.UnmanagedCallingConventionTypes,
-                    (ta, tb0) => ta.BuildOrWriteId(cx, tb0, symbolBeingDefined)
-                    );
-                trapFile.Write("]");
-            }
-
-            trapFile.Write('<');
-            trapFile.BuildList(",", funptr.Signature.Parameters,
-                (p, trap) =>
-                {
-                    p.Type.BuildOrWriteId(cx, trap, symbolBeingDefined);
-                    switch (p.RefKind)
-                    {
-                        case RefKind.Out:
-                            trap.Write(" out");
-                            break;
-                        case RefKind.In:
-                            trap.Write(" in");
-                            break;
-                        case RefKind.Ref:
-                            trap.Write(" ref");
-                            break;
-                    }
-                });
-
-            if (funptr.Signature.Parameters.Any())
-            {
-                trapFile.Write(",");
-            }
-
-            funptr.Signature.ReturnType.BuildOrWriteId(cx, trapFile, symbolBeingDefined);
-
-            if (funptr.Signature.ReturnsByRef)
-                trapFile.Write(" ref");
-            if (funptr.Signature.ReturnsByRefReadonly)
-                trapFile.Write(" readonly ref");
-
-            trapFile.Write('>');
+            BuildFunctionPointerSignature(funptr, trapFile, (s, tw) => s.BuildOrWriteId(cx, tw, symbolBeingDefined));
         }
 
         private static void BuildNamedTypeId(this INamedTypeSymbol named, Context cx, TextWriter trapFile, ISymbol symbolBeingDefined, bool addBaseClass, bool constructUnderlyingTupleType)
@@ -468,7 +426,8 @@ namespace Semmle.Extraction.CSharp
             }
         }
 
-        private static void BuildFunctionPointerTypeDisplayName(this IFunctionPointerTypeSymbol funptr, Context cx, TextWriter trapFile)
+        public static void BuildFunctionPointerSignature(IFunctionPointerTypeSymbol funptr, TextWriter trapFile,
+            Action<ITypeSymbol, TextWriter> buildNested)
         {
             trapFile.Write("delegate* ");
             trapFile.Write(funptr.Signature.CallingConvention.ToString().ToLowerInvariant());
@@ -476,10 +435,7 @@ namespace Semmle.Extraction.CSharp
             if (funptr.Signature.UnmanagedCallingConventionTypes.Any())
             {
                 trapFile.Write('[');
-                trapFile.BuildList(
-                    ",",
-                    funptr.Signature.UnmanagedCallingConventionTypes,
-                    (t, tb0) => t.BuildDisplayName(cx, tb0));
+                trapFile.BuildList(",", funptr.Signature.UnmanagedCallingConventionTypes, buildNested);
                 trapFile.Write("]");
             }
 
@@ -487,7 +443,7 @@ namespace Semmle.Extraction.CSharp
             trapFile.BuildList(",", funptr.Signature.Parameters,
                 (p, trap) =>
                 {
-                    p.Type.BuildDisplayName(cx, trapFile);
+                    buildNested(p.Type, trap);
                     switch (p.RefKind)
                     {
                         case RefKind.Out:
@@ -507,16 +463,21 @@ namespace Semmle.Extraction.CSharp
                 trapFile.Write(",");
             }
 
-            funptr.Signature.ReturnType.BuildDisplayName(cx, trapFile);
+            buildNested(funptr.Signature.ReturnType, trapFile);
 
             if (funptr.Signature.ReturnsByRef)
                 trapFile.Write(" ref");
             if (funptr.Signature.ReturnsByRefReadonly)
-                trapFile.Write(" readonly ref");
+                trapFile.Write(" ref readonly");
 
             trapFile.Write('>');
         }
 
+        private static void BuildFunctionPointerTypeDisplayName(this IFunctionPointerTypeSymbol funptr, Context cx, TextWriter trapFile)
+        {
+            BuildFunctionPointerSignature(funptr, trapFile, (s, tw) => s.BuildDisplayName(cx, tw));
+        }
+
         private static void BuildNamedTypeDisplayName(this INamedTypeSymbol namedType, Context cx, TextWriter trapFile, bool constructUnderlyingTupleType)
         {
             if (!constructUnderlyingTupleType && namedType.IsTupleType)

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

@@ -797,6 +797,63 @@ class DelegateType extends RefType, Parameterizable, @delegate_type {
   override string getAPrimaryQlClass() { result = "DelegateType" }
 }
 
+private newtype TCallingConvention =
+  MkCallingConvention(int i) { function_pointer_calling_conventions(_, i) }
+
+/**
+ * Represents a signature calling convention. Specifies how arguments in a given
+ * signature are passed from the caller to the callee.
+ */
+class CallingConvention extends TCallingConvention {
+  string toString() { result = "CallingConvention" }
+}
+
+/** Managed calling convention with fixed-length argument list. */
+class DefaultCallingConvention extends CallingConvention {
+  DefaultCallingConvention() { this = MkCallingConvention(0) }
+
+  override string toString() { result = "DefaultCallingConvention" }
+}
+
+/** Unmanaged C/C++-style calling convention where the call stack is cleaned by the caller. */
+class CDeclCallingConvention extends CallingConvention {
+  CDeclCallingConvention() { this = MkCallingConvention(1) }
+
+  override string toString() { result = "CDeclCallingConvention" }
+}
+
+/** Unmanaged calling convention where call stack is cleaned up by the callee. */
+class StdCallCallingConvention extends CallingConvention {
+  StdCallCallingConvention() { this = MkCallingConvention(2) }
+
+  override string toString() { result = "StdCallCallingConvention" }
+}
+
+/**
+ * Unmanaged C++-style calling convention for calling instance member functions
+ * with a fixed argument list.
+ */
+class ThisCallCallingConvention extends CallingConvention {
+  ThisCallCallingConvention() { this = MkCallingConvention(3) }
+
+  override string toString() { result = "ThisCallCallingConvention" }
+}
+
+/** Unmanaged calling convention where arguments are passed in registers when possible. */
+class FastCallCallingConvention extends CallingConvention {
+  FastCallCallingConvention() { this = MkCallingConvention(4) }
+
+  override string toString() { result = "FastCallCallingConvention" }
+}
+
+/** Managed calling convention for passing extra arguments. */
+class VarArgsCallingConvention extends CallingConvention {
+  VarArgsCallingConvention() { this = MkCallingConvention(5) }
+
+  override string toString() { result = "VarArgsCallingConvention" }
+}
+
+// Add sub classes
 /**
  * A function pointer type, for example
  *
@@ -809,7 +866,11 @@ class FunctionPointerType extends Type, Parameterizable, @function_pointer_type
   Type getReturnType() { function_pointer_return_type(this, getTypeRef(result)) }
 
   /** Gets the calling convention. */
-  int getCallingConvention() { function_pointer_calling_conventions(this, result) }
+  CallingConvention getCallingConvention() {
+    exists(int i |
+      function_pointer_calling_conventions(this, i) and result = MkCallingConvention(i)
+    )
+  }
 
   /** Gets the unmanaged calling convention at index `i`. */
   Type getUnmanagedCallingConvention(int i) {

csharp/ql/test/library-tests/csharp9/FunctionPointer.cs

@@ -13,7 +13,7 @@ public class FnPointer
             return 0;
         }
 
-        static void M1(delegate*<ref int, out object?, int> f)
+        static void M1(delegate*<ref int, out object?, ref readonly int> f)
         {
             int i = 42;
             int j = f(ref i, out object? o);

csharp/ql/test/library-tests/csharp9/FunctionPointer.expected

@@ -1,13 +1,13 @@
 type
-| file://:0:0:0:0 | delegate* default<A,B> | B | 0 |
-| file://:0:0:0:0 | delegate* default<B,A> | A | 0 |
-| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 in,Int32 ref> | Int32 | 0 |
-| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32> | Int32 | 0 |
-| file://:0:0:0:0 | delegate* default<Int32*,Void*> | Void* | 0 |
-| file://:0:0:0:0 | delegate* default<Int32> | Int32 | 0 |
-| file://:0:0:0:0 | delegate* default<T,Int32> | Int32 | 0 |
-| file://:0:0:0:0 | delegate* default<Void*,Int32*> | Int32* | 0 |
-| file://:0:0:0:0 | delegate* stdcall<Int32 ref,Object out,T,Void> | Void | 2 |
+| file://:0:0:0:0 | delegate* default<A,B> | B | DefaultCallingConvention |
+| file://:0:0:0:0 | delegate* default<B,A> | A | DefaultCallingConvention |
+| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 in,Int32 ref> | Int32 | DefaultCallingConvention |
+| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 ref readonly> | Int32 | DefaultCallingConvention |
+| file://:0:0:0:0 | delegate* default<Int32*,Void*> | Void* | DefaultCallingConvention |
+| file://:0:0:0:0 | delegate* default<Int32> | Int32 | DefaultCallingConvention |
+| file://:0:0:0:0 | delegate* default<T,Int32> | Int32 | DefaultCallingConvention |
+| file://:0:0:0:0 | delegate* default<Void*,Int32*> | Int32* | DefaultCallingConvention |
+| file://:0:0:0:0 | delegate* stdcall<Int32 ref,Object out,T,Void> | Void | StdCallCallingConvention |
 unmanagedCallingConvention
 parameter
 | file://:0:0:0:0 | delegate* default<A,B> | 0 | file://:0:0:0:0 |  | A |
@@ -15,8 +15,8 @@ parameter
 | file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 in,Int32 ref> | 0 | file://:0:0:0:0 |  | Int32 |
 | file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 in,Int32 ref> | 1 | file://:0:0:0:0 | `1 | Object |
 | file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 in,Int32 ref> | 2 | file://:0:0:0:0 | `2 | Int32 |
-| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32> | 0 | file://:0:0:0:0 |  | Int32 |
-| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32> | 1 | file://:0:0:0:0 | `1 | Object |
+| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 ref readonly> | 0 | file://:0:0:0:0 |  | Int32 |
+| file://:0:0:0:0 | delegate* default<Int32 ref,Object out,Int32 ref readonly> | 1 | file://:0:0:0:0 | `1 | Object |
 | file://:0:0:0:0 | delegate* default<Int32*,Void*> | 0 | file://:0:0:0:0 |  | Int32* |
 | file://:0:0:0:0 | delegate* default<T,Int32> | 0 | file://:0:0:0:0 |  | T |
 | file://:0:0:0:0 | delegate* default<Void*,Int32*> | 0 | file://:0:0:0:0 |  | Void* |

csharp/ql/test/library-tests/csharp9/FunctionPointer.ql

@@ -1,8 +1,8 @@
 import csharp
 
-query predicate type(FunctionPointerType fpt, string returnType, int callingConvention) {
+query predicate type(FunctionPointerType fpt, string returnType, string callingConvention) {
   fpt.getReturnType().toString() = returnType and
-  fpt.getCallingConvention() = callingConvention
+  fpt.getCallingConvention().toString() = callingConvention
 }
 
 query predicate unmanagedCallingConvention(FunctionPointerType fpt, int i, string callingConvention) {