Merge branch 'main' into rvermeulen/fix-issue-#311

Author: rvermeulen

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-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/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-14-fix-fp-m0-1-4.md

@@ -0,0 +1,4 @@
+- `M0-1-4` - `SingleUseMemberPODVariable.ql`:
+  - Address FP reported in #388. Include aggregrate initialization as a use of a member.
+  - Include indirect initialization of members. For example, casting a pointer to a buffer to a struct pointer.
+  - Reformat the alert message to adhere to the style-guide.

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.

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/M0-1-3/UnusedGlobalOrNamespaceVariable.ql

@@ -22,5 +22,7 @@ from PotentiallyUnusedGlobalOrNamespaceVariable v
 where
   not isExcluded(v, DeadCodePackage::unusedGlobalOrNamespaceVariableQuery()) and
   // No variable access
-  not exists(v.getAnAccess())
-select v, "Variable " + v.getQualifiedName() + " is unused."
+  not exists(v.getAnAccess()) and
+  // Exclude members whose value is compile time and is potentially used to inintialize a template
+  not maybeACompileTimeTemplateArgument(v)
+select v, "Variable '" + v.getQualifiedName() + "' is unused."

cpp/autosar/src/rules/M0-1-3/UnusedLocalVariable.ql

@@ -50,4 +50,4 @@ where
   // Local variable is never accessed
   not exists(v.getAnAccess()) and
   getUseCountConservatively(v) = 0
-select v, "Local variable " + v.getName() + " in " + v.getFunction().getName() + " is not used."
+select v, "Local variable '" + v.getName() + "' in '" + v.getFunction().getName() + "' is not used."

cpp/autosar/src/rules/M0-1-3/UnusedMemberVariable.ql

@@ -25,5 +25,7 @@ where
   // No variable access
   not exists(v.getAnAccess()) and
   // No explicit initialization in a constructor
-  not exists(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v)
-select v, "Member variable " + v.getName() + " is unused."
+  not exists(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v) and
+  // Exclude members whose value is compile time and is potentially used to inintialize a template
+  not maybeACompileTimeTemplateArgument(v)
+select v, "Member variable '" + v.getName() + "' is unused."

cpp/autosar/src/rules/M0-1-4/SingleUseMemberPODVariable.ql

@@ -24,5 +24,5 @@ where
   not isExcluded(v, DeadCodePackage::singleUseMemberPODVariableQuery()) and
   isSingleUseNonVolatilePODVariable(v)
 select v,
-  "Member POD variable " + v.getName() + " in " + v.getDeclaringType().getName() + " is only $@.",
-  getSingleUse(v), "used once"
+  "Member POD variable '" + v.getName() + "' in '" + v.getDeclaringType().getName() +
+    "' is only $@.", getSingleUse(v), "used once"

cpp/autosar/src/rules/M0-1-4/SingleUsePODVariable.qll

@@ -10,23 +10,68 @@ private string getConstExprValue(Variable v) {
   v.isConstexpr()
 }
 
+/**
+ * Gets the number of uses of variable `v` in an opaque assignment, where an opaqua assignment for example a cast from one type to the other and `v` is assumed to be a member of the resulting type.
+ * e.g.,
+ * struct foo {
+ *  int bar;
+ * }
+ *
+ * struct foo * v = (struct foo*)buffer;
+ */
+Expr getIndirectSubObjectAssignedValue(MemberVariable subobject) {
+  // struct foo * ptr = (struct foo*)buffer;
+  exists(Struct someStruct, Variable instanceOfSomeStruct | someStruct.getAMember() = subobject |
+    instanceOfSomeStruct.getType().(PointerType).getBaseType() = someStruct and
+    exists(Cast assignedValue |
+      // Exclude cases like struct foo * v = nullptr;
+      not assignedValue.isImplicit() and
+      // `v` is a subobject of another type that reinterprets another object. We count that as a use of `v`.
+      assignedValue.getExpr() = instanceOfSomeStruct.getAnAssignedValue() and
+      result = assignedValue
+    )
+    or
+    // struct foo; read(..., (char *)&foo);
+    instanceOfSomeStruct.getType() = someStruct and
+    exists(Call externalInitializerCall, Cast castToCharPointer, int n |
+      externalInitializerCall.getArgument(n).(AddressOfExpr).getOperand() =
+        instanceOfSomeStruct.getAnAccess() and
+      externalInitializerCall.getArgument(n) = castToCharPointer.getExpr() and
+      castToCharPointer.getType().(PointerType).getBaseType().getUnspecifiedType() instanceof
+        CharType and
+      result = externalInitializerCall
+    )
+    or
+    // the object this subject is part of is initialized and we assumes this initializes the subobject.
+    instanceOfSomeStruct.getType() = someStruct and
+    result = instanceOfSomeStruct.getInitializer().getExpr()
+  )
+}
+
 /** Gets a "use" count according to rule M0-1-4. */
 int getUseCount(Variable v) {
-  exists(int initializers |
-    // We enforce that it's a POD type variable, so if it has an initializer it is explicit
-    (if v.hasInitializer() then initializers = 1 else initializers = 0) and
-    result =
-      initializers +
-        count(VariableAccess access | access = v.getAnAccess() and not access.isCompilerGenerated())
-        + count(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v) +
-        // For constexpr variables used as template arguments, we don't see accesses (just the
-        // appropriate literals). We therefore take a conservative approach and count the number of
-        // template instantiations that use the given constant, and consider each one to be a use
-        // of the variable
-        count(ClassTemplateInstantiation cti |
-          cti.getTemplateArgument(_).(Expr).getValue() = getConstExprValue(v)
-        )
-  )
+  // We enforce that it's a POD type variable, so if it has an initializer it is explicit
+  result =
+    count(getAUserInitializedValue(v)) +
+      count(VariableAccess access | access = v.getAnAccess() and not access.isCompilerGenerated()) +
+      // For constexpr variables used as template arguments, we don't see accesses (just the
+      // appropriate literals). We therefore take a conservative approach and count the number of
+      // template instantiations that use the given constant, and consider each one to be a use
+      // of the variable
+      count(ClassTemplateInstantiation cti |
+        cti.getTemplateArgument(_).(Expr).getValue() = getConstExprValue(v)
+      ) + count(getIndirectSubObjectAssignedValue(v))
+}
+
+Expr getAUserInitializedValue(Variable v) {
+  (
+    result = v.getInitializer().getExpr()
+    or
+    exists(UserProvidedConstructorFieldInit cfi | cfi.getTarget() = v and result = cfi.getExpr())
+    or
+    exists(ClassAggregateLiteral l | not l.isCompilerGenerated() | result = l.getAFieldExpr(v))
+  ) and
+  not result.isCompilerGenerated()
 }
 
 /** Gets a single use of `v`, if `isSingleUseNonVolatilePODVariable` holds. */

cpp/autosar/src/rules/M16-1-1/DefinedMacro.qll

@@ -2,27 +2,27 @@ import cpp
 import codingstandards.cpp.autosar
 
 /**
- * A helper class describing macros wrapping defined operator
+ * A helper class describing macros wrapping the defined operator
  */
-class DefinedMacro extends Macro {
-  DefinedMacro() {
-    this.getBody().regexpMatch("defined\\s*\\(.*")
+class MacroUsesDefined extends Macro {
+  MacroUsesDefined() {
+    // Uses `defined` directly
+    exists(this.getBody().regexpFind("\\bdefined\\b", _, _))
     or
-    this.getBody().regexpMatch("defined[\\s]+|defined$")
+    // Uses a macro that uses `defined` (directly or indirectly)
+    exists(MacroUsesDefined dm | exists(this.getBody().regexpFind(dm.getRegexForMatch(), _, _)))
   }
 
-  Macro getAUse() {
-    result = this or
-    anyAliasing(result, this)
+  /**
+   * Gets a regex for matching uses of this macro.
+   */
+  string getRegexForMatch() {
+    exists(string arguments |
+      // If there are arguments
+      if getHead() = getName() then arguments = "" else arguments = "\\s*\\("
+    |
+      // Use word boundary matching to find identifiers that match
+      result = "\\b" + getName() + "\\b" + arguments
+    )
   }
 }
-
-predicate directAlias(Macro alias, Macro aliased) {
-  not alias.getBody() = alias.getBody().replaceAll(aliased.getHead(), "")
-}
-
-predicate anyAliasing(Macro alias, Macro inQ) {
-  directAlias(alias, inQ)
-  or
-  exists(Macro intermediate | anyAliasing(intermediate, inQ) and anyAliasing(alias, intermediate))
-}

cpp/autosar/src/rules/M16-1-1/DefinedPreProcessorOperatorGeneratedFromExpansionFound.ql

@@ -14,18 +14,12 @@
 
 import cpp
 import codingstandards.cpp.autosar
+import codingstandards.cpp.PreprocessorDirective
 import DefinedMacro
 
-from DefinedMacro m, PreprocessorBranch e
+from PreprocessorIfOrElif e, MacroUsesDefined m
 where
-  (
-    e instanceof PreprocessorIf or
-    e instanceof PreprocessorElif
-  ) and
-  (
-    e.getHead().regexpMatch(m.getAUse().getHead() + "\\s*\\(.*")
-    or
-    e.getHead().regexpMatch(m.getAUse().getHead().replaceAll("(", "\\(").replaceAll(")", "\\)"))
-  ) and
-  not isExcluded(e)
+  not isExcluded(e, MacrosPackage::definedPreProcessorOperatorInOneOfTheTwoStandardFormsQuery()) and
+  // A`#if` or `#elif` which uses a macro which uses `defined`
+  exists(e.getHead().regexpFind(m.getRegexForMatch(), _, _))
 select e, "The macro $@ expands to 'defined'", m, m.getName()

cpp/autosar/src/rules/M16-1-1/DefinedPreProcessorOperatorInOneOfTheTwoStandardForms.ql

@@ -15,6 +15,7 @@
 
 import cpp
 import codingstandards.cpp.autosar
+import codingstandards.cpp.PreprocessorDirective
 
 //get what comes after each 'defined' used with or without parenth
 string matchesDefinedOperator(PreprocessorBranch e) {
@@ -34,12 +35,8 @@ string matchesDefinedOperator(PreprocessorBranch e) {
   )
 }
 
-from PreprocessorBranch e, string arg
+from PreprocessorIfOrElif e, string arg
 where
-  (
-    e instanceof PreprocessorIf or
-    e instanceof PreprocessorElif
-  ) and
   arg = matchesDefinedOperator(e) and
   not arg.regexpMatch("^\\w*$") and
   not isExcluded(e, MacrosPackage::definedPreProcessorOperatorInOneOfTheTwoStandardFormsQuery())

cpp/autosar/test/rules/A18-0-1/CLibraryFacilitiesNotAccessedThroughCPPLibraryHeaders.expected

@@ -19,3 +19,4 @@
 | test.cpp:19:1:19:18 | #include <uchar.h> | C library "uchar.h" is included instead of the corresponding C++ library <cuchar>. |
 | test.cpp:20:1:20:18 | #include <wchar.h> | C library "wchar.h" is included instead of the corresponding C++ library <cwchar>. |
 | test.cpp:21:1:21:19 | #include <wctype.h> | C library "wctype.h" is included instead of the corresponding C++ library <cwctype>. |
+| test.cpp:45:1:45:17 | #include "time.h" | C library "time.h" is included instead of the corresponding C++ library <ctime>. |

cpp/autosar/test/rules/A18-0-1/lib/example.h

@@ -0,0 +1,4 @@
+#ifndef LIB_EXAMPLE_H_
+#define LIB_EXAMPLE_H_
+
+#endif

cpp/autosar/test/rules/A18-0-1/test.cpp

@@ -39,4 +39,7 @@
 #include <ctime>     // COMPLIANT
 #include <cuchar>    // COMPLIANT
 #include <cwchar>    // COMPLIANT
-#include <cwctype>   // COMPLIANT
+#include <cwctype>   // COMPLIANT
+
+#include "lib/example.h" // COMPLIANT
+#include "time.h"        // NON_COMPLIANT

cpp/autosar/test/rules/A18-0-1/time.h

@@ -0,0 +1,4 @@
+#ifndef LIB_TIME_EXAMPLE_H_
+#define LIB_TIME_EXAMPLE_H_
+// may be a user lib or a std lib checked into a project
+#endif

cpp/autosar/test/rules/M0-1-3/UnusedGlobalOrNamespaceVariable.expected

@@ -1,4 +1,4 @@
-| test_global_or_namespace.cpp:3:5:3:6 | g3 | Variable g3 is unused. |
-| test_global_or_namespace.cpp:18:4:18:4 | a | Variable a is unused. |
-| test_global_or_namespace.cpp:26:5:26:6 | x3 | Variable N1::x3 is unused. |
-| test_global_or_namespace.cpp:36:5:36:5 | a | Variable N1::a is unused. |
+| test_global_or_namespace.cpp:3:5:3:6 | g3 | Variable 'g3' is unused. |
+| test_global_or_namespace.cpp:18:4:18:4 | a | Variable 'a' is unused. |
+| test_global_or_namespace.cpp:26:5:26:6 | x3 | Variable 'N1::x3' is unused. |
+| test_global_or_namespace.cpp:36:5:36:5 | a | Variable 'N1::a' is unused. |

cpp/autosar/test/rules/M0-1-3/UnusedLocalVariable.expected

@@ -1,6 +1,6 @@
-| test.cpp:7:7:7:7 | y | Local variable y in test_simple is not used. |
-| test.cpp:14:13:14:13 | y | Local variable y in test_const is not used. |
-| test.cpp:17:7:17:7 | z | Local variable z in test_const is not used. |
-| test.cpp:23:5:23:5 | t | Local variable t in f1 is not used. |
-| test.cpp:23:5:23:5 | t | Local variable t in f1 is not used. |
-| test.cpp:44:6:44:6 | a | Local variable a in test_side_effect_init is not used. |
+| test.cpp:7:7:7:7 | y | Local variable 'y' in 'test_simple' is not used. |
+| test.cpp:14:13:14:13 | y | Local variable 'y' in 'test_const' is not used. |
+| test.cpp:17:7:17:7 | z | Local variable 'z' in 'test_const' is not used. |
+| test.cpp:23:5:23:5 | t | Local variable 't' in 'f1' is not used. |
+| test.cpp:23:5:23:5 | t | Local variable 't' in 'f1' is not used. |
+| test.cpp:44:6:44:6 | a | Local variable 'a' in 'test_side_effect_init' is not used. |

cpp/autosar/test/rules/M0-1-3/UnusedMemberVariable.expected

@@ -1,4 +1,4 @@
-| test_member.cpp:4:7:4:8 | m1 | Member variable m1 is unused. |
-| test_member.cpp:17:9:17:11 | pad | Member variable pad is unused. |
-| test_member.cpp:19:9:19:11 | sm2 | Member variable sm2 is unused. |
-| test_member.cpp:31:7:31:8 | m1 | Member variable m1 is unused. |
+| test_member.cpp:4:7:4:8 | m1 | Member variable 'm1' is unused. |
+| test_member.cpp:17:9:17:11 | pad | Member variable 'pad' is unused. |
+| test_member.cpp:19:9:19:11 | sm2 | Member variable 'sm2' is unused. |
+| test_member.cpp:31:7:31:8 | m1 | Member variable 'm1' is unused. |

cpp/autosar/test/rules/M0-1-3/test_global_or_namespace.cpp

@@ -41,4 +41,22 @@ void test_ns() { x2 = 1; }
 m1(); // ignore dead code in macros
 } // namespace N1
 
-int test_access_variable() { return N1::x5; }
+int test_access_variable() { return N1::x5; }
+
+template <int t> struct C1 {
+  int array[t]; // COMPLIANT
+};
+
+constexpr int g5 = 1; // COMPLIANT - used as template parameter
+
+namespace ns1 {
+constexpr int m1 = 1; // COMPLIANT - used a template parameter
+}
+
+void test_fp_reported_in_384() {
+  struct C1<g5> l1;
+  struct C1<ns1::m1> l2;
+
+  l1.array[0] = 1;
+  l2.array[0] = 1;
+}

cpp/autosar/test/rules/M0-1-3/test_member.cpp

@@ -47,4 +47,18 @@ void test_d() {
   d.getT();
 }
 
+template <int t> struct C1 {
+  int array[t]; // COMPLIANT
+};
+
+struct C2 {
+  static constexpr int m1 = 1; // COMPLIANT - used as template parameter
+};
+
+void test_fp_reported_in_384() {
+  struct C1<C2::m1> l1;
+
+  l1.array[0] = 1;
+}
+
 } // namespace test