First draft of Rule 0-1-1, unnecessary assignments.

Author: MichaelRFairhurst

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

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/SuccessorUnless.qll

@@ -0,0 +1,108 @@
+import cpp
+
+/**
+ * The configuration signature for the SuccessorUnless module.
+ *
+ * Typically, the `Node` type should be a `ControlFlowNode`, but it can be overridden to enable
+ * other kinds of graphs.
+ */
+signature module SuccessorUnlessConfigSig {
+  /** 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 isUnless(State state, Node node);
+
+  predicate isEnd(State state, Node node);
+}
+
+/**
+ * A module that finds successor of a node -- unless there is an intermediate node that satisfies
+ * a given condition.
+ *
+ * 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 SuccessorUnless<SomeContext>::ConfigSig {
+ *   predicate isStart(SomeContext state, ControlFlowNode node) {
+ *     node = state.someStartCondition()
+ *   }
+ *
+ *   predicate isUnless(SomeContext state, ControlFlowNode node) {
+ *    node = state.someUnlessCondition()
+ *   }
+ *
+ *   predicate isEnd(SomeContext state, ControlFlowNode node) {
+ *     node = state.someEndCondition()
+ *   }
+ * }
+ *
+ * import SuccessorUnless<SomeContext>::Make<MyConfig> as Successor
+ * ```
+ *
+ * ## 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
+ *     isUnless(mid)
+ *   )
+ * }
+ * ```
+ *
+ * This has an unintuitive trap case in looping CFGs:
+ *
+ * ```c
+ * while (cond) {
+ *   start();
+ *   end();
+ *   unless();
+ * }
+ * ```
+ *
+ * In the above code, `unless()` is a successor of `start()`, and `end()` is also a successor of
+ * `unless()` (via the loop back edge). However, there is no path from `start()` to `end()` that
+ * does not pass through `unless()`.
+ *
+ * This module will correctly handle this case. Forward exploration through the graph will stop
+ * at the `unless()` nodes, such that only paths from `start()` to `end()` that do not pass through
+ * `unless()` nodes will be found.
+ */
+module SuccessorUnless<SuccessorUnlessConfigSig 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::isUnless(state, mid) and
+        not Config::isEnd(state, prevMid)
+      )
+    )
+  }
+}

cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode5.qll

@@ -0,0 +1,26 @@
+//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY.  **/
+import cpp
+import RuleMetadata
+import codingstandards.cpp.exclusions.RuleMetadata
+
+newtype DeadCode5Query = TUnnecessaryWriteToLocalObjectQuery()
+
+predicate isDeadCode5QueryMetadata(Query query, string queryId, string ruleId, string category) {
+  query =
+    // `Query` instance for the `unnecessaryWriteToLocalObject` query
+    DeadCode5Package::unnecessaryWriteToLocalObjectQuery() and
+  queryId =
+    // `@id` for the `unnecessaryWriteToLocalObject` query
+    "cpp/misra/unnecessary-write-to-local-object" and
+  ruleId = "RULE-0-1-1" and
+  category = "advisory"
+}
+
+module DeadCode5Package {
+  Query unnecessaryWriteToLocalObjectQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `unnecessaryWriteToLocalObject` query
+      TQueryCPP(TDeadCode5PackageQuery(TUnnecessaryWriteToLocalObjectQuery()))
+  }
+}

cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll

@@ -16,6 +16,7 @@ import Const
 import Conversions
 import Conversions2
 import DeadCode
+import DeadCode5
 import Declarations
 import ExceptionSafety
 import Exceptions1
@@ -78,6 +79,7 @@ newtype TCPPQuery =
   TConversionsPackageQuery(ConversionsQuery q) or
   TConversions2PackageQuery(Conversions2Query q) or
   TDeadCodePackageQuery(DeadCodeQuery q) or
+  TDeadCode5PackageQuery(DeadCode5Query q) or
   TDeclarationsPackageQuery(DeclarationsQuery q) or
   TExceptionSafetyPackageQuery(ExceptionSafetyQuery q) or
   TExceptions1PackageQuery(Exceptions1Query q) or
@@ -140,6 +142,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat
   isConversionsQueryMetadata(query, queryId, ruleId, category) or
   isConversions2QueryMetadata(query, queryId, ruleId, category) or
   isDeadCodeQueryMetadata(query, queryId, ruleId, category) or
+  isDeadCode5QueryMetadata(query, queryId, ruleId, category) or
   isDeclarationsQueryMetadata(query, queryId, ruleId, category) or
   isExceptionSafetyQueryMetadata(query, queryId, ruleId, category) or
   isExceptions1QueryMetadata(query, queryId, ruleId, category) or