Merge pull request #771 from github/lcartey/rule-1-3-main

`RULE-1-3`: Improve detection of standard compliant main functions

Author: lcartey

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/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;
 }

change_notes/2024-10-21-rule-1-3-main.md

@@ -0,0 +1,3 @@
+ - `RULE-1-3` - `OccurrenceOfUndefinedBehavior.ql`:
+   - Improve alert message to report the undefined behavior triggered.
+   - Address both false positives and false negatives in identifying standard compliant main methods. Previously, `void main()` was considered permitted and `int main(void)` banned. In addition, we now detect main methods as standard compliant if they use typedefs, and if arrays are used in the definition of `argv`.

cpp/common/src/codingstandards/cpp/Pointers.qll

@@ -6,7 +6,7 @@ import cpp
 import codingstandards.cpp.Type
 
 /**
- * A type that is a pointer or array type.
+ * A type that is a pointer or array type after stripping top-level specifiers.
  */
 class PointerOrArrayType extends DerivedType {
   PointerOrArrayType() {
@@ -15,6 +15,16 @@ class PointerOrArrayType extends DerivedType {
   }
 }
 
+/**
+ * A type that is a pointer or array type.
+ */
+class UnspecifiedPointerOrArrayType extends DerivedType {
+  UnspecifiedPointerOrArrayType() {
+    this instanceof PointerType or
+    this instanceof ArrayType
+  }
+}
+
 /**
  * An expression which performs pointer arithmetic
  */