Merge branch 'initial-misra-cpp-2023-support' of https://github.com/github/codeql-coding-standards into initial-misra-cpp-2023-support

Author: mbaluda

.github/workflows/code-scanning-pack-gen.yml

@@ -99,8 +99,8 @@ jobs:
         run: |
           PATH=$PATH:$CODEQL_HOME/codeql
 
-          codeql query compile --threads 0 cpp
-          codeql query compile --threads 0 c
+          codeql query compile --precompile --threads 0 cpp
+          codeql query compile --precompile --threads 0 c
 
           cd ..
           zip -r codeql-coding-standards/code-scanning-cpp-query-pack.zip codeql-coding-standards/c/ codeql-coding-standards/cpp/ codeql-coding-standards/.codeqlmanifest.json codeql-coding-standards/supported_codeql_configs.json codeql-coding-standards/scripts/configuration codeql-coding-standards/scripts/reports codeql-coding-standards/scripts/shared codeql-coding-standards/scripts/guideline_recategorization codeql-coding-standards/scripts/shared codeql-coding-standards/scripts/schemas

c/cert/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cert-c-coding-standards
-version: 2.28.0-dev
+version: 2.29.0-dev
 description: CERT C 2016
 suites: codeql-suites
 license: MIT

c/cert/test/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cert-c-coding-standards-tests
-version: 2.28.0-dev
+version: 2.29.0-dev
 extractor: cpp
 license: MIT
 dependencies:

c/common/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/common-c-coding-standards
-version: 2.28.0-dev
+version: 2.29.0-dev
 license: MIT
 dependencies:
   codeql/common-cpp-coding-standards: '*'

c/common/test/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/common-c-coding-standards-tests
-version: 2.28.0-dev
+version: 2.29.0-dev
 extractor: cpp
 license: MIT
 dependencies:

c/common/test/rules/identifierhidden/IdentifierHidden.expected

@@ -1,5 +1,5 @@
-| test.c:4:7:4:9 | id1 | Variable is hiding variable $@. | test.c:1:5:1:7 | id1 | id1 |
-| test.c:7:13:7:15 | id1 | Variable is hiding variable $@. | test.c:1:5:1:7 | id1 | id1 |
-| test.c:10:12:10:14 | id1 | Variable is hiding variable $@. | test.c:1:5:1:7 | id1 | id1 |
-| test.c:11:14:11:16 | id1 | Variable is hiding variable $@. | test.c:10:12:10:14 | id1 | id1 |
-| test.c:24:24:24:26 | id2 | Variable is hiding variable $@. | test.c:22:5:22:7 | id2 | id2 |
+| test.c:4:7:4:9 | id1 | Declaration is hiding declaration $@. | test.c:1:5:1:7 | id1 | id1 |
+| test.c:7:13:7:15 | id1 | Declaration is hiding declaration $@. | test.c:1:5:1:7 | id1 | id1 |
+| test.c:10:12:10:14 | id1 | Declaration is hiding declaration $@. | test.c:1:5:1:7 | id1 | id1 |
+| test.c:11:14:11:16 | id1 | Declaration is hiding declaration $@. | test.c:10:12:10:14 | id1 | id1 |
+| test.c:24:24:24:26 | id2 | Declaration is hiding declaration $@. | test.c:22:5:22:7 | id2 | id2 |

c/misra/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/misra-c-coding-standards
-version: 2.28.0-dev
+version: 2.29.0-dev
 description: MISRA C 2012
 suites: codeql-suites
 license: MIT

c/misra/test/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/misra-c-coding-standards-tests
-version: 2.28.0-dev
+version: 2.29.0-dev
 extractor: cpp
 license: MIT
 dependencies:

change_notes/2024-02-27-identifier-hidden.md

@@ -0,0 +1,3 @@
+- `A2-10-1`, `RULE-5-3` - `IdentifierHiding.ql`, `IdentifierHidingC.ql`:
+  - Address FN reported in #118. Rule was missing detection of functions. Additionally omitted class template instantiations.
+  - Fix FP for identifiers in nested namespaces.

change_notes/2024-04-12-fix-fp-m9-3-3.md

@@ -0,0 +1,2 @@
+`M9-3-3`: `MemberFunctionConstIfPossible.ql`:
+    - Fix FP reported in 381. Omit member functions that return nonconst reference types.

cpp/autosar/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/autosar-cpp-coding-standards
-version: 2.28.0-dev
+version: 2.29.0-dev
 description: AUTOSAR C++14 Guidelines R22-11, R21-11, R20-11, R19-11 and R19-03
 suites: codeql-suites
 license: MIT

cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql

@@ -18,6 +18,7 @@ import codingstandards.cpp.autosar
 import codingstandards.cpp.TrivialType
 import codingstandards.cpp.SideEffect
 import semmle.code.cpp.controlflow.SSA
+import codingstandards.cpp.Expr
 
 predicate isZeroInitializable(Variable v) {
   not exists(v.getInitializer().getExpr()) and
@@ -34,78 +35,6 @@ predicate isTypeZeroInitializable(Type t) {
   t.getUnderlyingType() instanceof ArrayType
 }
 
-/**
- * An optimized set of expressions used to determine the flow through constexpr variables.
- */
-class VariableAccessOrCallOrLiteral extends Expr {
-  VariableAccessOrCallOrLiteral() {
-    this instanceof VariableAccess or
-    this instanceof Call or
-    this instanceof Literal
-  }
-}
-
-/**
- * Holds if the value of source flows through compile time evaluated variables to target.
- */
-predicate flowsThroughConstExprVariables(
-  VariableAccessOrCallOrLiteral source, VariableAccessOrCallOrLiteral target
-) {
-  (
-    source = target
-    or
-    source != target and
-    exists(SsaDefinition intermediateDef, StackVariable intermediate |
-      intermediateDef.getAVariable().getFunction() = source.getEnclosingFunction() and
-      intermediateDef.getAVariable().getFunction() = target.getEnclosingFunction() and
-      intermediateDef.getAVariable() = intermediate and
-      intermediate.isConstexpr()
-    |
-      DataFlow::localExprFlow(source, intermediateDef.getDefiningValue(intermediate)) and
-      flowsThroughConstExprVariables(intermediateDef.getAUse(intermediate), target)
-    )
-  )
-}
-
-/*
- * Returns true if the given call may be evaluated at compile time and is compile time evaluated because
- * all its arguments are compile time evaluated and its default values are compile time evaluated.
- */
-
-predicate isCompileTimeEvaluated(Call call) {
-  // 1. The call may be evaluated at compile time, because it is constexpr, and
-  call.getTarget().isConstexpr() and
-  // 2. all its arguments are compile time evaluated, and
-  forall(DataFlow::Node ultimateArgSource, DataFlow::Node argSource |
-    argSource = DataFlow::exprNode(call.getAnArgument()) and
-    DataFlow::localFlow(ultimateArgSource, argSource) and
-    not DataFlow::localFlowStep(_, ultimateArgSource)
-  |
-    (
-      ultimateArgSource.asExpr() instanceof Literal
-      or
-      any(Call c | isCompileTimeEvaluated(c)) = ultimateArgSource.asExpr()
-    ) and
-    // If the ultimate argument source is not the same as the argument source, then it must flow through
-    // constexpr variables.
-    (
-      ultimateArgSource != argSource
-      implies
-      flowsThroughConstExprVariables(ultimateArgSource.asExpr(), argSource.asExpr())
-    )
-  ) and
-  // 3. all the default values used are compile time evaluated.
-  forall(Expr defaultValue, Parameter parameterUsingDefaultValue, int idx |
-    parameterUsingDefaultValue = call.getTarget().getParameter(idx) and
-    not exists(call.getArgument(idx)) and
-    parameterUsingDefaultValue.getAnAssignedValue() = defaultValue
-  |
-    defaultValue instanceof Literal
-    or
-    any(Call c | isCompileTimeEvaluated(c)) = defaultValue
-  )
-}
-
 from Variable v
 where
   not isExcluded(v, ConstPackage::variableMissingConstexprQuery()) and
@@ -119,7 +48,7 @@ where
   (
     v.getInitializer().getExpr().isConstant()
     or
-    any(Call call | isCompileTimeEvaluated(call)) = v.getInitializer().getExpr()
+    any(Call call | isCompileTimeEvaluatedCall(call)) = v.getInitializer().getExpr()
     or
     isZeroInitializable(v)
     or

cpp/autosar/src/rules/M9-3-3/MemberFunctionConstIfPossible.ql

@@ -37,6 +37,13 @@ class NonConstMemberFunction extends MemberFunction {
   NonConstMemberFunction() { not this.hasSpecifier("const") }
 }
 
+/**
+ * References that are not const
+ */
+class NonConstReferenceType extends ReferenceType {
+  NonConstReferenceType() { not this.isConst() }
+}
+
 /**
  * `MemberFunction`s that are not const
  * and not `Constructor`s ect as const constructors are
@@ -57,7 +64,9 @@ class ConstMemberFunctionCandidate extends NonConstMemberFunction {
     this.hasDefinition() and
     // For uninstantiated templates we have only partial information that prevents us from determining
     // if the candidate calls non-const functions. Therefore we exclude these.
-    not this.isFromUninstantiatedTemplate(_)
+    not this.isFromUninstantiatedTemplate(_) and
+    // Cannot recommend const if it returns a non-const reference.
+    not this.getType() instanceof NonConstReferenceType
   }
 
   /**

cpp/autosar/test/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/autosar-cpp-coding-standards-tests
-version: 2.28.0-dev
+version: 2.29.0-dev
 extractor: cpp
 license: MIT
 dependencies:

cpp/autosar/test/rules/M9-3-3/test.cpp

@@ -193,3 +193,25 @@ void test_template() {
 class Z3 {
   void f(int) = delete; // COMPLIANT
 };
+
+class Z4 {
+public:
+  int values[128];
+  template <typename T>
+  void fill(const T &val) { // COMPLIANT[FALSE_NEGATIVE|TRUE_NEGATIVE] -
+                            // exception not specified in the
+    // standard, we opt to not raise an issue because the template can be both
+    // compliant and non-compliant depending on instantiations.
+    for (auto &elem : values) {
+      elem = val;
+    }
+  }
+  constexpr int &front() noexcept { return values[0]; } // COMPLIANT
+};
+
+void fp_reported_in_381() {
+  // added to test template initialization effects/lack thereof
+  Z4 z;
+  int i = z.front();
+  z.fill(i);
+}

cpp/cert/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cert-cpp-coding-standards
-version: 2.28.0-dev
+version: 2.29.0-dev
 description: CERT C++ 2016
 suites: codeql-suites
 license: MIT

cpp/cert/test/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cert-cpp-coding-standards-tests
-version: 2.28.0-dev
+version: 2.29.0-dev
 extractor: cpp
 license: MIT
 dependencies:

cpp/common/src/codingstandards/cpp/Expr.qll

@@ -191,8 +191,89 @@ module MisraExpr {
 }
 
 /**
+ * An optimized set of expressions used to determine the flow through constexpr variables.
+ */
+class VariableAccessOrCallOrLiteral extends Expr {
+  VariableAccessOrCallOrLiteral() {
+    this instanceof VariableAccess and this.(VariableAccess).getTarget().isConstexpr()
+    or
+    this instanceof Call
+    or
+    this instanceof Literal
+  }
+}
+
+/**
+ * Holds if the value of source flows through compile time evaluated variables to target.
+ */
+predicate flowsThroughConstExprVariables(
+  VariableAccessOrCallOrLiteral source, VariableAccessOrCallOrLiteral target
+) {
+  (
+    source = target
+    or
+    source != target and
+    exists(SsaDefinition intermediateDef, StackVariable intermediate |
+      intermediateDef.getAVariable().getFunction() = source.getEnclosingFunction() and
+      intermediateDef.getAVariable().getFunction() = target.getEnclosingFunction() and
+      intermediateDef.getAVariable() = intermediate and
+      intermediate.isConstexpr()
+    |
+      DataFlow::localExprFlow(source, intermediateDef.getDefiningValue(intermediate)) and
+      flowsThroughConstExprVariables(intermediateDef.getAUse(intermediate), target)
+    )
+  )
+}
+
+predicate isCompileTimeEvaluatedExpression(Expr expression) {
+  forall(DataFlow::Node ultimateSource, DataFlow::Node source |
+    source = DataFlow::exprNode(expression) and
+    DataFlow::localFlow(ultimateSource, source) and
+    not DataFlow::localFlowStep(_, ultimateSource)
+  |
+    isDirectCompileTimeEvaluatedExpression(ultimateSource.asExpr()) and
+    // If the ultimate source is not the same as the source, then it must flow through
+    // constexpr variables.
+    (
+      ultimateSource != source
+      implies
+      flowsThroughConstExprVariables(ultimateSource.asExpr(), source.asExpr())
+    )
+  )
+}
+
+predicate isDirectCompileTimeEvaluatedExpression(Expr expression) {
+  expression instanceof Literal
+  or
+  any(Call c | isCompileTimeEvaluatedCall(c)) = expression
+}
+
+/*
+ * Returns true if the given call may be evaluated at compile time and is compile time evaluated because
+ * all its arguments are compile time evaluated and its default values are compile time evaluated.
+ */
+
+predicate isCompileTimeEvaluatedCall(Call call) {
+  // 1. The call may be evaluated at compile time, because it is constexpr, and
+  call.getTarget().isConstexpr() and
+  // 2. all its arguments are compile time evaluated, and
+  forall(Expr argSource | argSource = call.getAnArgument() |
+    isCompileTimeEvaluatedExpression(argSource)
+  ) and
+  // 3. all the default values used are compile time evaluated.
+  forall(Expr defaultValue, Parameter parameterUsingDefaultValue, int idx |
+    parameterUsingDefaultValue = call.getTarget().getParameter(idx) and
+    not exists(call.getArgument(idx)) and
+    parameterUsingDefaultValue.getAnAssignedValue() = defaultValue
+  |
+    isDirectCompileTimeEvaluatedExpression(defaultValue)
+  )
+}
+
+/*
  * an operator that does not evaluate its operand
  */
+
 class UnevaluatedExprExtension extends Expr {
   UnevaluatedExprExtension() {
     this.getAChild().isUnevaluated()