Fix a bad join order on locations.

cpp/ql/lib/semmle/code/cpp/Declaration.qll

@@ -88,6 +88,7 @@ class Declaration extends Locatable, @declaration {
    *
    * See the 3-argument `hasQualifiedName` for examples.
    */
+  pragma[noinline]
   predicate hasQualifiedName(string namespaceQualifier, string baseName) {
     this.hasQualifiedName(namespaceQualifier, "", baseName)
   }

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

@@ -2011,13 +2011,23 @@ module ExprFlowCached {
     localFlowStep(n1, n2)
   }
 
+  /**
+   * Holds if `asExpr(n1)` doesn't have a result and `n1` flows to `n2` in a single
+   * dataflow step.
+   * 
+   * i.e. this is the transpose of `localStepFromNonExpr`
+   */
+  private predicate localStepFromNonExprR(Node n2, Node n1) {
+    localStepFromNonExpr(n1, n2)
+  }
+
   /**
    * Holds if `asExpr(n1)` doesn't have a result, `asExpr(n2) = e2` and
    * `n2` is the first node reachable from `n1` such that `asExpr(n2)` exists.
    */
   pragma[nomagic]
   private predicate localStepsToExpr(Node n1, Node n2, Expr e2) {
-    localStepFromNonExpr*(n1, n2) and
+    localStepFromNonExprR*(n2, n1) and
     e2 = asExprInternal(n2)
   }
 

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll

@@ -815,10 +815,15 @@ private predicate isRelatableMemoryLocation(VariableMemoryLocation vml) {
   vml.getStartBitOffset() != Ints::unknown()
 }
 
+pragma[noinline]
+private int getRank(VirtualVariable vvar, IntValue offset) {
+  offset = rank[result](IntValue offset_ | isRelevantOffset(vvar, offset_))
+}
+
 private predicate isCoveredOffset(Allocation var, int offsetRank, VariableMemoryLocation vml) {
   exists(int startRank, int endRank, VirtualVariable vvar |
-    vml.getStartBitOffset() = rank[startRank](IntValue offset_ | isRelevantOffset(vvar, offset_)) and
+    startRank = getRank(vvar, vml.getStartBitOffset()) and
-    vml.getEndBitOffset() = rank[endRank](IntValue offset_ | isRelevantOffset(vvar, offset_)) and
+    endRank = getRank(vvar, vml.getEndBitOffset()) and
     var = vml.getAnAllocation() and
     vvar = vml.getVirtualVariable() and
     isRelatableMemoryLocation(vml) and

cpp/ql/lib/semmle/code/cpp/ir/implementation/raw/internal/TranslatedGlobalVar.qll

@@ -117,27 +117,29 @@ class TranslatedStaticStorageDurationVarInit extends TranslatedRootElement,
    * directly accessed by the function.
    */
   final predicate hasUserVariable(Variable varUsed, CppType type) {
-    (
+    this.hasUserVariable1(varUsed) and
-      (
-        varUsed instanceof GlobalOrNamespaceVariable
-        or
-        varUsed instanceof StaticLocalVariable
-        or
-        varUsed instanceof MemberVariable and not varUsed instanceof Field
-      ) and
-      exists(VariableAccess access |
-        access.getTarget() = varUsed and
-        getEnclosingVariable(access) = var
-      )
-      or
-      var = varUsed
-      or
-      varUsed.(LocalScopeVariable).getEnclosingElement*() = var
-      or
-      varUsed.(Parameter).getCatchBlock().getEnclosingElement*() = var
-    ) and
     type = getTypeForPRValue(getVariableType(varUsed))
   }
+
+  private predicate hasUserVariable1(Variable varUsed) {
+    (
+      varUsed instanceof GlobalOrNamespaceVariable
+      or
+      varUsed instanceof StaticLocalVariable
+      or
+      varUsed instanceof MemberVariable and not varUsed instanceof Field
+    ) and
+    exists(VariableAccess access |
+      access.getTarget() = varUsed and
+      getEnclosingVariable(access) = var
+    )
+    or
+    var = varUsed
+    or
+    varUsed.(LocalScopeVariable).getEnclosingElement*() = var
+    or
+    varUsed.(Parameter).getCatchBlock().getEnclosingElement*() = var
+  }
 }
 
 TranslatedStaticStorageDurationVarInit getTranslatedVarInit(Variable var) { result.getAst() = var }

cpp/ql/src/experimental/Security/CWE/CWE-691/InsufficientControlFlowManagementAfterRefactoringTheCode.ql

@@ -44,6 +44,8 @@ class UsingWhileAfterWhile extends WhileStmt {
   }
 }
 
+
+
 /**
  * Using arithmetic in a condition.
  */
@@ -55,15 +57,15 @@ class UsingArithmeticInComparison extends BinaryArithmeticOperation {
    */
   UsingArithmeticInComparison() {
     this.getParent*() instanceof IfStmt and
-    not this.getAChild*().isConstant() and
+     not (this.getAChild*().isConstant() or
-    not this.getParent*() instanceof Call and
+     this.getParent*() instanceof Call or
-    not this.getParent*() instanceof AssignExpr and
+     this.getParent*() instanceof AssignExpr or
-    not this.getParent*() instanceof ArrayExpr and
+     this.getParent*() instanceof ArrayExpr or
-    not this.getParent*() instanceof RemExpr and
+     this.getParent*() instanceof RemExpr or
-    not this.getParent*() instanceof AssignBitwiseOperation and
+     this.getParent*() instanceof AssignBitwiseOperation or
-    not this.getParent*() instanceof AssignArithmeticOperation and
+     this.getParent*() instanceof AssignArithmeticOperation or
-    not this.getParent*() instanceof EqualityOperation and
+     this.getParent*() instanceof EqualityOperation or
-    not this.getParent*() instanceof RelationalOperation
+     this.getParent*() instanceof RelationalOperation)
   }
 
   /** Holds when the expression is inside the loop body. */

java/ql/consistency-queries/children.ql

@@ -13,6 +13,39 @@ predicate duplicateChildren(Element e, int i) {
   not e instanceof Constructor
 }
 
+predicate allowableGap(Element e, int i) {
+  // Annotations are child 0 upwards, 'implements' are -2 downwards,
+  // and there may or may not be an 'extends' for child -1.
+  e instanceof ClassOrInterface and i = -1
+  or
+  // A class instance creation expression has the type as child -3,
+  // may or may not have a qualifier as child -2, and will never have
+  // a child -1.
+  e instanceof ClassInstanceExpr and i = [-2, -1]
+  or
+  // Type access have annotations from -2 down, and type
+  // arguments from 0 up, but may or may not have a qualifier
+  // at -1.
+  e instanceof TypeAccess and i = -1
+  or
+  // Try statements have their 'finally' clause as child 2,
+  // and that may or may not exist.
+  e instanceof TryStmt and i = -2
+  or
+  // For statements may or may not declare a new variable (child 0), or
+  // have a condition (child 1).
+  e instanceof ForStmt and i = [0, 1]
+  or
+  // Pattern case statements legitimately have a TypeAccess (-2) and a pattern (0) but not a rule (-1)
+  i = -1 and e instanceof PatternCase and not e.(PatternCase).isRule()
+  or
+  // Pattern case statements can have a gap at -3 when they have more than one pattern but no guard.
+  i = -3 and count(e.(PatternCase).getAPattern()) > 1 and not exists(e.(PatternCase).getGuard())
+  or
+  // Instanceof with a record pattern is not expected to have a type access in position 1
+  i = 1 and e.(InstanceOfExpr).getPattern() instanceof RecordPatternExpr
+}
+
 predicate gapInChildren(Element e, int i) {
   exists(int left, int right |
     left = min(int l | exists(nthChildOf(e, l))) and
@@ -20,48 +53,24 @@ predicate gapInChildren(Element e, int i) {
     i in [left .. right] and
     not exists(nthChildOf(e, i))
   ) and
-  // Annotations are child 0 upwards, 'implements' are -2 downwards,
+  not allowableGap(e, i) and
-  // and there may or may not be an 'extends' for child -1.
+  not (
-  not (e instanceof ClassOrInterface and i = -1) and
+    // Pattern case statements may have some missing type accesses, depending on the nature of the direct child
-  // A class instance creation expression has the type as child -3,
+    (i = -2 or i < -4) and
-  // may or may not have a qualifier as child -2, and will never have
+    e instanceof PatternCase
-  // a child -1.
+  ) and
-  not (e instanceof ClassInstanceExpr and i = [-2, -1]) and
+  // RecordPatternExpr extracts type-accesses only for its LocalVariableDeclExpr children
-  // Type access have annotations from -2 down, and type
+  not (i < 0 and e instanceof RecordPatternExpr) and
-  // arguments from 0 up, but may or may not have a qualifier
-  // at -1.
-  not (e instanceof TypeAccess and i = -1) and
-  // Try statements have their 'finally' clause as child 2,
-  // and that may or may not exist.
-  not (e instanceof TryStmt and i = -2) and
-  // For statements may or may not declare a new variable (child 0), or
-  // have a condition (child 1).
-  not (e instanceof ForStmt and i = [0, 1]) and
-  // TODO: Clarify situation with Kotlin and MethodCall.
-  // -1 can be skipped (type arguments from -2 down, no qualifier at -1,
-  // then arguments from 0).
-  // Can we also skip arguments, e.g. due to defaults for parameters?
-  not (e instanceof MethodCall and e.getFile().isKotlinSourceFile()) and
   // Kotlin-extracted annotations can have missing children where a default
   // value should be, because kotlinc doesn't load annotation defaults and we
   // want to leave a space for another extractor to fill in the default if it
   // is able.
   not e instanceof Annotation and
-  // Pattern case statements legitimately have a TypeAccess (-2) and a pattern (0) but not a rule (-1)
+  // TODO: Clarify situation with Kotlin and MethodCall.
-  not (i = -1 and e instanceof PatternCase and not e.(PatternCase).isRule()) and
+  // -1 can be skipped (type arguments from -2 down, no qualifier at -1,
-  // Pattern case statements can have a gap at -3 when they have more than one pattern but no guard.
+  // then arguments from 0).
-  not (
+  // Can we also skip arguments, e.g. due to defaults for parameters?
-    i = -3 and count(e.(PatternCase).getAPattern()) > 1 and not exists(e.(PatternCase).getGuard())
+  not (e instanceof MethodCall and e.getFile().isKotlinSourceFile())
-  ) and
-  // Pattern case statements may have some missing type accesses, depending on the nature of the direct child
-  not (
-    (i = -2 or i < -4) and
-    e instanceof PatternCase
-  ) and
-  // Instanceof with a record pattern is not expected to have a type access in position 1
-  not (i = 1 and e.(InstanceOfExpr).getPattern() instanceof RecordPatternExpr) and
-  // RecordPatternExpr extracts type-accesses only for its LocalVariableDeclExpr children
-  not (i < 0 and e instanceof RecordPatternExpr)
 }
 
 predicate lateFirstChild(Element e, int i) {

javascript/ql/lib/semmle/javascript/ApiGraphs.qll

@@ -1372,7 +1372,7 @@ module API {
       exists(DataFlow::TypeBackTracker t, StepSummary summary, DataFlow::Node next |
         next = trackDefNode(nd, t) and
         StepSummary::step(prev, next, summary) and
-        result = t.prepend(summary)
+        result = t.prepend_cached(summary)
       )
     }
 

javascript/ql/lib/semmle/javascript/dataflow/TypeTracking.qll

@@ -224,8 +224,16 @@ class TypeBackTracker extends TTypeBackTracker {
   TypeBackTracker() { this = MkTypeBackTracker(hasReturn, prop) }
 
   /** Gets the summary resulting from prepending `step` to this type-tracking summary. */
+  bindingset[this, step]
+  pragma[inline_late]
+  TypeBackTracker prepend(StepSummary step) { result = this.prepend_cached(step) }
+
+  /**
+   * INTERNAL: DO NOT USE
+   * Raw version of prepend.
+   */
   cached
-  TypeBackTracker prepend(StepSummary step) {
+  TypeBackTracker prepend_cached(StepSummary step) {
     Stages::TypeTracking::ref() and
     step = LevelStep() and
     result = this

javascript/ql/lib/semmle/javascript/internal/CachedStages.qll

@@ -236,7 +236,7 @@ module Stages {
       or
       exists(any(DataFlow::TypeTracker t).append(_))
       or
-      exists(any(DataFlow::TypeBackTracker t).prepend(_))
+      exists(any(DataFlow::TypeBackTracker t).prepend_cached(_))
       or
       DataFlow::functionForwardingStep(_, _)
       or

javascript/ql/src/Statements/LabelInCase.ql

@@ -12,8 +12,11 @@
 
 import javascript
 
-from LabeledStmt l, Case c
+int labelInCaseStartColumn(Case c, LabeledStmt l) {
-where
   l = c.getAChildStmt+() and
-  l.getLocation().getStartColumn() = c.getLocation().getStartColumn()
+  result = l.getLocation().getStartColumn()
+}
+
+from LabeledStmt l, Case c
+where labelInCaseStartColumn(c, l) = c.getLocation().getStartColumn()
 select l.getChildExpr(0), "Non-case labels in switch statements are confusing."

ruby/ql/lib/codeql/ruby/frameworks/http_clients/NetHttp.qll

@@ -54,6 +54,14 @@ class NetHttpRequest extends Http::Client::Request::Range, DataFlow::CallNode {
   override DataFlow::Node getAUrlPart() {
     result = request.getArgument(0)
     or
+    result = this.getAUrlPartFromConstructor()
+  }
+
+  /**
+   * Gets a node that contributes to the URL of the request
+   * indirectly, through the constructor.
+   */
+  private DataFlow::Node getAUrlPartFromConstructor() {
     // Net::HTTP.new(...).get(...)
     exists(API::Node new |
       new = API::getTopLevelMember("Net").getMember("HTTP").getInstance() and

swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll

@@ -186,9 +186,17 @@ module Stmts {
       )
     }
 
+    private Element getLastElement() {
+      result = ast.getLastElement()
+    }
+
+    private Element getAnElement() {
+      result = ast.getAnElement()
+    }
+
     predicate lastInner(ControlFlowElement last, Completion c) {
       // Normal exit and no defer statements
-      astLast(ast.getLastElement().getFullyUnresolved(), last, c) and
+      astLast(this.getLastElement().getFullyUnresolved(), last, c) and
       not exists(this.getFirstDeferStmtBody()) and
       c instanceof NormalCompletion
       or
@@ -198,7 +206,7 @@ module Stmts {
       or
       // Abnormal exit without any defer statements
       not c instanceof NormalCompletion and
-      astLast(ast.getAnElement().getFullyUnresolved(), last, c) and
+      astLast(this.getAnElement().getFullyUnresolved(), last, c) and
       not exists(this.getFirstDeferStmtBody())
     }