Merge branch 'main' into lcartey/m5-3-3

Author: lcartey

change_notes/2024-01-30-fix-fp-for-a4-7-1.md

@@ -0,0 +1,2 @@
+`A4-7-1`: `IntegerExpressionLeadToDataLoss.ql`
+ - Fix #368: Incorrectly reporting `/=` as a cause for data loss.

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-a4-5-1.md

@@ -0,0 +1,4 @@
+`A4-5-1`: `EnumUsedInArithmeticContexts.ql`:
+    - Address incorrect exclusion of the binary operator `&`.
+    - Address incorrect inclusion of the unary operator `&`.
+    - Fix FP reported in #366.

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-02-13-fix-fn-M7-3-6.md

@@ -0,0 +1,2 @@
+- `M7-3-6` - `UsingDeclarationsUsedInHeaderFiles.ql`:
+  - Address FN reported in #400. Only using-declarations are exempted from class- and function-scope.

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/A4-5-1/EnumUsedInArithmeticContexts.ql

@@ -18,44 +18,26 @@
 
 import cpp
 import codingstandards.cpp.autosar
+import codingstandards.cpp.Operator
+import codingstandards.cpp.Type
 
-/*
- * Get an operand to all overloaded operator member functions, except:
- *   operator[]
- *   operator=
- *   operator==
- *   operator!=
- *   operator&
- *   operator<
- *   operator<=
- *   operator>
- *   operator>=
- */
-
-Expr getAnOperandOfAllowedOverloadedOperator(FunctionCall fc) {
-  fc.getAnArgument() = result and
-  fc.getTarget().getName().regexpMatch("operator(?!\\[]$|=$|==$|!=$|&$|<$|<=$|>$|>=$).+")
-}
-
-Expr getAnOperandOfAllowedOperation(Operation o) {
-  o.getAnOperand() = result and
-  not (
-    o instanceof AssignExpr or
-    o instanceof BitwiseAndExpr or
-    o instanceof ComparisonOperation
-  )
+class AllowedOperatorUse extends OperatorUse {
+  AllowedOperatorUse() {
+    this.getOperator() in ["[]", "=", "==", "!=", "<", "<=", ">", ">="]
+    or
+    this.(UnaryOperatorUse).getOperator() = "&"
+  }
 }
 
-from Expr e, Expr operand
+from OperatorUse operatorUse, Access access, Enum enum
 where
-  not isExcluded(e, ExpressionsPackage::enumUsedInArithmeticContextsQuery()) and
+  not isExcluded(access, ExpressionsPackage::enumUsedInArithmeticContextsQuery()) and
+  operatorUse.getAnOperand() = access and
   (
-    operand = getAnOperandOfAllowedOverloadedOperator(e)
-    or
-    operand = getAnOperandOfAllowedOperation(e)
+    access.(EnumConstantAccess).getTarget().getDeclaringEnum() = enum or
+    access.(VariableAccess).getType() = enum
   ) and
-  (
-    operand instanceof EnumConstantAccess or
-    operand.(VariableAccess).getType() instanceof Enum
-  )
-select e, "Enum $@ is used as an operand of arithmetic operation.", operand, "expression"
+  not operatorUse instanceof AllowedOperatorUse and
+  // Enums that implement the BitmaskType trait are an exception.
+  not enum instanceof BitmaskType
+select access, "Enum $@ is used as an operand of arithmetic operation.", enum, enum.getName()

cpp/autosar/src/rules/A4-7-1/IntegerExpressionLeadToDataLoss.ql

@@ -30,5 +30,7 @@ where
   not e instanceof MulExpr and
   // Not covered by this query - overflow/underflow in division is rare
   not e instanceof DivExpr and
-  not e instanceof RemExpr
+  not e instanceof AssignDivExpr and
+  not e instanceof RemExpr and
+  not e instanceof AssignRemExpr
 select e, "Binary expression ..." + e.getOperator() + "... may overflow."

cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.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/M7-3-6/UsingDeclarationsUsedInHeaderFiles.ql

@@ -28,5 +28,13 @@ predicate isInClassScope(UsingEntry u) { exists(Class c | u.getEnclosingElement(
 from UsingEntry u
 where
   not isExcluded(u, BannedSyntaxPackage::usingDeclarationsUsedInHeaderFilesQuery()) and
-  (isInHeaderFile(u) and not isInFunctionScope(u) and not isInClassScope(u))
+  isInHeaderFile(u) and
+  (
+    u instanceof UsingDeclarationEntry
+    implies
+    (
+      not isInFunctionScope(u) and
+      not isInClassScope(u)
+    )
+  )
 select u, "Using directive or declaration used in a header file " + u.getFile() + "."