update externs from closure-compiler

CPP: Add query for CWE-401 memory leak on unsuccessful call to realloc function

CONTRIBUTING.md

@@ -38,6 +38,8 @@ If you have an idea for a query that you would like to share with other CodeQL u
 
     - The queries and libraries must be autoformatted, for example using the "Format Document" command in [CodeQL for Visual Studio Code](https://help.semmle.com/codeql/codeql-for-vscode/procedures/about-codeql-for-vscode.html).
 
+    If you prefer, you can use this [pre-commit hook](misc/scripts/pre-commit) that automatically checks whether your files are correctly formatted. See the [pre-commit hook installation guide](docs/install-pre-commit-hook.md) for instructions on how to install the hook.
+
 4. **Compilation**
 
     - Compilation of the query and any associated libraries and tests must be resilient to future development of the [supported](docs/supported-queries.md) libraries. This means that the functionality cannot use internal libraries, cannot depend on the output of `getAQlClass`, and cannot make use of regexp matching on `toString`.

cpp/ql/src/Architecture/General Class-Level Information/ClassHierarchies.ql

@@ -4,9 +4,6 @@
  * @kind graph
  * @id cpp/architecture/class-hierarchies
  * @graph.layout organic
- * @workingset jhotdraw
- * @result succeed 48
- * @result_ondemand succeed 48
  * @tags maintainability
  */
 

cpp/ql/src/Architecture/General Class-Level Information/InheritanceDepthDistribution.ql

@@ -4,9 +4,6 @@
  * @kind chart
  * @id cpp/architecture/inheritance-depth-distribution
  * @chart.type line
- * @workingset jhotdraw
- * @result succeed 48
- * @result_ondemand succeed 48
  * @tags maintainability
  */
 

cpp/ql/src/Architecture/General Namespace-Level Information/GlobalNamespaceClasses.ql

@@ -1,7 +1,8 @@
 /**
  * @name Global namespace classes
  * @description Finds classes that belong to no namespace.
- * @kind table
+ * @kind problem
+ * @problem.severity recommendation
  * @id cpp/architecture/global-namespace-classes
  * @tags maintainability
  *       modularity

cpp/ql/src/Architecture/Refactoring Opportunities/ClassesWithManyDependencies.ql

@@ -4,9 +4,6 @@
  * @kind problem
  * @id cpp/architecture/classes-with-many-dependencies
  * @problem.severity recommendation
- * @workingset jhotdraw
- * @result succeed 20
- * @result_ondemand succeed 20
  * @tags maintainability
  *       statistical
  *       non-attributable

cpp/ql/src/Security/CWE/CWE-079/CgiXss.ql

@@ -29,6 +29,8 @@ class QueryString extends EnvironmentRead {
 }
 
 class Configuration extends TaintTrackingConfiguration {
+  override predicate isSource(Expr source) { source instanceof QueryString }
+
   override predicate isSink(Element tainted) {
     exists(PrintStdoutCall call | call.getAnArgument() = tainted)
   }

cpp/ql/src/Security/CWE/CWE-313/CleartextSqliteDatabase.ql

@@ -34,6 +34,10 @@ predicate sqlite_encryption_used() {
 }
 
 class Configuration extends TaintTrackingConfiguration {
+  override predicate isSource(Expr source) {
+    super.isSource(source) and source instanceof SensitiveExpr
+  }
+
   override predicate isSink(Element taintedArg) {
     exists(SqliteFunctionCall sqliteCall |
       taintedArg = sqliteCall.getASource() and

cpp/ql/src/experimental/Security/CWE/CWE-190/AllocMultiplicationOverflow.cpp

@@ -0,0 +1,19 @@
+
+image::image(int width, int height)
+{
+	int x, y;
+
+	// allocate width * height pixels
+	pixels = new uint32_t[width * height];
+
+	// fill width * height pixels
+	for (y = 0; y < height; y++)
+	{
+		for (x = 0; x < width; x++)
+		{
+			pixels[(y * width) + height] = 0;
+		}
+	}
+
+	// ...
+}

cpp/ql/src/experimental/Security/CWE/CWE-190/AllocMultiplicationOverflow.qhelp

@@ -0,0 +1,25 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>The result of a multiplication is used in the size of an allocation. If the multiplication can be made to overflow, a much smaller amount of memory may be allocated than the rest of the code expects. This may lead to overflowing writes when the buffer is accessed later.</p>
+</overview>
+
+<recommendation>
+<p>To fix this issue, ensure that the arithmetic used in the size of an allocation cannot overflow before memory is allocated.</p>
+</recommendation>
+
+<example>
+<p>In the following example, an array of size <code>width * height</code> is allocated and stored as <code>pixels</code>. If <code>width</code> and <code>height</code> are set such that the multiplication overflows and wraps to a small value (say, 4) then the initialization code that follows the allocation will write beyond the end of the array.</p>
+<sample src="AllocMultiplicationOverflow.cpp"/>
+</example>
+
+<references>
+<li>
+  Cplusplus.com: <a href="http://www.cplusplus.com/articles/DE18T05o/">Integer overflow</a>.
+</li>
+</references>
+
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-190/AllocMultiplicationOverflow.ql

@@ -0,0 +1,40 @@
+/**
+ * @name Multiplication result may overflow and be used in allocation
+ * @description Using a multiplication result that may overflow in the size of an allocation may lead to buffer overflows when the allocated memory is used.
+ * @kind path-problem
+ * @problem.severity warning
+ * @precision low
+ * @tags security
+ *       correctness
+ *       external/cwe/cwe-190
+ *       external/cwe/cwe-128
+ * @id cpp/multiplication-overflow-in-alloc
+ */
+
+import cpp
+import semmle.code.cpp.models.interfaces.Allocation
+import semmle.code.cpp.dataflow.DataFlow
+import DataFlow::PathGraph
+
+class MultToAllocConfig extends DataFlow::Configuration {
+  MultToAllocConfig() { this = "MultToAllocConfig" }
+
+  override predicate isSource(DataFlow::Node node) {
+    // a multiplication of two non-constant expressions
+    exists(MulExpr me |
+      me = node.asExpr() and
+      forall(Expr e | e = me.getAnOperand() | not exists(e.getValue()))
+    )
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    // something that affects an allocation size
+    node.asExpr() = any(AllocationExpr ae).getSizeExpr().getAChild*()
+  }
+}
+
+from MultToAllocConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
+select sink, source, sink,
+  "Potentially overflowing value from $@ is used in the size of this allocation.", source,
+  "multiplication"

cpp/ql/src/experimental/Security/CWE/CWE-401/MemoryLeakOnFailedCallToRealloc.c

@@ -0,0 +1,20 @@
+// BAD: on unsuccessful call to realloc, we will lose a pointer to a valid memory block
+if (currentSize < newSize)
+{
+	buffer = (unsigned char *)realloc(buffer, newSize);
+}
+
+
+
+// GOOD: this way we will exclude possible memory leak 
+unsigned char * tmp;
+if (currentSize < newSize)
+{
+	tmp = (unsigned char *)realloc(buffer, newSize);
+}
+if (tmp == NULL)
+{
+	free(buffer);
+} 
+else
+	buffer = tmp;

cpp/ql/src/experimental/Security/CWE/CWE-401/MemoryLeakOnFailedCallToRealloc.qhelp

@@ -0,0 +1,38 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Memory leak on failed call to realloc. 
+The expression <code>mem = realloc (mem, size)</code> is potentially dangerous, if the call fails, we will lose the pointer to the memory block.  
+An unsuccessful call is possible not only when trying to allocate a large amount of memory, but also when the process memory is strongly segmented.</p>
+
+<p>False positives include code in which immediately after calling the realloc function, the pointer is manipulated without first checking for validity. 
+In this case, an exception will occur in the program and it will terminate. 
+But from the point of view of safe coding, these places require the attention of developers.
+At this stage, false positives are also possible in situations where the exception handling is quite complicated and occurs outside the base block in which memory is redistributed.</p>
+
+</overview>
+<recommendation>
+
+<p>We recommend storing the result in a temporary variable and eliminating memory leak.</p>
+
+</recommendation>
+<example>
+<p>The following example demonstrates an erroneous and corrected use of the <code>realloc</code> function.</p>
+<sample src="MemoryLeakOnFailedCallToRealloc.c" />
+
+</example>
+<references>
+
+<li>
+  CERT C++ Coding Standard:
+<a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM51-CPP.+Properly+deallocate+dynamically+allocated+resources">MEM51-CPP. Properly deallocate dynamically allocated resources</a>.
+</li>
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/WIN30-C.+Properly+pair+allocation+and+deallocation+functions">WIN30-C. Properly pair allocation and deallocation functions</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-401/MemoryLeakOnFailedCallToRealloc.ql

@@ -0,0 +1,69 @@
+/**
+ * @name Memory leak on failed call to realloc
+ * @description The expression mem = realloc (mem, size) is potentially dangerous, if the call fails, we will lose the pointer to the memory block.
+ *              We recommend storing the result in a temporary variable and eliminating memory leak.
+ * @kind problem
+ * @id cpp/memory-leak-on-failed-call-to-realloc
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       security
+ *       external/cwe/cwe-401
+ */
+
+import cpp
+
+/**
+ * A call to `realloc` of the form `v = realloc(v, size)`, for some variable `v`.
+ */
+class ReallocCallLeak extends FunctionCall {
+  Variable v;
+
+  ReallocCallLeak() {
+    exists(AssignExpr ex, VariableAccess va1, VariableAccess va2 |
+      this.getTarget().hasName("realloc") and
+      this = ex.getRValue() and
+      va1 = ex.getLValue() and
+      va2 = this.getArgument(0) and
+      va1 = v.getAnAccess() and
+      va2 = v.getAnAccess()
+    )
+  }
+
+  predicate isExistsIfWithExitCall() {
+    exists(IfStmt ifc |
+      this.getArgument(0) = v.getAnAccess() and
+      ifc.getCondition().getAChild*() = v.getAnAccess() and
+      ifc.getEnclosingFunction() = this.getEnclosingFunction() and
+      ifc.getLocation().getStartLine() >= this.getArgument(0).getLocation().getStartLine() and
+      exists(FunctionCall fc |
+        fc.getTarget().hasName("exit") and
+        fc.getEnclosingFunction() = this.getEnclosingFunction() and
+        (ifc.getThen().getAChild*() = fc or ifc.getElse().getAChild*() = fc)
+      )
+      or
+      exists(FunctionCall fc, FunctionCall ftmp1, FunctionCall ftmp2 |
+        ftmp1.getTarget().hasName("exit") and
+        ftmp2.(ControlFlowNode).getASuccessor*() = ftmp1 and
+        fc = ftmp2.getEnclosingFunction().getACallToThisFunction() and
+        fc.getEnclosingFunction() = this.getEnclosingFunction() and
+        (ifc.getThen().getAChild*() = fc or ifc.getElse().getAChild*() = fc)
+      )
+    )
+  }
+
+  predicate isExistsAssertWithArgumentCall() {
+    exists(FunctionCall fc |
+      fc.getTarget().hasName("__assert_fail") and
+      this.getEnclosingFunction() = fc.getEnclosingFunction() and
+      fc.getLocation().getStartLine() > this.getArgument(0).getLocation().getEndLine() and
+      fc.getArgument(0).toString().matches("%" + this.getArgument(0).toString() + "%")
+    )
+  }
+}
+
+from ReallocCallLeak rcl
+where
+  not rcl.isExistsIfWithExitCall() and
+  not rcl.isExistsAssertWithArgumentCall()
+select rcl, "possible loss of original pointer on unsuccessful call realloc"

cpp/ql/src/semmle/code/cpp/Function.qll

@@ -391,20 +391,30 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
   /** Holds if this function has a `noexcept` exception specification. */
   predicate isNoExcept() { getADeclarationEntry().isNoExcept() }
 
-  /** Gets a function that overloads this one. */
+  /**
+   * Gets a function that overloads this one.
+   *
+   * Note: if _overrides_ are wanted rather than _overloads_ then
+   * `MemberFunction::getAnOverridingFunction` should be used instead.
+   */
   Function getAnOverload() {
-    result.getName() = getName() and
-    result.getNamespace() = getNamespace() and
-    result != this and
-    // If this function is declared in a class, only consider other
-    // functions from the same class. Conversely, if this function is not
-    // declared in a class, only consider other functions not declared in a
-    // class.
     (
-      if exists(getDeclaringType())
-      then result.getDeclaringType() = getDeclaringType()
-      else not exists(result.getDeclaringType())
+      // If this function is declared in a class, only consider other
+      // functions from the same class.
+      exists(string name, Class declaringType |
+        candGetAnOverloadMember(name, declaringType, this) and
+        candGetAnOverloadMember(name, declaringType, result)
+      )
+      or
+      // Conversely, if this function is not
+      // declared in a class, only consider other functions not declared in a
+      // class.
+      exists(string name, Namespace namespace |
+        candGetAnOverloadNonMember(name, namespace, this) and
+        candGetAnOverloadNonMember(name, namespace, result)
+      )
     ) and
+    result != this and
     // Instantiations and specializations don't participate in overload
     // resolution.
     not (
@@ -462,6 +472,19 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
   override AccessHolder getEnclosingAccessHolder() { result = this.getDeclaringType() }
 }
 
+pragma[noinline]
+private predicate candGetAnOverloadMember(string name, Class declaringType, Function f) {
+  f.getName() = name and
+  f.getDeclaringType() = declaringType
+}
+
+pragma[noinline]
+private predicate candGetAnOverloadNonMember(string name, Namespace namespace, Function f) {
+  f.getName() = name and
+  f.getNamespace() = namespace and
+  not exists(f.getDeclaringType())
+}
+
 /**
  * A particular declaration or definition of a C/C++ function. For example the
  * declaration and definition of `MyFunction` in the following code are each a

cpp/ql/src/semmle/code/cpp/commons/Printf.qll

@@ -900,6 +900,7 @@ class FormatLiteral extends Literal {
    */
   int getNumArgNeeded(int n) {
     exists(this.getConvSpecOffset(n)) and
+    exists(this.getConversionChar(n)) and
     result = count(int mode | hasFormatArgumentIndexFor(n, mode))
   }
 

cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

@@ -46,21 +46,23 @@ predicate predictableOnlyFlow(string name) {
 
 private DataFlow::Node getNodeForSource(Expr source) {
   isUserInput(source, _) and
-  (
-    result = DataFlow::exprNode(source)
-    or
-    // Some of the sources in `isUserInput` are intended to match the value of
-    // an expression, while others (those modeled below) are intended to match
-    // the taint that propagates out of an argument, like the `char *` argument
-    // to `gets`. It's impossible here to tell which is which, but the "access
-    // to argv" source is definitely not intended to match an output argument,
-    // and it causes false positives if we let it.
-    //
-    // This case goes together with the similar (but not identical) rule in
-    // `nodeIsBarrierIn`.
-    result = DataFlow::definitionByReferenceNodeFromArgument(source) and
-    not argv(source.(VariableAccess).getTarget())
-  )
+  result = getNodeForExpr(source)
+}
+
+private DataFlow::Node getNodeForExpr(Expr node) {
+  result = DataFlow::exprNode(node)
+  or
+  // Some of the sources in `isUserInput` are intended to match the value of
+  // an expression, while others (those modeled below) are intended to match
+  // the taint that propagates out of an argument, like the `char *` argument
+  // to `gets`. It's impossible here to tell which is which, but the "access
+  // to argv" source is definitely not intended to match an output argument,
+  // and it causes false positives if we let it.
+  //
+  // This case goes together with the similar (but not identical) rule in
+  // `nodeIsBarrierIn`.
+  result = DataFlow::definitionByReferenceNodeFromArgument(node) and
+  not argv(node.(VariableAccess).getTarget())
 }
 
 private class DefaultTaintTrackingCfg extends DataFlow::Configuration {
@@ -204,16 +206,27 @@ private predicate nodeIsBarrierIn(DataFlow::Node node) {
 
 cached
 private predicate commonTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
-  instructionToInstructionTaintStep(fromNode.asInstruction(), toNode.asInstruction())
-  or
   operandToInstructionTaintStep(fromNode.asOperand(), toNode.asInstruction())
   or
-  operandToOperandTaintStep(fromNode.asOperand(), toNode.asOperand())
+  instructionToOperandTaintStep(fromNode.asInstruction(), toNode.asOperand())
 }
 
-private predicate operandToOperandTaintStep(Operand fromOperand, Operand toOperand) {
+private predicate instructionToOperandTaintStep(Instruction fromInstr, Operand toOperand) {
+  // Propagate flow from the definition of an operand to the operand, even when the overlap is inexact.
+  // We only do this in certain cases:
+  // 1. The instruction's result must not be conflated, and
+  // 2. The instruction's result type is one the types where we expect element-to-object flow. Currently
+  // this is array types and union types. This matches the other two cases of element-to-object flow in
+  // `DefaultTaintTracking`.
+  toOperand.getAnyDef() = fromInstr and
+  not fromInstr.isResultConflated() and
+  (
+    fromInstr.getResultType() instanceof ArrayType or
+    fromInstr.getResultType() instanceof Union
+  )
+  or
   exists(ReadSideEffectInstruction readInstr |
-    fromOperand = readInstr.getArgumentOperand() and
+    fromInstr = readInstr.getArgumentDef() and
     toOperand = readInstr.getSideEffectOperand()
   )
 }
@@ -256,18 +269,18 @@ private predicate operandToInstructionTaintStep(Operand fromOperand, Instruction
         outInstr.getPrimaryInstruction() = call
       )
     )
-}
-
-private predicate instructionToInstructionTaintStep(Instruction i1, Instruction i2) {
+  or
   // Flow through pointer dereference
-  i2.(LoadInstruction).getSourceAddress() = i1
+  toInstr.(LoadInstruction).getSourceAddressOperand() = fromOperand
   or
   // Flow through partial reads of arrays and unions
-  i2.(LoadInstruction).getSourceValueOperand().getAnyDef() = i1 and
-  not i1.isResultConflated() and
-  (
-    i1.getResultType() instanceof ArrayType or
-    i1.getResultType() instanceof Union
+  toInstr.(LoadInstruction).getSourceValueOperand() = fromOperand and
+  exists(Instruction fromInstr | fromInstr = fromOperand.getAnyDef() |
+    not fromInstr.isResultConflated() and
+    (
+      fromInstr.getResultType() instanceof ArrayType or
+      fromInstr.getResultType() instanceof Union
+    )
   )
   or
   // Unary instructions tend to preserve enough information in practice that we
@@ -277,63 +290,54 @@ private predicate instructionToInstructionTaintStep(Instruction i1, Instruction
   // `FieldAddressInstruction` could cause flow into one field to come out an
   // unrelated field. This would happen across function boundaries, where the IR
   // would not be able to match loads to stores.
-  i2.(UnaryInstruction).getUnary() = i1 and
+  toInstr.(UnaryInstruction).getUnaryOperand() = fromOperand and
   (
-    not i2 instanceof FieldAddressInstruction
+    not toInstr instanceof FieldAddressInstruction
     or
-    i2.(FieldAddressInstruction).getField().getDeclaringType() instanceof Union
+    toInstr.(FieldAddressInstruction).getField().getDeclaringType() instanceof Union
   )
   or
-  // Flow out of definition-by-reference
-  i2.(ChiInstruction).getPartial() = i1.(WriteSideEffectInstruction) and
-  not i2.isResultConflated()
-  or
   // Flow from an element to an array or union that contains it.
-  i2.(ChiInstruction).getPartial() = i1 and
-  not i2.isResultConflated() and
-  exists(Type t | i2.getResultLanguageType().hasType(t, false) |
+  toInstr.(ChiInstruction).getPartialOperand() = fromOperand and
+  not toInstr.isResultConflated() and
+  exists(Type t | toInstr.getResultLanguageType().hasType(t, false) |
     t instanceof Union
     or
     t instanceof ArrayType
   )
   or
   exists(BinaryInstruction bin |
-    bin = i2 and
-    predictableInstruction(i2.getAnOperand().getDef()) and
-    i1 = i2.getAnOperand().getDef()
+    bin = toInstr and
+    predictableInstruction(toInstr.getAnOperand().getDef()) and
+    fromOperand = toInstr.getAnOperand()
   )
   or
   // This is part of the translation of `a[i]`, where we want taint to flow
   // from `a`.
-  i2.(PointerAddInstruction).getLeft() = i1
-  or
-  // Until we have from through indirections across calls, we'll take flow out
-  // of the parameter and into its indirection.
-  exists(IRFunction f, Parameter parameter |
-    i1 = getInitializeParameter(f, parameter) and
-    i2 = getInitializeIndirection(f, parameter)
-  )
+  toInstr.(PointerAddInstruction).getLeftOperand() = fromOperand
   or
   // Until we have flow through indirections across calls, we'll take flow out
   // of the indirection and into the argument.
   // When we get proper flow through indirections across calls, this code can be
   // moved to `adjusedSink` or possibly into the `DataFlow::ExprNode` class.
   exists(ReadSideEffectInstruction read |
-    read.getAnOperand().(SideEffectOperand).getAnyDef() = i1 and
-    read.getArgumentDef() = i2
+    read.getSideEffectOperand() = fromOperand and
+    read.getArgumentDef() = toInstr
   )
-}
-
-pragma[noinline]
-private InitializeIndirectionInstruction getInitializeIndirection(IRFunction f, Parameter p) {
-  result.getParameter() = p and
-  result.getEnclosingIRFunction() = f
-}
-
-pragma[noinline]
-private InitializeParameterInstruction getInitializeParameter(IRFunction f, Parameter p) {
-  result.getParameter() = p and
-  result.getEnclosingIRFunction() = f
+  or
+  // Until we have from through indirections across calls, we'll take flow out
+  // of the parameter and into its indirection.
+  // `InitializeIndirectionInstruction` only has a single operand: the address of the
+  // value whose indirection we are initializing. When initializing an indirection of a parameter `p`,
+  // the IR looks like this:
+  // ```
+  // m1 = InitializeParameter[p] : &r1
+  // r2 = Load[p] : r2, m1
+  // m3 = InitializeIndirection[p] : &r2
+  // ```
+  // So by having flow from `r2` to `m3` we're enabling flow from `m1` to `m3`. This relies on the
+  // `LoadOperand`'s overlap being exact.
+  toInstr.(InitializeIndirectionInstruction).getAnOperand() = fromOperand
 }
 
 /**
@@ -535,6 +539,9 @@ module TaintedWithPath {
    * a characteristic predicate.
    */
   class TaintTrackingConfiguration extends TSingleton {
+    /** Override this to specify which elements are sources in this configuration. */
+    predicate isSource(Expr source) { exists(getNodeForSource(source)) }
+
     /** Override this to specify which elements are sinks in this configuration. */
     abstract predicate isSink(Element e);
 
@@ -551,7 +558,11 @@ module TaintedWithPath {
   private class AdjustedConfiguration extends DataFlow3::Configuration {
     AdjustedConfiguration() { this = "AdjustedConfiguration" }
 
-    override predicate isSource(DataFlow::Node source) { source = getNodeForSource(_) }
+    override predicate isSource(DataFlow::Node source) {
+      exists(TaintTrackingConfiguration cfg, Expr e |
+        cfg.isSource(e) and source = getNodeForExpr(e)
+      )
+    }
 
     override predicate isSink(DataFlow::Node sink) {
       exists(TaintTrackingConfiguration cfg | cfg.isSink(adjustedSink(sink)))
@@ -594,7 +605,8 @@ module TaintedWithPath {
       exists(AdjustedConfiguration cfg, DataFlow3::Node sourceNode, DataFlow3::Node sinkNode |
         cfg.hasFlow(sourceNode, sinkNode)
       |
-        sourceNode = getNodeForSource(e)
+        sourceNode = getNodeForExpr(e) and
+        exists(TaintTrackingConfiguration ttCfg | ttCfg.isSource(e))
         or
         e = adjustedSink(sinkNode) and
         exists(TaintTrackingConfiguration ttCfg | ttCfg.isSink(e))
@@ -648,7 +660,7 @@ module TaintedWithPath {
 
   /** A PathNode whose `Element` is a source. It may also be a sink. */
   private class InitialPathNode extends EndpointPathNode {
-    InitialPathNode() { exists(getNodeForSource(this.inner())) }
+    InitialPathNode() { exists(TaintTrackingConfiguration cfg | cfg.isSource(this.inner())) }
   }
 
   /** A PathNode whose `Element` is a sink. It may also be a source. */
@@ -670,14 +682,14 @@ module TaintedWithPath {
     // Same for the first node
     exists(WrapPathNode sourceNode |
       DataFlow3::PathGraph::edges(sourceNode.inner(), b.(WrapPathNode).inner()) and
-      sourceNode.inner().getNode() = getNodeForSource(a.(InitialPathNode).inner())
+      sourceNode.inner().getNode() = getNodeForExpr(a.(InitialPathNode).inner())
     )
     or
     // Finally, handle the case where the path goes directly from a source to a
     // sink, meaning that they both need to be translated.
     exists(WrapPathNode sinkNode, WrapPathNode sourceNode |
       DataFlow3::PathGraph::edges(sourceNode.inner(), sinkNode.inner()) and
-      sourceNode.inner().getNode() = getNodeForSource(a.(InitialPathNode).inner()) and
+      sourceNode.inner().getNode() = getNodeForExpr(a.(InitialPathNode).inner()) and
       b.(FinalPathNode).inner() = adjustedSink(sinkNode.inner().getNode())
     )
   }
@@ -700,7 +712,7 @@ module TaintedWithPath {
   predicate taintedWithPath(Expr source, Element tainted, PathNode sourceNode, PathNode sinkNode) {
     exists(AdjustedConfiguration cfg, DataFlow3::Node flowSource, DataFlow3::Node flowSink |
       source = sourceNode.(InitialPathNode).inner() and
-      flowSource = getNodeForSource(source) and
+      flowSource = getNodeForExpr(source) and
       cfg.hasFlow(flowSource, flowSink) and
       tainted = adjustedSink(flowSink) and
       tainted = sinkNode.(FinalPathNode).inner()
@@ -722,8 +734,8 @@ module TaintedWithPath {
    * through a global variable.
    */
   predicate taintedWithoutGlobals(Element tainted) {
-    exists(PathNode sourceNode, FinalPathNode sinkNode |
-      sourceNode.(WrapPathNode).inner().getNode() = getNodeForSource(_) and
+    exists(AdjustedConfiguration cfg, PathNode sourceNode, FinalPathNode sinkNode |
+      cfg.isSource(sourceNode.(WrapPathNode).inner().getNode()) and
       edgesWithoutGlobals+(sourceNode, sinkNode) and
       tainted = sinkNode.inner()
     )

cpp/ql/src/semmle/code/cpp/models/implementations/Allocation.qll

@@ -82,7 +82,9 @@ private class AllocaAllocationFunction extends AllocationFunction {
     hasGlobalName([
         // --- stack allocation
         "alloca", // // alloca(size)
-        "__builtin_alloca" // __builtin_alloca(size)
+        "__builtin_alloca", // __builtin_alloca(size)
+        "_alloca", // _alloca(size)
+        "_malloca" // _malloca(size)
       ]) and
     sizeArg = 0
   }

cpp/ql/src/semmle/code/cpp/models/implementations/Strdup.qll

@@ -14,6 +14,7 @@ import semmle.code.cpp.models.interfaces.Taint
 private class StrdupFunction extends AllocationFunction, ArrayFunction, DataFlowFunction {
   StrdupFunction() {
     hasGlobalName([
+        // --- C library allocation
         "strdup", // strdup(str)
         "wcsdup", // wcsdup(str)
         "_strdup", // _strdup(str)
@@ -39,8 +40,8 @@ private class StrndupFunction extends AllocationFunction, ArrayFunction, DataFlo
   StrndupFunction() {
     exists(string name |
       hasGlobalName(name) and
-      // strndup(str, maxlen)
-      name = "strndup"
+      // --- C library allocation
+      name = "strndup" // strndup(str, maxlen)
     )
   }
 

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/AllocMultiplicationOverflow/AllocMultiplicationOverflow.expected

@@ -0,0 +1,17 @@
+edges
+| test.cpp:22:17:22:21 | ... * ... | test.cpp:23:33:23:37 | size1 |
+nodes
+| test.cpp:13:33:13:37 | ... * ... | semmle.label | ... * ... |
+| test.cpp:15:31:15:35 | ... * ... | semmle.label | ... * ... |
+| test.cpp:19:34:19:38 | ... * ... | semmle.label | ... * ... |
+| test.cpp:22:17:22:21 | ... * ... | semmle.label | ... * ... |
+| test.cpp:23:33:23:37 | size1 | semmle.label | size1 |
+| test.cpp:30:27:30:31 | ... * ... | semmle.label | ... * ... |
+| test.cpp:31:27:31:31 | ... * ... | semmle.label | ... * ... |
+#select
+| test.cpp:13:33:13:37 | ... * ... | test.cpp:13:33:13:37 | ... * ... | test.cpp:13:33:13:37 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:13:33:13:37 | ... * ... | multiplication |
+| test.cpp:15:31:15:35 | ... * ... | test.cpp:15:31:15:35 | ... * ... | test.cpp:15:31:15:35 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:15:31:15:35 | ... * ... | multiplication |
+| test.cpp:19:34:19:38 | ... * ... | test.cpp:19:34:19:38 | ... * ... | test.cpp:19:34:19:38 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:19:34:19:38 | ... * ... | multiplication |
+| test.cpp:23:33:23:37 | size1 | test.cpp:22:17:22:21 | ... * ... | test.cpp:23:33:23:37 | size1 | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:22:17:22:21 | ... * ... | multiplication |
+| test.cpp:30:27:30:31 | ... * ... | test.cpp:30:27:30:31 | ... * ... | test.cpp:30:27:30:31 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:30:27:30:31 | ... * ... | multiplication |
+| test.cpp:31:27:31:31 | ... * ... | test.cpp:31:27:31:31 | ... * ... | test.cpp:31:27:31:31 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:31:27:31:31 | ... * ... | multiplication |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/AllocMultiplicationOverflow/AllocMultiplicationOverflow.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-190/AllocMultiplicationOverflow.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/AllocMultiplicationOverflow/test.cpp

@@ -0,0 +1,32 @@
+
+typedef unsigned long size_t;
+void *malloc(size_t size);
+
+int getAnInt();
+
+void test()
+{
+	int x = getAnInt();
+	int y = getAnInt();
+
+	char *buffer1 = (char *)malloc(x + y); // GOOD
+	char *buffer2 = (char *)malloc(x * y); // BAD
+	int *buffer3 = (int *)malloc(x * sizeof(int)); // GOOD
+	int *buffer4 = (int *)malloc(x * y * sizeof(int)); // BAD
+
+	if ((x <= 1000) && (y <= 1000))
+	{
+		char *buffer5 = (char *)malloc(x * y); // GOOD [FALSE POSITIVE]
+	}
+
+	size_t size1 = x * y;
+	char *buffer5 = (char *)malloc(size1); // BAD
+
+	size_t size2 = x;
+	size2 *= y;
+	char *buffer6 = (char *)malloc(size2); // BAD [NOT DETECTED]
+
+	char *buffer7 = new char[x * 10]; // GOOD
+	char *buffer8 = new char[x * y]; // BAD
+	char *buffer9 = new char[x * x]; // BAD
+}

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-401/semmle/tests/MemoryLeakOnFailedCallToRealloc.expected

@@ -0,0 +1,4 @@
+| test.c:34:29:34:35 | call to realloc | possible loss of original pointer on unsuccessful call realloc |
+| test.c:63:29:63:35 | call to realloc | possible loss of original pointer on unsuccessful call realloc |
+| test.c:139:29:139:35 | call to realloc | possible loss of original pointer on unsuccessful call realloc |
+| test.c:186:29:186:35 | call to realloc | possible loss of original pointer on unsuccessful call realloc |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-401/semmle/tests/MemoryLeakOnFailedCallToRealloc.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-401/MemoryLeakOnFailedCallToRealloc.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-401/semmle/tests/test.c

@@ -0,0 +1,274 @@
+#define size_t int
+#define NULL ((void*)0)
+
+#define assert(x) if (!(x)) __assert_fail(#x,__FILE__,__LINE__)
+void __assert_fail(const char *assertion, const char *file, int line) {  }
+
+void aFakeFailed_1(int file, int line)
+{
+}
+void aFailed_1(int file, int line)
+{
+    exit(0);
+}
+void aFailed_2(int file, int line, int ex)
+{
+    if(ex == 1)
+	    exit(0);
+    else
+	    return;
+}
+#define F_NUM 1
+#define myASSERT_1(expr) \
+    if (!(expr)) \
+        aFailed_1(F_NUM, __LINE__)
+#define myASSERT_2(expr) \
+    if (!(expr)) \
+        aFailed_2(F_NUM, __LINE__, 1)
+
+unsigned char * badResize_0(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// BAD: on unsuccessful call to realloc, we will lose a pointer to a valid memory block
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	return buffer;
+}
+
+unsigned char * goodResize_0(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: this way we will exclude possible memory leak 
+	unsigned char * tmp;
+	if (currentSize < newSize)
+	{
+		tmp = (unsigned char *)realloc(buffer, newSize);
+	}
+	if (tmp == NULL)
+	{
+		free(buffer);
+		return NULL;
+	} 
+	else
+		buffer = tmp;
+	return buffer;
+}
+unsigned char * badResize_1_0(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	if(!buffer)
+		exit(0);
+	// BAD: on unsuccessful call to realloc, we will lose a pointer to a valid memory block
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	return buffer;
+}
+
+unsigned char * noBadResize_1_0(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	if(!buffer)
+		exit(0);
+	return buffer;
+}
+unsigned char * noBadResize_1_1(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	if(buffer)
+		return buffer;
+	else
+		exit(0);
+}
+unsigned char * noBadResize_1_2(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		if(buffer = (unsigned char *)realloc(buffer, newSize))
+		exit(0);
+	}
+	return buffer;
+}
+unsigned char * noBadResize_1_3(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	if(!buffer)
+		aFailed_1(1, 1);
+	return buffer;
+}
+unsigned char * noBadResize_1_4(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	if(buffer)
+		return buffer;
+	else
+		aFailed_1(1, 1);
+}
+unsigned char * noBadResize_1_5(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		if(buffer = (unsigned char *)realloc(buffer, newSize))
+		aFailed_1(1, 1);
+	}
+	return buffer;
+}
+unsigned char * badResize_1_1(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// BAD: on unsuccessful call to realloc, we will lose a pointer to a valid memory block
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	if(!buffer)
+		aFakeFailed_1(1, 1);
+	return buffer;
+}
+
+unsigned char * noBadResize_1_6(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	if(!buffer)
+		aFailed_2(1, 1, 1);
+	return buffer;
+}
+unsigned char * noBadResize_1_7(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	if(buffer)
+		return buffer;
+	else
+		aFailed_2(1, 1, 1);
+}
+unsigned char * noBadResize_1_8(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		if(buffer = (unsigned char *)realloc(buffer, newSize))
+		aFailed_2(1, 1, 1);
+	}
+	return buffer;
+}
+
+unsigned char * badResize_2_0(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// BAD: on unsuccessful call to realloc, we will lose a pointer to a valid memory block
+	assert(buffer!=0);
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	return buffer;
+}
+
+unsigned char * noBadResize_2_0(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+		assert(buffer!=0);
+	}
+	return buffer;
+}
+
+unsigned char * noBadResize2e_2_1(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	assert(buffer!=0);
+	return buffer;
+}
+unsigned char * noBadResize_2_2(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+		assert(buffer);
+	}
+	return buffer;
+}
+
+unsigned char * noBadResize_2_3(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	assert(buffer);
+	return buffer;
+}
+unsigned char * noBadResize_2_4(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+		myASSERT_1(buffer);
+	}
+	return buffer;
+}
+
+unsigned char * noBadResize_2_5(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	myASSERT_1(buffer);
+	return buffer;
+}
+unsigned char * noBadResize_2_6(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+		myASSERT_2(buffer);
+	}
+	return buffer;
+}
+
+unsigned char * noBadResize_2_7(unsigned char * buffer,size_t currentSize,size_t newSize)
+{
+	// GOOD: program to end
+	if (currentSize < newSize)
+	{
+		buffer = (unsigned char *)realloc(buffer, newSize);
+	}
+	myASSERT_2(buffer);
+	return buffer;
+}

cpp/ql/test/library-tests/dataflow/fields/conflated.cpp

@@ -0,0 +1,62 @@
+int user_input();
+void sink(int);
+
+struct A {
+  int* p;
+  int x;
+};
+
+void pointer_without_allocation(const A& ra) {
+  *ra.p = user_input();
+  sink(*ra.p); // $ MISSING: ast,ir
+}
+
+void argument_source(void*);
+void sink(void*);
+
+void pointer_without_allocation_2() {
+  char *raw;
+  argument_source(raw);
+  sink(raw); // $ ast MISSING: ir
+}
+
+A* makeA() {
+  return new A;
+}
+
+void no_InitializeDynamicAllocation_instruction() {
+  A* pa = makeA();
+  pa->x = user_input();
+  sink(pa->x); // $ ast MISSING: ir
+}
+
+void fresh_or_arg(A* arg, bool unknown) {
+  A* pa;
+  pa = unknown ? arg : new A;
+  pa->x = user_input();
+  sink(pa->x); // $ ast MISSING: ir
+}
+
+struct LinkedList {
+  LinkedList* next;
+  int y;
+
+  LinkedList() = default;
+  LinkedList(LinkedList* next) : next(next) {}
+};
+
+// Note: This example also suffers from #113: there is no ChiInstruction that merges the result of the
+// InitializeDynamicAllocation instruction into {AllAliasedMemory}. But even when that's fixed there's
+// still no dataflow because `ll->next->y = user_input()` writes to {AllAliasedMemory}. 
+void too_many_indirections() {
+  LinkedList* ll = new LinkedList;
+  ll->next = new LinkedList;
+  ll->next->y = user_input();
+  sink(ll->next->y); // $ ast MISSING: ir
+}
+
+void too_many_indirections_2(LinkedList* next) {
+  LinkedList* ll = new LinkedList(next);
+  ll->next->y = user_input();
+  sink(ll->next->y); // $ ast MISSING: ir
+}

cpp/ql/test/library-tests/dataflow/fields/dataflow-consistency.expected

@@ -121,6 +121,13 @@ postWithInFlow
 | by_reference.cpp:127:30:127:38 | inner_ptr [inner post update] | PostUpdateNode should not be the target of local flow. |
 | complex.cpp:11:22:11:23 | a_ [post update] | PostUpdateNode should not be the target of local flow. |
 | complex.cpp:12:22:12:23 | b_ [post update] | PostUpdateNode should not be the target of local flow. |
+| conflated.cpp:10:3:10:7 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
+| conflated.cpp:10:7:10:7 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
+| conflated.cpp:29:7:29:7 | x [post update] | PostUpdateNode should not be the target of local flow. |
+| conflated.cpp:36:7:36:7 | x [post update] | PostUpdateNode should not be the target of local flow. |
+| conflated.cpp:53:7:53:10 | next [post update] | PostUpdateNode should not be the target of local flow. |
+| conflated.cpp:54:13:54:13 | y [post update] | PostUpdateNode should not be the target of local flow. |
+| conflated.cpp:60:13:60:13 | y [post update] | PostUpdateNode should not be the target of local flow. |
 | constructors.cpp:20:24:20:25 | a_ [post update] | PostUpdateNode should not be the target of local flow. |
 | constructors.cpp:21:24:21:25 | b_ [post update] | PostUpdateNode should not be the target of local flow. |
 | qualifiers.cpp:9:36:9:36 | a [post update] | PostUpdateNode should not be the target of local flow. |

cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected

@@ -1,6 +1,10 @@
 uniqueEnclosingCallable
 uniqueType
 uniqueNodeLocation
+| E.cpp:15:31:15:33 | buf | Node should have one location but has 2. |
+| aliasing.cpp:2:11:2:13 | (unnamed parameter 0) | Node should have one location but has 2. |
+| conflated.cpp:2:11:2:13 | (unnamed parameter 0) | Node should have one location but has 2. |
+| conflated.cpp:14:22:14:25 | buf | Node should have one location but has 2. |
 | file://:0:0:0:0 | (unnamed parameter 0) | Node should have one location but has 0. |
 | file://:0:0:0:0 | (unnamed parameter 0) | Node should have one location but has 0. |
 | file://:0:0:0:0 | (unnamed parameter 0) | Node should have one location but has 0. |
@@ -129,6 +133,8 @@ postWithInFlow
 | complex.cpp:54:12:54:12 | Chi | PostUpdateNode should not be the target of local flow. |
 | complex.cpp:55:12:55:12 | Chi | PostUpdateNode should not be the target of local flow. |
 | complex.cpp:56:12:56:12 | Chi | PostUpdateNode should not be the target of local flow. |
+| conflated.cpp:45:39:45:42 | Chi | PostUpdateNode should not be the target of local flow. |
+| conflated.cpp:53:3:53:27 | Chi | PostUpdateNode should not be the target of local flow. |
 | constructors.cpp:20:24:20:29 | Chi | PostUpdateNode should not be the target of local flow. |
 | constructors.cpp:21:24:21:29 | Chi | PostUpdateNode should not be the target of local flow. |
 | constructors.cpp:23:28:23:28 | Chi | PostUpdateNode should not be the target of local flow. |

cpp/ql/test/library-tests/dataflow/fields/partial-definition-diff.expected

@@ -309,6 +309,22 @@
 | complex.cpp:62:7:62:8 | b2 | AST only |
 | complex.cpp:65:7:65:8 | b3 | AST only |
 | complex.cpp:68:7:68:8 | b4 | AST only |
+| conflated.cpp:10:3:10:7 | * ... | AST only |
+| conflated.cpp:10:4:10:5 | ra | AST only |
+| conflated.cpp:19:19:19:21 | raw | AST only |
+| conflated.cpp:20:8:20:10 | raw | AST only |
+| conflated.cpp:29:3:29:4 | pa | AST only |
+| conflated.cpp:29:7:29:7 | x | AST only |
+| conflated.cpp:36:3:36:4 | pa | AST only |
+| conflated.cpp:36:7:36:7 | x | AST only |
+| conflated.cpp:53:7:53:10 | next | AST only |
+| conflated.cpp:54:3:54:4 | ll | AST only |
+| conflated.cpp:54:7:54:10 | next | AST only |
+| conflated.cpp:54:13:54:13 | y | AST only |
+| conflated.cpp:59:35:59:38 | next | AST only |
+| conflated.cpp:60:3:60:4 | ll | AST only |
+| conflated.cpp:60:7:60:10 | next | AST only |
+| conflated.cpp:60:13:60:13 | y | AST only |
 | constructors.cpp:20:24:20:25 | a_ | AST only |
 | constructors.cpp:21:24:21:25 | b_ | AST only |
 | constructors.cpp:28:10:28:10 | f | AST only |

cpp/ql/test/library-tests/dataflow/fields/partial-definition-ir.expected

@@ -57,6 +57,7 @@
 | complex.cpp:54:6:54:10 | inner |
 | complex.cpp:55:6:55:10 | inner |
 | complex.cpp:56:6:56:10 | inner |
+| conflated.cpp:53:3:53:4 | ll |
 | constructors.cpp:20:24:20:25 | this |
 | constructors.cpp:21:24:21:25 | this |
 | qualifiers.cpp:9:30:9:33 | this |

cpp/ql/test/library-tests/dataflow/fields/partial-definition.expected

@@ -366,6 +366,23 @@
 | complex.cpp:62:7:62:8 | b2 |
 | complex.cpp:65:7:65:8 | b3 |
 | complex.cpp:68:7:68:8 | b4 |
+| conflated.cpp:10:3:10:7 | * ... |
+| conflated.cpp:10:4:10:5 | ra |
+| conflated.cpp:19:19:19:21 | raw |
+| conflated.cpp:20:8:20:10 | raw |
+| conflated.cpp:29:3:29:4 | pa |
+| conflated.cpp:29:7:29:7 | x |
+| conflated.cpp:36:3:36:4 | pa |
+| conflated.cpp:36:7:36:7 | x |
+| conflated.cpp:53:3:53:4 | ll |
+| conflated.cpp:53:7:53:10 | next |
+| conflated.cpp:54:3:54:4 | ll |
+| conflated.cpp:54:7:54:10 | next |
+| conflated.cpp:54:13:54:13 | y |
+| conflated.cpp:59:35:59:38 | next |
+| conflated.cpp:60:3:60:4 | ll |
+| conflated.cpp:60:7:60:10 | next |
+| conflated.cpp:60:13:60:13 | y |
 | constructors.cpp:20:24:20:25 | a_ |
 | constructors.cpp:20:24:20:25 | this |
 | constructors.cpp:21:24:21:25 | b_ |

cpp/ql/test/library-tests/dataflow/fields/path-flow.expected

@@ -336,6 +336,27 @@ edges
 | complex.cpp:62:7:62:8 | b2 [inner, f, b_] | complex.cpp:40:17:40:17 | b [inner, f, b_] |
 | complex.cpp:65:7:65:8 | b3 [inner, f, a_] | complex.cpp:40:17:40:17 | b [inner, f, a_] |
 | complex.cpp:65:7:65:8 | b3 [inner, f, b_] | complex.cpp:40:17:40:17 | b [inner, f, b_] |
+| conflated.cpp:19:19:19:21 | ref arg raw | conflated.cpp:20:8:20:10 | raw |
+| conflated.cpp:29:3:29:4 | pa [post update] [x] | conflated.cpp:30:8:30:9 | pa [x] |
+| conflated.cpp:29:3:29:22 | ... = ... | conflated.cpp:29:3:29:4 | pa [post update] [x] |
+| conflated.cpp:29:11:29:20 | call to user_input | conflated.cpp:29:3:29:22 | ... = ... |
+| conflated.cpp:30:8:30:9 | pa [x] | conflated.cpp:30:12:30:12 | x |
+| conflated.cpp:36:3:36:4 | pa [post update] [x] | conflated.cpp:37:8:37:9 | pa [x] |
+| conflated.cpp:36:3:36:22 | ... = ... | conflated.cpp:36:3:36:4 | pa [post update] [x] |
+| conflated.cpp:36:11:36:20 | call to user_input | conflated.cpp:36:3:36:22 | ... = ... |
+| conflated.cpp:37:8:37:9 | pa [x] | conflated.cpp:37:12:37:12 | x |
+| conflated.cpp:54:3:54:4 | ll [post update] [next, y] | conflated.cpp:55:8:55:9 | ll [next, y] |
+| conflated.cpp:54:3:54:28 | ... = ... | conflated.cpp:54:7:54:10 | next [post update] [y] |
+| conflated.cpp:54:7:54:10 | next [post update] [y] | conflated.cpp:54:3:54:4 | ll [post update] [next, y] |
+| conflated.cpp:54:17:54:26 | call to user_input | conflated.cpp:54:3:54:28 | ... = ... |
+| conflated.cpp:55:8:55:9 | ll [next, y] | conflated.cpp:55:12:55:15 | next [y] |
+| conflated.cpp:55:12:55:15 | next [y] | conflated.cpp:55:18:55:18 | y |
+| conflated.cpp:60:3:60:4 | ll [post update] [next, y] | conflated.cpp:61:8:61:9 | ll [next, y] |
+| conflated.cpp:60:3:60:28 | ... = ... | conflated.cpp:60:7:60:10 | next [post update] [y] |
+| conflated.cpp:60:7:60:10 | next [post update] [y] | conflated.cpp:60:3:60:4 | ll [post update] [next, y] |
+| conflated.cpp:60:17:60:26 | call to user_input | conflated.cpp:60:3:60:28 | ... = ... |
+| conflated.cpp:61:8:61:9 | ll [next, y] | conflated.cpp:61:12:61:15 | next [y] |
+| conflated.cpp:61:12:61:15 | next [y] | conflated.cpp:61:18:61:18 | y |
 | constructors.cpp:26:15:26:15 | f [a_] | constructors.cpp:28:10:28:10 | f [a_] |
 | constructors.cpp:26:15:26:15 | f [b_] | constructors.cpp:29:10:29:10 | f [b_] |
 | constructors.cpp:28:10:28:10 | f [a_] | constructors.cpp:28:12:28:12 | call to a |
@@ -827,6 +848,32 @@ nodes
 | complex.cpp:62:7:62:8 | b2 [inner, f, b_] | semmle.label | b2 [inner, f, b_] |
 | complex.cpp:65:7:65:8 | b3 [inner, f, a_] | semmle.label | b3 [inner, f, a_] |
 | complex.cpp:65:7:65:8 | b3 [inner, f, b_] | semmle.label | b3 [inner, f, b_] |
+| conflated.cpp:19:19:19:21 | ref arg raw | semmle.label | ref arg raw |
+| conflated.cpp:20:8:20:10 | raw | semmle.label | raw |
+| conflated.cpp:29:3:29:4 | pa [post update] [x] | semmle.label | pa [post update] [x] |
+| conflated.cpp:29:3:29:22 | ... = ... | semmle.label | ... = ... |
+| conflated.cpp:29:11:29:20 | call to user_input | semmle.label | call to user_input |
+| conflated.cpp:30:8:30:9 | pa [x] | semmle.label | pa [x] |
+| conflated.cpp:30:12:30:12 | x | semmle.label | x |
+| conflated.cpp:36:3:36:4 | pa [post update] [x] | semmle.label | pa [post update] [x] |
+| conflated.cpp:36:3:36:22 | ... = ... | semmle.label | ... = ... |
+| conflated.cpp:36:11:36:20 | call to user_input | semmle.label | call to user_input |
+| conflated.cpp:37:8:37:9 | pa [x] | semmle.label | pa [x] |
+| conflated.cpp:37:12:37:12 | x | semmle.label | x |
+| conflated.cpp:54:3:54:4 | ll [post update] [next, y] | semmle.label | ll [post update] [next, y] |
+| conflated.cpp:54:3:54:28 | ... = ... | semmle.label | ... = ... |
+| conflated.cpp:54:7:54:10 | next [post update] [y] | semmle.label | next [post update] [y] |
+| conflated.cpp:54:17:54:26 | call to user_input | semmle.label | call to user_input |
+| conflated.cpp:55:8:55:9 | ll [next, y] | semmle.label | ll [next, y] |
+| conflated.cpp:55:12:55:15 | next [y] | semmle.label | next [y] |
+| conflated.cpp:55:18:55:18 | y | semmle.label | y |
+| conflated.cpp:60:3:60:4 | ll [post update] [next, y] | semmle.label | ll [post update] [next, y] |
+| conflated.cpp:60:3:60:28 | ... = ... | semmle.label | ... = ... |
+| conflated.cpp:60:7:60:10 | next [post update] [y] | semmle.label | next [post update] [y] |
+| conflated.cpp:60:17:60:26 | call to user_input | semmle.label | call to user_input |
+| conflated.cpp:61:8:61:9 | ll [next, y] | semmle.label | ll [next, y] |
+| conflated.cpp:61:12:61:15 | next [y] | semmle.label | next [y] |
+| conflated.cpp:61:18:61:18 | y | semmle.label | y |
 | constructors.cpp:26:15:26:15 | f [a_] | semmle.label | f [a_] |
 | constructors.cpp:26:15:26:15 | f [b_] | semmle.label | f [b_] |
 | constructors.cpp:28:10:28:10 | f [a_] | semmle.label | f [a_] |
@@ -1028,6 +1075,11 @@ nodes
 | complex.cpp:42:18:42:18 | call to a | complex.cpp:55:19:55:28 | call to user_input | complex.cpp:42:18:42:18 | call to a | call to a flows from $@ | complex.cpp:55:19:55:28 | call to user_input | call to user_input |
 | complex.cpp:43:18:43:18 | call to b | complex.cpp:54:19:54:28 | call to user_input | complex.cpp:43:18:43:18 | call to b | call to b flows from $@ | complex.cpp:54:19:54:28 | call to user_input | call to user_input |
 | complex.cpp:43:18:43:18 | call to b | complex.cpp:56:19:56:28 | call to user_input | complex.cpp:43:18:43:18 | call to b | call to b flows from $@ | complex.cpp:56:19:56:28 | call to user_input | call to user_input |
+| conflated.cpp:20:8:20:10 | raw | conflated.cpp:19:19:19:21 | ref arg raw | conflated.cpp:20:8:20:10 | raw | raw flows from $@ | conflated.cpp:19:19:19:21 | ref arg raw | ref arg raw |
+| conflated.cpp:30:12:30:12 | x | conflated.cpp:29:11:29:20 | call to user_input | conflated.cpp:30:12:30:12 | x | x flows from $@ | conflated.cpp:29:11:29:20 | call to user_input | call to user_input |
+| conflated.cpp:37:12:37:12 | x | conflated.cpp:36:11:36:20 | call to user_input | conflated.cpp:37:12:37:12 | x | x flows from $@ | conflated.cpp:36:11:36:20 | call to user_input | call to user_input |
+| conflated.cpp:55:18:55:18 | y | conflated.cpp:54:17:54:26 | call to user_input | conflated.cpp:55:18:55:18 | y | y flows from $@ | conflated.cpp:54:17:54:26 | call to user_input | call to user_input |
+| conflated.cpp:61:18:61:18 | y | conflated.cpp:60:17:60:26 | call to user_input | conflated.cpp:61:18:61:18 | y | y flows from $@ | conflated.cpp:60:17:60:26 | call to user_input | call to user_input |
 | constructors.cpp:28:12:28:12 | call to a | constructors.cpp:34:11:34:20 | call to user_input | constructors.cpp:28:12:28:12 | call to a | call to a flows from $@ | constructors.cpp:34:11:34:20 | call to user_input | call to user_input |
 | constructors.cpp:28:12:28:12 | call to a | constructors.cpp:36:11:36:20 | call to user_input | constructors.cpp:28:12:28:12 | call to a | call to a flows from $@ | constructors.cpp:36:11:36:20 | call to user_input | call to user_input |
 | constructors.cpp:29:12:29:12 | call to b | constructors.cpp:35:14:35:23 | call to user_input | constructors.cpp:29:12:29:12 | call to b | call to b flows from $@ | constructors.cpp:35:14:35:23 | call to user_input | call to user_input |

cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/ast_ir_gvn.expected

@@ -0,0 +1,46 @@
+| test.cpp:5:3:5:3 | GVN | 5:c3-c3 6:c3-c3 |
+| test.cpp:5:7:5:8 | GVN | 5:c7-c8 6:c7-c8 |
+| test.cpp:5:7:5:13 | GVN | 5:c7-c13 6:c7-c13 7:c7-c7 |
+| test.cpp:5:12:5:13 | GVN | 5:c12-c13 6:c12-c13 |
+| test.cpp:16:3:16:3 | GVN | 16:c3-c3 17:c3-c3 |
+| test.cpp:16:7:16:8 | GVN | 16:c7-c8 17:c7-c8 |
+| test.cpp:16:7:16:13 | GVN | 16:c7-c13 17:c7-c13 |
+| test.cpp:16:7:16:24 | GVN | 16:c7-c24 17:c7-c24 18:c7-c7 |
+| test.cpp:16:12:16:13 | GVN | 16:c12-c13 17:c12-c13 |
+| test.cpp:16:17:16:24 | GVN | 16:c17-c24 17:c17-c24 |
+| test.cpp:29:3:29:3 | GVN | 29:c3-c3 31:c3-c3 |
+| test.cpp:29:7:29:8 | GVN | 29:c7-c8 31:c7-c8 |
+| test.cpp:29:7:29:13 | GVN | 29:c7-c13 31:c7-c13 |
+| test.cpp:29:12:29:13 | GVN | 29:c12-c13 31:c12-c13 |
+| test.cpp:31:7:31:24 | GVN | 31:c7-c24 32:c7-c7 |
+| test.cpp:43:3:43:3 | GVN | 43:c3-c3 45:c3-c3 |
+| test.cpp:43:7:43:8 | GVN | 43:c7-c8 45:c7-c8 |
+| test.cpp:43:7:43:13 | GVN | 43:c7-c13 45:c7-c13 |
+| test.cpp:43:7:43:24 | GVN | 43:c7-c24 45:c7-c24 46:c7-c7 |
+| test.cpp:43:12:43:13 | GVN | 43:c12-c13 45:c12-c13 |
+| test.cpp:43:17:43:24 | GVN | 43:c17-c24 45:c17-c24 |
+| test.cpp:44:3:44:5 | GVN | 44:c3-c5 44:c4-c5 |
+| test.cpp:53:10:53:13 | GVN | 53:c10-c13 56:c21-c24 |
+| test.cpp:53:10:53:13 | GVN | 53:c10-c13 56:c21-c24 |
+| test.cpp:53:11:53:13 | GVN | 53:c11-c13 56:c22-c24 |
+| test.cpp:53:18:53:21 | GVN | 53:c18-c21 56:c39-c42 59:c17-c20 |
+| test.cpp:56:13:56:16 | GVN | 56:c13-c16 56:c31-c34 59:c9-c12 |
+| test.cpp:56:13:56:16 | GVN | 56:c13-c16 56:c31-c34 59:c9-c12 |
+| test.cpp:56:14:56:16 | GVN | 56:c14-c16 56:c32-c34 56:c47-c49 59:c10-c12 |
+| test.cpp:62:5:62:10 | GVN | 62:c5-c10 65:c10-c15 |
+| test.cpp:77:20:77:28 | GVN | 77:c20-c28 79:c7-c7 |
+| test.cpp:79:11:79:14 | GVN | 79:c11-c14 79:c24-c27 |
+| test.cpp:92:11:92:16 | GVN | 92:c11-c16 92:c15-c16 93:c10-c10 |
+| test.cpp:105:11:105:12 | GVN | 105:c11-c12 106:c33-c34 |
+| test.cpp:105:11:105:12 | GVN | 105:c11-c12 106:c33-c34 107:c11-c12 |
+| test.cpp:105:15:105:15 | GVN | 105:c15-c15 107:c15-c15 109:c10-c10 |
+| test.cpp:113:3:113:5 | GVN | 113:c3-c5 115:c3-c5 |
+| test.cpp:125:11:125:12 | GVN | 125:c11-c12 126:c11-c12 128:c3-c4 129:c11-c12 |
+| test.cpp:125:15:125:15 | GVN | 125:c15-c15 126:c15-c15 |
+| test.cpp:128:11:128:11 | GVN | 128:c11-c11 129:c15-c15 |
+| test.cpp:136:11:136:18 | GVN | 136:c11-c18 137:c11-c18 139:c3-c10 |
+| test.cpp:136:21:136:21 | GVN | 136:c21-c21 137:c21-c21 |
+| test.cpp:144:11:144:12 | GVN | 144:c11-c12 145:c11-c12 147:c3-c4 149:c11-c12 |
+| test.cpp:144:15:144:15 | GVN | 144:c15-c15 149:c15-c15 |
+| test.cpp:153:11:153:18 | GVN | 153:c11-c18 154:c11-c18 156:c3-c10 |
+| test.cpp:153:21:153:21 | GVN | 153:c21-c21 154:c21-c21 |

cpp/ql/test/library-tests/valuenumbering/GlobalValueNumbering/ast_ir_gvn.ql

@@ -0,0 +1,11 @@
+import cpp
+import semmle.code.cpp.ir.internal.ASTValueNumbering
+
+from GVN g
+where strictcount(g.getAnExpr()) > 1
+select g,
+  strictconcat(Location loc |
+    loc = g.getAnExpr().getLocation()
+  |
+    loc.getStartLine() + ":c" + loc.getStartColumn() + "-c" + loc.getEndColumn(), " "
+  )

cpp/ql/test/query-tests/Critical/NewFree/NewFreeMismatch.expected

@@ -18,3 +18,6 @@
 | test.cpp:235:2:235:5 | call to free | There is a new/free mismatch between this free and the corresponding $@. | test.cpp:227:7:227:13 | new | new |
 | test.cpp:239:2:239:5 | call to free | There is a new/free mismatch between this free and the corresponding $@. | test.cpp:228:7:228:17 | new[] | new[] |
 | test.cpp:272:3:272:6 | call to free | There is a new/free mismatch between this free and the corresponding $@. | test.cpp:265:7:265:13 | new | new |
+| test.cpp:441:2:441:10 | delete | There is a malloc/delete mismatch between this delete and the corresponding $@. | test.cpp:434:13:434:18 | call to strdup | malloc |
+| test.cpp:443:2:443:10 | delete | There is a malloc/delete mismatch between this delete and the corresponding $@. | test.cpp:436:13:436:19 | call to strndup | malloc |
+| test.cpp:445:2:445:10 | delete | There is a malloc/delete mismatch between this delete and the corresponding $@. | test.cpp:438:16:438:21 | call to wcsdup | malloc |

cpp/ql/test/query-tests/Critical/NewFree/test.cpp

@@ -424,3 +424,24 @@ void test13()
 
 	delete myPointer3.getPointer(); // GOOD
 }
+
+char *strdup(const char *s1);
+char *strndup(const char *s1, size_t n);
+wchar_t* wcsdup(const wchar_t* s1);
+
+void test14()
+{
+	char *s1 = strdup("string");
+	char *s2 = strdup("string");
+	char *s3 = strndup("string", 3);
+	char *s4 = strndup("string", 3);
+	wchar_t *s5 = wcsdup(L"string");
+	wchar_t *s6 = wcsdup(L"string");
+
+	delete s1; // BAD: strdup -> delete
+	free(s2); // GOOD
+	delete s3; // BAD: strndup -> delete
+	free(s4); // GOOD
+	delete s5; // BAD: wcsdup -> delete
+	free(s6); // GOOD
+}

cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/test.c

@@ -42,4 +42,8 @@ void test(int i, const char *str)
 	}
 
 	printf("%@ %i %i", 1, 2); // GOOD
+
+	printf("%Y", 1, 2); // GOOD (unknown format character, this might be correct)
+	printf("%1.1Y", 1, 2); // GOOD (unknown format character, this might be correct)
+	printf("%*.*Y", 1, 2); // GOOD (unknown format character, this might be correct)
 }

cpp/ql/test/query-tests/Security/CWE/CWE-134/semmle/globalVars/UncontrolledFormatStringThroughGlobalVar.expected

@@ -4,12 +4,14 @@ edges
 | globalVars.c:8:7:8:10 | copy | globalVars.c:27:9:27:12 | copy |
 | globalVars.c:8:7:8:10 | copy | globalVars.c:30:15:30:18 | copy |
 | globalVars.c:8:7:8:10 | copy | globalVars.c:30:15:30:18 | copy |
+| globalVars.c:8:7:8:10 | copy | globalVars.c:30:15:30:18 | copy |
 | globalVars.c:8:7:8:10 | copy | globalVars.c:35:11:35:14 | copy |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:38:9:38:13 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:41:15:41:19 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:41:15:41:19 | copy2 |
+| globalVars.c:9:7:9:11 | copy2 | globalVars.c:41:15:41:19 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 |
 | globalVars.c:9:7:9:11 | copy2 | globalVars.c:50:9:50:13 | copy2 |
@@ -25,9 +27,15 @@ edges
 | globalVars.c:24:11:24:14 | argv | globalVars.c:11:22:11:25 | argv |
 | globalVars.c:27:9:27:12 | copy | globalVars.c:27:9:27:12 | (const char *)... |
 | globalVars.c:27:9:27:12 | copy | globalVars.c:27:9:27:12 | copy |
+| globalVars.c:30:15:30:18 | copy | globalVars.c:30:15:30:18 | copy |
+| globalVars.c:30:15:30:18 | copy | globalVars.c:30:15:30:18 | copy |
+| globalVars.c:30:15:30:18 | copy | globalVars.c:30:15:30:18 | copy |
 | globalVars.c:35:11:35:14 | copy | globalVars.c:15:21:15:23 | val |
 | globalVars.c:38:9:38:13 | copy2 | globalVars.c:38:9:38:13 | (const char *)... |
 | globalVars.c:38:9:38:13 | copy2 | globalVars.c:38:9:38:13 | copy2 |
+| globalVars.c:41:15:41:19 | copy2 | globalVars.c:41:15:41:19 | copy2 |
+| globalVars.c:41:15:41:19 | copy2 | globalVars.c:41:15:41:19 | copy2 |
+| globalVars.c:41:15:41:19 | copy2 | globalVars.c:41:15:41:19 | copy2 |
 | globalVars.c:50:9:50:13 | copy2 | globalVars.c:50:9:50:13 | (const char *)... |
 | globalVars.c:50:9:50:13 | copy2 | globalVars.c:50:9:50:13 | copy2 |
 nodes

csharp/change-notes/2020-12-17-format-method-empty-overload.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* For string formatting methods, such as `System.Console.WriteLine(string format, params object[] arg)`, we now also recognize overloads without insertion parameters as string formatting methods. For example, `System.Console.WriteLine(string value)` is now also a member of the class `FormatMethod` in `frameworks/Format.qll`.

csharp/change-notes/2020-12-18-extract-custom-modifiers.md

@@ -0,0 +1,4 @@
+lgtm,codescanning
+* CIL extraction has been improved to store `modreq` and `modopt` custom modifiers.
+The extracted information is surfaced through the `CustomModifierReceiver` class. Additionally,
+the information is also used to evaluate the new `Setter::isInitOnly` predicate.

csharp/change-notes/2020-12-21-merge-format-queries.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The queries `FormatInvalid.ql`, `FormatMissingArgument.ql`, and `FormatUnusedArgument.ql` have been merged into a single `FormatInvalid.ql` query.

csharp/extractor/Semmle.Extraction.CIL/Context.cs

@@ -1,6 +1,5 @@
 using System;
 using System.IO;
-using System.Linq;
 using System.Reflection.Metadata;
 using System.Reflection.PortableExecutable;
 
@@ -83,7 +82,7 @@ namespace Semmle.Extraction.CIL
             trapFile.Write(GetString(def.Name));
             trapFile.Write('_');
             trapFile.Write(def.Version.ToString());
-            trapFile.Write("::");
+            trapFile.Write(Entities.Type.AssemblyTypeNameSeparator);
         }
 
         public Entities.TypeSignatureDecoder TypeSignatureDecoder { get; }

csharp/extractor/Semmle.Extraction.CIL/Entities/ArrayType.cs

@@ -27,27 +27,26 @@ namespace Semmle.Extraction.CIL.Entities
             return obj is ArrayType array && elementType.Equals(array.elementType) && rank == array.rank;
         }
 
-        public override int GetHashCode()
-        {
-            return elementType.GetHashCode() * 5 + rank;
-        }
+        public override int GetHashCode() => HashCode.Combine(elementType, rank);
 
         public override void WriteId(TextWriter trapFile, bool inContext)
         {
-            elementType.GetId(trapFile, inContext);
+            elementType.WriteId(trapFile, inContext);
             trapFile.Write('[');
             for (var i = 1; i < rank; ++i)
+            {
                 trapFile.Write(',');
+            }
             trapFile.Write(']');
         }
 
         public override string Name => elementType.Name + "[]";
 
-        public override Namespace Namespace => Cx.SystemNamespace;
+        public override Namespace ContainingNamespace => Cx.SystemNamespace;
 
         public override Type? ContainingType => null;
 
-        public override int ThisTypeParameters => elementType.ThisTypeParameters;
+        public override int ThisTypeParameterCount => elementType.ThisTypeParameterCount;
 
         public override CilTypeKind Kind => CilTypeKind.Array;
 
@@ -71,7 +70,5 @@ namespace Semmle.Extraction.CIL.Entities
         public override IEnumerable<Type> GenericArguments => elementType.GenericArguments;
 
         public override IEnumerable<Type> TypeParameters => elementType.TypeParameters;
-
-        public override IEnumerable<Type> MethodParameters => throw new NotImplementedException();
     }
 }

csharp/extractor/Semmle.Extraction.CIL/Entities/Attribute.cs

@@ -43,33 +43,38 @@ namespace Semmle.Extraction.CIL.Entities
                 {
                     decoded = attrib.DecodeValue(new CustomAttributeDecoder(Cx));
                 }
-                catch (NotImplementedException)
+                catch
                 {
-                    // Attribute decoding is only partial at this stage.
+                    Cx.Cx.Extractor.Logger.Log(Util.Logging.Severity.Info,
+                        $"Attribute decoding is partial. Decoding attribute {constructor.DeclaringType.GetQualifiedName()} failed on {@object}.");
                     yield break;
                 }
 
                 for (var index = 0; index < decoded.FixedArguments.Length; ++index)
                 {
-                    var value = decoded.FixedArguments[index].Value;
-                    var stringValue = GetStringValue(value);
+                    var stringValue = GetStringValue(decoded.FixedArguments[index].Type, decoded.FixedArguments[index].Value);
                     yield return Tuples.cil_attribute_positional_argument(this, index, stringValue);
                 }
 
                 foreach (var p in decoded.NamedArguments)
                 {
-                    var value = p.Value;
-                    var stringValue = GetStringValue(value);
-                    yield return Tuples.cil_attribute_named_argument(this, p.Name, stringValue);
+                    var stringValue = GetStringValue(p.Type, p.Value);
+                    yield return Tuples.cil_attribute_named_argument(this, p.Name!, stringValue);
                 }
             }
         }
 
-        private static string GetStringValue(object? value)
+        private static string GetStringValue(Type type, object? value)
         {
             if (value is System.Collections.Immutable.ImmutableArray<CustomAttributeTypedArgument<Type>> values)
             {
-                return "[" + string.Join(",", values.Select(v => GetStringValue(v.Value))) + "]";
+                return "[" + string.Join(",", values.Select(v => GetStringValue(v.Type, v.Value))) + "]";
+            }
+
+            if (type.GetQualifiedName() == "System.Type" &&
+                value is Type t)
+            {
+                return t.GetQualifiedName();
             }
 
             return value?.ToString() ?? "null";

csharp/extractor/Semmle.Extraction.CIL/Entities/ConstructedType.cs

@@ -17,36 +17,18 @@ namespace Semmle.Extraction.CIL.Entities
         // Either null or notEmpty
         private readonly Type[]? thisTypeArguments;
 
-        public override IEnumerable<Type> ThisTypeArguments => thisTypeArguments.EnumerateNull();
-
-        public override IEnumerable<Type> ThisGenericArguments => thisTypeArguments.EnumerateNull();
-
-        public override IEnumerable<IExtractionProduct> Contents
-        {
-            get
-            {
-                foreach (var c in base.Contents)
-                    yield return c;
-
-                var i = 0;
-                foreach (var type in ThisGenericArguments)
-                {
-                    yield return type;
-                    yield return Tuples.cil_type_argument(this, i++, type);
-                }
-            }
-        }
-
-        public override Type SourceDeclaration => unboundGenericType;
+        private readonly Type? containingType;
+        private readonly NamedTypeIdWriter idWriter;
 
         public ConstructedType(Context cx, Type unboundType, IEnumerable<Type> typeArguments) : base(cx)
         {
+            idWriter = new NamedTypeIdWriter(this);
             var suppliedArgs = typeArguments.Count();
-            if (suppliedArgs != unboundType.TotalTypeParametersCheck)
+            if (suppliedArgs != unboundType.TotalTypeParametersCount)
                 throw new InternalError("Unexpected number of type arguments in ConstructedType");
 
             unboundGenericType = unboundType;
-            var thisParams = unboundType.ThisTypeParameters;
+            var thisParams = unboundType.ThisTypeParameterCount;
 
             if (typeArguments.Count() == thisParams)
             {
@@ -88,14 +70,29 @@ namespace Semmle.Extraction.CIL.Entities
             return h;
         }
 
-        private readonly Type? containingType;
+        public override IEnumerable<IExtractionProduct> Contents
+        {
+            get
+            {
+                foreach (var c in base.Contents)
+                    yield return c;
+
+                var i = 0;
+                foreach (var type in ThisTypeArguments)
+                {
+                    yield return type;
+                    yield return Tuples.cil_type_argument(this, i++, type);
+                }
+            }
+        }
+
+        public override Type SourceDeclaration => unboundGenericType;
+
         public override Type? ContainingType => containingType;
 
         public override string Name => unboundGenericType.Name;
 
-        public override Namespace Namespace => unboundGenericType.Namespace!;
-
-        public override int ThisTypeParameters => thisTypeArguments == null ? 0 : thisTypeArguments.Length;
+        public override Namespace ContainingNamespace => unboundGenericType.ContainingNamespace!;
 
         public override CilTypeKind Kind => unboundGenericType.Kind;
 
@@ -106,40 +103,17 @@ namespace Semmle.Extraction.CIL.Entities
 
         public override void WriteId(TextWriter trapFile, bool inContext)
         {
-            if (ContainingType != null)
-            {
-                ContainingType.GetId(trapFile, inContext);
-                trapFile.Write('.');
-            }
-            else
-            {
-                WriteAssemblyPrefix(trapFile);
-
-                if (!Namespace.IsGlobalNamespace)
-                {
-                    Namespace.WriteId(trapFile);
-                    trapFile.Write('.');
-                }
-            }
-            trapFile.Write(unboundGenericType.Name);
-
-            if (thisTypeArguments != null && thisTypeArguments.Any())
-            {
-                trapFile.Write('<');
-                var index = 0;
-                foreach (var t in thisTypeArguments)
-                {
-                    trapFile.WriteSeparator(",", ref index);
-                    t.WriteId(trapFile);
-                }
-                trapFile.Write('>');
-            }
+            idWriter.WriteId(trapFile, inContext);
         }
 
         public override void WriteAssemblyPrefix(TextWriter trapFile) => unboundGenericType.WriteAssemblyPrefix(trapFile);
 
+        public override int ThisTypeParameterCount => thisTypeArguments?.Length ?? 0;
+
         public override IEnumerable<Type> TypeParameters => GenericArguments;
 
-        public override IEnumerable<Type> MethodParameters => throw new NotImplementedException();
+        public override IEnumerable<Type> ThisTypeArguments => thisTypeArguments.EnumerateNull();
+
+        public override IEnumerable<Type> ThisGenericArguments => thisTypeArguments.EnumerateNull();
     }
 }

csharp/extractor/Semmle.Extraction.CIL/Entities/CustomAttributeDecoder.cs

@@ -14,7 +14,7 @@ namespace Semmle.Extraction.CIL.Entities
 
         public Type GetPrimitiveType(PrimitiveTypeCode typeCode) => cx.Create(typeCode);
 
-        public Type GetSystemType() => throw new NotImplementedException();
+        public Type GetSystemType() => new NoMetadataHandleType(cx, "System.Type");
 
         public Type GetSZArrayType(Type elementType) =>
             cx.Populate(new ArrayType(cx, elementType));
@@ -25,10 +25,23 @@ namespace Semmle.Extraction.CIL.Entities
         public Type GetTypeFromReference(MetadataReader reader, TypeReferenceHandle handle, byte rawTypeKind) =>
             (Type)cx.Create(handle);
 
-        public Type GetTypeFromSerializedName(string name) => throw new NotImplementedException();
+        public Type GetTypeFromSerializedName(string name) => new NoMetadataHandleType(cx, name);
 
-        public PrimitiveTypeCode GetUnderlyingEnumType(Type type) => throw new NotImplementedException();
+        public PrimitiveTypeCode GetUnderlyingEnumType(Type type)
+        {
+            if (type is TypeDefinitionType tdt &&
+                tdt.GetUnderlyingEnumType() is PrimitiveTypeCode underlying)
+            {
+                return underlying;
+            }
 
-        public bool IsSystemType(Type type) => type is PrimitiveType; // ??
+            var name = type.GetQualifiedName();
+            cx.Cx.Extractor.Logger.Log(Util.Logging.Severity.Info, $"Couldn't get underlying enum type for {name}");
+
+            // We can't fall back to Int32, because the type returned here defines how many bytes are read from the
+            // stream and how those bytes are interpreted.
+            throw new NotImplementedException();
+        }
+        public bool IsSystemType(Type type) => type.GetQualifiedName() == "System.Type";
     }
 }

csharp/extractor/Semmle.Extraction.CIL/Entities/DefinitionMethod.cs

@@ -76,9 +76,10 @@ namespace Semmle.Extraction.CIL.Entities
 
                 var typeSignature = md.DecodeSignature(Cx.TypeSignatureDecoder, this);
 
-                Parameters = MakeParameters(typeSignature.ParameterTypes).ToArray();
+                var parameters = GetParameterExtractionProducts(typeSignature.ParameterTypes).ToArray();
+                Parameters = parameters.OfType<Parameter>().ToArray();
 
-                foreach (var c in Parameters)
+                foreach (var c in parameters)
                     yield return c;
 
                 foreach (var c in PopulateFlags)
@@ -95,7 +96,12 @@ namespace Semmle.Extraction.CIL.Entities
                 }
 
                 yield return Tuples.metadata_handle(this, Cx.Assembly, MetadataTokens.GetToken(handle));
-                yield return Tuples.cil_method(this, Name, declaringType, typeSignature.ReturnType);
+
+                foreach (var m in GetMethodExtractionProducts(Name, declaringType, typeSignature.ReturnType))
+                {
+                    yield return m;
+                }
+
                 yield return Tuples.cil_method_source_declaration(this, this);
                 yield return Tuples.cil_method_location(this, Cx.Assembly);
 
@@ -178,7 +184,7 @@ namespace Semmle.Extraction.CIL.Entities
             }
         }
 
-        private IEnumerable<IExtractionProduct> Decode(byte[] ilbytes, Dictionary<int, Instruction> jump_table)
+        private IEnumerable<IExtractionProduct> Decode(byte[]? ilbytes, Dictionary<int, Instruction> jump_table)
         {
             // Sequence points are stored in order of offset.
             // We use an enumerator to locate the correct sequence point for each instruction.
@@ -203,9 +209,9 @@ namespace Semmle.Extraction.CIL.Entities
             }
 
             var child = 0;
-            for (var offset = 0; offset < ilbytes.Length;)
+            for (var offset = 0; offset < (ilbytes?.Length ?? 0);)
             {
-                var instruction = new Instruction(Cx, this, ilbytes, offset, child++);
+                var instruction = new Instruction(Cx, this, ilbytes!, offset, child++);
                 yield return instruction;
 
                 if (nextSequencePoint != null && offset >= nextSequencePoint.Current.Offset)
@@ -245,12 +251,12 @@ namespace Semmle.Extraction.CIL.Entities
                     var ilbytes = body.GetILBytes();
 
                     var child = 0;
-                    for (var offset = 0; offset < ilbytes.Length;)
+                    for (var offset = 0; offset < (ilbytes?.Length ?? 0);)
                     {
                         Instruction decoded;
                         try
                         {
-                            decoded = new Instruction(Cx, this, ilbytes, offset, child++);
+                            decoded = new Instruction(Cx, this, ilbytes!, offset, child++);
                             offset += decoded.Width;
                         }
                         catch  // lgtm[cs/catch-of-all-exceptions]

csharp/extractor/Semmle.Extraction.CIL/Entities/ErrorType.cs

@@ -16,18 +16,16 @@ namespace Semmle.Extraction.CIL.Entities
 
         public override string Name => "!error";
 
-        public override Namespace Namespace => Cx.GlobalNamespace;
+        public override Namespace ContainingNamespace => Cx.GlobalNamespace;
 
         public override Type? ContainingType => null;
 
-        public override int ThisTypeParameters => 0;
+        public override int ThisTypeParameterCount => 0;
 
         public override void WriteAssemblyPrefix(TextWriter trapFile) => throw new NotImplementedException();
 
         public override IEnumerable<Type> TypeParameters => throw new NotImplementedException();
 
-        public override IEnumerable<Type> MethodParameters => throw new NotImplementedException();
-
         public override Type Construct(IEnumerable<Type> typeArguments) => throw new NotImplementedException();
     }
 }

csharp/extractor/Semmle.Extraction.CIL/Entities/Field.cs

@@ -8,7 +8,7 @@ namespace Semmle.Extraction.CIL.Entities
     /// <summary>
     /// An entity representing a field.
     /// </summary>
-    internal abstract class Field : GenericContext, IMember
+    internal abstract class Field : GenericContext, IMember, ICustomModifierReceiver
     {
         protected Field(Context cx) : base(cx)
         {
@@ -45,7 +45,13 @@ namespace Semmle.Extraction.CIL.Entities
         {
             get
             {
-                yield return Tuples.cil_field(this, DeclaringType, Name, Type);
+                var t = Type;
+                if (t is ModifiedType mt)
+                {
+                    t = mt.Unmodified;
+                    yield return Tuples.cil_custom_modifiers(this, mt.Modifier, mt.IsRequired);
+                }
+                yield return Tuples.cil_field(this, DeclaringType, Name, t);
             }
         }
 

csharp/extractor/Semmle.Extraction.CIL/Entities/GenericsHelper.cs

@@ -0,0 +1,59 @@
+using System.Collections.Generic;
+using System.Reflection.Metadata;
+
+namespace Semmle.Extraction.CIL.Entities
+{
+    internal static class GenericsHelper
+    {
+        public static TypeTypeParameter[] MakeTypeParameters(Type container, int count)
+        {
+            var newTypeParams = new TypeTypeParameter[count];
+            for (var i = 0; i < newTypeParams.Length; ++i)
+            {
+                newTypeParams[i] = new TypeTypeParameter(container, i);
+            }
+            return newTypeParams;
+        }
+
+        public static string GetNonGenericName(StringHandle name, MetadataReader reader)
+        {
+            var n = reader.GetString(name);
+            return GetNonGenericName(n);
+        }
+
+        public static string GetNonGenericName(string name)
+        {
+            var tick = name.LastIndexOf('`');
+            return tick == -1
+                ? name
+                : name.Substring(0, tick);
+        }
+
+        public static int GetGenericTypeParameterCount(StringHandle name, MetadataReader reader)
+        {
+            var n = reader.GetString(name);
+            return GetGenericTypeParameterCount(n);
+        }
+
+        public static int GetGenericTypeParameterCount(string name)
+        {
+            var tick = name.LastIndexOf('`');
+            return tick == -1
+                ? 0
+                : int.Parse(name.Substring(tick + 1));
+        }
+
+        public static IEnumerable<Type> GetAllTypeParameters(Type? container, IEnumerable<TypeTypeParameter> thisTypeParameters)
+        {
+            if (container != null)
+            {
+                foreach (var t in container.TypeParameters)
+                    yield return t;
+            }
+
+            foreach (var t in thisTypeParameters)
+                yield return t;
+
+        }
+    }
+}

csharp/extractor/Semmle.Extraction.CIL/Entities/MemberReferenceMethod.cs

@@ -76,12 +76,16 @@ namespace Semmle.Extraction.CIL.Entities
 
                 var typeSignature = mr.DecodeMethodSignature(Cx.TypeSignatureDecoder, this);
 
-                Parameters = MakeParameters(typeSignature.ParameterTypes).ToArray();
-                foreach (var p in Parameters) yield return p;
+                var parameters = GetParameterExtractionProducts(typeSignature.ParameterTypes).ToArray();
+                Parameters = parameters.OfType<Parameter>().ToArray();
+                foreach (var p in parameters) yield return p;
 
                 foreach (var f in PopulateFlags) yield return f;
 
-                yield return Tuples.cil_method(this, Name, DeclaringType, typeSignature.ReturnType);
+                foreach (var m in GetMethodExtractionProducts(Name, DeclaringType, typeSignature.ReturnType))
+                {
+                    yield return m;
+                }
 
                 if (SourceDeclaration != null)
                     yield return Tuples.cil_method_source_declaration(this, SourceDeclaration);

csharp/extractor/Semmle.Extraction.CIL/Entities/Method.cs

@@ -3,14 +3,13 @@ using System.Reflection.Metadata;
 using System.Collections.Generic;
 using System.Linq;
 using System.IO;
-using Semmle.Util;
 
 namespace Semmle.Extraction.CIL.Entities
 {
     /// <summary>
     /// A method entity.
     /// </summary>
-    internal abstract class Method : TypeContainer, IMember
+    internal abstract class Method : TypeContainer, IMember, ICustomModifierReceiver
     {
         protected MethodTypeParameter[]? genericParams;
         protected GenericContext gc;
@@ -21,6 +20,8 @@ namespace Semmle.Extraction.CIL.Entities
             this.gc = gc;
         }
 
+        public ITypeSignature ReturnType => signature.ReturnType;
+
         public override IEnumerable<Type> TypeParameters => gc.TypeParameters.Concat(DeclaringType.TypeParameters);
 
         public override IEnumerable<Type> MethodParameters =>
@@ -76,7 +77,7 @@ namespace Semmle.Extraction.CIL.Entities
 
         public abstract bool IsStatic { get; }
 
-        protected IEnumerable<Parameter> MakeParameters(IEnumerable<Type> parameterTypes)
+        protected IEnumerable<IExtractionProduct> GetParameterExtractionProducts(IEnumerable<Type> parameterTypes)
         {
             var i = 0;
 
@@ -86,7 +87,26 @@ namespace Semmle.Extraction.CIL.Entities
             }
 
             foreach (var p in parameterTypes)
-                yield return Cx.Populate(new Parameter(Cx, this, i++, p));
+            {
+                var t = p;
+                if (t is ModifiedType mt)
+                {
+                    t = mt.Unmodified;
+                    yield return Tuples.cil_custom_modifiers(this, mt.Modifier, mt.IsRequired);
+                }
+                yield return Cx.Populate(new Parameter(Cx, this, i++, t));
+            }
+        }
+
+        protected IEnumerable<IExtractionProduct> GetMethodExtractionProducts(string name, Type declaringType, Type returnType)
+        {
+            var t = returnType;
+            if (t is ModifiedType mt)
+            {
+                t = mt.Unmodified;
+                yield return Tuples.cil_custom_modifiers(this, mt.Modifier, mt.IsRequired);
+            }
+            yield return Tuples.cil_method(this, name, declaringType, t);
         }
     }
 }

csharp/extractor/Semmle.Extraction.CIL/Entities/MethodSpecificationMethod.cs

@@ -77,14 +77,19 @@ namespace Semmle.Extraction.CIL.Entities
                         throw new InternalError($"Unexpected constructed method handle kind {ms.Method.Kind}");
                 }
 
-                Parameters = MakeParameters(constructedTypeSignature.ParameterTypes).ToArray();
-                foreach (var p in Parameters)
+                var parameters = GetParameterExtractionProducts(constructedTypeSignature.ParameterTypes).ToArray();
+                Parameters = parameters.OfType<Parameter>().ToArray();
+                foreach (var p in parameters)
                     yield return p;
 
                 foreach (var f in PopulateFlags)
                     yield return f;
 
-                yield return Tuples.cil_method(this, Name, DeclaringType, constructedTypeSignature.ReturnType);
+                foreach (var m in GetMethodExtractionProducts(Name, DeclaringType, constructedTypeSignature.ReturnType))
+                {
+                    yield return m;
+                }
+
                 yield return Tuples.cil_method_source_declaration(this, SourceDeclaration);
 
                 if (typeParams.Length != unboundMethod.GenericParameterCount)

csharp/extractor/Semmle.Extraction.CIL/Entities/MethodTypeParameter.cs

@@ -41,8 +41,6 @@ namespace Semmle.Extraction.CIL.Entities
 
         public override IEnumerable<Type> TypeParameters => throw new NotImplementedException();
 
-        public override IEnumerable<Type> MethodParameters => throw new NotImplementedException();
-
         public override IEnumerable<IExtractionProduct> Contents
         {
             get

csharp/extractor/Semmle.Extraction.CIL/Entities/ModifiedType.cs

@@ -0,0 +1,42 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+
+namespace Semmle.Extraction.CIL.Entities
+{
+    /// <summary>
+    /// Modified types are not written directly to trap files. Instead, the modifiers are stored
+    /// on the modifiable entity (field type, property/method/function pointer parameter or return types).
+    /// </summary>
+    internal sealed class ModifiedType : Type
+    {
+        public ModifiedType(Context cx, Type unmodified, Type modifier, bool isRequired) : base(cx)
+        {
+            Unmodified = unmodified;
+            Modifier = modifier;
+            IsRequired = isRequired;
+        }
+
+        public Type Unmodified { get; }
+        public Type Modifier { get; }
+        public bool IsRequired { get; }
+
+        public override CilTypeKind Kind => throw new NotImplementedException();
+
+        public override Namespace? ContainingNamespace => throw new NotImplementedException();
+
+        public override Type? ContainingType => throw new NotImplementedException();
+
+        public override IEnumerable<Type> TypeParameters => throw new NotImplementedException();
+
+        public override int ThisTypeParameterCount => throw new NotImplementedException();
+
+        public override Type Construct(IEnumerable<Type> typeArguments) => throw new NotImplementedException();
+
+        public override string Name => $"{Unmodified.Name} {(IsRequired ? "modreq" : "modopt")}({Modifier.Name})";
+
+        public override void WriteAssemblyPrefix(TextWriter trapFile) => throw new NotImplementedException();
+
+        public override void WriteId(TextWriter trapFile, bool inContext) => throw new NotImplementedException();
+    }
+}

csharp/extractor/Semmle.Extraction.CIL/Entities/NamedTypeIdWriter.cs

@@ -0,0 +1,63 @@
+using Microsoft.CodeAnalysis;
+using System.Linq;
+using System.IO;
+
+namespace Semmle.Extraction.CIL.Entities
+{
+    internal class NamedTypeIdWriter
+    {
+        private readonly Type type;
+
+        public NamedTypeIdWriter(Type type)
+        {
+            this.type = type;
+        }
+
+        public void WriteId(TextWriter trapFile, bool inContext)
+        {
+            if (type.IsPrimitiveType)
+            {
+                Type.WritePrimitiveTypeId(trapFile, type.Name);
+                return;
+            }
+
+            var ct = type.ContainingType;
+            if (ct != null)
+            {
+                ct.WriteId(trapFile, inContext);
+                trapFile.Write('.');
+            }
+            else
+            {
+                type.WriteAssemblyPrefix(trapFile);
+
+                var ns = type.ContainingNamespace!;
+                if (!ns.IsGlobalNamespace)
+                {
+                    ns.WriteId(trapFile);
+                    trapFile.Write('.');
+                }
+            }
+
+            trapFile.Write(type.Name);
+
+            var thisTypeArguments = type.ThisTypeArguments;
+            if (thisTypeArguments != null && thisTypeArguments.Any())
+            {
+                trapFile.Write('<');
+                var index = 0;
+                foreach (var t in thisTypeArguments)
+                {
+                    trapFile.WriteSeparator(",", ref index);
+                    t.WriteId(trapFile);
+                }
+                trapFile.Write('>');
+            }
+            else if (type.ThisTypeParameterCount > 0)
+            {
+                trapFile.Write('`');
+                trapFile.Write(type.ThisTypeParameterCount);
+            }
+        }
+    }
+}

csharp/extractor/Semmle.Extraction.CIL/Entities/NoMetadataHandleType.FullyQualifiedNameParser.cs

@@ -0,0 +1,163 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using Semmle.Util;
+
+namespace Semmle.Extraction.CIL.Entities
+{
+    internal sealed partial class NoMetadataHandleType
+    {
+        /// <summary>
+        /// Parser to split a fully qualified name into short name, namespace or declaring type name, assembly name, and
+        /// type argument names. Names are in the following format:
+        /// <c>N1.N2.T1`2+T2`2[T3,[T4, A1, Version=...],T5,T6], A2, Version=...</c>
+        /// </summary>
+        /// <example>
+        /// <code>typeof(System.Collections.Generic.List<int>.Enumerator)
+        /// -> System.Collections.Generic.List`1+Enumerator[[System.Int32, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]], System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e
+        /// typeof(System.Collections.Generic.List<>.Enumerator)
+        /// -> System.Collections.Generic.List`1+Enumerator, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e
+        /// </code>
+        /// </example>
+        private class FullyQualifiedNameParser
+        {
+            public string ShortName { get; internal set; }
+            public string? AssemblyName { get; internal set; }
+            public IEnumerable<string>? TypeArguments { get; internal set; }
+            public string? UnboundGenericTypeName { get; internal set; }
+            public string ContainerName { get; internal set; }
+            public bool IsContainerNamespace { get; internal set; }
+
+            private string AssemblySuffix => string.IsNullOrWhiteSpace(AssemblyName) ? "" : $", {AssemblyName}";
+
+            public FullyQualifiedNameParser(string name)
+            {
+                ExtractAssemblyName(ref name, out var lastBracketIndex);
+                ExtractTypeArguments(ref name, lastBracketIndex, out var containerTypeArguments);
+                ContainerName = ExtractContainer(ref name, containerTypeArguments);
+
+                ShortName = name;
+            }
+
+            private void ExtractTypeArguments(ref string name, int lastBracketIndex, out string containerTypeArguments)
+            {
+                var firstBracketIndex = name.IndexOf('[');
+                if (firstBracketIndex < 0)
+                {
+                    // not generic or non-constructed generic
+                    TypeArguments = null;
+                    containerTypeArguments = "";
+                    UnboundGenericTypeName = null;
+                    return;
+                }
+
+                // "T3,[T4, Assembly1, Version=...],T5,T6"
+                string typeArgs;
+                (name, _, typeArgs, _) = name.Split(firstBracketIndex, firstBracketIndex + 1, lastBracketIndex - firstBracketIndex - 1);
+
+                var thisTypeArgCount = GenericsHelper.GetGenericTypeParameterCount(name);
+                if (thisTypeArgCount == 0)
+                {
+                    // not generic or non-constructed generic; container is constructed
+                    TypeArguments = null;
+                    containerTypeArguments = $"[{typeArgs}]";
+                    UnboundGenericTypeName = null;
+                    return;
+                }
+
+                // constructed generic
+                // "T3,[T4, Assembly1, Version=...]", ["T5", "T6"]
+                var (containerTypeArgs, thisTypeArgs) = ParseTypeArgumentStrings(typeArgs, thisTypeArgCount);
+
+                TypeArguments = thisTypeArgs;
+
+                containerTypeArguments = string.IsNullOrWhiteSpace(containerTypeArgs)
+                    ? ""                            // containing type is not constructed generics
+                    : $"[{containerTypeArgs}]";     // "T3,[T4, Assembly1, Version=...],,]"
+
+                UnboundGenericTypeName = $"{name}{AssemblySuffix}";
+            }
+
+            private string ExtractContainer(ref string name, string containerTypeArguments)
+            {
+                var lastPlusIndex = name.LastIndexOf('+');
+                IsContainerNamespace = lastPlusIndex < 0;
+                if (IsContainerNamespace)
+                {
+                    return ExtractContainerNamespace(ref name);
+                }
+
+                return ExtractContainerType(ref name, containerTypeArguments, lastPlusIndex);
+            }
+
+            private static string ExtractContainerNamespace(ref string name)
+            {
+                var lastDotIndex = name.LastIndexOf('.');
+                if (lastDotIndex >= 0)
+                {
+                    string containerName;
+                    (containerName, _, name) = name.Split(lastDotIndex, lastDotIndex + 1);
+                    return containerName;
+                }
+
+                return ""; // global namespace name
+            }
+
+            private string ExtractContainerType(ref string name, string containerTypeArguments, int lastPlusIndex)
+            {
+                string containerName;
+                (containerName, _, name) = name.Split(lastPlusIndex, lastPlusIndex + 1);
+                return $"{containerName}{containerTypeArguments}{AssemblySuffix}";
+            }
+
+            private void ExtractAssemblyName(ref string name, out int lastBracketIndex)
+            {
+                lastBracketIndex = name.LastIndexOf(']');
+                var assemblyCommaIndex = name.IndexOf(',', lastBracketIndex < 0 ? 0 : lastBracketIndex);
+                if (assemblyCommaIndex >= 0)
+                {
+                    // "Assembly2, Version=..."
+                    (name, _, AssemblyName) = name.Split(assemblyCommaIndex, assemblyCommaIndex + 2);
+                }
+            }
+
+            private static (string, IEnumerable<string>) ParseTypeArgumentStrings(string typeArgs, int thisTypeArgCount)
+            {
+                var thisTypeArgs = new Stack<string>(thisTypeArgCount);
+                while (typeArgs.Length > 0 && thisTypeArgCount > 0)
+                {
+                    int startCurrentType;
+                    if (typeArgs[^1] != ']')
+                    {
+                        startCurrentType = typeArgs.LastIndexOf(',') + 1;
+                        thisTypeArgs.Push(typeArgs.Substring(startCurrentType));
+                    }
+                    else
+                    {
+                        var bracketCount = 1;
+                        for (startCurrentType = typeArgs.Length - 2; startCurrentType >= 0 && bracketCount > 0; startCurrentType--)
+                        {
+                            if (typeArgs[startCurrentType] == ']')
+                            {
+                                bracketCount++;
+                            }
+                            else if (typeArgs[startCurrentType] == '[')
+                            {
+                                bracketCount--;
+                            }
+                        }
+                        startCurrentType++;
+                        thisTypeArgs.Push(typeArgs[(startCurrentType + 1)..^1]);
+                    }
+
+                    typeArgs = startCurrentType != 0
+                        ? typeArgs.Substring(0, startCurrentType - 1)
+                        : "";
+
+                    thisTypeArgCount--;
+                }
+                return (typeArgs, thisTypeArgs.ToList());
+            }
+        }
+    }
+}

csharp/extractor/Semmle.Extraction.CIL/Entities/NoMetadataHandleType.cs

@@ -0,0 +1,158 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Reflection;
+using Semmle.Util;
+
+namespace Semmle.Extraction.CIL.Entities
+{
+    internal sealed partial class NoMetadataHandleType : Type
+    {
+        private readonly string originalName;
+        private readonly string name;
+        private readonly string? assemblyName;
+        private readonly string containerName;
+        private readonly bool isContainerNamespace;
+
+        private readonly Lazy<TypeTypeParameter[]>? typeParams;
+
+        // Either null or notEmpty
+        private readonly Type[]? thisTypeArguments;
+        private readonly Type unboundGenericType;
+        private readonly Type? containingType;
+
+        private readonly NamedTypeIdWriter idWriter;
+
+        public NoMetadataHandleType(Context cx, string originalName) : base(cx)
+        {
+            this.originalName = originalName;
+            this.idWriter = new NamedTypeIdWriter(this);
+
+            var nameParser = new FullyQualifiedNameParser(originalName);
+
+            name = nameParser.ShortName;
+            assemblyName = nameParser.AssemblyName;
+            isContainerNamespace = nameParser.IsContainerNamespace;
+            containerName = nameParser.ContainerName;
+
+            unboundGenericType = nameParser.UnboundGenericTypeName == null
+                ? this
+                : new NoMetadataHandleType(Cx, nameParser.UnboundGenericTypeName);
+
+            if (nameParser.TypeArguments != null)
+            {
+                thisTypeArguments = nameParser.TypeArguments.Select(t => new NoMetadataHandleType(Cx, t)).ToArray();
+            }
+            else
+            {
+                typeParams = new Lazy<TypeTypeParameter[]>(GenericsHelper.MakeTypeParameters(this, ThisTypeParameterCount));
+            }
+
+            containingType = isContainerNamespace
+                ? null
+                : new NoMetadataHandleType(Cx, containerName);
+
+            Populate();
+        }
+
+        private void Populate()
+        {
+            if (isContainerNamespace &&
+                !ContainingNamespace!.IsGlobalNamespace)
+            {
+                Cx.Populate(ContainingNamespace);
+            }
+
+            Cx.Populate(this);
+        }
+
+        public override bool Equals(object? obj)
+        {
+            return obj is NoMetadataHandleType t && originalName.Equals(t.originalName, StringComparison.Ordinal);
+        }
+
+        public override int GetHashCode()
+        {
+            return originalName.GetHashCode(StringComparison.Ordinal);
+        }
+
+        public override IEnumerable<IExtractionProduct> Contents
+        {
+            get
+            {
+                foreach (var tp in typeParams?.Value ?? Array.Empty<TypeTypeParameter>())
+                    yield return tp;
+
+                foreach (var c in base.Contents)
+                    yield return c;
+
+                var i = 0;
+                foreach (var type in ThisTypeArguments)
+                {
+                    yield return type;
+                    yield return Tuples.cil_type_argument(this, i++, type);
+                }
+            }
+        }
+
+        public override CilTypeKind Kind => CilTypeKind.ValueOrRefType;
+
+        public override string Name => GenericsHelper.GetNonGenericName(name);
+
+        public override Namespace? ContainingNamespace => isContainerNamespace
+            ? containerName == Cx.GlobalNamespace.Name
+                ? Cx.GlobalNamespace
+                : new Namespace(Cx, containerName)
+            : null;
+
+        public override Type? ContainingType => containingType;
+
+        public override Type SourceDeclaration => unboundGenericType;
+
+        public override Type Construct(IEnumerable<Type> typeArguments)
+        {
+            if (TotalTypeParametersCount != typeArguments.Count())
+                throw new InternalError("Mismatched type arguments");
+
+            return Cx.Populate(new ConstructedType(Cx, this, typeArguments));
+        }
+
+        public override void WriteAssemblyPrefix(TextWriter trapFile)
+        {
+            if (!string.IsNullOrWhiteSpace(assemblyName))
+            {
+                var an = new AssemblyName(assemblyName);
+                trapFile.Write(an.Name);
+                trapFile.Write('_');
+                trapFile.Write((an.Version ?? new Version(0, 0, 0, 0)).ToString());
+                trapFile.Write(Type.AssemblyTypeNameSeparator);
+            }
+            else
+            {
+                Cx.WriteAssemblyPrefix(trapFile);
+            }
+        }
+
+        public override void WriteId(TextWriter trapFile, bool inContext)
+        {
+            idWriter.WriteId(trapFile, inContext);
+        }
+
+        public override int ThisTypeParameterCount => unboundGenericType == this
+            ? GenericsHelper.GetGenericTypeParameterCount(name)
+            : thisTypeArguments!.Length;
+
+        public override IEnumerable<Type> TypeParameters => unboundGenericType == this
+            ? GenericsHelper.GetAllTypeParameters(containingType, typeParams!.Value)
+            : GenericArguments;
+
+        public override IEnumerable<Type> ThisTypeArguments => unboundGenericType == this
+            ? base.ThisTypeArguments
+            : thisTypeArguments!;
+
+        public override IEnumerable<Type> ThisGenericArguments => unboundGenericType == this
+            ? typeParams!.Value
+            : thisTypeArguments!;
+    }
+}

csharp/extractor/Semmle.Extraction.CIL/Entities/PointerType.cs

@@ -18,11 +18,7 @@ namespace Semmle.Extraction.CIL.Entities
             return obj is PointerType pt && pointee.Equals(pt.pointee);
         }
 
-
-        public override int GetHashCode()
-        {
-            return pointee.GetHashCode() * 29;
-        }
+        public override int GetHashCode() => HashCode.Combine(pointee, nameof(PointerType));
 
         public override void WriteId(TextWriter trapFile, bool inContext)
         {
@@ -32,13 +28,13 @@ namespace Semmle.Extraction.CIL.Entities
 
         public override string Name => pointee.Name + "*";
 
-        public override Namespace? Namespace => pointee.Namespace;
+        public override Namespace? ContainingNamespace => pointee.ContainingNamespace;
 
         public override Type? ContainingType => pointee.ContainingType;
 
         public override TypeContainer Parent => pointee.Parent;
 
-        public override int ThisTypeParameters => 0;
+        public override int ThisTypeParameterCount => 0;
 
         public override CilTypeKind Kind => CilTypeKind.Pointer;
 
@@ -46,8 +42,6 @@ namespace Semmle.Extraction.CIL.Entities
 
         public override IEnumerable<Type> TypeParameters => throw new NotImplementedException();
 
-        public override IEnumerable<Type> MethodParameters => throw new NotImplementedException();
-
         public override Type Construct(IEnumerable<Type> typeArguments) => throw new NotImplementedException();
 
         public override IEnumerable<IExtractionProduct> Contents

csharp/extractor/Semmle.Extraction.CIL/Entities/PrimitiveType.cs

@@ -18,24 +18,20 @@ namespace Semmle.Extraction.CIL.Entities
             return obj is PrimitiveType pt && typeCode == pt.typeCode;
         }
 
-        public override int GetHashCode()
-        {
-            return 1337 * (int)typeCode;
-        }
+        public override int GetHashCode() => typeCode.GetHashCode();
 
         public override void WriteId(TextWriter trapFile, bool inContext)
         {
-            trapFile.Write("builtin:");
-            trapFile.Write(Name);
+            Type.WritePrimitiveTypeId(trapFile, Name);
         }
 
         public override string Name => typeCode.Id();
 
-        public override Namespace Namespace => Cx.SystemNamespace;
+        public override Namespace ContainingNamespace => Cx.SystemNamespace;
 
         public override Type? ContainingType => null;
 
-        public override int ThisTypeParameters => 0;
+        public override int ThisTypeParameterCount => 0;
 
         public override CilTypeKind Kind => CilTypeKind.ValueOrRefType;
 
@@ -43,8 +39,6 @@ namespace Semmle.Extraction.CIL.Entities
 
         public override IEnumerable<Type> TypeParameters => throw new NotImplementedException();
 
-        public override IEnumerable<Type> MethodParameters => throw new NotImplementedException();
-
         public override Type Construct(IEnumerable<Type> typeArguments) => throw new NotImplementedException();
     }
 }

csharp/extractor/Semmle.Extraction.CIL/Entities/Property.cs

@@ -8,7 +8,7 @@ namespace Semmle.Extraction.CIL.Entities
     /// <summary>
     /// A property.
     /// </summary>
-    internal sealed class Property : LabelledEntity
+    internal sealed class Property : LabelledEntity, ICustomModifierReceiver
     {
         private readonly Handle handle;
         private readonly Type type;
@@ -54,7 +54,15 @@ namespace Semmle.Extraction.CIL.Entities
                 yield return Tuples.metadata_handle(this, Cx.Assembly, MetadataTokens.GetToken(handle));
                 var sig = pd.DecodeSignature(Cx.TypeSignatureDecoder, type);
 
-                yield return Tuples.cil_property(this, type, Cx.ShortName(pd.Name), sig.ReturnType);
+                var name = Cx.ShortName(pd.Name);
+
+                var t = sig.ReturnType;
+                if (t is ModifiedType mt)
+                {
+                    t = mt.Unmodified;
+                    yield return Tuples.cil_custom_modifiers(this, mt.Modifier, mt.IsRequired);
+                }
+                yield return Tuples.cil_property(this, type, name, t);
 
                 var accessors = pd.GetAccessors();
                 if (!accessors.Getter.IsNil)

csharp/extractor/Semmle.Extraction.CIL/Entities/SignatureDecoder.cs

@@ -1,6 +1,7 @@
 using System.Reflection.Metadata;
 using System.Collections.Immutable;
 using System.IO;
+using System.Linq;
 
 namespace Semmle.Extraction.CIL.Entities
 {
@@ -22,7 +23,9 @@ namespace Semmle.Extraction.CIL.Entities
                 elementType.WriteId(trapFile, gc);
                 trapFile.Write('[');
                 for (var i = 1; i < shape.Rank; ++i)
+                {
                     trapFile.Write(',');
+                }
                 trapFile.Write(']');
             }
         }
@@ -136,21 +139,28 @@ namespace Semmle.Extraction.CIL.Entities
         private class Modified : ITypeSignature
         {
             private readonly ITypeSignature unmodifiedType;
+            private readonly ITypeSignature modifier;
+            private readonly bool isRequired;
 
-            public Modified(ITypeSignature unmodifiedType)
+            public Modified(ITypeSignature unmodifiedType, ITypeSignature modifier, bool isRequired)
             {
                 this.unmodifiedType = unmodifiedType;
+                this.modifier = modifier;
+                this.isRequired = isRequired;
             }
 
             public void WriteId(TextWriter trapFile, GenericContext gc)
             {
                 unmodifiedType.WriteId(trapFile, gc);
+                trapFile.Write(isRequired ? " modreq(" : " modopt(");
+                modifier.WriteId(trapFile, gc);
+                trapFile.Write(")");
             }
         }
 
         ITypeSignature ISignatureTypeProvider<ITypeSignature, object>.GetModifiedType(ITypeSignature modifier, ITypeSignature unmodifiedType, bool isRequired)
         {
-            return new Modified(unmodifiedType);
+            return new Modified(unmodifiedType, modifier, isRequired);
         }
 
         private class Pinned : ITypeSignature

csharp/extractor/Semmle.Extraction.CIL/Entities/Type.cs

@@ -1,8 +1,8 @@
 using System.Reflection.Metadata;
 using System.Collections.Generic;
-using Semmle.Util;
 using System.IO;
 using System.Diagnostics.CodeAnalysis;
+using System.Linq;
 
 namespace Semmle.Extraction.CIL.Entities
 {
@@ -12,8 +12,10 @@ namespace Semmle.Extraction.CIL.Entities
     public abstract class Type : TypeContainer, IMember
     {
         public override string IdSuffix => ";cil-type";
+        internal const string AssemblyTypeNameSeparator = "::";
+        internal const string PrimitiveTypePrefix = "builtin" + AssemblyTypeNameSeparator + "System.";
 
-        public sealed override void WriteId(TextWriter trapFile) => WriteId(trapFile, false);
+        protected Type(Context cx) : base(cx) { }
 
         /// <summary>
         /// Find the method in this type matching the name and signature.
@@ -29,29 +31,6 @@ namespace Semmle.Extraction.CIL.Entities
             return null;
         }
 
-        public IEnumerable<Type> TypeArguments
-        {
-            get
-            {
-                if (ContainingType != null)
-                {
-                    foreach (var t in ContainingType.TypeArguments)
-                        yield return t;
-                }
-                foreach (var t in ThisTypeArguments)
-                    yield return t;
-
-            }
-        }
-
-        public virtual IEnumerable<Type> ThisTypeArguments
-        {
-            get
-            {
-                yield break;
-            }
-        }
-
         /// <summary>
         /// Writes the assembly identifier of this type.
         /// </summary>
@@ -67,22 +46,24 @@ namespace Semmle.Extraction.CIL.Entities
         /// </param>
         public abstract void WriteId(TextWriter trapFile, bool inContext);
 
-        public void GetId(TextWriter trapFile, bool inContext)
+        public sealed override void WriteId(TextWriter trapFile) => WriteId(trapFile, false);
+
+        /// <summary>
+        /// Returns the friendly qualified name of types, such as
+        /// ``"System.Collection.Generic.List`1"`` or
+        /// ``"System.Collection.Generic.List<System.Int32>"``.
+        ///
+        /// Note that method/type generic type parameters never show up in the returned name.
+        /// </summary>
+        public string GetQualifiedName()
         {
-            if (inContext)
-                WriteId(trapFile, true);
-            else
-                WriteId(trapFile);
+            using var writer = new StringWriter();
+            WriteId(writer, false);
+            var name = writer.ToString();
+            return name.Substring(name.IndexOf(AssemblyTypeNameSeparator) + 2);
         }
 
-        protected Type(Context cx) : base(cx) { }
-
-        public abstract CilTypeKind Kind
-        {
-            get;
-        }
-
-        public virtual TypeContainer Parent => (TypeContainer?)ContainingType ?? Namespace!;
+        public abstract CilTypeKind Kind { get; }
 
         public override IEnumerable<IExtractionProduct> Contents
         {
@@ -92,35 +73,43 @@ namespace Semmle.Extraction.CIL.Entities
             }
         }
 
-        /// <summary>
-        /// Whether this type is visible outside the current assembly.
-        /// </summary>
-        public virtual bool IsVisible => true;
-
         public abstract string Name { get; }
 
-        public abstract Namespace? Namespace { get; }
+        public abstract Namespace? ContainingNamespace { get; }
 
         public abstract Type? ContainingType { get; }
 
+        public virtual TypeContainer Parent => (TypeContainer?)ContainingType ?? ContainingNamespace!;
+
         public abstract Type Construct(IEnumerable<Type> typeArguments);
 
         /// <summary>
-        /// The number of type arguments, or 0 if this isn't generic.
-        /// The containing type may also have type arguments.
+        /// Returns the type arguments of constructed types. For non-constructed types it returns an
+        /// empty collection.
         /// </summary>
-        public abstract int ThisTypeParameters { get; }
+        public virtual IEnumerable<Type> ThisTypeArguments
+        {
+            get
+            {
+                yield break;
+            }
+        }
 
         /// <summary>
-        /// The total number of type parameters (including parent types).
+        /// The number of type parameters for non-constructed generic types, the number of type arguments
+        /// for constructed types, or 0.
+        /// </summary>
+        public abstract int ThisTypeParameterCount { get; }
+
+        /// <summary>
+        /// The total number of type parameters/type arguments (including parent types).
         /// This is used for internal consistency checking only.
         /// </summary>
-        public int TotalTypeParametersCheck =>
-            ContainingType == null ? ThisTypeParameters : ThisTypeParameters + ContainingType.TotalTypeParametersCheck;
+        public int TotalTypeParametersCount =>
+            ThisTypeParameterCount + (ContainingType?.TotalTypeParametersCount ?? 0);
 
         /// <summary>
-        /// Returns all bound/unbound generic arguments
-        /// of a constructed/unbound generic type.
+        /// Returns all bound/unbound generic arguments of a constructed/unbound generic type.
         /// </summary>
         public virtual IEnumerable<Type> ThisGenericArguments
         {
@@ -139,6 +128,7 @@ namespace Semmle.Extraction.CIL.Entities
                     foreach (var t in ContainingType.GenericArguments)
                         yield return t;
                 }
+
                 foreach (var t in ThisGenericArguments)
                     yield return t;
             }
@@ -146,12 +136,34 @@ namespace Semmle.Extraction.CIL.Entities
 
         public virtual Type SourceDeclaration => this;
 
-        public void PrimitiveTypeId(TextWriter trapFile)
+        public static void WritePrimitiveTypeId(TextWriter trapFile, string name)
         {
-            trapFile.Write("builtin:");
-            trapFile.Write(Name);
+            trapFile.Write(PrimitiveTypePrefix);
+            trapFile.Write(name);
         }
 
+        private static readonly Dictionary<string, PrimitiveTypeCode> primitiveTypeCodeMapping = new Dictionary<string, PrimitiveTypeCode>
+        {
+            {"Boolean", PrimitiveTypeCode.Boolean},
+            {"Object", PrimitiveTypeCode.Object},
+            {"Byte", PrimitiveTypeCode.Byte},
+            {"SByte", PrimitiveTypeCode.SByte},
+            {"Int16", PrimitiveTypeCode.Int16},
+            {"UInt16", PrimitiveTypeCode.UInt16},
+            {"Int32", PrimitiveTypeCode.Int32},
+            {"UInt32", PrimitiveTypeCode.UInt32},
+            {"Int64", PrimitiveTypeCode.Int64},
+            {"UInt64", PrimitiveTypeCode.UInt64},
+            {"Single", PrimitiveTypeCode.Single},
+            {"Double", PrimitiveTypeCode.Double},
+            {"String", PrimitiveTypeCode.String},
+            {"Void", PrimitiveTypeCode.Void},
+            {"IntPtr", PrimitiveTypeCode.IntPtr},
+            {"UIntPtr", PrimitiveTypeCode.UIntPtr},
+            {"Char", PrimitiveTypeCode.Char},
+            {"TypedReference", PrimitiveTypeCode.TypedReference}
+        };
+
         /// <summary>
         /// Gets the primitive type corresponding to this type, if possible.
         /// </summary>
@@ -171,72 +183,20 @@ namespace Semmle.Extraction.CIL.Entities
 
         private bool TryGetPrimitiveTypeCode(out PrimitiveTypeCode code)
         {
-            if (ContainingType == null && Namespace?.Name == Cx.SystemNamespace.Name)
+            if (ContainingType == null &&
+                ContainingNamespace?.Name == Cx.SystemNamespace.Name &&
+                primitiveTypeCodeMapping.TryGetValue(Name, out code))
             {
-                switch (Name)
-                {
-                    case "Boolean":
-                        code = PrimitiveTypeCode.Boolean;
-                        return true;
-                    case "Object":
-                        code = PrimitiveTypeCode.Object;
-                        return true;
-                    case "Byte":
-                        code = PrimitiveTypeCode.Byte;
-                        return true;
-                    case "SByte":
-                        code = PrimitiveTypeCode.SByte;
-                        return true;
-                    case "Int16":
-                        code = PrimitiveTypeCode.Int16;
-                        return true;
-                    case "UInt16":
-                        code = PrimitiveTypeCode.UInt16;
-                        return true;
-                    case "Int32":
-                        code = PrimitiveTypeCode.Int32;
-                        return true;
-                    case "UInt32":
-                        code = PrimitiveTypeCode.UInt32;
-                        return true;
-                    case "Int64":
-                        code = PrimitiveTypeCode.Int64;
-                        return true;
-                    case "UInt64":
-                        code = PrimitiveTypeCode.UInt64;
-                        return true;
-                    case "Single":
-                        code = PrimitiveTypeCode.Single;
-                        return true;
-                    case "Double":
-                        code = PrimitiveTypeCode.Double;
-                        return true;
-                    case "String":
-                        code = PrimitiveTypeCode.String;
-                        return true;
-                    case "Void":
-                        code = PrimitiveTypeCode.Void;
-                        return true;
-                    case "IntPtr":
-                        code = PrimitiveTypeCode.IntPtr;
-                        return true;
-                    case "UIntPtr":
-                        code = PrimitiveTypeCode.UIntPtr;
-                        return true;
-                    case "Char":
-                        code = PrimitiveTypeCode.Char;
-                        return true;
-                    case "TypedReference":
-                        code = PrimitiveTypeCode.TypedReference;
-                        return true;
-                }
+                return true;
             }
 
-            code = default(PrimitiveTypeCode);
+            code = default;
             return false;
         }
 
-        protected bool IsPrimitiveType => TryGetPrimitiveTypeCode(out _);
+        protected internal bool IsPrimitiveType => TryGetPrimitiveTypeCode(out _);
+
+        public sealed override IEnumerable<Type> MethodParameters => Enumerable.Empty<Type>();
 
         public static Type DecodeType(GenericContext gc, TypeSpecificationHandle handle) =>
             gc.Cx.MdReader.GetTypeSpecification(handle).DecodeSignature(gc.Cx.TypeSignatureDecoder, gc);

csharp/extractor/Semmle.Extraction.CIL/Entities/TypeDefinitionType.cs

@@ -6,6 +6,7 @@ using System.Linq;
 using System.Collections.Generic;
 using System.Reflection;
 using System.IO;
+using System.Reflection.Metadata.Ecma335;
 
 namespace Semmle.Extraction.CIL.Entities
 {
@@ -14,11 +15,15 @@ namespace Semmle.Extraction.CIL.Entities
     /// </summary>
     public sealed class TypeDefinitionType : Type
     {
-        private readonly Handle handle;
+        private readonly TypeDefinitionHandle handle;
         private readonly TypeDefinition td;
+        private readonly Lazy<IEnumerable<TypeTypeParameter>> typeParams;
+        private readonly Type? declType;
+        private readonly NamedTypeIdWriter idWriter;
 
         public TypeDefinitionType(Context cx, TypeDefinitionHandle handle) : base(cx)
         {
+            idWriter = new NamedTypeIdWriter(this);
             td = cx.MdReader.GetTypeDefinition(handle);
             this.handle = handle;
 
@@ -39,66 +44,22 @@ namespace Semmle.Extraction.CIL.Entities
 
         public override void WriteId(TextWriter trapFile, bool inContext)
         {
-            if (IsPrimitiveType)
-            {
-                PrimitiveTypeId(trapFile);
-                return;
-            }
-
-            var name = Cx.GetString(td.Name);
-
-            if (ContainingType != null)
-            {
-                ContainingType.GetId(trapFile, inContext);
-                trapFile.Write('.');
-            }
-            else
-            {
-                WriteAssemblyPrefix(trapFile);
-
-                var ns = Namespace;
-                if (!ns.IsGlobalNamespace)
-                {
-                    ns.WriteId(trapFile);
-                    trapFile.Write('.');
-                }
-            }
-
-            trapFile.Write(name);
+            idWriter.WriteId(trapFile, inContext);
         }
 
-        public override string Name
-        {
-            get
-            {
-                var name = Cx.GetString(td.Name);
-                var tick = name.IndexOf('`');
-                return tick == -1 ? name : name.Substring(0, tick);
-            }
-        }
+        public override string Name => GenericsHelper.GetNonGenericName(td.Name, Cx.MdReader);
 
-        public override Namespace Namespace => Cx.Create(td.NamespaceDefinition);
-
-        private readonly Type? declType;
+        public override Namespace ContainingNamespace => Cx.Create(td.NamespaceDefinition);
 
         public override Type? ContainingType => declType;
 
-        public override int ThisTypeParameters
-        {
-            get
-            {
-                var containingType = td.GetDeclaringType();
-                var parentTypeParameters = containingType.IsNil ? 0 :
-                    Cx.MdReader.GetTypeDefinition(containingType).GetGenericParameters().Count;
-
-                return td.GetGenericParameters().Count - parentTypeParameters;
-            }
-        }
-
         public override CilTypeKind Kind => CilTypeKind.ValueOrRefType;
 
         public override Type Construct(IEnumerable<Type> typeArguments)
         {
+            if (TotalTypeParametersCount != typeArguments.Count())
+                throw new InternalError("Mismatched type arguments");
+
             return Cx.Populate(new ConstructedType(Cx, this, typeArguments));
         }
 
@@ -108,52 +69,50 @@ namespace Semmle.Extraction.CIL.Entities
             if (ct is null)
                 Cx.WriteAssemblyPrefix(trapFile);
             else if (IsPrimitiveType)
-                trapFile.Write("builtin:");
+                trapFile.Write(Type.PrimitiveTypePrefix);
             else
                 ct.WriteAssemblyPrefix(trapFile);
         }
 
         private IEnumerable<TypeTypeParameter> MakeTypeParameters()
         {
-            if (ThisTypeParameters == 0)
+            if (ThisTypeParameterCount == 0)
                 return Enumerable.Empty<TypeTypeParameter>();
 
-            var newTypeParams = new TypeTypeParameter[ThisTypeParameters];
+            var newTypeParams = new TypeTypeParameter[ThisTypeParameterCount];
             var genericParams = td.GetGenericParameters();
             var toSkip = genericParams.Count - newTypeParams.Length;
 
             // Two-phase population because type parameters can be mutually dependent
             for (var i = 0; i < newTypeParams.Length; ++i)
-                newTypeParams[i] = Cx.Populate(new TypeTypeParameter(this, this, i));
+                newTypeParams[i] = Cx.Populate(new TypeTypeParameter(this, i));
             for (var i = 0; i < newTypeParams.Length; ++i)
                 newTypeParams[i].PopulateHandle(genericParams[i + toSkip]);
             return newTypeParams;
         }
 
-        private readonly Lazy<IEnumerable<TypeTypeParameter>> typeParams;
-
-        public override IEnumerable<Type> MethodParameters => Enumerable.Empty<Type>();
-
-        public override IEnumerable<Type> TypeParameters
+        public override int ThisTypeParameterCount
         {
             get
             {
-                if (declType != null)
-                {
-                    foreach (var t in declType.TypeParameters)
-                        yield return t;
-                }
+                var containingType = td.GetDeclaringType();
+                var parentTypeParameters = containingType.IsNil
+                    ? 0
+                    : Cx.MdReader.GetTypeDefinition(containingType).GetGenericParameters().Count;
 
-                foreach (var t in typeParams.Value)
-                    yield return t;
+                return td.GetGenericParameters().Count - parentTypeParameters;
             }
         }
 
+        public override IEnumerable<Type> TypeParameters => GenericsHelper.GetAllTypeParameters(declType, typeParams!.Value);
+
+        public override IEnumerable<Type> ThisGenericArguments => typeParams.Value;
+
         public override IEnumerable<IExtractionProduct> Contents
         {
             get
             {
-                yield return Tuples.metadata_handle(this, Cx.Assembly, handle.GetHashCode());
+                yield return Tuples.metadata_handle(this, Cx.Assembly, MetadataTokens.GetToken(handle));
 
                 foreach (var c in base.Contents) yield return c;
 

csharp/extractor/Semmle.Extraction.CIL/Entities/TypeParameter.cs

@@ -16,11 +16,11 @@ namespace Semmle.Extraction.CIL.Entities
             this.gc = gc;
         }
 
-        public override Namespace? Namespace => null;
+        public override Namespace? ContainingNamespace => null;
 
         public override Type? ContainingType => null;
 
-        public override int ThisTypeParameters => 0;
+        public override int ThisTypeParameterCount => 0;
 
         public override CilTypeKind Kind => CilTypeKind.TypeParameter;
 

csharp/extractor/Semmle.Extraction.CIL/Entities/TypeReferenceType.cs

@@ -16,12 +16,14 @@ namespace Semmle.Extraction.CIL.Entities
         private readonly TypeReferenceHandle handle;
         private readonly TypeReference tr;
         private readonly Lazy<TypeTypeParameter[]> typeParams;
+        private readonly NamedTypeIdWriter idWriter;
 
         public TypeReferenceType(Context cx, TypeReferenceHandle handle) : base(cx)
         {
-            this.typeParams = new Lazy<TypeTypeParameter[]>(MakeTypeParameters);
+            this.idWriter = new NamedTypeIdWriter(this);
             this.handle = handle;
             this.tr = cx.MdReader.GetTypeReference(handle);
+            this.typeParams = new Lazy<TypeTypeParameter[]>(GenericsHelper.MakeTypeParameters(this, ThisTypeParameterCount));
         }
 
         public override bool Equals(object? obj)
@@ -34,16 +36,6 @@ namespace Semmle.Extraction.CIL.Entities
             return handle.GetHashCode();
         }
 
-        private TypeTypeParameter[] MakeTypeParameters()
-        {
-            var newTypeParams = new TypeTypeParameter[ThisTypeParameters];
-            for (var i = 0; i < newTypeParams.Length; ++i)
-            {
-                newTypeParams[i] = new TypeTypeParameter(this, this, i);
-            }
-            return newTypeParams;
-        }
-
         public override IEnumerable<IExtractionProduct> Contents
         {
             get
@@ -56,45 +48,17 @@ namespace Semmle.Extraction.CIL.Entities
             }
         }
 
-        public override string Name
-        {
-            get
-            {
-                var name = Cx.GetString(tr.Name);
-                var tick = name.IndexOf('`');
-                return tick == -1 ? name : name.Substring(0, tick);
-            }
-        }
+        public override string Name => GenericsHelper.GetNonGenericName(tr.Name, Cx.MdReader);
 
-        public override Namespace Namespace => Cx.CreateNamespace(tr.Namespace);
-
-        public override int ThisTypeParameters
-        {
-            get
-            {
-                // Parse the name
-                var name = Cx.GetString(tr.Name);
-                var tick = name.IndexOf('`');
-                return tick == -1 ? 0 : int.Parse(name.Substring(tick + 1));
-            }
-        }
-
-        public override IEnumerable<Type> ThisGenericArguments
-        {
-            get
-            {
-                foreach (var t in typeParams.Value)
-                    yield return t;
-            }
-        }
+        public override Namespace ContainingNamespace => Cx.CreateNamespace(tr.Namespace);
 
         public override Type? ContainingType
         {
             get
             {
-                if (tr.ResolutionScope.Kind == HandleKind.TypeReference)
-                    return (Type)Cx.Create((TypeReferenceHandle)tr.ResolutionScope);
-                return null;
+                return tr.ResolutionScope.Kind == HandleKind.TypeReference
+                    ? (Type)Cx.Create((TypeReferenceHandle)tr.ResolutionScope)
+                    : null;
             }
         }
 
@@ -112,7 +76,7 @@ namespace Semmle.Extraction.CIL.Entities
                     trapFile.Write(Cx.GetString(assemblyDef.Name));
                     trapFile.Write('_');
                     trapFile.Write(assemblyDef.Version.ToString());
-                    trapFile.Write("::");
+                    trapFile.Write(Entities.Type.AssemblyTypeNameSeparator);
                     break;
                 default:
                     Cx.WriteAssemblyPrefix(trapFile);
@@ -120,43 +84,20 @@ namespace Semmle.Extraction.CIL.Entities
             }
         }
 
-        public override IEnumerable<Type> TypeParameters => typeParams.Value;
+        public override int ThisTypeParameterCount => GenericsHelper.GetGenericTypeParameterCount(tr.Name, Cx.MdReader);
 
-        public override IEnumerable<Type> MethodParameters => throw new InternalError("This type does not have method parameters");
+        public override IEnumerable<Type> TypeParameters => GenericsHelper.GetAllTypeParameters(ContainingType, typeParams!.Value);
+
+        public override IEnumerable<Type> ThisGenericArguments => typeParams.Value;
 
         public override void WriteId(TextWriter trapFile, bool inContext)
         {
-            if (IsPrimitiveType)
-            {
-                PrimitiveTypeId(trapFile);
-                return;
-            }
-
-            var ct = ContainingType;
-            if (ct != null)
-            {
-                ct.GetId(trapFile, inContext);
-            }
-            else
-            {
-                if (tr.ResolutionScope.Kind == HandleKind.AssemblyReference)
-                {
-                    WriteAssemblyPrefix(trapFile);
-                }
-
-                if (!Namespace.IsGlobalNamespace)
-                {
-                    Namespace.WriteId(trapFile);
-                }
-            }
-
-            trapFile.Write('.');
-            trapFile.Write(Cx.GetString(tr.Name));
+            idWriter.WriteId(trapFile, inContext);
         }
 
         public override Type Construct(IEnumerable<Type> typeArguments)
         {
-            if (TotalTypeParametersCheck != typeArguments.Count())
+            if (TotalTypeParametersCount != typeArguments.Count())
                 throw new InternalError("Mismatched type arguments");
 
             return Cx.Populate(new ConstructedType(Cx, this, typeArguments));

csharp/extractor/Semmle.Extraction.CIL/Entities/TypeSignatureDecoder.cs

@@ -35,7 +35,7 @@ namespace Semmle.Extraction.CIL.Entities
             genericContext.GetGenericTypeParameter(index);
 
         Type ISignatureTypeProvider<Type, GenericContext>.GetModifiedType(Type modifier, Type unmodifiedType, bool isRequired) =>
-            unmodifiedType; // !! Not implemented properly
+            new ModifiedType(cx, unmodifiedType, modifier, isRequired);
 
         Type ISignatureTypeProvider<Type, GenericContext>.GetPinnedType(Type elementType) =>
             cx.Populate(new PointerType(cx, elementType));

csharp/extractor/Semmle.Extraction.CIL/Entities/TypeTypeParameter.cs

@@ -9,7 +9,7 @@ namespace Semmle.Extraction.CIL.Entities
         private readonly Type type;
         private readonly int index;
 
-        public TypeTypeParameter(GenericContext cx, Type t, int i) : base(cx)
+        public TypeTypeParameter(Type t, int i) : base(t)
         {
             index = i;
             type = t;
@@ -37,8 +37,6 @@ namespace Semmle.Extraction.CIL.Entities
 
         public override IEnumerable<Type> TypeParameters => Enumerable.Empty<Type>();
 
-        public override IEnumerable<Type> MethodParameters => Enumerable.Empty<Type>();
-
         public override IEnumerable<IExtractionProduct> Contents
         {
             get

csharp/extractor/Semmle.Extraction.CIL/GenericContext.cs

@@ -17,7 +17,7 @@ namespace Semmle.Extraction.CIL
         }
 
         /// <summary>
-        /// The list of generic type parameters, including type parameters of
+        /// The list of generic type parameters/arguments, including type parameters/arguments of
         /// containing types.
         /// </summary>
         public abstract IEnumerable<Entities.Type> TypeParameters { get; }

csharp/extractor/Semmle.Extraction.CIL/ICustomModifierReceiver.cs

@@ -0,0 +1,6 @@
+namespace Semmle.Extraction.CIL
+{
+    internal interface ICustomModifierReceiver
+    {
+    }
+}

csharp/extractor/Semmle.Extraction.CIL/Tuples.cs

@@ -188,6 +188,9 @@ namespace Semmle.Extraction.CIL
         internal static Tuple cil_virtual(Method method) =>
             new Tuple("cil_virtual", method);
 
+        internal static Tuple cil_custom_modifiers(ICustomModifierReceiver receiver, Type modifier, bool isRequired) =>
+            new Tuple("cil_custom_modifiers", receiver, modifier, isRequired ? 1 : 0);
+
         internal static Tuple containerparent(Folder parent, IFileOrFolder child) =>
             new Tuple("containerparent", parent, child);
 

csharp/extractor/Semmle.Extraction.CSharp/Entities/Attribute.cs

@@ -66,12 +66,25 @@ namespace Semmle.Extraction.CSharp.Entities
 
         private void ExtractArguments(TextWriter trapFile)
         {
+            var ctorArguments = attributeSyntax?.ArgumentList?.Arguments.Where(a => a.NameEquals == null).ToList();
+
             var childIndex = 0;
-            foreach (var constructorArgument in symbol.ConstructorArguments)
+            for (var i = 0; i < symbol.ConstructorArguments.Length; i++)
             {
+                var constructorArgument = symbol.ConstructorArguments[i];
+                var paramName = symbol.AttributeConstructor?.Parameters[i].Name;
+                var argSyntax = ctorArguments?.SingleOrDefault(a => a.NameColon != null && a.NameColon.Name.Identifier.Text == paramName);
+
+                if (argSyntax == null &&                            // couldn't find named argument
+                    ctorArguments?.Count > childIndex &&            // there're more arguments
+                    ctorArguments[childIndex].NameColon == null)    // the argument is positional
+                {
+                    argSyntax = ctorArguments[childIndex];
+                }
+
                 CreateExpressionFromArgument(
                     constructorArgument,
-                    attributeSyntax?.ArgumentList.Arguments[childIndex].Expression,
+                    argSyntax?.Expression,
                     this,
                     childIndex++);
             }

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Factory.cs

@@ -84,6 +84,9 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
                     case SyntaxKind.ObjectCreationExpression:
                         return ExplicitObjectCreation.Create(info);
 
+                    case SyntaxKind.ImplicitObjectCreationExpression:
+                        return ImplicitObjectCreation.Create(info);
+
                     case SyntaxKind.ArrayCreationExpression:
                         return NormalArrayCreation.Create(info);
 
@@ -179,7 +182,7 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
                         return ImplicitArrayCreation.Create(info);
 
                     case SyntaxKind.AnonymousObjectCreationExpression:
-                        return ImplicitObjectCreation.Create(info);
+                        return AnonymousObjectCreation.Create(info);
 
                     case SyntaxKind.ComplexElementInitializerExpression:
                         return CollectionInitializer.Create(info);

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ObjectCreation.cs

@@ -1,137 +0,0 @@
-using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.CSharp;
-using Microsoft.CodeAnalysis.CSharp.Syntax;
-using Semmle.Extraction.Entities;
-using Semmle.Extraction.Kinds;
-using System.IO;
-using System.Linq;
-
-namespace Semmle.Extraction.CSharp.Entities.Expressions
-{
-    internal abstract class ObjectCreation<TExpressionSyntax> : Expression<TExpressionSyntax>
-        where TExpressionSyntax : ExpressionSyntax
-    {
-        protected ObjectCreation(ExpressionNodeInfo info)
-            : base(info) { }
-    }
-
-    // new Foo(...) { ... }.
-    internal class ExplicitObjectCreation : ObjectCreation<ObjectCreationExpressionSyntax>
-    {
-        private static bool IsDynamicObjectCreation(Context cx, ObjectCreationExpressionSyntax node)
-        {
-            return node.ArgumentList != null && node.ArgumentList.Arguments.Any(arg => IsDynamic(cx, arg.Expression));
-        }
-
-        private static ExprKind GetKind(Context cx, ObjectCreationExpressionSyntax node)
-        {
-            var si = cx.GetModel(node).GetSymbolInfo(node.Type);
-            return Entities.Type.IsDelegate(si.Symbol as INamedTypeSymbol) ? ExprKind.EXPLICIT_DELEGATE_CREATION : ExprKind.OBJECT_CREATION;
-        }
-
-        private ExplicitObjectCreation(ExpressionNodeInfo info)
-            : base(info.SetKind(GetKind(info.Context, (ObjectCreationExpressionSyntax)info.Node))) { }
-
-        public static Expression Create(ExpressionNodeInfo info) => new ExplicitObjectCreation(info).TryPopulate();
-
-        protected override void PopulateExpression(TextWriter trapFile)
-        {
-            if (Syntax.ArgumentList != null)
-            {
-                PopulateArguments(trapFile, Syntax.ArgumentList, 0);
-            }
-
-            var target = cx.GetModel(Syntax).GetSymbolInfo(Syntax);
-            var method = (IMethodSymbol)target.Symbol;
-
-            if (method != null)
-            {
-                trapFile.expr_call(this, Method.Create(cx, method));
-            }
-
-            if (IsDynamicObjectCreation(cx, Syntax))
-            {
-                var name = GetDynamicName(Syntax.Type);
-                if (name.HasValue)
-                    trapFile.dynamic_member_name(this, name.Value.Text);
-                else
-                    cx.ModelError(Syntax, "Unable to get name for dynamic object creation.");
-            }
-
-            if (Syntax.Initializer != null)
-            {
-                switch (Syntax.Initializer.Kind())
-                {
-                    case SyntaxKind.CollectionInitializerExpression:
-                        CollectionInitializer.Create(new ExpressionNodeInfo(cx, Syntax.Initializer, this, -1) { Type = Type });
-                        break;
-                    case SyntaxKind.ObjectInitializerExpression:
-                        ObjectInitializer.Create(new ExpressionNodeInfo(cx, Syntax.Initializer, this, -1) { Type = Type });
-                        break;
-                    default:
-                        cx.ModelError("Unhandled initializer in object creation");
-                        break;
-                }
-            }
-
-            TypeMention.Create(cx, Syntax.Type, this, Type);
-        }
-
-        private static SyntaxToken? GetDynamicName(CSharpSyntaxNode name)
-        {
-            switch (name.Kind())
-            {
-                case SyntaxKind.IdentifierName:
-                    return ((IdentifierNameSyntax)name).Identifier;
-                case SyntaxKind.GenericName:
-                    return ((GenericNameSyntax)name).Identifier;
-                case SyntaxKind.QualifiedName:
-                    // We ignore any qualifiers, for now
-                    return GetDynamicName(((QualifiedNameSyntax)name).Right);
-                default:
-                    return null;
-            }
-        }
-    }
-
-    internal class ImplicitObjectCreation : ObjectCreation<AnonymousObjectCreationExpressionSyntax>
-    {
-        public ImplicitObjectCreation(ExpressionNodeInfo info)
-            : base(info.SetKind(ExprKind.OBJECT_CREATION)) { }
-
-        public static Expression Create(ExpressionNodeInfo info) =>
-            new ImplicitObjectCreation(info).TryPopulate();
-
-        protected override void PopulateExpression(TextWriter trapFile)
-        {
-            var target = cx.GetSymbolInfo(Syntax);
-            var method = (IMethodSymbol)target.Symbol;
-
-            if (method != null)
-            {
-                trapFile.expr_call(this, Method.Create(cx, method));
-            }
-            var child = 0;
-
-            var objectInitializer = Syntax.Initializers.Any() ?
-                new Expression(new ExpressionInfo(cx, Type, Location, ExprKind.OBJECT_INIT, this, -1, false, null)) :
-                null;
-
-            foreach (var init in Syntax.Initializers)
-            {
-                // Create an "assignment"
-                var property = cx.GetModel(init).GetDeclaredSymbol(init);
-                var propEntity = Property.Create(cx, property);
-                var type = Entities.Type.Create(cx, property.GetAnnotatedType());
-                var loc = cx.Create(init.GetLocation());
-
-                var assignment = new Expression(new ExpressionInfo(cx, type, loc, ExprKind.SIMPLE_ASSIGN, objectInitializer, child++, false, null));
-                Create(cx, init.Expression, assignment, 0);
-                Property.Create(cx, property);
-
-                var access = new Expression(new ExpressionInfo(cx, type, loc, ExprKind.PROPERTY_ACCESS, assignment, 1, false, null));
-                trapFile.expr_access(access, propEntity);
-            }
-        }
-    }
-}

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ObjectCreation/AnonymousObjectCreation.cs

@@ -0,0 +1,50 @@
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Semmle.Extraction.Entities;
+using Semmle.Extraction.Kinds;
+using System.IO;
+
+namespace Semmle.Extraction.CSharp.Entities.Expressions
+{
+    internal class AnonymousObjectCreation : Expression<AnonymousObjectCreationExpressionSyntax>
+    {
+        public AnonymousObjectCreation(ExpressionNodeInfo info)
+            : base(info.SetKind(ExprKind.OBJECT_CREATION)) { }
+
+        public static Expression Create(ExpressionNodeInfo info) =>
+            new AnonymousObjectCreation(info).TryPopulate();
+
+        protected override void PopulateExpression(TextWriter trapFile)
+        {
+            var target = cx.GetSymbolInfo(Syntax);
+            var method = (IMethodSymbol)target.Symbol;
+
+            if (method != null)
+            {
+                trapFile.expr_call(this, Method.Create(cx, method));
+            }
+            var child = 0;
+
+            var objectInitializer = Syntax.Initializers.Any() ?
+                new Expression(new ExpressionInfo(cx, Type, Location, ExprKind.OBJECT_INIT, this, -1, false, null)) :
+                null;
+
+            foreach (var init in Syntax.Initializers)
+            {
+                // Create an "assignment"
+                var property = cx.GetModel(init).GetDeclaredSymbol(init);
+                var propEntity = Property.Create(cx, property);
+                var type = Entities.Type.Create(cx, property.GetAnnotatedType());
+                var loc = cx.Create(init.GetLocation());
+
+                var assignment = new Expression(new ExpressionInfo(cx, type, loc, ExprKind.SIMPLE_ASSIGN, objectInitializer, child++, false, null));
+                Create(cx, init.Expression, assignment, 0);
+                Property.Create(cx, property);
+
+                var access = new Expression(new ExpressionInfo(cx, type, loc, ExprKind.PROPERTY_ACCESS, assignment, 1, false, null));
+                trapFile.expr_access(access, propEntity);
+            }
+        }
+    }
+}

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ObjectCreation/BaseObjectCreation.cs

@@ -0,0 +1,75 @@
+using System.IO;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Semmle.Extraction.Kinds;
+
+namespace Semmle.Extraction.CSharp.Entities.Expressions
+{
+    internal abstract class BaseObjectCreation<TExpressionSyntax> : Expression<TExpressionSyntax>
+        where TExpressionSyntax : BaseObjectCreationExpressionSyntax
+    {
+        protected BaseObjectCreation(ExpressionNodeInfo info)
+            : base(info.SetKind(GetKind(info.Context, (BaseObjectCreationExpressionSyntax)info.Node)))
+        {
+        }
+
+        protected override void PopulateExpression(TextWriter trapFile)
+        {
+            if (Syntax.ArgumentList != null)
+            {
+                PopulateArguments(trapFile, Syntax.ArgumentList, 0);
+            }
+
+            var target = cx.GetModel(Syntax).GetSymbolInfo(Syntax);
+            if (target.Symbol is IMethodSymbol method)
+            {
+                trapFile.expr_call(this, Method.Create(cx, method));
+            }
+
+            if (IsDynamicObjectCreation(cx, Syntax))
+            {
+                if (cx.GetModel(Syntax).GetTypeInfo(Syntax).Type is INamedTypeSymbol type &&
+                    !string.IsNullOrEmpty(type.Name))
+                {
+                    trapFile.dynamic_member_name(this, type.Name);
+                }
+                else
+                {
+                    cx.ModelError(Syntax, "Unable to get name for dynamic object creation.");
+                }
+            }
+
+            if (Syntax.Initializer != null)
+            {
+                switch (Syntax.Initializer.Kind())
+                {
+                    case SyntaxKind.CollectionInitializerExpression:
+                        CollectionInitializer.Create(new ExpressionNodeInfo(cx, Syntax.Initializer, this, -1) { Type = Type });
+                        break;
+                    case SyntaxKind.ObjectInitializerExpression:
+                        ObjectInitializer.Create(new ExpressionNodeInfo(cx, Syntax.Initializer, this, -1) { Type = Type });
+                        break;
+                    default:
+                        cx.ModelError("Unhandled initializer in object creation");
+                        break;
+                }
+            }
+        }
+
+        private static ExprKind GetKind(Context cx, BaseObjectCreationExpressionSyntax node)
+        {
+            var type = cx.GetModel(node).GetTypeInfo(node).Type;
+            return Entities.Type.IsDelegate(type as INamedTypeSymbol)
+                ? ExprKind.EXPLICIT_DELEGATE_CREATION
+                : ExprKind.OBJECT_CREATION;
+        }
+
+        private static bool IsDynamicObjectCreation(Context cx, BaseObjectCreationExpressionSyntax node)
+        {
+            return node.ArgumentList != null &&
+                node.ArgumentList.Arguments.Any(arg => IsDynamic(cx, arg.Expression));
+        }
+    }
+}

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ObjectCreation/ExplicitObjectCreation.cs

@@ -0,0 +1,20 @@
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using System.IO;
+
+namespace Semmle.Extraction.CSharp.Entities.Expressions
+{
+    // new Foo(...) { ... }.
+    internal class ExplicitObjectCreation : BaseObjectCreation<ObjectCreationExpressionSyntax>
+    {
+        private ExplicitObjectCreation(ExpressionNodeInfo info) : base(info) { }
+
+        public static Expression Create(ExpressionNodeInfo info) => new ExplicitObjectCreation(info).TryPopulate();
+
+        protected override void PopulateExpression(TextWriter trapFile)
+        {
+            base.PopulateExpression(trapFile);
+
+            TypeMention.Create(cx, Syntax.Type, this, Type);
+        }
+    }
+}

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ObjectCreation/ImplicitObjectCreation.cs

@@ -0,0 +1,19 @@
+using System.IO;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+
+namespace Semmle.Extraction.CSharp.Entities.Expressions
+{
+    internal class ImplicitObjectCreation : BaseObjectCreation<ImplicitObjectCreationExpressionSyntax>
+    {
+        private ImplicitObjectCreation(ExpressionNodeInfo info) : base(info) { }
+
+        public static Expression Create(ExpressionNodeInfo info) => new ImplicitObjectCreation(info).TryPopulate();
+
+        protected override void PopulateExpression(TextWriter trapFile)
+        {
+            base.PopulateExpression(trapFile);
+
+            trapFile.implicitly_typed_object_creation(this);
+        }
+    }
+}

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

@@ -87,6 +87,11 @@ namespace Semmle.Extraction.CSharp.Entities
                 foreach (var l in Locations)
                     trapFile.type_location(this, l);
             }
+
+            if (symbol.IsAnonymousType)
+            {
+                trapFile.anonymous_types(this);
+            }
         }
 
         private readonly Lazy<Type[]> typeArgumentsLazy;

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

@@ -271,6 +271,11 @@ namespace Semmle.Extraction.CSharp
             trapFile.WriteTuple("extend", type, super);
         }
 
+        internal static void anonymous_types(this TextWriter trapFile, Type type)
+        {
+            trapFile.WriteTuple("anonymous_types", type);
+        }
+
         internal static void field_location(this TextWriter trapFile, Field field, Location location)
         {
             trapFile.WriteTuple("field_location", field, location);
@@ -301,6 +306,11 @@ namespace Semmle.Extraction.CSharp
             trapFile.WriteTuple("implicitly_typed_array_creation", array);
         }
 
+        internal static void implicitly_typed_object_creation(this TextWriter trapFile, Expression expression)
+        {
+            trapFile.WriteTuple("implicitly_typed_object_creation", expression);
+        }
+
         internal static void indexer_location(this TextWriter trapFile, Indexer indexer, Location location)
         {
             trapFile.WriteTuple("indexer_location", indexer, location);

csharp/extractor/Semmle.Extraction/AssemblyScope.cs

@@ -0,0 +1,30 @@
+using Microsoft.CodeAnalysis;
+
+namespace Semmle.Extraction
+{
+    /// <summary>
+    /// The scope of symbols in an assembly.
+    /// </summary>
+    public class AssemblyScope : IExtractionScope
+    {
+        private readonly IAssemblySymbol assembly;
+        private readonly string filepath;
+
+        public AssemblyScope(IAssemblySymbol symbol, string path, bool isOutput)
+        {
+            assembly = symbol;
+            filepath = path;
+            IsGlobalScope = isOutput;
+        }
+
+        public bool IsGlobalScope { get; }
+
+        public bool InFileScope(string path) => path == filepath;
+
+        public bool InScope(ISymbol symbol) =>
+            SymbolEqualityComparer.Default.Equals(symbol.ContainingAssembly, assembly) ||
+            SymbolEqualityComparer.Default.Equals(symbol, assembly);
+
+        public bool FromSource => false;
+    }
+}

csharp/extractor/Semmle.Extraction/Context.cs

@@ -215,7 +215,7 @@ namespace Semmle.Extraction
                 }
                 catch (Exception ex)  // lgtm[cs/catch-of-all-exceptions]
                 {
-                    ExtractionError("Uncaught exception", ex.Message, GeneratedLocation.Create(this), ex.StackTrace);
+                    ExtractionError("Uncaught exception", ex.Message, Entities.Location.Create(this), ex.StackTrace);
                 }
             }
         }
@@ -250,6 +250,8 @@ namespace Semmle.Extraction
 
         private IExtractionScope scope { get; }
 
+        public SyntaxTree? SourceTree => scope is SourceScope sc ? sc.SourceTree : null;
+
         /// <summary>
         ///     Whether the given symbol needs to be defined in this context.
         ///     This is the case if the symbol is contained in the source/assembly, or
@@ -451,7 +453,7 @@ namespace Semmle.Extraction
             }
             else
             {
-                ExtractionError(message, "", GeneratedLocation.Create(this));
+                ExtractionError(message, "", Entities.Location.Create(this));
             }
         }
 
@@ -522,13 +524,21 @@ namespace Semmle.Extraction
                 Message message;
 
                 if (node != null)
+                {
                     message = Message.Create(context, ex.Message, node, ex.StackTrace);
+                }
                 else if (symbol != null)
+                {
                     message = Message.Create(context, ex.Message, symbol, ex.StackTrace);
+                }
                 else if (ex is InternalError ie)
+                {
                     message = new Message(ie.Text, ie.EntityText, Entities.Location.Create(context, ie.Location), ex.StackTrace);
+                }
                 else
-                    message = new Message("Uncaught exception", ex.Message, GeneratedLocation.Create(context), ex.StackTrace);
+                {
+                    message = new Message("Uncaught exception", ex.Message, Entities.Location.Create(context), ex.StackTrace);
+                }
 
                 context.ExtractionError(message);
             }

csharp/extractor/Semmle.Extraction/Entities/ExtractionError.cs

@@ -14,7 +14,7 @@ namespace Semmle.Extraction.Entities
 
         protected override void Populate(TextWriter trapFile)
         {
-            trapFile.extractor_messages(this, msg.Severity, "C# extractor", msg.Text, msg.EntityText, msg.Location ?? GeneratedLocation.Create(cx), msg.StackTrace);
+            trapFile.extractor_messages(this, msg.Severity, "C# extractor", msg.Text, msg.EntityText, msg.Location ?? Location.Create(cx), msg.StackTrace);
         }
 
         public override TrapStackBehaviour TrapStackBehaviour => TrapStackBehaviour.NoLabel;

csharp/extractor/Semmle.Extraction/Entities/GeneratedLocation.cs

@@ -28,7 +28,7 @@ namespace Semmle.Extraction.Entities
 
         public override bool Equals(object? obj) => obj != null && obj.GetType() == typeof(GeneratedLocation);
 
-        public static GeneratedLocation Create(Context cx) => GeneratedLocationFactory.Instance.CreateEntity(cx, typeof(GeneratedLocation), null);
+        public static new GeneratedLocation Create(Context cx) => GeneratedLocationFactory.Instance.CreateEntity(cx, typeof(GeneratedLocation), null);
 
         private class GeneratedLocationFactory : ICachedEntityFactory<string?, GeneratedLocation>
         {

csharp/extractor/Semmle.Extraction/Entities/Location.cs

@@ -1,4 +1,6 @@
 
+using Microsoft.CodeAnalysis.Text;
+
 namespace Semmle.Extraction.Entities
 {
     public abstract class Location : CachedEntity<Microsoft.CodeAnalysis.Location?>
@@ -13,6 +15,13 @@ namespace Semmle.Extraction.Entities
                     ? NonGeneratedSourceLocation.Create(cx, loc)
                     : Assembly.Create(cx, loc);
 
+        public static Location Create(Context cx)
+        {
+            return cx.SourceTree == null
+                ? GeneratedLocation.Create(cx)
+                : Create(cx, Microsoft.CodeAnalysis.Location.Create(cx.SourceTree, TextSpan.FromBounds(0, 0)));
+        }
+
         public override Microsoft.CodeAnalysis.Location? ReportingLocation => symbol;
 
         public override TrapStackBehaviour TrapStackBehaviour => TrapStackBehaviour.OptionalLabel;

csharp/extractor/Semmle.Extraction/ExtractionScope.cs

@@ -1,78 +0,0 @@
-using Microsoft.CodeAnalysis;
-using System.Linq;
-
-namespace Semmle.Extraction
-{
-    /// <summary>
-    /// Defines which entities belong in the trap file
-    /// for the currently extracted entity. This is used to ensure that
-    /// trap files do not contain redundant information. Generally a symbol
-    /// should have an affinity with exactly one trap file, except for constructed
-    /// symbols.
-    /// </summary>
-    public interface IExtractionScope
-    {
-        /// <summary>
-        /// Whether the given symbol belongs in the trap file.
-        /// </summary>
-        /// <param name="symbol">The symbol to populate.</param>
-        bool InScope(ISymbol symbol);
-
-        /// <summary>
-        /// Whether the given file belongs in the trap file.
-        /// </summary>
-        /// <param name="path">The path to populate.</param>
-        bool InFileScope(string path);
-
-        bool IsGlobalScope { get; }
-
-        bool FromSource { get; }
-    }
-
-    /// <summary>
-    /// The scope of symbols in an assembly.
-    /// </summary>
-    public class AssemblyScope : IExtractionScope
-    {
-        private readonly IAssemblySymbol assembly;
-        private readonly string filepath;
-
-        public AssemblyScope(IAssemblySymbol symbol, string path, bool isOutput)
-        {
-            assembly = symbol;
-            filepath = path;
-            IsGlobalScope = isOutput;
-        }
-
-        public bool IsGlobalScope { get; }
-
-        public bool InFileScope(string path) => path == filepath;
-
-        public bool InScope(ISymbol symbol) =>
-            SymbolEqualityComparer.Default.Equals(symbol.ContainingAssembly, assembly) ||
-            SymbolEqualityComparer.Default.Equals(symbol, assembly);
-
-        public bool FromSource => false;
-    }
-
-    /// <summary>
-    /// The scope of symbols in a source file.
-    /// </summary>
-    public class SourceScope : IExtractionScope
-    {
-        private readonly SyntaxTree sourceTree;
-
-        public SourceScope(SyntaxTree tree)
-        {
-            sourceTree = tree;
-        }
-
-        public bool IsGlobalScope => false;
-
-        public bool InFileScope(string path) => path == sourceTree.FilePath;
-
-        public bool InScope(ISymbol symbol) => symbol.Locations.Any(loc => loc.SourceTree == sourceTree);
-
-        public bool FromSource => true;
-    }
-}

csharp/extractor/Semmle.Extraction/IExtractionScope.cs

@@ -0,0 +1,30 @@
+using Microsoft.CodeAnalysis;
+
+namespace Semmle.Extraction
+{
+    /// <summary>
+    /// Defines which entities belong in the trap file
+    /// for the currently extracted entity. This is used to ensure that
+    /// trap files do not contain redundant information. Generally a symbol
+    /// should have an affinity with exactly one trap file, except for constructed
+    /// symbols.
+    /// </summary>
+    public interface IExtractionScope
+    {
+        /// <summary>
+        /// Whether the given symbol belongs in the trap file.
+        /// </summary>
+        /// <param name="symbol">The symbol to populate.</param>
+        bool InScope(ISymbol symbol);
+
+        /// <summary>
+        /// Whether the given file belongs in the trap file.
+        /// </summary>
+        /// <param name="path">The path to populate.</param>
+        bool InFileScope(string path);
+
+        bool IsGlobalScope { get; }
+
+        bool FromSource { get; }
+    }
+}

csharp/extractor/Semmle.Extraction/SourceScope.cs

@@ -0,0 +1,27 @@
+using Microsoft.CodeAnalysis;
+using System.Linq;
+
+namespace Semmle.Extraction
+{
+
+    /// <summary>
+    /// The scope of symbols in a source file.
+    /// </summary>
+    public class SourceScope : IExtractionScope
+    {
+        public SyntaxTree SourceTree { get; }
+
+        public SourceScope(SyntaxTree tree)
+        {
+            SourceTree = tree;
+        }
+
+        public bool IsGlobalScope => false;
+
+        public bool InFileScope(string path) => path == SourceTree.FilePath;
+
+        public bool InScope(ISymbol symbol) => symbol.Locations.Any(loc => loc.SourceTree == SourceTree);
+
+        public bool FromSource => true;
+    }
+}

csharp/extractor/Semmle.Util/StringExtensions.cs

@@ -0,0 +1,41 @@
+using System.Collections.Generic;
+using System.Linq;
+
+namespace Semmle.Util
+{
+    public static class StringExtensions
+    {
+        public static (string, string) Split(this string self, int index0)
+        {
+            var split = self.Split(new[] { index0 });
+            return (split[0], split[1]);
+        }
+
+        public static (string, string, string) Split(this string self, int index0, int index1)
+        {
+            var split = self.Split(new[] { index0, index1 });
+            return (split[0], split[1], split[2]);
+        }
+
+        public static (string, string, string, string) Split(this string self, int index0, int index1, int index2)
+        {
+            var split = self.Split(new[] { index0, index1, index2 });
+            return (split[0], split[1], split[2], split[4]);
+        }
+
+        private static List<string> Split(this string self, params int[] indices)
+        {
+            var ret = new List<string>();
+            var previousIndex = 0;
+            foreach (var index in indices.OrderBy(i => i))
+            {
+                ret.Add(self.Substring(previousIndex, index - previousIndex));
+                previousIndex = index;
+            }
+
+            ret.Add(self.Substring(previousIndex));
+
+            return ret;
+        }
+    }
+}

csharp/ql/src/API Abuse/FormatInvalid.qhelp

@@ -4,28 +4,46 @@
 <qhelp>
 <overview>
 <p>
-The format string supplied to formatting methods (such as <code>string.Format()</code>)
-must be formatted correctly, otherwise the exception <code>System.FormatException</code> 
-will be thrown.
+When using string formatting methods (such as <code>string.Format()</code>), the following
+should be taken into account:
 </p>
+<ol>
+  <li>
+    The formatting string must be formatted correctly, otherwise the exception
+    <code>System.FormatException</code> will be thrown.
+  </li>
+  <li>
+    All passed arguments should be used by the formatting string, otherwise such
+    arguments will be ignored.
+  </li>
+  <li>
+    Missing arguments will result in a <code>System.FormatException</code> exception
+    being thrown.
+  </li>
+</ol>
 
 </overview>
 
 <recommendation>
 
-<p>
-Change the format string so that it is correctly formatted. Ensure that each
-format item adheres to the syntax:
-</p>
-
-<blockquote>
-  <p><b>{</b><i>index</i>[<b>,</b><i>alignment</i>][<b>:</b><i>formatString</i>]<b>}</b></p>
-</blockquote>
-
-<p>
-When literals <code>{</code> or <code>}</code> are required, replace them with <code>{{</code> and
-<code>}}</code>, respectively, or supply them as arguments.
-</p>
+<ol>
+  <li>
+    Change the format string so that it is correctly formatted. Ensure that each
+    format item adheres to the syntax:
+    <blockquote>
+      <p><b>{</b><i>index</i>[<b>,</b><i>alignment</i>][<b>:</b><i>formatString</i>]<b>}</b></p>
+    </blockquote>
+    When literals <code>{</code> or <code>}</code> are required, replace them with <code>{{</code> and
+    <code>}}</code>, respectively, or supply them as arguments.
+  </li>
+  <li>
+    Change the format string to use the highlighted argument, or remove the unnecessary argument.
+  </li>
+  <li>
+    Supply the correct number of arguments to the format method, or change the format string 
+    to use the correct arguments.
+  </li>
+</ol>
 
 </recommendation>
 
@@ -40,8 +58,39 @@ literals are not properly escaped.
 In the revised example, the literals are properly escaped.
 </p>
 <sample src="FormatInvalidGood.cs" />
-
 </example>
+
+<example>
+<p>
+Here are three examples where the format string does not use all the arguments.
+</p>
+<sample src="FormatUnusedArgumentBad.cs"/>
+<ul>
+  <li>On line 7, the second argument (<code>ex.HResult</code>) is not logged.</li>
+  <li>On line 8, the first argument (<code>ex</code>) is not logged but the second 
+    argument (<code>ex.HResult</code>) is logged twice.</li>
+  <li>On line 9, a C-style format string is used, which is incorrect, and neither 
+    argument will be logged.</li>
+</ul>  
+</example>
+
+<example>
+<p>
+Here are two examples where the call to <code>String.Format()</code> is missing arguments.
+</p>
+<sample src="FormatMissingArgumentBad.cs"/>
+<ul>
+  <li>On line 7, the second argument (<code>last</code>) is not supplied.</li>
+  <li>On line 8, the format items are numbered <code>{1}</code> and <code>{2}</code>,
+    instead of <code>{0}</code> and <code>{1}</code> as they should be.</li>
+</ul>
+
+<p>
+In the revised example, both arguments are supplied.
+</p>
+<sample src="FormatMissingArgumentGood.cs"/>
+</example>
+
 <references>
   <li>MSDN: <a href="https://msdn.microsoft.com/en-us/library/system.string.format.aspx">String.Format Method</a>.</li>
   <li>Microsoft: <a href="https://docs.microsoft.com/en-us/dotnet/standard/base-types/composite-formatting">Composite Formatting</a>.</li>

csharp/ql/src/API Abuse/FormatInvalid.ql

@@ -1,18 +1,86 @@
 /**
- * @name Invalid format string
- * @description Using a format string with an incorrect format causes a 'System.FormatException'.
+ * @name Invalid string formatting
+ * @description Calling 'string.Format()' with either an invalid format string or incorrect
+ *              number of arguments may result in dropped arguments or a 'System.FormatException'.
  * @kind path-problem
  * @problem.severity error
  * @precision high
- * @id cs/invalid-format-string
+ * @id cs/invalid-string-formatting
  * @tags reliability
  *       maintainability
  */
 
 import csharp
 import semmle.code.csharp.frameworks.Format
-import FormatFlow
+import DataFlow::PathGraph
 
-from FormatCall s, InvalidFormatString src, PathNode source, PathNode sink
-where hasFlowPath(src, source, s, sink)
-select src, source, sink, "Invalid format string used in $@ formatting call.", s, "this"
+private class FormatConfiguration extends DataFlow::Configuration {
+  FormatConfiguration() { this = "format" }
+
+  override predicate isSource(DataFlow::Node n) { n.asExpr() instanceof StringLiteral }
+
+  override predicate isSink(DataFlow::Node n) {
+    exists(FormatCall c | n.asExpr() = c.getFormatExpr())
+  }
+}
+
+private predicate invalidFormatString(
+  InvalidFormatString src, DataFlow::PathNode source, DataFlow::PathNode sink, string msg,
+  FormatCall call, string callString
+) {
+  source.getNode().asExpr() = src and
+  sink.getNode().asExpr() = call.getFormatExpr() and
+  any(FormatConfiguration conf).hasFlowPath(source, sink) and
+  call.hasInsertions() and
+  msg = "Invalid format string used in $@ formatting call." and
+  callString = "this"
+}
+
+private predicate unusedArgument(
+  FormatCall call, DataFlow::PathNode source, DataFlow::PathNode sink, string msg,
+  ValidFormatString src, string srcString, Expr unusedExpr, string unusedString
+) {
+  exists(int unused |
+    source.getNode().asExpr() = src and
+    sink.getNode().asExpr() = call.getFormatExpr() and
+    any(FormatConfiguration conf).hasFlowPath(source, sink) and
+    unused = call.getASuppliedArgument() and
+    not unused = src.getAnInsert() and
+    not src.getValue() = "" and
+    msg = "The $@ ignores $@." and
+    srcString = "format string" and
+    unusedExpr = call.getSuppliedExpr(unused) and
+    unusedString = "this supplied value"
+  )
+}
+
+private predicate missingArgument(
+  FormatCall call, DataFlow::PathNode source, DataFlow::PathNode sink, string msg,
+  ValidFormatString src, string srcString
+) {
+  exists(int used, int supplied |
+    source.getNode().asExpr() = src and
+    sink.getNode().asExpr() = call.getFormatExpr() and
+    any(FormatConfiguration conf).hasFlowPath(source, sink) and
+    used = src.getAnInsert() and
+    supplied = call.getSuppliedArguments() and
+    used >= supplied and
+    msg = "Argument '{" + used + "}' has not been supplied to $@ format string." and
+    srcString = "this"
+  )
+}
+
+from
+  Element alert, DataFlow::PathNode source, DataFlow::PathNode sink, string msg, Element extra1,
+  string extra1String, Element extra2, string extra2String
+where
+  invalidFormatString(alert, source, sink, msg, extra1, extra1String) and
+  extra2 = extra1 and
+  extra2String = extra1String
+  or
+  unusedArgument(alert, source, sink, msg, extra1, extra1String, extra2, extra2String)
+  or
+  missingArgument(alert, source, sink, msg, extra1, extra1String) and
+  extra2 = extra1 and
+  extra2String = extra1String
+select alert, source, sink, msg, extra1, extra1String, extra2, extra2String

csharp/ql/src/API Abuse/FormatInvalidBad.cs

@@ -1,9 +1,9 @@
 using System;
 
-class Bad
+class Bad1
 {
     string GenerateEmptyClass(string c)
     {
-        return string.Format("class {0} { }");
+        return string.Format("class {0} { }", "C");
     }
 }

csharp/ql/src/API Abuse/FormatInvalidGood.cs

@@ -1,9 +1,9 @@
 using System;
 
-class Good
+class Good1
 {
     string GenerateEmptyClass(string c)
     {
-        return string.Format("class {0} {{ }}");
+        return string.Format("class {0} {{ }}", "C");
     }
 }

csharp/ql/src/API Abuse/FormatMissingArgument.qhelp

@@ -1,41 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>
-Formatting methods (such as <code>String.Format()</code>) that are missing arguments will 
-throw the exception <code>System.FormatException</code>. This is caused by the format 
-string not matching the actual arguments supplied or an incorrect format string.
-</p>
-</overview>
-
-<recommendation>
-<p>
-Supply the correct number of arguments to the format method, or change the format string 
-to use the correct arguments.
-</p>
-</recommendation>
-
-<example>
-<p>
-Here are two examples where the call to <code>String.Format()</code> is missing arguments.
-</p>
-<sample src="FormatMissingArgumentBad.cs"/>
-<ul>
-  <li>On line 5, the second argument (<code>last</code>) is not supplied.</li>
-  <li>On line 6, the format items are numbered <code>{1}</code> and <code>{2}</code>,
-    instead of <code>{0}</code> and <code>{1}</code> as they should be.</li>
-</ul>
-
-<p>
-In the revised example, both arguments are supplied.
-</p>
-<sample src="FormatMissingArgumentGood.cs"/>
-</example>
-
-<references>
-  <li>MSDN: <a href="https://msdn.microsoft.com/en-us/library/system.string.format.aspx">String.Format Method</a>.</li>
-  <li>Microsoft: <a href="https://docs.microsoft.com/en-us/dotnet/standard/base-types/composite-formatting">Composite Formatting</a>.</li>
-</references>
-</qhelp>

csharp/ql/src/API Abuse/FormatMissingArgument.ql

@@ -1,24 +0,0 @@
-/**
- * @name Missing format argument
- * @description Supplying too few arguments to a format string causes a 'System.FormatException'.
- * @kind path-problem
- * @problem.severity error
- * @precision high
- * @id cs/format-argument-missing
- * @tags reliability
- *       maintainability
- */
-
-import csharp
-import semmle.code.csharp.frameworks.Format
-import FormatFlow
-
-from
-  FormatCall format, ValidFormatString src, int used, int supplied, PathNode source, PathNode sink
-where
-  hasFlowPath(src, source, format, sink) and
-  used = src.getAnInsert() and
-  supplied = format.getSuppliedArguments() and
-  used >= supplied
-select format, source, sink, "Argument '{" + used + "}' has not been supplied to $@ format string.",
-  src, "this"

csharp/ql/src/API Abuse/FormatMissingArgumentBad.cs

@@ -1,6 +1,6 @@
 using System;
 
-class Bad
+class Bad3
 {
     void Hello(string first, string last)
     {

csharp/ql/src/API Abuse/FormatMissingArgumentGood.cs

@@ -1,6 +1,6 @@
 using System;
 
-class Good
+class Good3
 {
     void Hello(string first, string last)
     {

csharp/ql/src/API Abuse/FormatUnusedArgument.qhelp

@@ -1,37 +0,0 @@
-<!DOCTYPE qhelp PUBLIC
-  "-//Semmle//qhelp//EN"
-  "qhelp.dtd">
-<qhelp>
-<overview>
-<p>
-Arguments which are passed to formatting methods (such as <code>String.Format()</code>)
-but are not used, are either unnecessary or mean that the format string is incorrect. The result
-is that the argument will be ignored, which may not be the intended behavior.
-</p>
-</overview>
-
-<recommendation>
-<p>
-Change the format string to use the highlighted argument, or remove the unnecessary argument.
-</p>
-</recommendation>
-
-<example>
-<p>
-Here are three examples where the format string does not use all the arguments.
-</p>
-<sample src="FormatUnusedArgumentBad.cs"/>
-<ul>
-  <li>On line 5, the second argument (<code>ex.HResult</code>) is not logged.</li>
-  <li>On line 6, the first argument (<code>ex</code>) is not logged but the second 
-    argument (<code>ex.HResult</code>) is logged twice.</li>
-  <li>On line 4, a C-style format string is used, which is incorrect, and neither 
-    argument will be logged.</li>
-</ul>  
-</example>
-
-<references>
-  <li>MSDN: <a href="https://msdn.microsoft.com/en-us/library/system.string.format.aspx">String.Format Method</a>.</li>
-  <li>Microsoft: <a href="https://docs.microsoft.com/en-us/dotnet/standard/base-types/composite-formatting">Composite Formatting</a>.</li>
-</references>
-</qhelp>

csharp/ql/src/API Abuse/FormatUnusedArgument.ql

@@ -1,22 +0,0 @@
-/**
- * @name Unused format argument
- * @description Supplying more arguments than are required for a format string may indicate an error in the format string.
- * @kind path-problem
- * @problem.severity warning
- * @precision high
- * @id cs/format-argument-unused
- * @tags reliability
- *       maintainability
- */
-
-import csharp
-import semmle.code.csharp.frameworks.Format
-import FormatFlow
-
-from FormatCall format, int unused, ValidFormatString src, PathNode source, PathNode sink
-where
-  hasFlowPath(src, source, format, sink) and
-  unused = format.getAnUnusedArgument(src) and
-  not src.getValue() = ""
-select format, source, sink, "The $@ ignores $@.", src, "format string",
-  format.getSuppliedExpr(unused), "this supplied value"

csharp/ql/src/API Abuse/FormatUnusedArgumentBad.cs

@@ -1,6 +1,6 @@
 using System;
 
-class Bad
+class Bad2
 {
     void M(Exception ex)
     {

csharp/ql/src/Security Features/CWE-134/UncontrolledFormatString.ql

@@ -27,7 +27,7 @@ class FormatStringConfiguration extends TaintTracking::Configuration {
   }
 
   override predicate isSink(DataFlow::Node sink) {
-    sink.asExpr() = any(FormatCall call).getFormatExpr()
+    sink.asExpr() = any(FormatCall call | call.hasInsertions()).getFormatExpr()
   }
 }