Use shared queries / generally report obsolescent features even if redundant.

Redundant reports should not be a common user issue; these features are
obsolescent and likely rarely used and less often to be excepted.

Implement ungetc() on a zero-offset stream and specific banning of gets(), as
the redundant rules for those obsolescent features report a far wider set of
issues than banned by RULE-1-5.

Implementation of banning ungetc() on a zero-offset stream is not thorough or
comprehensive. This should be fine. False positives should not create any user
issues because the call of the function overall is banned. And false negatives
should not be an issue, for the same reason.

Author: MichaelRFairhurst

c/common/test/rules/functiontypesnotinprototypeformshared/FunctionTypesNotInPrototypeFormShared.expected

@@ -0,0 +1,4 @@
+| test.c:3:6:3:7 | f1 | Function f1 declares parameter that is unnamed. |
+| test.c:4:6:4:7 | f2 | Function f2 does not specify void for no parameters present. |
+| test.c:5:6:5:7 | f3 | Function f3 does not specify void for no parameters present. |
+| test.c:7:5:7:6 | f5 | Function f5 declares parameter in unsupported declaration list. |

c/misra/test/rules/RULE-8-2/FunctionTypesNotInPrototypeForm.expected.clang renamed to c/common/test/rules/functiontypesnotinprototypeformshared/FunctionTypesNotInPrototypeFormShared.expected.clang

@@ -0,0 +1,4 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.functiontypesnotinprototypeformshared.FunctionTypesNotInPrototypeFormShared
+
+class TestFileQuery extends FunctionTypesNotInPrototypeFormSharedSharedQuery, TestQuery { }

c/common/test/rules/functiontypesnotinprototypeformshared/FunctionTypesNotInPrototypeFormShared.ql

@@ -1,8 +1,3 @@
-// Note: A subset of these cases are also tested in c/misra/test/rules/RULE-1-5
-// via a FunctionTypesNotInPrototypeForm.qlref and .expected file in that
-// directory. Changes to these tests may require updating the test code or
-// expectations in that directory as well.
-
 void f(int x);    // COMPLIANT
 void f0(void);    // COMPLIANT
 void f1(int);     // NON_COMPLIANT

c/misra/test/rules/RULE-8-2/test.c renamed to c/common/test/rules/functiontypesnotinprototypeformshared/test.c

@@ -0,0 +1 @@
+| test.c:2:12:2:12 | declaration of g | The redeclaration of $@ with internal linkage misses the static specifier. | test.c:1:12:1:12 | definition of g | g |

c/misra/test/rules/RULE-8-2/test.c.clang renamed to c/common/test/rules/functiontypesnotinprototypeformshared/test.c.clang

@@ -0,0 +1,5 @@
+// GENERATED FILE - DO NOT MODIFY
+import codingstandards.cpp.rules.missingstaticspecifierobjectredeclarationshared.MissingStaticSpecifierObjectRedeclarationShared
+
+class TestFileQuery extends MissingStaticSpecifierObjectRedeclarationSharedSharedQuery, TestQuery {
+}

c/common/test/rules/missingstaticspecifierobjectredeclarationshared/MissingStaticSpecifierObjectRedeclarationShared.expected

@@ -0,0 +1,8 @@
+static int g = 0;
+extern int g; // NON_COMPLIANT
+
+static int g1;
+static int g1 = 0; // COMPLIANT
+
+int g2;
+int g2 = 0; // COMPLIANT

c/common/test/rules/missingstaticspecifierobjectredeclarationshared/MissingStaticSpecifierObjectRedeclarationShared.ql

@@ -0,0 +1,22 @@
+/**
+ * @id c/misra/call-to-obsolescent-function-gets
+ * @name RULE-1-5: Disallowed usage of obsolescent function 'gets'
+ * @description The function 'gets' is an obsolescent language feature which was removed in C11.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/misra/id/rule-1-5
+ *       external/misra/c/2012/amendment3
+ *       security
+ *       maintainability
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import codingstandards.c.misra
+
+from FunctionCall fc
+where
+  not isExcluded(fc, Language4Package::callToObsolescentFunctionGetsQuery()) and
+  fc.getTarget().hasGlobalOrStdName("gets")
+select fc, "Call to obsolescent function 'gets'."

c/common/test/rules/missingstaticspecifierobjectredeclarationshared/test.c

@@ -0,0 +1,23 @@
+/**
+ * @id c/misra/function-types-not-in-prototype-form-obsolete
+ * @name RULE-1-5: Function types shall be in prototype form with named parameters
+ * @description The use of non-prototype format parameter type declarators is an obsolescent
+ *              language feature.
+ * @kind problem
+ * @precision medium
+ * @problem.severity error
+ * @tags external/misra/id/rule-1-5
+ *       correctness
+ *       external/misra/c/2012/amendment3
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.cpp.rules.functiontypesnotinprototypeformshared.FunctionTypesNotInPrototypeFormShared
+
+class FunctionTypesNotInPrototypeFormObsoleteQuery extends FunctionTypesNotInPrototypeFormSharedSharedQuery {
+  FunctionTypesNotInPrototypeFormObsoleteQuery() {
+    this = Language4Package::functionTypesNotInPrototypeFormObsoleteQuery()
+  }
+}

c/misra/src/rules/RULE-1-5/CallToObsolescentFunctionGets.ql

@@ -0,0 +1,23 @@
+/**
+ * @id c/misra/missing-static-specifier-func-redeclaration-obsolete
+ * @name RULE-1-5: If a function has internal linkage then all re-declarations shall include the static storage class
+ * @description Declaring a function with internal linkage without the static storage class
+ *              specifier is an obselescent feature.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity warning
+ * @tags external/misra/id/rule-1-5
+ *       readability
+ *       external/misra/c/2012/amendment3
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.cpp.rules.missingstaticspecifierfunctionredeclarationshared.MissingStaticSpecifierFunctionRedeclarationShared
+
+class MissingStaticSpecifierFuncRedeclarationObsoleteQuery extends MissingStaticSpecifierFunctionRedeclarationSharedSharedQuery {
+  MissingStaticSpecifierFuncRedeclarationObsoleteQuery() {
+    this = Language4Package::missingStaticSpecifierFuncRedeclarationObsoleteQuery()
+  }
+}

c/misra/src/rules/RULE-1-5/FunctionTypesNotInPrototypeFormObsolete.ql

@@ -0,0 +1,23 @@
+/**
+ * @id c/misra/missing-static-specifier-object-redeclaration-obsolete
+ * @name RULE-1-5: If an object has internal linkage then all re-declarations shall include the static storage class
+ * @description Declaring an identifier with internal linkage without the static storage class
+ *              specifier is an obselescent feature.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity warning
+ * @tags external/misra/id/rule-1-5
+ *       readability
+ *       external/misra/c/2012/amendment3
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import codingstandards.c.misra
+import codingstandards.cpp.rules.missingstaticspecifierobjectredeclarationshared.MissingStaticSpecifierObjectRedeclarationShared
+
+class MissingStaticSpecifierObjectRedeclarationObsoleteQuery extends MissingStaticSpecifierObjectRedeclarationSharedSharedQuery {
+  MissingStaticSpecifierObjectRedeclarationObsoleteQuery() {
+    this = Language4Package::missingStaticSpecifierObjectRedeclarationObsoleteQuery()
+  }
+}

c/misra/src/rules/RULE-1-5/MissingStaticSpecifierFuncRedeclarationObsolete.ql

@@ -0,0 +1,69 @@
+/**
+ * @id c/misra/ungetc-call-on-stream-position-zero
+ * @name RULE-1-5: Disallowed obsolescent usage of 'ungetc' on a file stream at position zero
+ * @description Calling the function 'ungetc' on a file stream with a position of zero is an
+ *              obsolescent language feature.
+ * @kind problem
+ * @precision medium
+ * @problem.severity error
+ * @tags external/misra/id/rule-1-5
+ *       external/misra/c/2012/amendment3
+ *       security
+ *       maintainability
+ *       external/misra/obligation/required
+ */
+
+import cpp
+import semmle.code.cpp.dataflow.new.DataFlow
+import semmle.code.cpp.controlflow.Dominance
+import codingstandards.c.misra
+
+/**
+ * This is an inconclusive list, which is adequate, as RULE-21-3 provides
+ * assurance we won't have false negatives, or care too much about false
+ * positives.
+ */
+class MoveStreamPositionCall extends FunctionCall {
+  Expr streamArgument;
+
+  MoveStreamPositionCall() {
+    getTarget().hasGlobalOrStdName("fgetc") and
+    streamArgument = getArgument(0)
+    or
+    getTarget().hasGlobalOrStdName("getc") and
+    streamArgument = getArgument(0)
+    or
+    getTarget().hasGlobalOrStdName("fget") and
+    streamArgument = getArgument(2)
+    or
+    getTarget().hasGlobalOrStdName("fscanf") and
+    streamArgument = getArgument(0)
+    or
+    getTarget().hasGlobalOrStdName("fsetpos") and
+    streamArgument = getArgument(0)
+    or
+    getTarget().hasGlobalOrStdName("fseek") and
+    streamArgument = getArgument(0)
+    or
+    getTarget().hasGlobalOrStdName("fread") and
+    streamArgument = getArgument(3)
+  }
+
+  Expr getStreamArgument() { result = streamArgument }
+}
+
+from FunctionCall ungetc, DataFlow::Node file
+where
+  not isExcluded(ungetc, Language4Package::ungetcCallOnStreamPositionZeroQuery()) and
+  // ungetc() called on file stream
+  ungetc.getTarget().hasGlobalOrStdName("ungetc") and
+  DataFlow::localFlow(file, DataFlow::exprNode(ungetc.getArgument(1))) and
+  // ungetc() is not dominated by a fread() etc to that file stream
+  not exists(MoveStreamPositionCall moveStreamCall |
+    DataFlow::localFlow(file, DataFlow::exprNode(moveStreamCall.getStreamArgument())) and
+    dominates(moveStreamCall, ungetc)
+  )
+  // the file stream is the root of the local data flow
+  and not DataFlow::localFlow(any(DataFlow::Node n | not n = file), file)
+select ungetc, "Obsolescent call to ungetc on file stream $@ at position zero.", file,
+  file.toString()

c/misra/src/rules/RULE-1-5/MissingStaticSpecifierObjectRedeclarationObsolete.ql

@@ -14,46 +14,10 @@
 
 import cpp
 import codingstandards.c.misra
-import codingstandards.cpp.Identifiers
+import codingstandards.cpp.rules.functiontypesnotinprototypeformshared.FunctionTypesNotInPrototypeFormShared
 
-/**
- * `Parameter`s without names
- */
-class UnnamedParameter extends Parameter {
-  UnnamedParameter() { not this.isNamed() }
+class FunctionTypesNotInPrototypeFormQuery extends FunctionTypesNotInPrototypeFormSharedSharedQuery {
+  FunctionTypesNotInPrototypeFormQuery() {
+    this = Declarations4Package::functionTypesNotInPrototypeFormQuery()
+  }
 }
-
-/*
- * This is a copy of the private `hasZeroParamDecl` predicate from the standard set of
- * queries as of the `codeql-cli/2.11.2` tag in `github/codeql`.
- */
-
-predicate hasZeroParamDecl(Function f) {
-  exists(FunctionDeclarationEntry fde | fde = f.getADeclarationEntry() |
-    not fde.isImplicit() and
-    not fde.hasVoidParamList() and
-    fde.getNumberOfParameters() = 0 and
-    not fde.isDefinition()
-  )
-}
-
-from Function f, string msg
-where
-  not isExcluded(f, Declarations4Package::functionTypesNotInPrototypeFormQuery()) and
-  f instanceof InterestingIdentifiers and
-  (
-    f.getAParameter() instanceof UnnamedParameter and
-    msg = "Function " + f + " declares parameter that is unnamed."
-    or
-    hasZeroParamDecl(f) and
-    msg = "Function " + f + " does not specify void for no parameters present."
-    or
-    //parameters declared in declaration list (not in function signature)
-    //have placeholder file location associated only
-    exists(Parameter p |
-      p.getFunction() = f and
-      not p.getFile() = f.getFile() and
-      msg = "Function " + f + " declares parameter in unsupported declaration list."
-    )
-  )
-select f, msg

c/misra/src/rules/RULE-1-5/UngetcCallOnStreamPositionZero.ql

@@ -14,15 +14,10 @@
 
 import cpp
 import codingstandards.c.misra
+import codingstandards.cpp.rules.missingstaticspecifierobjectredeclarationshared.MissingStaticSpecifierObjectRedeclarationShared
 
-from VariableDeclarationEntry redeclaration, VariableDeclarationEntry de
-where
-  not isExcluded(redeclaration,
-    Declarations5Package::missingStaticSpecifierObjectRedeclarationCQuery()) and
-  //following implies de != redeclaration
-  de.hasSpecifier("static") and
-  not redeclaration.hasSpecifier("static") and
-  de.getDeclaration().isTopLevel() and
-  redeclaration.getDeclaration() = de.getDeclaration()
-select redeclaration, "The redeclaration of $@ with internal linkage misses the static specifier.",
-  de, de.getName()
+class MissingStaticSpecifierObjectRedeclarationCQuery extends MissingStaticSpecifierObjectRedeclarationSharedSharedQuery {
+  MissingStaticSpecifierObjectRedeclarationCQuery() {
+    this = Declarations5Package::missingStaticSpecifierObjectRedeclarationCQuery()
+  }
+}

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

@@ -0,0 +1 @@
+| test.c:36:3:36:6 | call to gets | Call to obsolescent function 'gets'. |

c/misra/src/rules/RULE-8-8/MissingStaticSpecifierObjectRedeclarationC.ql

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

c/misra/test/rules/RULE-1-5/CallToObsolescentFunctionGets.expected

@@ -0,0 +1 @@
+c/common/test/rules/functiontypesnotinprototypeformshared/FunctionTypesNotInPrototypeFormShared.ql

c/misra/test/rules/RULE-1-5/CallToObsolescentFunctionGets.qlref

@@ -0,0 +1 @@
+c/common/test/rules/missingstaticspecifierfunctionredeclarationshared/MissingStaticSpecifierFunctionRedeclarationShared.ql

c/misra/test/rules/RULE-1-5/FunctionTypesNotInPrototypeForm.expected

@@ -0,0 +1 @@
+c/common/test/rules/missingstaticspecifierobjectredeclarationshared/MissingStaticSpecifierObjectRedeclarationShared.ql

c/misra/test/rules/RULE-1-5/FunctionTypesNotInPrototypeForm.qlref

@@ -0,0 +1 @@
+| test.c:40:3:40:8 | call to ungetc | Obsolescent call to ungetc on file stream $@ at position zero. | test.c:38:16:38:20 | call to fopen | call to fopen |

c/misra/test/rules/RULE-1-5/FunctionTypesNotInPrototypeFormObsolete.testref

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

c/misra/test/rules/RULE-1-5/MemoryAllocDeallocFunctionsOfStdlibhUsed.expected

@@ -4,8 +4,8 @@
 #include "stdlib.h"
 
 void f1(void) {
-  // malloc() is not obsolete, but banned by Rule 21.3
-  int *t = malloc(10); // COMPLIANT[False Negative]
+  // malloc() is not obsolete, though it is banned by Rule 21.3
+  int *t = malloc(10); // COMPLIANT
 
   // Obsolete usage of realloc.
   realloc(t, 0); // NON-COMPLIANT
@@ -28,32 +28,19 @@ const extern int g2; // NON-COMPLIANT
 _Atomic int g3 = ATOMIC_VAR_INIT(18); // NON-COMPLIANT
 _Atomic int g4 = 18;                  // COMPLIANT
 
-// The following cases are already covered by other rules:
-
-// Rule 8.8:
-static int g5 = 3; // COMPLIANT
-extern int g5;     // NON-COMPLIANT
-
-// Rule 8.2:
-void f2();     // NON-COMPLIANT
-void f3(void); // COMPLIANT
-
-void f4(int p1){}; // COMPLIANT
-int f5(x)          // NON_COMPLIANT
-int x;
-{ return 1; }
+// `gets` was removed from C11.
+extern char* gets(FILE *stream);
 
 // Rule 21.6 covers the below cases:
 void f6(void) {
-  // `gets` was removed from C11.
-  // gets(stdin); // NON_COMPLIANT
+  gets(stdin); // NON_COMPLIANT
 
   FILE *file = fopen("", 0);
   // Obsolete usage of ungetc.
   ungetc('c', file); // NON-COMPLIANT
 
   char buf[10];
   fread(buf, sizeof(buf), 10, file);
-  // This is not an obsolete usage of ungetc, but ungetc isn't allowed.
-  ungetc('c', file); // NON-COMPLIANT[FALSE NEGATIVE]
+  // This is not an obsolete usage of ungetc, though ungetc isn't allowed by 21-3.
+  ungetc('c', file); // COMPLIANT
 }

c/misra/test/rules/RULE-1-5/MemoryAllocDeallocFunctionsOfStdlibhUsed.qlref

@@ -0,0 +1 @@
+c/common/test/rules/functiontypesnotinprototypeformshared/FunctionTypesNotInPrototypeFormShared.ql

c/misra/test/rules/RULE-1-5/MissingStaticSpecifierFuncRedeclarationObsolete.testref

@@ -0,0 +1 @@
+c/common/test/rules/missingstaticspecifierobjectredeclarationshared/MissingStaticSpecifierObjectRedeclarationShared.ql