Merge pull request #784 from github/michaelrfairhurst/implement-deadc…

…ode-2-unused-object-definitions

Implement RULE-2-8, project should not contain unused object definitions

c/misra/src/codeql-suites/misra-c-default.qls

@@ -7,4 +7,5 @@
 - exclude:
     tags contain:
     - external/misra/audit
+    - external/misra/strict
     - external/misra/default-disabled

c/misra/src/codeql-suites/misra-c-strict.qls

@@ -0,0 +1,8 @@
+- description: MISRA C 2012 (Strict)
+- qlpack: codeql/misra-c-coding-standards
+- include:
+    kind:
+    - problem
+    - path-problem
+    tags contain:
+    - external/misra/strict

c/misra/src/rules/RULE-2-8/UnusedObjectDefinition.ql

@@ -0,0 +1,24 @@
+/**
+ * @id c/misra/unused-object-definition
+ * @name RULE-2-8: A project should not contain unused object definitions
+ * @description Object definitions which are unused should be removed.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity recommendation
+ * @tags external/misra/id/rule-2-8
+ *       maintainability
+ *       performance
+ *       external/misra/c/2012/amendment4
+ *       external/misra/obligation/advisory
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.cpp.deadcode.UnusedObjects
+
+from ReportDeadObject report
+where
+  not isExcluded(report.getPrimaryElement(), DeadCode2Package::unusedObjectDefinitionQuery()) and
+  not report.hasAttrUnused()
+select report.getPrimaryElement(), report.getMessage(), report.getOptionalPlaceholderLocation(),
+  report.getOptionalPlaceholderMessage()

c/misra/src/rules/RULE-2-8/UnusedObjectDefinitionStrict.ql

@@ -0,0 +1,26 @@
+/**
+ * @id c/misra/unused-object-definition-strict
+ * @name RULE-2-8: A project should not contain '__attribute__((unused))' object definitions
+ * @description A strict query which reports all unused object definitions with
+ *              '__attribute__((unused))'.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity recommendation
+ * @tags external/misra/id/rule-2-8
+ *       maintainability
+ *       performance
+ *       external/misra/c/2012/amendment4
+ *       external/misra/c/strict
+ *       external/misra/obligation/advisory
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.cpp.deadcode.UnusedObjects
+
+from ReportDeadObject report
+where
+  not isExcluded(report.getPrimaryElement(), DeadCode2Package::unusedObjectDefinitionStrictQuery()) and
+  report.hasAttrUnused()
+select report.getPrimaryElement(), report.getMessage(), report.getOptionalPlaceholderLocation(),
+  report.getOptionalPlaceholderMessage()

c/misra/test/rules/RULE-2-8/UnusedObjectDefinition.expected

@@ -0,0 +1,12 @@
+| test.c:6:5:6:6 | definition of g2 | Unused object 'g2'. | test.c:6:5:6:6 | test.c:6:5:6:6 | (ignored) |
+| test.c:9:5:9:6 | definition of g3 | Unused object 'g3'. | test.c:9:5:9:6 | test.c:9:5:9:6 | (ignored) |
+| test.c:20:7:20:8 | definition of l2 | Unused object 'l2'. | test.c:20:7:20:8 | test.c:20:7:20:8 | (ignored) |
+| test.c:27:7:27:8 | definition of l5 | Unused object 'l5'. | test.c:27:7:27:8 | test.c:27:7:27:8 | (ignored) |
+| test.c:37:10:37:11 | definition of g5 | Unused object 'g5'. | test.c:37:10:37:11 | test.c:37:10:37:11 | (ignored) |
+| test.c:45:9:45:10 | definition of g6 | Unused object 'g6'. | test.c:45:9:45:10 | test.c:45:9:45:10 | (ignored) |
+| test.c:51:5:51:6 | definition of g7 | Unused object 'g7'. | test.c:51:5:51:6 | test.c:51:5:51:6 | (ignored) |
+| test.c:64:3:64:18 | ONLY_DEF_VAR(x) | Invocation of macro '$@' defines unused object 'l2'. | test.c:60:1:60:34 | test.c:60:1:60:34 | ONLY_DEF_VAR |
+| test.c:68:1:71:5 | #define ALSO_DEF_VAR(x) int x = 0; while (1) ; | Macro 'ALSO_DEF_VAR' defines unused object with an invocation-dependent name, for example, '$@'. | test.c:73:16:73:17 | test.c:73:16:73:17 | l1 |
+| test.c:77:1:82:3 | #define DEF_UNUSED_INNER_VAR() { int _v = 0; while (1) ; } | Macro 'DEF_UNUSED_INNER_VAR' defines unused object '_v'. | test.c:77:1:82:3 | test.c:77:1:82:3 | (ignored) |
+| test.c:119:11:119:13 | definition of g10 | Unused object 'g10'. | test.c:119:11:119:13 | test.c:119:11:119:13 | (ignored) |
+| test.c:124:13:124:14 | definition of l2 | Unused object 'l2'. | test.c:124:13:124:14 | test.c:124:13:124:14 | (ignored) |

c/misra/test/rules/RULE-2-8/UnusedObjectDefinition.qlref

@@ -0,0 +1 @@
+rules/RULE-2-8/UnusedObjectDefinition.ql

c/misra/test/rules/RULE-2-8/UnusedObjectDefinitionStrict.expected

@@ -0,0 +1,4 @@
+| test.c:87:29:87:30 | definition of g8 | Unused object 'g8'. | test.c:87:29:87:30 | test.c:87:29:87:30 | (ignored) |
+| test.c:92:3:92:30 | ONLY_DEF_ATTR_UNUSED_VAR(x) | Invocation of macro '$@' defines unused object 'l2'. | test.c:88:1:88:70 | test.c:88:1:88:70 | ONLY_DEF_ATTR_UNUSED_VAR |
+| test.c:96:1:99:5 | #define ALSO_DEF_ATTR_UNUSED_VAR(x) __attribute__((unused)) int x = 0; while (1) ; | Macro 'ALSO_DEF_ATTR_UNUSED_VAR' defines unused object with an invocation-dependent name, for example, '$@'. | test.c:101:28:101:29 | test.c:101:28:101:29 | l1 |
+| test.c:106:1:111:3 | #define DEF_ATTR_UNUSED_INNER_VAR() { __attribute__((unused)) int _v = 0; while (1) ; } | Macro 'DEF_ATTR_UNUSED_INNER_VAR' defines unused object '_v'. | test.c:106:1:111:3 | test.c:106:1:111:3 | (ignored) |

c/misra/test/rules/RULE-2-8/UnusedObjectDefinitionStrict.qlref

@@ -0,0 +1 @@
+rules/RULE-2-8/UnusedObjectDefinitionStrict.ql

c/misra/test/rules/RULE-2-8/test.c

@@ -0,0 +1,133 @@
+// Not a definition, only a declaration:
+extern int g1; // COMPLIANT
+
+// Both declared + defined:
+extern int g2; // COMPLIANT
+int g2 = 1;    // NON_COMPLIANT
+
+// Definition is only declaration:
+int g3 = 1; // NON_COMPLIANT
+
+// Definition, but value is required for program to compile:
+int g4 = 1; // COMPLIANT
+void f1() { g4; }
+
+// Local variables:
+void f2() {
+  int l1; // COMPLIANT
+  l1;
+
+  int l2; // NON-COMPLIANT
+
+  // Value is required for the program to compile:
+  int l3; // COMPLIANT
+  sizeof(l3);
+
+  int l4, // COMPLIANT
+      l5; // NON-COMPLIANT
+  l4;
+}
+
+// Struct fields are not objects:
+struct s {
+  int x; // COMPLIANT
+};
+
+// Declaration of type struct is an object:
+struct s g5; // NON-COMPLIANT
+
+// Struct fields are not objects:
+union u {
+  int x; // COMPLIANT
+};
+
+// Declaration of type union is an object:
+union u g6; // NON-COMPLIANT
+
+// Typedefs are not objects:
+typedef int td1; // COMPLIANT
+
+// Declaration of typedef type object:
+td1 g7; // NON-COMPLIANT
+
+// Function parameters are not objects:
+void f3(int p) {} // COMPLIANT
+
+// Function type parameters are not objects:
+typedef int td2(int x); // COMPLIANT
+
+// Macros that define unused vars tests:
+#define ONLY_DEF_VAR(x) int x = 0;
+void f4() {
+  ONLY_DEF_VAR(l1); // COMPLIANT
+  l1;
+  ONLY_DEF_VAR(l2); // NON-COMPLIANT
+}
+
+// NON-COMPLIANT
+#define ALSO_DEF_VAR(x)                                                        \
+  int x = 0;                                                                   \
+  while (1)                                                                    \
+    ;
+void f5() {
+  ALSO_DEF_VAR(l1); // COMPLIANT
+  ALSO_DEF_VAR(l2); // COMPLIANT
+}
+
+#define DEF_UNUSED_INNER_VAR()                                                 \
+  {                                                                            \
+    int _v = 0;                                                                \
+    while (1)                                                                  \
+      ;                                                                        \
+  } // NON-COMPLIANT
+void f6() {
+  DEF_UNUSED_INNER_VAR(); // COMPLIANT
+}
+
+__attribute__((unused)) int g8 = 1; // NON-COMPLIANT
+#define ONLY_DEF_ATTR_UNUSED_VAR(x) __attribute__((unused)) int x = 0;
+void f7() {
+  ONLY_DEF_ATTR_UNUSED_VAR(l1); // COMPLIANT
+  l1;
+  ONLY_DEF_ATTR_UNUSED_VAR(l2); // NON-COMPLIANT
+}
+
+// NON-COMPLIANT
+#define ALSO_DEF_ATTR_UNUSED_VAR(x)                                            \
+  __attribute__((unused)) int x = 0;                                           \
+  while (1)                                                                    \
+    ;
+void f8() {
+  ALSO_DEF_ATTR_UNUSED_VAR(l1); // COMPLIANT
+  ALSO_DEF_ATTR_UNUSED_VAR(l2); // COMPLIANT
+}
+
+// NON-COMPLIANT
+#define DEF_ATTR_UNUSED_INNER_VAR()                                            \
+  {                                                                            \
+    __attribute__((unused)) int _v = 0;                                        \
+    while (1)                                                                  \
+      ;                                                                        \
+  }
+
+void f9() {
+  DEF_ATTR_UNUSED_INNER_VAR(); // COMPLIANT
+}
+
+// Const variable tests:
+const int g9 = 1;  // COMPLIANT
+const int g10 = 1; // NON-COMPLIANT
+
+void f10() {
+  g9;
+  const int l1 = 1; // COMPLIANT
+  const int l2 = 1; // NON-COMPLIANT
+  l1;
+}
+
+// Side effects should not disable this rule:
+void f11() {
+  int l1 = 1;    // COMPLIANT
+  int l2 = l1++; // COMPLIANT
+  l2;
+}

cpp/common/src/codingstandards/cpp/AlertReporting.qll

@@ -18,24 +18,46 @@ module MacroUnwrapper<ResultType ResultElement> {
   }
 
   /**
-   * Gets the primary macro that generated the result element.
+   * Gets the primary macro invocation that generated the result element.
    */
-  Macro getPrimaryMacro(ResultElement re) {
+  MacroInvocation getPrimaryMacroInvocation(ResultElement re) {
     exists(MacroInvocation mi |
       mi = getAMacroInvocation(re) and
       // No other more specific macro that expands to element
       not exists(MacroInvocation otherMi |
         otherMi = getAMacroInvocation(re) and otherMi.getParentInvocation() = mi
       ) and
-      result = mi.getMacro()
+      result = mi
     )
   }
 
+  /**
+   * Gets the primary macro that generated the result element.
+   */
+  Macro getPrimaryMacro(ResultElement re) { result = getPrimaryMacroInvocation(re).getMacro() }
+
   /**
    * If a result element is expanded from a macro invocation, then return the "primary" macro that
    * generated the element, otherwise return the element itself.
    */
   Element unwrapElement(ResultElement re) {
     if exists(getPrimaryMacro(re)) then result = getPrimaryMacro(re) else result = re
   }
+
+  /* Final class so we can extend it */
+  final private class FinalMacroInvocation = MacroInvocation;
+
+  /* A macro invocation that expands to create a `ResultElement` */
+  class ResultMacroExpansion extends FinalMacroInvocation {
+    ResultElement re;
+
+    ResultMacroExpansion() { re = getAnExpandedElement() }
+
+    ResultElement getResultElement() { result = re }
+  }
+
+  /* The most specific macro invocation that expands to create this `ResultElement`. */
+  class PrimaryMacroExpansion extends ResultMacroExpansion {
+    PrimaryMacroExpansion() { this = getPrimaryMacroInvocation(re) }
+  }
 }