Merge branch 'main' into michaelrfairhurst/fix-path-problem-nodes-from-outside-workspace

Author: mbaluda

c/common/src/codingstandards/c/Objects.qll

@@ -1,5 +1,5 @@
 import cpp
-import codingstandards.c.StorageDuration
+import codingstandards.cpp.lifetimes.StorageDuration
 import codingstandards.c.IdentifierLinkage
 import semmle.code.cpp.valuenumbering.HashCons
 import codingstandards.cpp.Clvalues

change_notes/2026-01-31-move-iterators-shared-library.md

@@ -0,0 +1,4 @@
+- `A-23-0-1`, `A-23-0-2`, `CTR-51-CPP`, `CTR-52-CPP`, `CTR-53-CPP`, `CTR-54-CPP`, `CTR-55-CPP`, `STR-52-CPP` - `IteratorImplicitlyConvertedToConstIterator.ql`, `ValidContainerElementAccess.ql`, `UsesValidContainerElementAccess.ql`, `GuaranteeGenericCppLibraryFunctionsDoNotOverflow.ql`, `UseValidIteratorRanges.ql`, `DoNotSubtractIteratorsForDifferentContainers.ql`, `DoNotUseAnAdditiveOperatorOnAnIterator.ql`, `UseValidReferencesForElementsOfString.ql`:
+  - Iterator access methods `rbegin`, `rend`, `crbegin`, `crend` are now recognized on containers.
+  - Shared library `Iterators.qll` has been refactored by splitting out container type logic into a separate library and add logic to differentiate types of containers, such as associative, indexed, and strings.
+  - Shared library `Iterators.qll`, used by many queries, has been moved.

cpp/autosar/src/rules/A23-0-1/IteratorImplicitlyConvertedToConstIterator.ql

@@ -13,7 +13,7 @@
 
 import cpp
 import codingstandards.cpp.autosar
-import codingstandards.cpp.Iterators
+import codingstandards.cpp.standardlibrary.Iterators
 
 /*
  *  Due to inconsistent typedefs across STL containers the way this is parsed

cpp/autosar/test/rules/A23-0-1/IteratorImplicitlyConvertedToConstIterator.expected

@@ -1,8 +1,8 @@
 | test.cpp:10:39:10:48 | call to __iterator | Non-const version of container call immediately converted to a `const_iterator`. |
 | test.cpp:13:38:13:42 | call to begin | Non-const version of container call immediately converted to a `const_iterator`. |
 | test.cpp:16:44:16:48 | call to begin | Non-const version of container call immediately converted to a `const_iterator`. |
-| test.cpp:19:43:19:47 | call to begin | Non-const version of container call immediately converted to a `const_iterator`. |
-| test.cpp:22:50:22:54 | call to begin | Non-const version of container call immediately converted to a `const_iterator`. |
+| test.cpp:19:41:19:50 | call to __iterator | Non-const version of container call immediately converted to a `const_iterator`. |
+| test.cpp:22:47:22:57 | call to __iterator | Non-const version of container call immediately converted to a `const_iterator`. |
 | test.cpp:25:8:25:16 | call to __iterator | Non-const version of container call immediately converted to a `const_iterator`. |
 | test.cpp:27:10:27:14 | call to begin | Non-const version of container call immediately converted to a `const_iterator`. |
 | test.cpp:29:11:29:15 | call to begin | Non-const version of container call immediately converted to a `const_iterator`. |

cpp/cert/src/rules/CTR52-CPP/GuaranteeGenericCppLibraryFunctionsDoNotOverflow.ql

@@ -18,7 +18,7 @@
 
 import cpp
 import codingstandards.cpp.cert
-import codingstandards.cpp.Iterators
+import codingstandards.cpp.standardlibrary.Iterators
 import codingstandards.cpp.rules.containeraccesswithoutrangecheck.ContainerAccessWithoutRangeCheck as ContainerAccessWithoutRangeCheck
 import semmle.code.cpp.controlflow.Guards
 import semmle.code.cpp.dataflow.TaintTracking

cpp/cert/src/rules/CTR53-CPP/UseValidIteratorRanges.ql

@@ -18,7 +18,7 @@
 
 import cpp
 import codingstandards.cpp.cert
-import codingstandards.cpp.Iterators
+import codingstandards.cpp.standardlibrary.Iterators
 import semmle.code.cpp.dataflow.DataFlow
 
 predicate startEndArgumentsDoNotPointToTheSameContainer(

cpp/cert/src/rules/CTR54-CPP/DoNotSubtractIteratorsForDifferentContainers.ql

@@ -18,7 +18,7 @@
 
 import cpp
 import codingstandards.cpp.cert
-import codingstandards.cpp.Iterators
+import codingstandards.cpp.standardlibrary.Iterators
 
 /** Models the subtraction operator */
 class SubtractionOperator extends Function {

cpp/cert/src/rules/CTR55-CPP/DoNotUseAnAdditiveOperatorOnAnIterator.ql

@@ -18,7 +18,7 @@
 
 import cpp
 import codingstandards.cpp.cert
-import codingstandards.cpp.Iterators
+import codingstandards.cpp.standardlibrary.Iterators
 import semmle.code.cpp.controlflow.Dominance
 import semmle.code.cpp.dataflow.DataFlow
 

cpp/common/src/codingstandards/cpp/Iterators.qll

@@ -252,78 +252,6 @@ class AdditiveOperatorFunctionCall extends FunctionCall {
   }
 }
 
-/**
- * Models a collection of STL container classes.
- */
-class STLContainer extends Class {
-  STLContainer() {
-    getNamespace() instanceof StdNS and
-    getSimpleName() in [
-        "vector", "list", "deque", "set", "multiset", "map", "multimap", "stack", "queue",
-        "priority_queue", "string", "forward_list", "unordered_set", "unordered_multiset",
-        "unordered_map", "unordered_multimap", "valarray", "string", "basic_string"
-      ]
-    or
-    getSimpleName() = "string"
-    or
-    getSimpleName() = "basic_string"
-  }
-
-  /**
-   * Get a call to a named function of this class.
-   */
-  FunctionCall getACallTo(string name) {
-    exists(Function f |
-      f = getAMemberFunction() and
-      f.hasName(name) and
-      result = f.getACallToThisFunction()
-    )
-  }
-
-  /**
-   * Gets all calls to all functions of this class.
-   */
-  FunctionCall getACallToAFunction() {
-    exists(Function f |
-      f = getAMemberFunction() and
-      result = f.getACallToThisFunction()
-    )
-  }
-
-  FunctionCall getACallToSize() { result = getACallTo("size") }
-
-  FunctionCall getANonConstIteratorBeginFunctionCall() { result = getACallTo("begin") }
-
-  IteratorSource getAConstIteratorBeginFunctionCall() { result = getACallTo("cbegin") }
-
-  IteratorSource getANonConstIteratorEndFunctionCall() { result = getACallTo("end") }
-
-  IteratorSource getAConstIteratorEndFunctionCall() { result = getACallTo("cend") }
-
-  IteratorSource getANonConstIteratorFunctionCall() {
-    result = getACallToAFunction() and
-    result.getTarget().getType() instanceof NonConstIteratorType
-  }
-
-  IteratorSource getAConstIteratorFunctionCall() {
-    result = getACallToAFunction() and
-    result.getTarget().getType() instanceof ConstIteratorType
-  }
-
-  IteratorSource getAnIteratorFunctionCall() {
-    result = getANonConstIteratorFunctionCall() or result = getAConstIteratorFunctionCall()
-  }
-
-  IteratorSource getAnIteratorBeginFunctionCall() {
-    result = getANonConstIteratorBeginFunctionCall() or
-    result = getAConstIteratorBeginFunctionCall()
-  }
-
-  IteratorSource getAnIteratorEndFunctionCall() {
-    result = getANonConstIteratorEndFunctionCall() or result = getAConstIteratorEndFunctionCall()
-  }
-}
-
 /**
  * Models the set of iterator sources. Useful for encapsulating dataflow coming
  * from a function call producing an iterator.
@@ -338,13 +266,6 @@ class IteratorSource extends FunctionCall {
   Variable getOwner() { result = getQualifier().(VariableAccess).getTarget() }
 }
 
-/**
- * Models a variable that is a `STLContainer`
- */
-class STLContainerVariable extends Variable {
-  STLContainerVariable() { getType() instanceof STLContainer }
-}
-
 /**
  * Usually an iterator range consists of two sequential iterator arguments, for
  * example, the start and end. However, there are exceptions to this rule so

cpp/common/src/codingstandards/cpp/dominance/FeasiblePath.qll

@@ -0,0 +1,108 @@
+import cpp
+
+/**
+ * The configuration signature for the FeasiblePath module.
+ *
+ * Typically, the `Node` type should be a `ControlFlowNode`, but it can be overridden to enable
+ * other kinds of graphs.
+ */
+signature module FeasiblePathConfigSig {
+  /** The state type used to carry context through the CFG exploration. */
+  class State;
+
+  /** The node type used to represent CFG nodes. Overridable. */
+  class Node {
+    Node getASuccessor();
+  }
+
+  predicate isStart(State state, Node node);
+
+  predicate isExcludedPath(State state, Node node);
+
+  predicate isEnd(State state, Node node);
+}
+
+/**
+ * A module that finds whether a feasible path exists between two control flow nodes, and
+ * additionally support configuration that should not be traversed.
+ *
+ * Also accepts a state parameter to allow a context to flow through the CFG.
+ *
+ * ## Usage
+ *
+ * Implement the module `ConfigSig`, with some context type:
+ *
+ * ```ql
+ * module MyConfig implements FeasiblePathConfigSig {
+ *   predicate isStart(SomeContext state, ControlFlowNode node) {
+ *     node = state.someStartCondition()
+ *   }
+ *
+ *   predicate isExcludedPath(SomeContext state, ControlFlowNode node) {
+ *    node = state.someExcludedPathCondition()
+ *   }
+ *
+ *   predicate isEnd(SomeContext state, ControlFlowNode node) {
+ *     node = state.someEndCondition()
+ *   }
+ * }
+ *
+ * import FeasiblePath<MyConfig> as MyFeasiblePath
+ * ```
+ *
+ * ## Rationale
+ *
+ * Why does this module exist? While it may be tempting to write:
+ *
+ * ```ql
+ * exists(ControlFlowNode start, ControlFlowNode end) {
+ *   isStart(start) and
+ *   isEnd(end) and
+ *   end = start.getASuccessor*() and
+ *   not exists(ControlFlowNode mid |
+ *     mid = start.getASuccessor+() and
+ *     end = mid.getASuccessor*() and
+ *     isExcludedPath(mid)
+ *   )
+ * }
+ * ```
+ *
+ * This has an unintuitive trap case in looping CFGs:
+ *
+ * ```c
+ * while (cond) {
+ *   start();
+ *   end();
+ *   excluded();
+ * }
+ * ```
+ *
+ * In the above code, `excluded()` is a successor of `start()`, and `end()` is also a successor of
+ * `excluded()` (via the loop back edge). However, there is no path from `start()` to `end()` that
+ * does not pass through `excluded()`.
+ *
+ * This module will correctly handle this case. Forward exploration through the graph will stop
+ * at the `excluded()` nodes, such that only paths from `start()` to `end()` that do not pass
+ * through `excluded()` nodes will be found.
+ */
+module FeasiblePath<FeasiblePathConfigSig Config> {
+  predicate isSuccessor(Config::State state, Config::Node start, Config::Node end) {
+    isMid(state, start, end) and
+    Config::isEnd(state, end)
+  }
+
+  private predicate isMid(Config::State state, Config::Node start, Config::Node mid) {
+    // TODO: Explore if forward-reverse pruning would be beneficial for performance here.
+    Config::isStart(state, start) and
+    (
+      mid = start
+      or
+      exists(Config::Node prevMid |
+        isMid(state, start, prevMid) and
+        mid = prevMid.getASuccessor() and
+        not Config::isExcludedPath(state, mid) and
+        not Config::isEnd(state, prevMid)
+      )
+    )
+  }
+}