Merge branch 'main' into rvermeulen/fix-366

Author: rvermeulen

change_notes/2023-11-22-remove-parameters-a7-1-1.md

@@ -0,0 +1 @@
+ * `A7-1-1` - no longer report parameters as contravening this rule. This is inline with the rule intent as described in the referenced C++ Core Guidelines rule [CON.1](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#con1-by-default-make-objects-immutable), which states "To avoid confusion and lots of false positives, don’t enforce this rule for function parameters."

change_notes/2024-01-12-fix-reported-fp-a3-9-1.md

@@ -0,0 +1,5 @@
+- `A3-9-1` - `VariableWidthIntegerTypesUsed.ql`:
+    - Exclude the plain char type. Still includes `signed char` and `unsigned char`.
+    - Include CV-qualified variable width integer types.
+- `A3-9-1` - `VariableWidthPlainCharTypeUsed.ql`:
+    - New query to support fine grained deviation support for the plain char type.

change_notes/2024-01-17-a4-7-1-exclude-pointers.md

@@ -0,0 +1 @@
+ * `A4-7-1` - exclude pointer increment and decrement operators from this rule.

change_notes/2024-01-17-fix-reported-fp-for-a2-3-1.md

@@ -0,0 +1,2 @@
+`A2-3-1`: ` cpp/autosar/invalid-character-in-string-literal`
+    - Fixes #311. Exclude wide string literals and utf8 string literal.

change_notes/2024-01-24-throwing-functions-exclude-noexcept.md

@@ -0,0 +1,6 @@
+ * Exceptions are no longer propagated from calls to `noexcept` functions, or calls functions with dynamic exception specifications where the exception is not permitted. This is consistent with the default behaviour specified in `[expect.spec]` which indicates that `std::terminate` is called. This has the following impact:
+   - `A15-4-2`, `ERR55-CPP` - reduce false positives for `noexcept` functions which call other `noexcept` function which may throw.
+   - `A15-2-2` - reduce false positives for constructors which call `noexcept` functions.
+   - `A15-4-5` - reduce false positives for checked exceptions that are thrown from `noexcept` functions called by the original function.
+   - `DCL57-CPP` - do not report exceptions thrown from `noexcept` functions called by deallocation functions or destructors.
+   - `A15-5-1`, `M15-3-1` - do not report exceptions thrown from `noexcept` functions called by special functions.

change_notes/2024-01-25-exclusion-m9-3-3.md

@@ -0,0 +1,2 @@
+`M9-3-3` - `MemberFunctionConstIfPossible.ql`, `MemberFunctionStaticIfPossible.ql`:
+    - Fixes #413. Exclude deleted member functions.

change_notes/2024-01-25-fix-reported-fp-for-a8-4-7.md

@@ -0,0 +1,3 @@
+`A8-4-7` - `InParametersForCheapToCopyTypesNotPassedByValue.ql`, `InParametersForNotCheapToCopyTypesNotPassedByReference.ql`:
+    - Fixes #397. Exclude user defined operators and move constructors.`
+    - Exclude parameters for instantiated templates because the declaration location of the function does not contain enough information about the type used in the instantiation to make an actionable alert. 

change_notes/2024-01-26-exclusion-a5-0-2.md

@@ -0,0 +1,2 @@
+`A5-0-2` - `NonBooleanIfStmt.qll`, `NonBooleanIterationStmt.qll`:
+    - Exclude compiler generated conditions.

change_notes/2024-01-30-exclusion-a13-3-1.md

@@ -0,0 +1,2 @@
+`A13-3-1` - `FunctionThatContainsForwardingReferenceAsItsArgumentOverloaded.ql`:
+    - Fixes #399. Exclude functions that have different number of parameters.

change_notes/2024-01-31-exclusion-a16-0-1.md

@@ -0,0 +1,2 @@
+`A16-0-1` - `PreProcessorShallOnlyBeUsedForCertainDirectivesPatterns.ql`:
+    - Exclude all preprocessor elses and also consider elifs separately (ie do not affect valid ifs) but not valid if not meeting the same criteria as an ifdef etc.

change_notes/2024-01-31-fix-fp-a7-1-2.md

@@ -0,0 +1,2 @@
+`A7-1-2` - `VariableMissingConstexpr.ql`:
+    - Fix FP reported in #466. Addresses incorrect assumption that calls to `constexpr` functions are always compile-time evaluated.

change_notes/2024-01-31-fix-fp-m9-3-3.md

@@ -0,0 +1,2 @@
+`M9-3-3`: `MemberFunctionConstIfPossible.ql`:
+    - Fix FP reported in 467. Excluding candidates in uninstantiated templates.

change_notes/2024-02-01-fix-fp-reported-for-a7-1-1.md

@@ -0,0 +1,2 @@
+`A7-1-1` - `DeclarationUnmodifiedObjectMissingConstSpecifier.ql`
+    - Fix FP reported in #372. Exclude compiler generated variables.

cpp/autosar/src/rules/A13-3-1/FunctionThatContainsForwardingReferenceAsItsArgumentOverloaded.ql

@@ -21,10 +21,36 @@ class Candidate extends TemplateFunction {
   }
 }
 
-from Candidate c, Function f
+from
+  Candidate c, Function f, Function overload, Function overloaded, string msg,
+  string firstMsgSegment
 where
   not isExcluded(f,
     OperatorsPackage::functionThatContainsForwardingReferenceAsItsArgumentOverloadedQuery()) and
   not f.isDeleted() and
-  f = c.getAnOverload()
-select f, "Function overloads a $@ with a forwarding reference parameter.", c, "function"
+  f = c.getAnOverload() and
+  // allow for overloading with different number of parameters, because there is no
+  // confusion on what function will be called.
+  f.getNumberOfParameters() = c.getNumberOfParameters() and
+  //build a dynamic select statement that guarantees to read that the overloading function is the explicit one
+  if
+    (f instanceof CopyConstructor or f instanceof MoveConstructor) and
+    f.isCompilerGenerated()
+  then (
+    (
+      f instanceof CopyConstructor and
+      msg = "implicit copy constructor"
+      or
+      f instanceof MoveConstructor and
+      msg = "implicit move constructor"
+    ) and
+    firstMsgSegment = " with a forwarding reference parameter " and
+    overloaded = f and
+    overload = c
+  ) else (
+    msg = "function with a forwarding reference parameter" and
+    firstMsgSegment = " " and
+    overloaded = c and
+    overload = f
+  )
+select overload, "Function" + firstMsgSegment + "overloads a $@.", overloaded, msg

cpp/autosar/src/rules/A16-0-1/PreProcessorShallOnlyBeUsedForCertainDirectivesPatterns.ql

@@ -21,13 +21,26 @@ import cpp
 import codingstandards.cpp.autosar
 import codingstandards.cpp.FunctionLikeMacro
 
+class PermittedInnerDirectiveType extends PreprocessorDirective {
+  PermittedInnerDirectiveType() {
+    //permissive listing for directives that can be used in a valid wrapper
+    this instanceof MacroWrapper or
+    this instanceof PreprocessorEndif or
+    this instanceof Include or
+    this instanceof PermittedMacro or
+    this instanceof PreprocessorElif or
+    this instanceof PreprocessorElse
+  }
+}
+
 class PermittedDirectiveType extends PreprocessorDirective {
   PermittedDirectiveType() {
     //permissive listing in case directive types modelled in ql ever expands (example non valid directives)
     this instanceof MacroWrapper or
     this instanceof PreprocessorEndif or
     this instanceof Include or
-    this instanceof PermittedMacro
+    this instanceof PermittedMacro or
+    this instanceof PreprocessorElse
   }
 }
 
@@ -40,9 +53,9 @@ pragma[noinline]
 predicate isPreprocConditionalRange(
   PreprocessorBranch pb, string filepath, int startLine, int endLine
 ) {
-  exists(PreprocessorEndif end | pb.getEndIf() = end |
-    isPreprocFileAndLine(pb, filepath, startLine) and
-    isPreprocFileAndLine(end, filepath, endLine)
+  isPreprocFileAndLine(pb, filepath, startLine) and
+  exists(PreprocessorDirective end |
+    pb.getNext() = end and isPreprocFileAndLine(end, filepath, endLine)
   )
 }
 
@@ -73,7 +86,7 @@ class MacroWrapper extends PreprocessorIfndef {
 class AcceptableWrapper extends PreprocessorBranch {
   AcceptableWrapper() {
     forall(Element inner | not inner instanceof Comment and this = getAGuard(inner) |
-      inner instanceof PermittedDirectiveType
+      inner instanceof PermittedInnerDirectiveType
     )
   }
 }

cpp/autosar/src/rules/A2-3-1/InvalidCharacterInStringLiteral.ql

@@ -17,6 +17,7 @@
 
 import cpp
 import codingstandards.cpp.autosar
+import codingstandards.cpp.Literals
 
 bindingset[s]
 string getCharOutsideBasicSourceCharSet(string s) {
@@ -27,6 +28,9 @@ string getCharOutsideBasicSourceCharSet(string s) {
 from StringLiteral s, string ch
 where
   not isExcluded(s, NamingPackage::invalidCharacterInStringLiteralQuery()) and
-  ch = getCharOutsideBasicSourceCharSet(s.getValueText())
+  ch = getCharOutsideBasicSourceCharSet(s.getValueText()) and
+  // wide string and utf8 string literals are exempted.
+  not s instanceof WideStringLiteral and
+  not s instanceof Utf8StringLiteral
 select s,
   "String literal uses the character '" + ch + "' that is outside the language basic character set."

cpp/autosar/src/rules/A3-9-1/VariableWidthIntegerTypesUsed.ql

@@ -1,8 +1,8 @@
 /**
  * @id cpp/autosar/variable-width-integer-types-used
  * @name A3-9-1: Use fixed-width integer types instead of basic, variable-width, integer types
- * @description The basic numerical types of char, int, short, long are not supposed to be used. The
- *              specific-length types from <cstdint> header need be used instead.
+ * @description The basic numerical types of signed/unsigned char, int, short, long are not supposed
+ *              to be used. The specific-length types from <cstdint> header need be used instead.
  * @kind problem
  * @precision very-high
  * @problem.severity error
@@ -19,15 +19,16 @@ import cpp
 import codingstandards.cpp.autosar
 import codingstandards.cpp.EncapsulatingFunctions
 import codingstandards.cpp.BuiltInNumericTypes
+import codingstandards.cpp.Type
 
-from Variable v
+from Variable v, Type typeStrippedOfSpecifiers
 where
   not isExcluded(v, DeclarationsPackage::variableWidthIntegerTypesUsedQuery()) and
+  typeStrippedOfSpecifiers = stripSpecifiers(v.getType()) and
   (
-    v.getType() instanceof BuiltInIntegerType or
-    v.getType() instanceof PlainCharType or
-    v.getType() instanceof UnsignedCharType or
-    v.getType() instanceof SignedCharType
+    typeStrippedOfSpecifiers instanceof BuiltInIntegerType or
+    typeStrippedOfSpecifiers instanceof UnsignedCharType or
+    typeStrippedOfSpecifiers instanceof SignedCharType
   ) and
   not v instanceof ExcludedVariable
 select v, "Variable '" + v.getName() + "' has variable-width type."

cpp/autosar/src/rules/A3-9-1/VariableWidthPlainCharTypeUsed.ql

@@ -0,0 +1,26 @@
+/**
+ * @id cpp/autosar/variable-width-plain-char-type-used
+ * @name A3-9-1: Use a fixed-width integer type instead of a char type
+ * @description The basic numerical type char is not supposed to be used. The specific-length types
+ *              from <cstdint> header need be used instead.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/autosar/id/a3-9-1
+ *       correctness
+ *       security
+ *       maintainability
+ *       external/autosar/allocated-target/implementation
+ *       external/autosar/enforcement/automated
+ *       external/autosar/obligation/required
+ */
+
+import cpp
+import codingstandards.cpp.autosar
+import codingstandards.cpp.Type
+
+from Variable variable
+where
+  not isExcluded(variable, DeclarationsPackage::variableWidthPlainCharTypeUsedQuery()) and
+  stripSpecifiers(variable.getType()) instanceof PlainCharType
+select variable, "Variable '" + variable.getName() + "' has variable-width char type."

cpp/autosar/src/rules/A7-1-1/DeclarationUnmodifiedObjectMissingConstSpecifier.ql

@@ -36,5 +36,6 @@ where
     else cond = " is used for an object"
   ) and
   not exists(LambdaExpression lc | lc.getACapture().getField() = v) and
-  not v.isFromUninstantiatedTemplate(_)
+  not v.isFromUninstantiatedTemplate(_) and
+  not v.isCompilerGenerated()
 select v, "Non-constant variable " + v.getName() + cond + " and is not modified."

cpp/autosar/src/rules/A7-1-1/DeclarationUnmodifiedParamMissingConstSpecifier.ql

@@ -17,6 +17,7 @@ import cpp
 import codingstandards.cpp.autosar
 import codingstandards.cpp.TrivialType
 import codingstandards.cpp.SideEffect
+import semmle.code.cpp.controlflow.SSA
 
 predicate isZeroInitializable(Variable v) {
   not exists(v.getInitializer().getExpr()) and
@@ -33,6 +34,78 @@ 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
@@ -46,7 +119,7 @@ where
   (
     v.getInitializer().getExpr().isConstant()
     or
-    v.getInitializer().getExpr().(Call).getTarget().isConstexpr()
+    any(Call call | isCompileTimeEvaluated(call)) = v.getInitializer().getExpr()
     or
     isZeroInitializable(v)
     or

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

@@ -17,6 +17,7 @@ import cpp
 import codingstandards.cpp.autosar
 import TriviallySmallType
 import codingstandards.cpp.CommonTypes as CommonTypes
+import codingstandards.cpp.Class
 
 /*
  * For the purposes of this rule, "cheap to copy" is defined as a trivially copyable type that is no
@@ -34,8 +35,10 @@ where
   ) and
   t.isConst() and
   not exists(CatchBlock cb | cb.getParameter() = v) and
-  not exists(CopyConstructor cc | cc.getAParameter() = v) and
-  not v.isFromUninstantiatedTemplate(_)
+  not exists(SpecialMemberFunction cc | cc.getAParameter() = v) and
+  not exists(Operator op | op.getAParameter() = v) and
+  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 '" + t.getName() +
+    "' but it is passed by reference instead of by value."