Merge branch 'main' into unreachable4

Author: geoffw0

.vscode/settings.json

@@ -1,5 +1,6 @@
 {
   "omnisharp.autoStart": false,
   "cmake.sourceDirectory": "${workspaceFolder}/swift",
-  "cmake.buildDirectory": "${workspaceFolder}/bazel-cmake-build"
+  "cmake.buildDirectory": "${workspaceFolder}/bazel-cmake-build",
+  "editor.suggest.matchOnWordStartOnly": false
 }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/TypeFlow.qll

@@ -159,7 +159,7 @@ private module Input implements TypeFlowInput<Location> {
     )
   }
 
-  predicate joinStep(TypeFlowNode n1, TypeFlowNode n2) {
+  predicate step(TypeFlowNode n1, TypeFlowNode n2) {
     // instruction -> phi
     getAnUltimateLocalDefinition(n2.asInstruction()) = n1.asInstruction()
     or
@@ -179,6 +179,8 @@ private module Input implements TypeFlowInput<Location> {
       n1.asInstruction() = arg and
       n2.asInstruction() = p
     )
+    or
+    instructionStep(n1.asInstruction(), n2.asInstruction())
   }
 
   /**
@@ -199,10 +201,6 @@ private module Input implements TypeFlowInput<Location> {
     i2.(PointerArithmeticInstruction).getLeft() = i1
   }
 
-  predicate step(TypeFlowNode n1, TypeFlowNode n2) {
-    instructionStep(n1.asInstruction(), n2.asInstruction())
-  }
-
   predicate isNullValue(TypeFlowNode n) { n.isNullValue() }
 
   private newtype TType =
@@ -245,11 +243,7 @@ private module Input implements TypeFlowInput<Location> {
 
   pragma[nomagic]
   private predicate upcastCand(TypeFlowNode n, Type t1, Type t2) {
-    exists(TypeFlowNode next |
-      step(n, next)
-      or
-      joinStep(n, next)
-    |
+    exists(TypeFlowNode next | step(n, next) |
       n.getType() = t1 and
       next.getType() = t2 and
       t1 != t2

cpp/ql/src/Likely Bugs/Format/WrongNumberOfFormatArguments.ql

@@ -16,6 +16,20 @@
 
 import cpp
 
+class SyntaxError extends CompilerError {
+  SyntaxError() { this.getTag().matches("exp_%") }
+
+  predicate affects(Element e) {
+    exists(Location l1, Location l2 |
+      l1 = this.getLocation() and
+      l2 = e.getLocation()
+    |
+      l1.getFile() = l2.getFile() and
+      l1.getStartLine() = l2.getStartLine()
+    )
+  }
+}
+
 from FormatLiteral fl, FormattingFunctionCall ffc, int expected, int given, string ffcName
 where
   ffc = fl.getUse() and
@@ -27,7 +41,10 @@ where
     if ffc.isInMacroExpansion()
     then ffcName = ffc.getTarget().getName() + " (in a macro expansion)"
     else ffcName = ffc.getTarget().getName()
-  )
+  ) and
+  // A typical problem is that string literals are concatenated, but if one of the string
+  // literals is an undefined macro, then this just leads to a syntax error.
+  not exists(SyntaxError e | e.affects(fl))
 select ffc,
   "Format for " + ffcName + " expects " + expected.toString() + " arguments but given " +
     given.toString()

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

@@ -0,0 +1,7 @@
+// semmle-extractor-options: --expect_errors
+
+extern int printf(const char *fmt, ...);
+
+void test_syntax_error() {
+    printf("Error code %d: " FMT_MSG, 0, "");
+}

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPrivate.qll

@@ -13,6 +13,7 @@ private import semmle.code.csharp.Unification
 private import semmle.code.csharp.controlflow.Guards
 private import semmle.code.csharp.dispatch.Dispatch
 private import semmle.code.csharp.frameworks.EntityFramework
+private import semmle.code.csharp.frameworks.system.linq.Expressions
 private import semmle.code.csharp.frameworks.NHibernate
 private import semmle.code.csharp.frameworks.Razor
 private import semmle.code.csharp.frameworks.system.Collections
@@ -1146,7 +1147,11 @@ private module Cached {
     TPrimaryConstructorParameterContent(Parameter p) {
       p.getCallable() instanceof PrimaryConstructor
     } or
-    TCapturedVariableContent(VariableCapture::CapturedVariable v)
+    TCapturedVariableContent(VariableCapture::CapturedVariable v) or
+    TDelegateCallArgumentContent(int i) {
+      i = [0 .. max(any(DelegateLikeCall dc).getNumberOfArguments()) - 1]
+    } or
+    TDelegateCallReturnContent()
 
   cached
   newtype TContentSet =
@@ -1162,7 +1167,9 @@ private module Cached {
     TPrimaryConstructorParameterApproxContent(string firstChar) {
       firstChar = approximatePrimaryConstructorParameterContent(_)
     } or
-    TCapturedVariableContentApprox(VariableCapture::CapturedVariable v)
+    TCapturedVariableContentApprox(VariableCapture::CapturedVariable v) or
+    TDelegateCallArgumentApproxContent() or
+    TDelegateCallReturnApproxContent()
 
   pragma[nomagic]
   private predicate commonSubTypeGeneral(DataFlowTypeOrUnifiable t1, RelevantGvnType t2) {
@@ -2273,6 +2280,21 @@ private predicate recordProperty(RecordType t, ContentSet c, string name) {
   )
 }
 
+/**
+ * Holds if data can flow from `node1` to `node2` via an assignment to
+ * the content set `c` of a delegate call.
+ *
+ * If there is a delegate call f(x), then we store "x" on "f"
+ * using a delegate argument content set.
+ */
+private predicate storeStepDelegateCall(ExplicitArgumentNode node1, ContentSet c, Node node2) {
+  exists(ExplicitDelegateLikeDataFlowCall call, int i |
+    node1.argumentOf(call, TPositionalArgumentPosition(i)) and
+    lambdaCall(call, _, node2.(PostUpdateNode).getPreUpdateNode()) and
+    c.isDelegateCallArgument(i)
+  )
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` via an assignment to
  * content `c`.
@@ -2305,6 +2327,8 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
   or
   FlowSummaryImpl::Private::Steps::summaryStoreStep(node1.(FlowSummaryNode).getSummaryNode(), c,
     node2.(FlowSummaryNode).getSummaryNode())
+  or
+  storeStepDelegateCall(node1, c, node2)
 }
 
 private class ReadStepConfiguration extends ControlFlowReachabilityConfiguration {
@@ -2425,6 +2449,21 @@ private predicate readContentStep(Node node1, Content c, Node node2) {
   VariableCapture::readStep(node1, c, node2)
 }
 
+/**
+ * Holds if data can flow from `node1` to `node2` via an assignment to
+ * the content set `c` of a delegate call.
+ *
+ * If there is a delegate call f(x), then we read the return of the delegate
+ * call.
+ */
+private predicate readStepDelegateCall(Node node1, ContentSet c, OutNode node2) {
+  exists(ExplicitDelegateLikeDataFlowCall call |
+    lambdaCall(call, _, node1) and
+    node2.getCall(TNormalReturnKind()) = call and
+    c.isDelegateCallReturn()
+  )
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` via a read of content `c`.
  */
@@ -2443,6 +2482,8 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
   or
   FlowSummaryImpl::Private::Steps::summaryReadStep(node1.(FlowSummaryNode).getSummaryNode(), c,
     node2.(FlowSummaryNode).getSummaryNode())
+  or
+  readStepDelegateCall(node1, c, node2)
 }
 
 private predicate clearsCont(Node n, Content c) {
@@ -3037,6 +3078,12 @@ class ContentApprox extends TContentApprox {
     exists(VariableCapture::CapturedVariable v |
       this = TCapturedVariableContentApprox(v) and result = "captured " + v
     )
+    or
+    this = TDelegateCallArgumentApproxContent() and
+    result = "approximated delegate call argument"
+    or
+    this = TDelegateCallReturnApproxContent() and
+    result = "approximated delegate call return"
   }
 }
 
@@ -3073,6 +3120,12 @@ ContentApprox getContentApprox(Content c) {
     TPrimaryConstructorParameterApproxContent(approximatePrimaryConstructorParameterContent(c))
   or
   result = TCapturedVariableContentApprox(VariableCapture::getCapturedVariableContent(c))
+  or
+  c instanceof DelegateCallArgumentContent and
+  result = TDelegateCallArgumentApproxContent()
+  or
+  c instanceof DelegateCallReturnContent and
+  result = TDelegateCallReturnApproxContent()
 }
 
 /**

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll

@@ -3,6 +3,7 @@ private import DataFlowDispatch
 private import DataFlowPrivate
 private import semmle.code.csharp.controlflow.Guards
 private import semmle.code.csharp.Unification
+private import semmle.code.csharp.frameworks.system.linq.Expressions
 
 /**
  * An element, viewed as a node in a data flow graph. Either an expression
@@ -238,6 +239,30 @@ class PropertyContent extends Content, TPropertyContent {
   override Location getLocation() { result = p.getLocation() }
 }
 
+/**
+ * A reference to the index of an argument of a delegate call.
+ */
+class DelegateCallArgumentContent extends Content, TDelegateCallArgumentContent {
+  private int i;
+
+  DelegateCallArgumentContent() { this = TDelegateCallArgumentContent(i) }
+
+  override string toString() { result = "delegate argument at position " + i }
+
+  override Location getLocation() { result instanceof EmptyLocation }
+}
+
+/**
+ * A reference to the return of a delegate call.
+ */
+class DelegateCallReturnContent extends Content, TDelegateCallReturnContent {
+  DelegateCallReturnContent() { this = TDelegateCallReturnContent() }
+
+  override string toString() { result = "delegate return" }
+
+  override Location getLocation() { result instanceof EmptyLocation }
+}
+
 /**
  * A reference to a synthetic field corresponding to a
  * primary constructor parameter.
@@ -299,6 +324,16 @@ class ContentSet extends TContentSet {
    */
   predicate isProperty(Property p) { this = TPropertyContentSet(p) }
 
+  /**
+   * Holds if this content set represents the `i`th argument of a delegate call.
+   */
+  predicate isDelegateCallArgument(int i) { this.isSingleton(TDelegateCallArgumentContent(i)) }
+
+  /**
+   * Holds if this content set represents the return of a delegate call.
+   */
+  predicate isDelegateCallReturn() { this.isSingleton(TDelegateCallReturnContent()) }
+
   /** Holds if this content set represents the field `f`. */
   predicate isField(Field f) { this.isSingleton(TFieldContent(f)) }
 

csharp/ql/src/utils/modelgenerator/internal/CaptureModels.qll

@@ -101,7 +101,9 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
     api = any(FlowSummaryImpl::Public::NeutralSinkCallable sc | sc.hasManualModel())
   }
 
-  predicate isUninterestingForDataFlowModels(Callable api) { isHigherOrder(api) }
+  predicate isUninterestingForDataFlowModels(Callable api) { none() }
+
+  predicate isUninterestingForHeuristicDataFlowModels(Callable api) { isHigherOrder(api) }
 
   class SourceOrSinkTargetApi extends Callable {
     SourceOrSinkTargetApi() { relevant(this) }
@@ -174,8 +176,15 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
    * Gets the underlying type of the content `c`.
    */
   private CS::Type getUnderlyingContType(DataFlow::Content c) {
-    result = c.(DataFlow::FieldContent).getField().getType() or
+    result = c.(DataFlow::FieldContent).getField().getType()
+    or
     result = c.(DataFlow::SyntheticFieldContent).getField().getType()
+    or
+    // Use System.Object as the type of delegate arguments and returns as the content doesn't
+    // contain any type information.
+    c instanceof DataFlow::DelegateCallArgumentContent and result instanceof ObjectType
+    or
+    c instanceof DataFlow::DelegateCallReturnContent and result instanceof ObjectType
   }
 
   Type getUnderlyingContentType(DataFlow::ContentSet c) {
@@ -309,6 +318,10 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
     c.isField(_) or c.isSyntheticField(_) or c.isProperty(_)
   }
 
+  predicate isCallback(DataFlow::ContentSet c) {
+    c.isDelegateCallArgument(_) or c.isDelegateCallReturn()
+  }
+
   string getSyntheticName(DataFlow::ContentSet c) {
     exists(CS::Field f |
       not f.isEffectivelyPublic() and
@@ -342,6 +355,10 @@ module ModelGeneratorInput implements ModelGeneratorInputSig<Location, CsharpDat
     or
     c.isElement() and
     result = "Element"
+    or
+    exists(int i | c.isDelegateCallArgument(i) and result = "Parameter[" + i + "]")
+    or
+    c.isDelegateCallReturn() and result = "ReturnValue"
   }
 
   predicate partialModel = ExternalFlow::partialModel/6;