Merge branch 'main' into codeql/upgrade-to-2.16.6

c/common/src/codingstandards/c/UndefinedBehavior.qll

@@ -1,33 +1,44 @@
 import cpp
+import codingstandards.cpp.Pointers
 import codingstandards.cpp.UndefinedBehavior
 
 /**
  * Library for modeling undefined behavior.
  */
 abstract class CUndefinedBehavior extends UndefinedBehavior { }
 
+/**
+ * A function which has the signature - but not the name - of a main function.
+ */
 class C99MainFunction extends Function {
   C99MainFunction() {
     this.getNumberOfParameters() = 2 and
-    this.getType() instanceof IntType and
-    this.getParameter(0).getType() instanceof IntType and
-    this.getParameter(1).getType().(PointerType).getBaseType().(PointerType).getBaseType()
-      instanceof CharType
+    this.getType().getUnderlyingType() instanceof IntType and
+    this.getParameter(0).getType().getUnderlyingType() instanceof IntType and
+    this.getParameter(1)
+        .getType()
+        .getUnderlyingType()
+        .(UnspecifiedPointerOrArrayType)
+        .getBaseType()
+        .(UnspecifiedPointerOrArrayType)
+        .getBaseType() instanceof CharType
     or
     this.getNumberOfParameters() = 0 and
-    this.getType() instanceof VoidType
+    // Must be explicitly declared as `int main(void)`.
+    this.getADeclarationEntry().hasVoidParamList() and
+    this.getType().getUnderlyingType() instanceof IntType
   }
 }
 
 class CUndefinedMainDefinition extends CUndefinedBehavior, Function {
   CUndefinedMainDefinition() {
     // for testing purposes, we use the prefix ____codeql_coding_standards`
-    (this.getName() = "main" or this.getName().indexOf("____codeql_coding_standards") = 0) and
+    (this.getName() = "main" or this.getName().indexOf("____codeql_coding_standards_main") = 0) and
     not this instanceof C99MainFunction
   }
 
   override string getReason() {
     result =
-      "The behavior of the program is undefined because the main function is not defined according to the C standard."
+      "main function may trigger undefined behavior because it is not in one of the formats specified by the C standard."
   }
 }

c/misra/src/rules/RULE-1-3/OccurrenceOfUndefinedBehavior.ql

@@ -18,4 +18,4 @@ import codingstandards.c.UndefinedBehavior
 
 from CUndefinedBehavior c
 where not isExcluded(c, Language3Package::occurrenceOfUndefinedBehaviorQuery())
-select c, "May result in undefined behavior."
+select c, c.getReason()

c/misra/src/rules/RULE-5-4/MacroIdentifiersNotDistinct.ql

@@ -15,8 +15,56 @@
 
 import cpp
 import codingstandards.c.misra
+import codingstandards.cpp.Macro
+import codingstandards.cpp.Includes
+import codingstandards.cpp.PreprocessorDirective
 
-from Macro m, Macro m2
+/**
+ * Gets a top level element that this macro is expanded to, e.g. an element which does not also have
+ * an enclosing element in the macro.
+ */
+Element getATopLevelElement(MacroInvocation mi) {
+  result = mi.getAnExpandedElement() and
+  not result.getEnclosingElement() = mi.getAnExpandedElement() and
+  not result instanceof Conversion
+}
+
+/**
+ * Gets a link target that this macro is expanded in.
+ */
+LinkTarget getALinkTarget(Macro m) {
+  exists(MacroInvocation mi, Element e |
+    mi = m.getAnInvocation() and
+    e = getATopLevelElement(mi)
+  |
+    result = e.(Expr).getEnclosingFunction().getALinkTarget()
+    or
+    result = e.(Stmt).getEnclosingFunction().getALinkTarget()
+    or
+    exists(GlobalOrNamespaceVariable g |
+      result = g.getALinkTarget() and
+      g = e.(Expr).getEnclosingDeclaration()
+    )
+  )
+}
+
+/**
+ * Holds if the m1 and m2 are unconditionally included from a common file.
+ *
+ * Extracted out for performance reasons - otherwise the call to determine the file path for the
+ * message was specializing the calls to `getAnUnconditionallyIncludedFile*(..)` and causing
+ * slow performance.
+ */
+bindingset[m1, m2]
+pragma[inline_late]
+private predicate isIncludedUnconditionallyFromCommonFile(Macro m1, Macro m2) {
+  exists(File f |
+    getAnUnconditionallyIncludedFile*(f) = m1.getFile() and
+    getAnUnconditionallyIncludedFile*(f) = m2.getFile()
+  )
+}
+
+from Macro m, Macro m2, string message
 where
   not isExcluded(m, Declarations1Package::macroIdentifiersNotDistinctQuery()) and
   not m = m2 and
@@ -25,12 +73,40 @@ where
     //C90 states the first 31 characters of macro identifiers are significant and is not currently considered by this rule
     //ie an identifier differing on the 32nd character would be indistinct for C90 but distinct for C99
     //and is currently not reported by this rule
-    if m.getName().length() >= 64
-    then m.getName().prefix(63) = m2.getName().prefix(63)
-    else m.getName() = m2.getName()
+    if m.getName().length() >= 64 and not m.getName() = m2.getName()
+    then (
+      m.getName().prefix(63) = m2.getName().prefix(63) and
+      message =
+        "Macro identifer " + m.getName() + " is nondistinct in first 63 characters, compared to $@."
+    ) else (
+      m.getName() = m2.getName() and
+      message =
+        "Definition of macro " + m.getName() +
+          " is not distinct from alternative definition of $@ in " +
+          m2.getLocation().getFile().getRelativePath() + "."
+    )
   ) and
   //reduce double report since both macros are in alert, arbitrary ordering
-  m.getLocation().getStartLine() >= m2.getLocation().getStartLine()
-select m,
-  "Macro identifer " + m.getName() + " is nondistinct in first 63 characters, compared to $@.", m2,
-  m2.getName()
+  m.getLocation().getStartLine() >= m2.getLocation().getStartLine() and
+  // Not within an #ifndef MACRO_NAME
+  not exists(PreprocessorIfndef ifBranch |
+    m.getAGuard() = ifBranch or
+    m2.getAGuard() = ifBranch
+  |
+    ifBranch.getHead() = m.getName()
+  ) and
+  // Must be included unconditionally from the same file, otherwise m1 may not be defined
+  // when m2 is defined
+  isIncludedUnconditionallyFromCommonFile(m, m2) and
+  // Macros can't be mutually exclusive
+  not mutuallyExclusiveBranchDirectiveMacros(m, m2) and
+  not mutuallyExclusiveBranchDirectiveMacros(m2, m) and
+  // If at least one invocation exists for at least one of the macros, then they must share a link
+  // target - i.e. must both be expanded in the same context
+  (
+    (exists(m.getAnInvocation()) and exists(m2.getAnInvocation()))
+    implies
+    // Must share a link target - e.g. must both be expanded in the same context
+    getALinkTarget(m) = getALinkTarget(m2)
+  )
+select m, message, m2, m2.getName()

c/misra/src/rules/RULE-8-13/PointerShouldPointToConstTypeWhenPossible.ql

@@ -18,29 +18,54 @@ import cpp
 import codingstandards.c.misra
 import codingstandards.cpp.Pointers
 import codingstandards.cpp.SideEffect
+import codingstandards.cpp.alertreporting.HoldsForAllCopies
 
-from Variable ptr, PointerOrArrayType type
+class NonConstPointerVariableCandidate extends Variable {
+  NonConstPointerVariableCandidate() {
+    // Ignore parameters in functions without bodies
+    (this instanceof Parameter implies exists(this.(Parameter).getFunction().getBlock())) and
+    // Ignore variables in functions that use ASM commands
+    not exists(AsmStmt a |
+      a.getEnclosingFunction() = this.(LocalScopeVariable).getFunction()
+      or
+      // In a type declared locally
+      this.(Field).getDeclaringType+().getEnclosingFunction() = a.getEnclosingFunction()
+    ) and
+    exists(PointerOrArrayType type |
+      // include only pointers which point to a const-qualified type
+      this.getType() = type and
+      not type.isDeeplyConstBelow()
+    ) and
+    // exclude pointers passed as arguments to functions which take a
+    // parameter that points to a non-const-qualified type
+    not exists(FunctionCall fc, int i |
+      fc.getArgument(i) = this.getAnAccess() and
+      not fc.getTarget().getParameter(i).getType().isDeeplyConstBelow()
+    ) and
+    // exclude any pointers which have their underlying data modified
+    not exists(VariableEffect effect |
+      effect.getTarget() = this and
+      // but not pointers that are only themselves modified
+      not effect.(AssignExpr).getLValue() = this.getAnAccess() and
+      not effect.(CrementOperation).getOperand() = this.getAnAccess()
+    ) and
+    // exclude pointers assigned to another pointer to a non-const-qualified type
+    not exists(Variable a |
+      a.getAnAssignedValue() = this.getAnAccess() and
+      not a.getType().(PointerOrArrayType).isDeeplyConstBelow()
+    )
+  }
+}
+
+/**
+ * Ensure that all copies of a variable are considered to be missing const qualification to avoid
+ * false positives where a variable is only used/modified in a single copy.
+ */
+class NonConstPointerVariable =
+  HoldsForAllCopies<NonConstPointerVariableCandidate, Variable>::LogicalResultElement;
+
+from NonConstPointerVariable ptr
 where
-  not isExcluded(ptr, Pointers1Package::pointerShouldPointToConstTypeWhenPossibleQuery()) and
-  // include only pointers which point to a const-qualified type
-  ptr.getType() = type and
-  not type.isDeeplyConstBelow() and
-  // exclude pointers passed as arguments to functions which take a
-  // parameter that points to a non-const-qualified type
-  not exists(FunctionCall fc, int i |
-    fc.getArgument(i) = ptr.getAnAccess() and
-    not fc.getTarget().getParameter(i).getType().isDeeplyConstBelow()
-  ) and
-  // exclude any pointers which have their underlying data modified
-  not exists(VariableEffect effect |
-    effect.getTarget() = ptr and
-    // but not pointers that are only themselves modified
-    not effect.(AssignExpr).getLValue() = effect.getAnAccess() and
-    not effect.(CrementOperation).getOperand() = effect.getAnAccess()
-  ) and
-  // exclude pointers assigned to another pointer to a non-const-qualified type
-  not exists(Variable a |
-    a.getAnAssignedValue() = ptr.getAnAccess() and
-    not a.getType().(PointerOrArrayType).isDeeplyConstBelow()
-  )
-select ptr, "$@ points to a non-const-qualified type.", ptr, ptr.getName()
+  not isExcluded(ptr.getAnElementInstance(),
+    Pointers1Package::pointerShouldPointToConstTypeWhenPossibleQuery())
+select ptr, "$@ points to a non-const-qualified type.", ptr, ptr.getAnElementInstance().getName()

c/misra/test/rules/RULE-1-3/OccurrenceOfUndefinedBehavior.expected

@@ -1,5 +1,8 @@
-| test.c:8:6:8:35 | ____codeql_coding_standards_m2 | May result in undefined behavior. |
-| test.c:11:5:11:34 | ____codeql_coding_standards_m3 | May result in undefined behavior. |
-| test.c:15:5:15:34 | ____codeql_coding_standards_m4 | May result in undefined behavior. |
-| test.c:19:5:19:34 | ____codeql_coding_standards_m5 | May result in undefined behavior. |
-| test.c:23:5:23:34 | ____codeql_coding_standards_m6 | May result in undefined behavior. |
+| test.c:4:6:4:38 | ____codeql_coding_standards_main1 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. |
+| test.c:8:5:8:37 | ____codeql_coding_standards_main2 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. |
+| test.c:27:5:27:37 | ____codeql_coding_standards_main6 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. |
+| test.c:32:6:32:38 | ____codeql_coding_standards_main7 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. |
+| test.c:36:5:36:37 | ____codeql_coding_standards_main8 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. |
+| test.c:40:5:40:37 | ____codeql_coding_standards_main9 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. |
+| test.c:44:5:44:38 | ____codeql_coding_standards_main10 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. |
+| test.c:48:5:48:38 | ____codeql_coding_standards_main11 | main function may trigger undefined behavior because it is not in one of the formats specified by the C standard. |

c/misra/test/rules/RULE-1-3/test.c

@@ -1,25 +1,50 @@
-void main(void) { // COMPLIANT
+int main(void) { // COMPLIANT
 }
 
-int ____codeql_coding_standards_m1(int argc, char **argv) { // NON_COMPLIANT
+void ____codeql_coding_standards_main1(void) { // NON_COMPLIANT
   return 0;
 }
 
-void ____codeql_coding_standards_m2(char *argc, char **argv) { // NON_COMPLIANT
+int ____codeql_coding_standards_main2() { // NON_COMPLIANT
+  return 0;
+}
+
+int ____codeql_coding_standards_main3(int argc, char **argv) { // COMPLIANT
+  return 0;
+}
+
+int ____codeql_coding_standards_main4(int argc, char argv[][]) { // COMPLIANT
+  return 0;
+}
+
+int ____codeql_coding_standards_main5(int argc, char *argv[]) { // COMPLIANT
+  return 0;
+}
+
+typedef int MY_INT;
+typedef char *MY_CHAR_PTR;
+
+int ____codeql_coding_standards_main6(MY_INT argc,
+                                      MY_CHAR_PTR argv[]) { // COMPLIANT
+  return 0;
+}
+
+void ____codeql_coding_standards_main7(char *argc,
+                                       char **argv) { // NON_COMPLIANT
 }
 
-int ____codeql_coding_standards_m3(int argc, char *argv) { // NON_COMPLIANT
+int ____codeql_coding_standards_main8(int argc, char *argv) { // NON_COMPLIANT
   return 0;
 }
 
-int ____codeql_coding_standards_m4() { // NON_COMPLIANT
+int ____codeql_coding_standards_main9() { // NON_COMPLIANT
   return 0;
 }
 
-int ____codeql_coding_standards_m5(int argc, int *argv) { // NON_COMPLIANT
+int ____codeql_coding_standards_main10(int argc, int *argv) { // NON_COMPLIANT
   return 0;
 }
 
-int ____codeql_coding_standards_m6(int argc, int **argv) { // NON_COMPLIANT
+int ____codeql_coding_standards_main11(int argc, int **argv) { // NON_COMPLIANT
   return 0;
 }

c/misra/test/rules/RULE-5-4/MacroIdentifiersNotDistinct.expected

@@ -1,2 +1,4 @@
+| header3.h:7:1:7:24 | #define MULTIPLE_INCLUDE | Definition of macro MULTIPLE_INCLUDE is not distinct from alternative definition of $@ in rules/RULE-5-4/header4.h. | header4.h:1:1:1:24 | #define MULTIPLE_INCLUDE | MULTIPLE_INCLUDE |
+| header3.h:14:1:14:21 | #define NOT_PROTECTED | Definition of macro NOT_PROTECTED is not distinct from alternative definition of $@ in rules/RULE-5-4/header4.h. | header4.h:12:1:12:23 | #define NOT_PROTECTED 1 | NOT_PROTECTED |
 | test.c:2:1:2:72 | #define iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyB | Macro identifer iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyB is nondistinct in first 63 characters, compared to $@. | test.c:1:1:1:72 | #define iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyA | iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyA |
-| test.c:8:1:8:31 | #define FUNCTION_MACRO(X) X + 1 | Macro identifer FUNCTION_MACRO is nondistinct in first 63 characters, compared to $@. | test.c:7:1:7:57 | #define FUNCTION_MACRO(FUNCTION_MACRO) FUNCTION_MACRO + 1 | FUNCTION_MACRO |
+| test.c:8:1:8:31 | #define FUNCTION_MACRO(X) X + 1 | Definition of macro FUNCTION_MACRO is not distinct from alternative definition of $@ in rules/RULE-5-4/test.c. | test.c:7:1:7:57 | #define FUNCTION_MACRO(FUNCTION_MACRO) FUNCTION_MACRO + 1 | FUNCTION_MACRO |

c/misra/test/rules/RULE-5-4/conditional.h

@@ -0,0 +1,11 @@
+#ifdef FOO
+#include "header1.h"
+#else
+#include "header2.h"
+#endif
+
+#ifdef FOO
+#define A_MACRO 1 // COMPLIANT
+#else
+#define A_MACRO 2 // COMPLIANT
+#endif

c/misra/test/rules/RULE-5-4/header1.h

@@ -0,0 +1 @@
+#define REPEATED 11 // COMPLIANT

c/misra/test/rules/RULE-5-4/header2.h

@@ -0,0 +1 @@
+#define REPEATED 1 // COMPLIANT

c/misra/test/rules/RULE-5-4/header3.h

@@ -0,0 +1,16 @@
+#ifndef HEADER3_H
+#define HEADER3_H
+
+// We should ignore the header guards in this file
+
+// This is defined unconditionally by both header3.h and header4.h
+#define MULTIPLE_INCLUDE // NON_COMPLIANT
+
+// This is redefined in header3.h, but only if it isn't already defined
+#define PROTECTED // COMPLIANT
+
+// This is redefined in header3.h, but is conditional on some other condition,
+// so this is redefined
+#define NOT_PROTECTED // NON_COMPLIANT
+
+#endif

c/misra/test/rules/RULE-5-4/header4.h

@@ -0,0 +1,13 @@
+#define MULTIPLE_INCLUDE // NON_COMPLIANT
+
+// This case is triggered from root2.c
+// because PROTECTED isn't defined in
+// that case
+#ifndef PROTECTED
+#define PROTECTED // COMPLIANT - checked by guard
+#endif
+
+// Always enabled, so conflicts in root1.c case
+#ifdef MULTIPLE_INCLUDE
+#define NOT_PROTECTED 1 // NON_COMPLIANT
+#endif

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

@@ -0,0 +1,6 @@
+#define FOO 1
+#include "conditional.h"
+
+// Both headers define MULTIPLE_INCLUDE
+#include "header3.h"
+#include "header4.h"

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

@@ -0,0 +1,3 @@
+#include "conditional.h"
+
+#include "header4.h"

c/misra/test/rules/RULE-8-13/PointerShouldPointToConstTypeWhenPossible.expected

@@ -12,3 +12,4 @@
 | test.c:66:23:66:24 | p1 | $@ points to a non-const-qualified type. | test.c:66:23:66:24 | p1 | p1 |
 | test.c:71:17:71:18 | p1 | $@ points to a non-const-qualified type. | test.c:71:17:71:18 | p1 | p1 |
 | test.c:75:15:75:16 | p1 | $@ points to a non-const-qualified type. | test.c:75:15:75:16 | p1 | p1 |
+| test.c:103:30:103:30 | s | $@ points to a non-const-qualified type. | test.c:103:30:103:30 | s | s |