Merge pull request #94 from knewbury01/knewbury01/Declarations3

Package Declarations3

Author: knewbury01

c/cert/src/rules/DCL31-C/DeclareIdentifiersBeforeUsingThem.md

@@ -153,7 +153,7 @@ Search for [vulnerabilities](https://wiki.sei.cmu.edu/confluence/display/c/BB.+D
 
 ## Implementation notes
 
-This query does not check for implicit function declarations as this is partially compiler checked.
+This query does not check for implicitly typed parameters, typedefs or member declarations as this is partially compiler checked.
 
 ## References
 

c/cert/src/rules/DCL31-C/DeclareIdentifiersBeforeUsingThem.ql

@@ -13,16 +13,10 @@
 
 import cpp
 import codingstandards.c.cert
+import codingstandards.cpp.rules.typeomitted.TypeOmitted
 
-from Declaration d
-where
-  not isExcluded(d, Declarations1Package::declareIdentifiersBeforeUsingThemQuery()) and
-  d.hasSpecifier("implicit_int") and
-  exists(Type t |
-    (d.(Variable).getType() = t or d.(Function).getType() = t) and
-    // Exclude "short" or "long", as opposed to "short int" or "long int".
-    t instanceof IntType and
-    // Exclude "signed" or "unsigned", as opposed to "signed int" or "unsigned int".
-    not exists(IntegralType it | it = t | it.isExplicitlySigned() or it.isExplicitlyUnsigned())
-  )
-select d, "Declaration " + d.getName() + " is missing a type specifier."
+class DeclareIdentifiersBeforeUsingThem extends TypeOmittedSharedQuery {
+  DeclareIdentifiersBeforeUsingThem() {
+    this = Declarations1Package::declareIdentifiersBeforeUsingThemQuery()
+  }
+}

c/cert/test/rules/DCL31-C/DeclareIdentifiersBeforeUsingThem.qlref

@@ -0,0 +1 @@
+c/common/test/rules/typeomitted/TypeOmitted.ql

c/cert/test/rules/DCL31-C/DeclareIdentifiersBeforeUsingThem.testref

@@ -0,0 +1,16 @@
+import cpp
+
+//Identifiers that are candidates for checking uniqueness
+class InterestingIdentifiers extends Declaration {
+  InterestingIdentifiers() {
+    not this.isFromTemplateInstantiation(_) and
+    not this.isFromUninstantiatedTemplate(_) and
+    not this instanceof TemplateParameter and
+    not this.hasDeclaringType() and
+    not this instanceof Operator and
+    not this.hasName("main") and
+    exists(this.getADeclarationLocation())
+  }
+
+  string getSignificantName() { result = this.getName().prefix(31) }
+}

c/common/src/codingstandards/c/Identifiers.qll

@@ -0,0 +1,5 @@
+| test.c:4:7:4:9 | id1 | Variable is hiding variable $@. | test.c:1:5:1:7 | id1 | id1 |
+| test.c:7:13:7:15 | id1 | Variable is hiding variable $@. | test.c:1:5:1:7 | id1 | id1 |
+| test.c:10:12:10:14 | id1 | Variable is hiding variable $@. | test.c:1:5:1:7 | id1 | id1 |
+| test.c:11:14:11:16 | id1 | Variable is hiding variable $@. | test.c:10:12:10:14 | id1 | id1 |
+| test.c:24:24:24:26 | id2 | Variable is hiding variable $@. | test.c:22:5:22:7 | id2 | id2 |

c/common/test/rules/identifierhidden/IdentifierHidden.expected

@@ -0,0 +1,2 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.identifierhidden.IdentifierHidden

c/common/test/rules/identifierhidden/IdentifierHidden.ql

@@ -0,0 +1,30 @@
+int id1;
+
+void f1() {
+  int id1; // NON_COMPLIANT
+}
+
+void f2(int id1) {} // NON_COMPLIANT
+
+void f3() {
+  for (int id1; id1 < 1; id1++) { // NON_COMPLIANT
+    for (int id1; id1 < 1; id1++) {
+    } // NON_COMPLIANT
+  }
+}
+
+struct astruct {
+  int id1;
+};
+
+extern void g(struct astruct *p);
+
+int id2 = 0;
+
+void f4(struct astruct id2) { // NON_COMPLIANT
+  g(&id2);
+}
+
+void f5(struct astruct id3) { // COMPLIANT
+  g(&id2);
+}

c/common/test/rules/identifierhidden/test.c

@@ -0,0 +1,2 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.typeomitted.TypeOmitted

c/cert/test/rules/DCL31-C/DeclareIdentifiersBeforeUsingThem.expected renamed to c/common/test/rules/typeomitted/TypeOmitted.expected

@@ -13,3 +13,13 @@ int f1(void) { // COMPLIANT
 short g2;                 // COMPLIANT
 long g3;                  // COMPLIANT
 signed g4() { return 1; } // COMPLIANT
+
+typedef *newtype3; // NON_COMPLIANT[FALSE_NEGATIVE]
+
+int f2(const x) { // NON_COMPLIANT[FALSE_NEGATIVE]
+  return 1;
+}
+
+struct str {
+  const y; // NON_COMPLIANT[FALSE_NEGATIVE]
+} s;

c/common/test/rules/typeomitted/TypeOmitted.ql

@@ -0,0 +1,24 @@
+/**
+ * @id c/misra/identifier-hiding-c
+ * @name RULE-5-3: An identifier declared in an inner scope shall not hide an identifier declared in an outer scope
+ * @description Use of an identifier declared in an inner scope with an identical name to an
+ *              identifier in an outer scope can lead to inadvertent errors if the incorrect
+ *              identifier is modified.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity warning
+ * @tags external/misra/id/rule-5-3
+ *       readability
+ *       maintainability
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.cpp.rules.identifierhidden.IdentifierHidden
+
+class IdentifierHidingCQuery extends IdentifierHiddenSharedQuery {
+  IdentifierHidingCQuery() {
+    this = Declarations3Package::identifierHidingCQuery()
+  }
+}

c/cert/test/rules/DCL31-C/test.c renamed to c/common/test/rules/typeomitted/test.c

@@ -0,0 +1,31 @@
+/**
+ * @id c/misra/identifiers-not-distinct-from-macro-names
+ * @name RULE-5-5: Identifiers shall be distinct from macro names
+ * @description Reusing a macro name compared to the name of any other identifier can cause
+ *              confusion and make code harder to read.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/misra/id/rule-5-5
+ *       readability
+ *       maintainability
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.c.Identifiers
+
+from Macro m, InterestingIdentifiers i, string mName, string iName
+where
+  not isExcluded(m, Declarations3Package::identifiersNotDistinctFromMacroNamesQuery()) and
+  not isExcluded(i, Declarations3Package::identifiersNotDistinctFromMacroNamesQuery()) and
+  mName = iName and
+  (
+    //C99 states the first 31 characters of external identifiers are significant
+    //C90 states the first 6 characters of external identifiers are significant and case is not required to be significant
+    //C90 is not currently considered by this rule
+    if m.getName().length() > 31 then mName = m.getName().prefix(31) else mName = m.getName()
+  ) and
+  if i.getName().length() > 31 then iName = i.getSignificantName() else iName = i.getName()
+select m, "Macro name is nonunique compared to $@.", i, i.getName()

c/misra/src/rules/RULE-5-3/IdentifierHidingC.ql

@@ -0,0 +1,28 @@
+/**
+ * @id c/misra/typedef-name-not-unique
+ * @name RULE-5-6: A typedef name shall be a unique identifier
+ * @description Reusing a typedef name compared to the name of any other identifier can cause
+ *              confusion and make code harder to read.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/misra/id/rule-5-6
+ *       readability
+ *       maintainability
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.c.Identifiers
+
+from TypedefType t, InterestingIdentifiers d
+where
+  not isExcluded(t, Declarations3Package::typedefNameNotUniqueQuery()) and
+  not isExcluded(d, Declarations3Package::typedefNameNotUniqueQuery()) and
+  not t.getADeclarationLocation() = d.getADeclarationLocation() and
+  t.getName() = d.getName() and
+  //exception cases
+  not d.(Struct).getName() = t.getBaseType().toString() and
+  not d.(Enum).getName() = t.getBaseType().toString()
+select t, "Typedef name is nonunique compared to $@.", d, d.getName()

c/misra/src/rules/RULE-5-5/IdentifiersNotDistinctFromMacroNames.ql

@@ -0,0 +1,28 @@
+/**
+ * @id c/misra/tag-name-not-unique
+ * @name RULE-5-7: A tag name shall be a unique identifier
+ * @description Reusing a tag name compared to the name of any tag can cause confusion and make code
+ *              harder to read.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/misra/id/rule-5-7
+ *       readability
+ *       maintainability
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.c.Identifiers
+
+from Struct s, InterestingIdentifiers s2
+where
+  not isExcluded(s, Declarations3Package::tagNameNotUniqueQuery()) and
+  not isExcluded(s2, Declarations3Package::tagNameNotUniqueQuery()) and
+  not s = s2 and
+  s.getName() = s2.getName() and
+  not s.getName() = "struct <unnamed>" and
+  not s.getName() = "union <unnamed>" and
+  not s.getName() = s2.(TypedefType).getBaseType().toString()
+select s, "Tag name is nonunique compared to $@.", s2, s2.getName()

c/misra/src/rules/RULE-5-6/TypedefNameNotUnique.ql

@@ -0,0 +1,20 @@
+/**
+ * @id c/misra/explicitly-declare-types
+ * @name RULE-8-1: Declare identifiers before using them
+ * @description Omission of type specifiers may not be supported by some compilers.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/misra/id/rule-8-1
+ *       correctness
+ *       readability
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.cpp.rules.typeomitted.TypeOmitted
+
+class ExplicitlyDeclareTypesQuery extends TypeOmittedSharedQuery {
+  ExplicitlyDeclareTypesQuery() { this = Declarations3Package::explicitlyDeclareTypesQuery() }
+}

c/misra/src/rules/RULE-5-7/TagNameNotUnique.ql

@@ -0,0 +1 @@
+c/common/test/rules/identifierhidden/IdentifierHidden.ql

c/misra/src/rules/RULE-8-1/ExplicitlyDeclareTypes.ql

@@ -0,0 +1,2 @@
+| test.c:1:1:1:23 | #define Sum(x,y) x + y | Macro name is nonunique compared to $@. | test.c:4:5:4:7 | Sum | Sum |
+| test.c:6:1:6:42 | #define iltiqzxgfqsgigwfuyntzghvzltueeaZ ; | Macro name is nonunique compared to $@. | test.c:7:12:7:43 | iltiqzxgfqsgigwfuyntzghvzltueeaQ | iltiqzxgfqsgigwfuyntzghvzltueeaQ |

c/misra/test/rules/RULE-5-3/IdentifierHidingC.testref

@@ -0,0 +1 @@
+rules/RULE-5-5/IdentifiersNotDistinctFromMacroNames.ql

c/misra/test/rules/RULE-5-5/IdentifiersNotDistinctFromMacroNames.expected

@@ -0,0 +1,7 @@
+#define Sum(x, y) x + y // NON_COMPLIANT
+#undef Sum
+
+int Sum;
+
+#define iltiqzxgfqsgigwfuyntzghvzltueeaZ ;   // NON_COMPLIANT - length 32
+static int iltiqzxgfqsgigwfuyntzghvzltueeaQ; // NON_COMPLIANT - length 32

c/misra/test/rules/RULE-5-5/IdentifiersNotDistinctFromMacroNames.qlref

@@ -0,0 +1,2 @@
+| test.c:11:15:11:19 | test1 | Typedef name is nonunique compared to $@. | test.c:13:17:13:21 | test1 | test1 |
+| test.c:30:3:30:7 | chain | Typedef name is nonunique compared to $@. | test.c:26:10:26:14 | chain | chain |

c/misra/test/rules/RULE-5-5/test.c

@@ -0,0 +1 @@
+rules/RULE-5-6/TypedefNameNotUnique.ql

c/misra/test/rules/RULE-5-6/TypedefNameNotUnique.expected

@@ -0,0 +1,30 @@
+#include "test.h"
+void f() {
+  {
+    typedef unsigned char test; // NON_COMPLIANT
+  }
+  {
+    typedef unsigned char test; // NON_COMPLIANT
+  }
+}
+
+typedef float test1; // NON_COMPLIANT
+
+void f2() { int test1 = 0; }
+
+typedef struct list {
+  int i;
+} list; // COMPLIANT
+
+typedef struct BIGList1 {
+  int i;
+} list1; // COMPLIANT
+
+typedef enum enum1 { testenum } enum1; // COMPLIANT
+
+typedef struct {
+  struct chain {
+    int ii;
+  } s1;
+  int i;
+} chain; // NON_COMPLIANT

c/misra/test/rules/RULE-5-6/TypedefNameNotUnique.qlref

@@ -0,0 +1 @@
+typedef int headertest; // COMPLIANT

c/misra/test/rules/RULE-5-6/test.c

@@ -0,0 +1 @@
+#include "test.h"

c/misra/test/rules/RULE-5-6/test.h

@@ -0,0 +1,4 @@
+| test.c:5:8:5:9 | s1 | Tag name is nonunique compared to $@. | test.c:12:10:12:11 | s1 | s1 |
+| test.c:5:8:5:9 | s1 | Tag name is nonunique compared to $@. | test.c:17:17:17:18 | s1 | s1 |
+| test.c:12:10:12:11 | s1 | Tag name is nonunique compared to $@. | test.c:5:8:5:9 | s1 | s1 |
+| test.c:12:10:12:11 | s1 | Tag name is nonunique compared to $@. | test.c:17:17:17:18 | s1 | s1 |

c/misra/test/rules/RULE-5-6/test1.c

@@ -0,0 +1 @@
+rules/RULE-5-7/TagNameNotUnique.ql

c/misra/test/rules/RULE-5-7/TagNameNotUnique.expected

@@ -0,0 +1,33 @@
+typedef struct s {
+  int i;
+} s; // COMPLIANT
+
+struct s1 { // NON_COMPLIANT
+  int i;
+};
+
+struct s1 a1 = {0}; // COMPLIANT
+
+void f() {
+  struct s1 { // NON_COMPLIANT
+    int i;
+  };
+}
+
+void f1() { int s1 = 0; }
+
+typedef struct {
+  int i;
+} sunnamed; // COMPLIANT
+
+typedef struct {
+  int i;
+} sunnamed2; // COMPLIANT
+
+typedef union {
+  int i;
+} U; // COMPLIANT
+
+typedef union {
+  int i;
+}; // COMPLIANT

c/misra/test/rules/RULE-5-7/TagNameNotUnique.qlref

@@ -0,0 +1 @@
+c/common/test/rules/typeomitted/TypeOmitted.ql

c/misra/test/rules/RULE-5-7/test.c

@@ -19,11 +19,8 @@
 
 import cpp
 import codingstandards.cpp.autosar
-import codingstandards.cpp.Scope
+import codingstandards.cpp.rules.identifierhidden.IdentifierHidden
 
-from UserVariable v1, UserVariable v2
-where
-  not isExcluded(v1, NamingPackage::identifierHidingQuery()) and
-  not isExcluded(v2, NamingPackage::identifierHidingQuery()) and
-  hides(v1, v2)
-select v2, "Variable is hiding variable $@.", v1, v1.getName()
+class IdentifierHidingCQuery extends IdentifierHiddenSharedQuery {
+  IdentifierHidingCQuery() { this = NamingPackage::identifierHidingQuery() }
+}

c/misra/test/rules/RULE-8-1/ExplicitlyDeclareTypes.testref

@@ -0,0 +1 @@
+cpp/common/test/rules/identifierhidden/IdentifierHidden.ql