Merge branch 'main' into rvermeulen/fix-396

Author: rvermeulen

README.md

@@ -6,15 +6,12 @@ This repository contains CodeQL queries and libraries which support various Codi
 
 _Carnegie Mellon and CERT are registered trademarks of Carnegie Mellon University._
 
-This repository contains CodeQL queries and libraries which support various Coding Standards for the [C++14](https://www.iso.org/standard/64029.html) programming language.
+This repository contains CodeQL queries and libraries which support various Coding Standards for the [C++14](https://www.iso.org/standard/64029.html), [C99](https://www.iso.org/standard/29237.html) and [C11](https://www.iso.org/standard/57853.html) programming languages.
 
 The following coding standards are supported:
 - [AUTOSAR - Guidelines for the use of C++14 language in critical and safety-related systems (Releases R22-11, R20-11, R19-11 and R19-03)](https://www.autosar.org/fileadmin/standards/R22-11/AP/AUTOSAR_RS_CPP14Guidelines.pdf). 
 - [MISRA C++:2008](https://www.misra.org.uk) (support limited to the rules specified in AUTOSAR).
 - [SEI CERT C++ Coding Standard: Rules for Developing Safe, Reliable, and Secure Systems (2016 Edition)](https://resources.sei.cmu.edu/library/asset-view.cfm?assetID=494932)
-
-In addition, the following Coding Standards for the C programming language are under development:
-
 - [SEI CERT C Coding Standard: Rules for Developing Safe, Reliable, and Secure Systems (2016 Edition)](https://resources.sei.cmu.edu/downloads/secure-coding/assets/sei-cert-c-coding-standard-2016-v01.pdf)
 - [MISRA C 2012](https://www.misra.org.uk/product/misra-c2012-third-edition-first-revision/).
 

c/misra/src/rules/RULE-20-8/ControllingExpressionIfDirective.ql

@@ -14,40 +14,35 @@
 
 import cpp
 import codingstandards.c.misra
+import codingstandards.cpp.PreprocessorDirective
 
 /*  A controlling expression is evaluated if it is not excluded (guarded by another controlling expression that is not taken). This translates to it either being taken or not taken.  */
 predicate isEvaluated(PreprocessorBranch b) { b.wasTaken() or b.wasNotTaken() }
 
-class IfOrElifPreprocessorBranch extends PreprocessorBranch {
-  IfOrElifPreprocessorBranch() {
-    this instanceof PreprocessorIf or this instanceof PreprocessorElif
-  }
-}
-
 /**
  * Looks like it contains a single macro, which may be undefined
  */
-class SimpleMacroPreprocessorBranch extends IfOrElifPreprocessorBranch {
+class SimpleMacroPreprocessorBranch extends PreprocessorIfOrElif {
   SimpleMacroPreprocessorBranch() { this.getHead().regexpMatch("[a-zA-Z_][a-zA-Z0-9_]+") }
 }
 
-class SimpleNumericPreprocessorBranch extends IfOrElifPreprocessorBranch {
+class SimpleNumericPreprocessorBranch extends PreprocessorIfOrElif {
   SimpleNumericPreprocessorBranch() { this.getHead().regexpMatch("[0-9]+") }
 }
 
 class ZeroOrOnePreprocessorBranch extends SimpleNumericPreprocessorBranch {
   ZeroOrOnePreprocessorBranch() { this.getHead().regexpMatch("[0|1]") }
 }
 
-predicate containsOnlySafeOperators(IfOrElifPreprocessorBranch b) {
+predicate containsOnlySafeOperators(PreprocessorIfOrElif b) {
   containsOnlyDefinedOperator(b)
   or
   //logic: comparison operators eval last, so they make it safe?
   b.getHead().regexpMatch(".*[\\&\\&|\\|\\||>|<|==].*")
 }
 
 //all defined operators is definitely safe
-predicate containsOnlyDefinedOperator(IfOrElifPreprocessorBranch b) {
+predicate containsOnlyDefinedOperator(PreprocessorIfOrElif b) {
   forall(string portion |
     portion =
       b.getHead()
@@ -65,7 +60,7 @@ class BinaryValuedMacro extends Macro {
   BinaryValuedMacro() { this.getBody().regexpMatch("\\(?(0|1)\\)?") }
 }
 
-from IfOrElifPreprocessorBranch b, string msg
+from PreprocessorIfOrElif b, string msg
 where
   not isExcluded(b, Preprocessor3Package::controllingExpressionIfDirectiveQuery()) and
   isEvaluated(b) and

change_notes/2023-10-04-aggregate-literals-from-variadic-templates.md

@@ -0,0 +1,2 @@
+ `M8-5-2` - `AggregateLiteralEnhancements.qll`:
+   - recognise aggregate literals initialized with parameters from variadic templates.

change_notes/2023-11-03-identifier-hiding-improvements.md

@@ -0,0 +1,3 @@
+ - `A2-10-1`, `RULE-5-3`:
+   - Reduce false positives by considering point of declaration for local variables.
+   - Reduce false negatives by considering catch block parameters to be in scope in the catch block.

change_notes/2023-11-05-m6-5-5-const-refs.md

@@ -0,0 +1,3 @@
+ - `M6-5-5`:
+   - Reduce false positives by no longer considering the taking of a const reference as a modification.
+   - Improve detection of non-local modification of loop iteration variables to reduce false positives.

change_notes/2023-12-06-m16-1-1-perf.md

@@ -0,0 +1,4 @@
+ - `M16-1-1` - `DefinedPreProcessorOperatorGeneratedFromExpansionFound.ql`:
+   - Optimize query to improve performance
+   - Improve detection of macros whose body contains the `defined` operator after the start of the macro (e.g. `#define X Y || defined(Z)`).
+   - Enable exclusions to be applied for this rule.

change_notes/2023-12-07-fix-deviations.md

@@ -0,0 +1,2 @@
+ - The following queries have been updated to address issues with applying deviations:
+   - `A18-5-11`, `A23-0-1`, `A9-3-1`, `M0-1-2`, `M3-1-2`, `M3-2-1`, `M3-2-3`, `M3-9-1`, `M4-5-3`, `M5-0-2`, `M5-2-10`, `A23-0-2`, `CTR51-CPP`, `STR52-CPP`

change_notes/2024-01-16-m5-2-10-arith-only.md

@@ -0,0 +1,2 @@
+ `M5-2-10` - `IncrementAndDecrementOperatorsMixedWithOtherOperatorsInExpression.ql`:
+   - only report use of the increment and decrement operations in conjunction with arithmetic operators, as specified by the rule. Notably we no longer report the expressions of the form `*p++`, which combine increment and dereferencing operations.

change_notes/2024-01-30-fix-fp-for-a8-4-8.md

@@ -0,0 +1,7 @@
+- `A8-4-8` - `OutParametersUsed.ql`
+  - Fixes #370 - Non-member user-defined assignment operator and stream insertion/extraction parameters that are required to be out parameters are excluded.
+  - Broadens the definition of out parameter by considering assignment and crement operators as modifications to an out parameter candidate.
+- `FIO51-CPP` - `CloseFilesWhenTheyAreNoLongerNeeded.ql`:
+  - Broadened definition of `IStream` and `OStream` types may result in reduced false negatives.
+- `A5-1-1` - `LiteralValueUsedOutsideTypeInit.ql`:
+  - Broadened definition of `IStream` types may result in reduced false positives because more file stream function calls may be detected as logging operations that will be excluded from the results.

change_notes/2024-02-12-improve-a18-0-1.md

@@ -0,0 +1,2 @@
+- `A18-0-1` - `CLibraryFacilitiesNotAccessedThroughCPPLibraryHeaders.ql`:
+    - Fix issue #7 - improve query logic to only match on exact standard library names (e.g., now excludes sys/header.h type headers from the results as those are not C standard libraries).

change_notes/2024-02-13-fix-fp-a15-4-4.md

@@ -0,0 +1,2 @@
+-`A15-4-4` - `MissingNoExcept.ql`:
+    - Fix FP reported in #424. Exclude functions calling `std::string::reserve` or `std::string::append` that may throw even if their signatures don't specify it.

change_notes/2024-02-15-fix-fp-m0-1-3.md

@@ -0,0 +1,4 @@
+- `M0-1-3` - `UnusedMemberVariable.ql`, `UnusedGlobalOrNamespaceVariable.ql`:
+  - Address FP reported in #384. Exclude variables with compile time values that may have been used as a template argument.
+  - Exclude uninstantiated template members.
+  - Reformat the alert message to adhere to the style-guide.

change_notes/2024-02-16-fix-fps-a5-1-1.md

@@ -0,0 +1,7 @@
+- `A5-1-1` - `LiteralValueUsedOutsideTypeInit.ql`:
+  - Address FP reported in #371. Exclude literals generated by uses of constexpr variables.
+  - Exclude literals used in class template instantiations.
+  - Update the alert message to adhere to the style-guide.
+  - Exclude boolean literals used as template arguments.
+  - Exclude `u` and `U` prefixed char literals.
+  - Exclude literals part of a class aggregate literal.

change_notes/2024-02-22-fix-fp-a5-0-2-and-change-alert-m5-3-1.md

@@ -0,0 +1,4 @@
+- `A5-0-2` - `NonBooleanIterationCondition.ql`:
+  - Address FP reported in #10. Exclude conditions in uninstantiated templates.
+- `M5-3-1` - `EachOperandOfTheOperatorTheLogicalAndOrTheLogicalOperatorsShallHaveTypeBool.ql`:
+  - Adjust the alert message to comply with the style guide.

change_notes/2024-02-26-exclusion-M5-14-1.md

@@ -0,0 +1,2 @@
+- `M5-14-1` - `RightHandOperandOfALogicalAndOperatorsContainSideEffects.ql`:
+    - Fix FP reported in #375. Addresses incorrect detection of side effects in unevaluated contexts.

change_notes/2024-03-04-fix-fp-a16-2-2.md

@@ -0,0 +1,2 @@
+- `A16-2-2` - `UnusedIncludeDirectives.ql`:
+  - Address FP reported in #453. Exclude reporting of redundant include directives indirectly included by included files.

change_notes/2024-03-04-improve-a8-4-7.md

@@ -0,0 +1,3 @@
+- `A8-4-7` - `InParametersForNotCheapToCopyTypesNotPassedByReference.ql`, `InParametersForCheapToCopyTypesNotPassedByValue.ql`:
+  - Improve coverage of the query by additionally alerting to non-trivially-copyable types being passed by value.
+  - Non-trivially-copyable types not passed by value will no longer be incorrectly reported.

cpp/autosar/src/rules/A16-2-2/UnusedIncludeDirectives.ql

@@ -223,10 +223,19 @@ private predicate firstReliableProvide(File f, File g, int line) {
 
 cached
 predicate mayProvideFirst(IncludeDepends i, File g) {
-  // i may provide g and does not come after a reliable include of g.
+  // i may provide g
   i.provides(g) and
-  not exists(int line | firstReliableProvide(i.getFile(), g, line) |
-    line < i.getLocation().getStartLine()
+  (
+    // and does not come after a reliable include of g.
+    not exists(int line | firstReliableProvide(i.getFile(), g, line) |
+      line < i.getLocation().getStartLine()
+    )
+    or
+    // or it comes after a reliable include of g, and although redundant,
+    // is not necessarily an issue e.g. in the case of libraries with
+    // public header forwards to an internal header.
+    // therefore, hold for transitive includes as well to exclude those results.
+    not i.getIncludedFile() = g
   )
 }
 

cpp/autosar/src/rules/A18-0-1/CLibraryFacilitiesNotAccessedThroughCPPLibraryHeaders.ql

@@ -28,7 +28,7 @@ where
    *    not use any of 'signal.h's facilities, for example.
    */
 
-  filename = i.getIncludedFile().getBaseName() and
+  filename = i.getIncludeText().substring(1, i.getIncludeText().length() - 1) and
   filename in [
       "assert.h", "ctype.h", "errno.h", "fenv.h", "float.h", "inttypes.h", "limits.h", "locale.h",
       "math.h", "setjmp.h", "signal.h", "stdarg.h", "stddef.h", "stdint.h", "stdio.h", "stdlib.h",

cpp/autosar/src/rules/A18-5-11/OperatorNewAndOperatorDeleteNotDefinedLocally.ql

@@ -19,7 +19,8 @@ import codingstandards.cpp.autosar
 
 from MemberFunction operator_new, Class c
 where
-  not isExcluded(operator_new) and
+  not isExcluded(operator_new,
+    DeclarationsPackage::operatorNewAndOperatorDeleteNotDefinedLocallyQuery()) and
   not isExcluded(c, DeclarationsPackage::operatorNewAndOperatorDeleteNotDefinedLocallyQuery()) and
   operator_new.hasName("operator new") and
   operator_new.getDeclaringType() = c and

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

@@ -40,8 +40,8 @@ import codingstandards.cpp.Iterators
 
 from ConstIteratorVariable v, STLContainer c, Expr e
 where
-  not isExcluded(v) and
-  not isExcluded(e) and
+  not isExcluded(v, IteratorsPackage::iteratorImplicitlyConvertedToConstIteratorQuery()) and
+  not isExcluded(e, IteratorsPackage::iteratorImplicitlyConvertedToConstIteratorQuery()) and
   e = v.getAnAssignedValue() and
   e.getAChild*() = /* see note at top of query */ c.getANonConstIteratorFunctionCall()
 select e, "Non-const version of container call immediately converted to a `const_iterator`."

cpp/autosar/src/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.ql

@@ -18,6 +18,7 @@ import cpp
 import codingstandards.cpp.autosar
 import codingstandards.cpp.LoggingOperation
 import codingstandards.cpp.Literals
+import codingstandards.cpp.Cpp14Literal
 
 from Literal l
 where
@@ -35,11 +36,11 @@ where
   // Exclude literal 0
   not l.getValue() = "0" and
   // Exclude character literals
-  not l instanceof CharLiteral and
+  not l instanceof Cpp14Literal::CharLiteral and
   // Exclude `nullptr`
   not l.getType() instanceof NullPointerType and
   // Exclude boolean `true` and `false`
-  not l.getType() instanceof BoolType and
+  not l instanceof BoolLiteral and
   // Exclude empty string
   not l.getValue() = "" and
   // Template functions use literals to represent calls which are unknown
@@ -51,7 +52,14 @@ where
   // Aggregate literal
   not l = any(ArrayOrVectorAggregateLiteral aal).getAnElementExpr(_).getAChild*() and
   // Ignore x - 1 expressions
-  not exists(SubExpr se | se.getRightOperand() = l and l.getValue() = "1")
-select l,
-  "Literal value " + getTruncatedLiteralText(l) + " used outside of type initialization " +
-    l.getAPrimaryQlClass()
+  not exists(SubExpr se | se.getRightOperand() = l and l.getValue() = "1") and
+  // Exclude compile time computed integral literals as they can appear as integral literals
+  // when used as non-type template arguments.
+  // We limit ourselves to integral literals, because floating point literals as non-type
+  // template arguments are not supported in C++ 14. Those are supported shince C++ 20.
+  not l instanceof CompileTimeComputedIntegralLiteral and
+  // Exclude literals to instantiate a class template per example in the standard
+  // where an type of std::array is intialized with size 5.
+  not l = any(ClassTemplateInstantiation cti).getATemplateArgument() and
+  not l = any(ClassAggregateLiteral cal).getAFieldExpr(_)
+select l, "Literal value '" + getTruncatedLiteralText(l) + "' used outside of type initialization."

cpp/autosar/src/rules/A8-4-7/InParametersForCheapToCopyTypesNotPassedByValue.ql

@@ -15,7 +15,7 @@
 
 import cpp
 import codingstandards.cpp.autosar
-import TriviallySmallType
+import TriviallyCopyableSmallType
 import codingstandards.cpp.CommonTypes as CommonTypes
 import codingstandards.cpp.Class
 
@@ -26,7 +26,7 @@ import codingstandards.cpp.Class
  * In this rule, we will look cases where a "cheap to copy" type is not passed by value.
  */
 
-from Parameter v, TriviallySmallType t
+from Parameter v, TriviallyCopyableSmallType t
 where
   not isExcluded(v, ClassesPackage::inParametersForCheapToCopyTypesNotPassedByValueQuery()) and
   exists(ReferenceType rt |
@@ -40,5 +40,6 @@ where
   not v.isFromUninstantiatedTemplate(_) and
   not v.isFromTemplateInstantiation(_)
 select v,
-  "Parameter '" + v.getName() + "' is the trivially copyable type '" + t.getName() +
-    "' but it is passed by reference instead of by value."
+  "Parameter '" + v.getName() +
+    "' is the trivially copyable type $@ but it is passed by reference instead of by value.", t,
+  t.getName()

cpp/autosar/src/rules/A8-4-7/InParametersForNotCheapToCopyTypesNotPassedByReference.ql

@@ -15,7 +15,7 @@
 
 import cpp
 import codingstandards.cpp.autosar
-import TriviallySmallType
+import TriviallyCopyableSmallType
 import codingstandards.cpp.CommonTypes as CommonTypes
 
 /*
@@ -28,12 +28,12 @@ import codingstandards.cpp.CommonTypes as CommonTypes
 from Parameter v
 where
   not isExcluded(v, ClassesPackage::inParametersForNotCheapToCopyTypesNotPassedByReferenceQuery()) and
-  not v.getType() instanceof TriviallySmallType and
+  not v.getType() instanceof TriviallyCopyableSmallType and
   not v.getType().getUnderlyingType() instanceof ReferenceType and
   not exists(CatchBlock cb | cb.getParameter() = v) and
   not v.isFromUninstantiatedTemplate(_) and
   not v.isFromTemplateInstantiation(_)
 select v,
-  "Parameter " + v.getName() +
-    " is the trivially non-copyable type $@ but it is passed by value instead of by reference.",
+  "Parameter '" + v.getName() +
+    "' is the trivially non-copyable type $@ but it is passed by value instead of by reference.",
   v.getType(), v.getType().getName()

cpp/autosar/src/rules/A8-4-7/TriviallySmallType.qll renamed to cpp/autosar/src/rules/A8-4-7/TriviallyCopyableSmallType.qll

@@ -1,5 +1,6 @@
 import cpp
 import codingstandards.cpp.autosar
+import codingstandards.cpp.TrivialType
 
 /**
  * Get the largest word size, in bytes. Some projects may have multiple different
@@ -13,6 +14,9 @@ int wordSize() { result = max(VoidPointerType v | | v.getSize()) }
 bindingset[bytes]
 int bytesToWords(int bytes) { result = bytes / wordSize() }
 
-class TriviallySmallType extends Type {
-  TriviallySmallType() { exists(int size | size = this.getSize() | bytesToWords(size) <= 2) }
+class TriviallyCopyableSmallType extends Type {
+  TriviallyCopyableSmallType() {
+    isTriviallyCopyableType(this) and
+    exists(int size | size = this.getSize() | bytesToWords(size) <= 2)
+  }
 }

cpp/autosar/src/rules/A8-4-8/OutputParametersUsed.ql

@@ -23,31 +23,60 @@ import codingstandards.cpp.ConstHelpers
 import codingstandards.cpp.Operator
 
 /**
- * Non-const T& and T* `Parameter`s to `Function`s
+ * Holds if p is passed as a non-const reference or pointer and is modified.
+ * This holds for in-out or out-only parameters.
  */
-class NonConstReferenceOrPointerParameterCandidate extends FunctionParameter {
-  NonConstReferenceOrPointerParameterCandidate() {
-    this instanceof NonConstReferenceParameter
-    or
-    this instanceof NonConstPointerParameter
-  }
+predicate isOutParameter(NonConstPointerorReferenceParameter p) {
+  any(VariableEffect ve).getTarget() = p
+}
+
+/**
+ * Holds if parameter `p` is a parameter to a user defined assignment operator that
+ * is defined outside of a class body.
+ * These require an in-out parameter as the first argument.
+ */
+predicate isNonMemberUserAssignmentParameter(NonConstPointerorReferenceParameter p) {
+  p.getFunction() instanceof UserAssignmentOperator and
+  not p.isMember()
+}
+
+/**
+ * Holds if parameter `p` is a parameter to a stream insertion operator that
+ * is defined outside of a class body.
+ * These require an in-out parameter as the first argument.
+ *
+ * e.g., `std::ostream& operator<<(std::ostream& os, const T& obj)`
+ */
+predicate isStreamInsertionStreamParameter(NonConstPointerorReferenceParameter p) {
+  exists(StreamInsertionOperator op | not op.isMember() | op.getParameter(0) = p)
 }
 
-pragma[inline]
-predicate isFirstAccess(VariableAccess va) {
-  not exists(VariableAccess otherVa |
-    otherVa.getTarget() = va.getTarget() or
-    otherVa.getQualifier().(VariableAccess).getTarget() = va.getTarget()
-  |
-    otherVa.getASuccessor() = va
+/**
+ * Holds if parameter `p` is a parameter to a stream insertion operator that
+ * is defined outside of a class body.
+ * These require an in-out parameter as the first argument and an out parameter for the second.
+ *
+ * e.g., `std::istream& operator>>(std::istream& is, T& obj)`
+ */
+predicate isStreamExtractionParameter(NonConstPointerorReferenceParameter p) {
+  exists(StreamExtractionOperator op | not op.isMember() |
+    op.getParameter(0) = p
+    or
+    op.getParameter(1) = p
   )
 }
 
-from NonConstReferenceOrPointerParameterCandidate p, VariableEffect ve
+predicate isException(NonConstPointerorReferenceParameter p) {
+  isNonMemberUserAssignmentParameter(p) and p.getIndex() = 0
+  or
+  isStreamInsertionStreamParameter(p)
+  or
+  isStreamExtractionParameter(p)
+}
+
+from NonConstPointerorReferenceParameter p
 where
   not isExcluded(p, ConstPackage::outputParametersUsedQuery()) and
-  ve.getTarget() = p and
-  isFirstAccess(ve.getAnAccess()) and
-  not ve instanceof AnyAssignOperation and
-  not ve instanceof CrementOperation
-select p, "Out parameter " + p.getName() + " that is modified before being read."
+  isOutParameter(p) and
+  not isException(p)
+select p, "Out parameter '" + p.getName() + "' used."