Merge branch 'main' into lcartey/contracts

lcartey · web-flow · commit 2794921d67c9 · 2024-01-30T10:21:27.000Z
diff --git a/c/common/src/codingstandards/c/Literals.qll b/c/common/src/codingstandards/c/Literals.qll
@@ -0,0 +1,4 @@
+// Reuse the `IntegerLiteral` class
+import codingstandards.cpp.Cpp14Literal
+
+class IntegerLiteral = Cpp14Literal::IntegerLiteral;
diff --git a/c/misra/src/rules/RULE-7-3/LowercaseCharacterLUsedInLiteralSuffix.ql b/c/misra/src/rules/RULE-7-3/LowercaseCharacterLUsedInLiteralSuffix.ql
@@ -14,10 +14,10 @@
 
 import cpp
 import codingstandards.c.misra
+import codingstandards.c.Literals
 
-from Literal l
+from IntegerLiteral l
 where
   not isExcluded(l, SyntaxPackage::lowercaseCharacterLUsedInLiteralSuffixQuery()) and
-  not l instanceof StringLiteral and
   exists(l.getValueText().indexOf("l"))
 select l, "Lowercase 'l' used as a literal suffix."
diff --git a/c/misra/test/rules/RULE-7-3/cpp/LowercaseCharacterLUsedInLiteralSuffix.expected b/c/misra/test/rules/RULE-7-3/cpp/LowercaseCharacterLUsedInLiteralSuffix.expected
diff --git a/c/misra/test/rules/RULE-7-3/cpp/LowercaseCharacterLUsedInLiteralSuffix.qlref b/c/misra/test/rules/RULE-7-3/cpp/LowercaseCharacterLUsedInLiteralSuffix.qlref
@@ -0,0 +1 @@
+rules/RULE-7-3/LowercaseCharacterLUsedInLiteralSuffix.ql
diff --git a/c/misra/test/rules/RULE-7-3/cpp/README.md b/c/misra/test/rules/RULE-7-3/cpp/README.md
@@ -0,0 +1 @@
+This test case was added to validate FP report [#319](https://github.com/github/codeql-coding-standards/issues/319) that occurs when this rule is run on a translation unit with language mode c++.
diff --git a/c/misra/test/rules/RULE-7-3/cpp/options b/c/misra/test/rules/RULE-7-3/cpp/options
@@ -0,0 +1 @@
+semmle-extractor-options:--clang -std=c++14 --edg --diag_error=implicit_func_decl -nostdinc -I../../../../../cpp/common/test/includes/standard-library
diff --git a/c/misra/test/rules/RULE-7-3/cpp/test.cpp b/c/misra/test/rules/RULE-7-3/cpp/test.cpp
@@ -0,0 +1 @@
+int x = false; // COMPLIANT - reported as FP in #319
diff --git a/c/misra/test/rules/RULE-7-3/test.c b/c/misra/test/rules/RULE-7-3/test.c
@@ -41,4 +41,4 @@ long d9 = 001LU; // COMPLIANT
 
 char *e1 = "";
 char *e2 = "ul";
-char *e3 = "UL";
+char *e3 = "UL";
diff --git a/change_notes/2024-01-12-fix-reported-fp-a3-9-1.md b/change_notes/2024-01-12-fix-reported-fp-a3-9-1.md
@@ -0,0 +1,5 @@
+- `A3-9-1` - `VariableWidthIntegerTypesUsed.ql`:
+    - Exclude the plain char type. Still includes `signed char` and `unsigned char`.
+    - Include CV-qualified variable width integer types.
+- `A3-9-1` - `VariableWidthPlainCharTypeUsed.ql`:
+    - New query to support fine grained deviation support for the plain char type.
diff --git a/change_notes/2024-01-17-fix-reported-fp-for-a2-3-1.md b/change_notes/2024-01-17-fix-reported-fp-for-a2-3-1.md
@@ -0,0 +1,2 @@
+`A2-3-1`: ` cpp/autosar/invalid-character-in-string-literal`
+    - Fixes #311. Exclude wide string literals and utf8 string literal.
diff --git a/change_notes/2024-01-18-fix-reported-fp-for-rule-7-3.md b/change_notes/2024-01-18-fix-reported-fp-for-rule-7-3.md
@@ -0,0 +1,2 @@
+`RULE-7-3`: `c/misra/lowercase-character-l-used-in-literal-suffix`
+    - Exclude non integer literals. This removes a false positive triggered when analyzing C++ code containing the `false` literal.
diff --git a/cpp/autosar/src/rules/A2-3-1/InvalidCharacterInStringLiteral.ql b/cpp/autosar/src/rules/A2-3-1/InvalidCharacterInStringLiteral.ql
@@ -17,6 +17,7 @@
 
 import cpp
 import codingstandards.cpp.autosar
+import codingstandards.cpp.Literals
 
 bindingset[s]
 string getCharOutsideBasicSourceCharSet(string s) {
@@ -27,6 +28,9 @@ string getCharOutsideBasicSourceCharSet(string s) {
 from StringLiteral s, string ch
 where
   not isExcluded(s, NamingPackage::invalidCharacterInStringLiteralQuery()) and
-  ch = getCharOutsideBasicSourceCharSet(s.getValueText())
+  ch = getCharOutsideBasicSourceCharSet(s.getValueText()) and
+  // wide string and utf8 string literals are exempted.
+  not s instanceof WideStringLiteral and
+  not s instanceof Utf8StringLiteral
 select s,
   "String literal uses the character '" + ch + "' that is outside the language basic character set."
diff --git a/cpp/autosar/src/rules/A3-9-1/VariableWidthIntegerTypesUsed.ql b/cpp/autosar/src/rules/A3-9-1/VariableWidthIntegerTypesUsed.ql
@@ -1,8 +1,8 @@
 /**
  * @id cpp/autosar/variable-width-integer-types-used
  * @name A3-9-1: Use fixed-width integer types instead of basic, variable-width, integer types
- * @description The basic numerical types of char, int, short, long are not supposed to be used. The
- *              specific-length types from <cstdint> header need be used instead.
+ * @description The basic numerical types of signed/unsigned char, int, short, long are not supposed
+ *              to be used. The specific-length types from <cstdint> header need be used instead.
  * @kind problem
  * @precision very-high
  * @problem.severity error
@@ -19,15 +19,16 @@ import cpp
 import codingstandards.cpp.autosar
 import codingstandards.cpp.EncapsulatingFunctions
 import codingstandards.cpp.BuiltInNumericTypes
+import codingstandards.cpp.Type
 
-from Variable v
+from Variable v, Type typeStrippedOfSpecifiers
 where
   not isExcluded(v, DeclarationsPackage::variableWidthIntegerTypesUsedQuery()) and
+  typeStrippedOfSpecifiers = stripSpecifiers(v.getType()) and
   (
-    v.getType() instanceof BuiltInIntegerType or
-    v.getType() instanceof PlainCharType or
-    v.getType() instanceof UnsignedCharType or
-    v.getType() instanceof SignedCharType
+    typeStrippedOfSpecifiers instanceof BuiltInIntegerType or
+    typeStrippedOfSpecifiers instanceof UnsignedCharType or
+    typeStrippedOfSpecifiers instanceof SignedCharType
   ) and
   not v instanceof ExcludedVariable
 select v, "Variable '" + v.getName() + "' has variable-width type."
diff --git a/cpp/autosar/src/rules/A3-9-1/VariableWidthPlainCharTypeUsed.ql b/cpp/autosar/src/rules/A3-9-1/VariableWidthPlainCharTypeUsed.ql
@@ -0,0 +1,26 @@
+/**
+ * @id cpp/autosar/variable-width-plain-char-type-used
+ * @name A3-9-1: Use a fixed-width integer type instead of a char type
+ * @description The basic numerical type char is not supposed to be used. The specific-length types
+ *              from <cstdint> header need be used instead.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/autosar/id/a3-9-1
+ *       correctness
+ *       security
+ *       maintainability
+ *       external/autosar/allocated-target/implementation
+ *       external/autosar/enforcement/automated
+ *       external/autosar/obligation/required
+ */
+
+import cpp
+import codingstandards.cpp.autosar
+import codingstandards.cpp.Type
+
+from Variable variable
+where
+  not isExcluded(variable, DeclarationsPackage::variableWidthPlainCharTypeUsedQuery()) and
+  stripSpecifiers(variable.getType()) instanceof PlainCharType
+select variable, "Variable '" + variable.getName() + "' has variable-width char type."
diff --git a/cpp/autosar/test/rules/A2-3-1/InvalidCharacterInComment.expected b/cpp/autosar/test/rules/A2-3-1/InvalidCharacterInComment.expected
@@ -1,2 +1,2 @@
 | test.cpp:3:1:3:37 | // Invalid character \u00ce\u00b1 NON_COMPLIANT | Comment uses the character '\u00ce\u00b1' that is outside the language basic character set. |
-| test.cpp:10:1:12:2 | /*\nInvalid character \u00e2\u0086\u00a6 NON_COMPLIANT\n*/ | Comment uses the character '\u00e2\u0086\u00a6' that is outside the language basic character set. |
+| test.cpp:12:1:14:2 | /*\nInvalid character \u00e2\u0086\u00a6 NON_COMPLIANT\n*/ | Comment uses the character '\u00e2\u0086\u00a6' that is outside the language basic character set. |
diff --git a/cpp/autosar/test/rules/A2-3-1/InvalidCharacterInStringLiteral.expected b/cpp/autosar/test/rules/A2-3-1/InvalidCharacterInStringLiteral.expected
@@ -1 +1 @@
-| test.cpp:7:20:7:22 | \u00ce\u00b1 | String literal uses the character '\u03b1' that is outside the language basic character set. |
+| test.cpp:7:21:7:23 | \u00ce\u00b1 | String literal uses the character '\u03b1' that is outside the language basic character set. |
diff --git a/cpp/autosar/test/rules/A2-3-1/test.cpp b/cpp/autosar/test/rules/A2-3-1/test.cpp
@@ -4,9 +4,15 @@
 double α = 2.;                   // NON_COMPLIANT; U+03b1
 void *to_𐆅_and_beyond = nullptr; // NON_COMPLIANT; U+10185
 int l1_\u00A8;                   // COMPLIANT[FALSE_POSITIVE]
-const char *euro = "α";          // NON_COMPLIANT
+const char *euro1 = "α";         // NON_COMPLIANT
+const wchar_t *euro2 = L"α";     // COMPLIANT
+const char *euro3 = u8"α";       // COMPLIANT
 
 int valid;
 /*
 Invalid character ↦ NON_COMPLIANT
+*/
+
+/*
+Valid character @ in comments COMPLIANT
 */
diff --git a/cpp/autosar/test/rules/A3-9-1/VariableWidthIntegerTypesUsed.expected b/cpp/autosar/test/rules/A3-9-1/VariableWidthIntegerTypesUsed.expected
@@ -1,4 +1,3 @@
-| test.cpp:4:8:4:8 | c | Variable 'c' has variable-width type. |
 | test.cpp:5:17:5:18 | uc | Variable 'uc' has variable-width type. |
 | test.cpp:6:15:6:16 | sc | Variable 'sc' has variable-width type. |
 | test.cpp:8:7:8:7 | i | Variable 'i' has variable-width type. |
@@ -12,3 +11,29 @@
 | test.cpp:18:8:18:8 | l | Variable 'l' has variable-width type. |
 | test.cpp:19:17:19:18 | ul | Variable 'ul' has variable-width type. |
 | test.cpp:20:15:20:16 | sl | Variable 'sl' has variable-width type. |
+| test.cpp:39:23:39:25 | uc1 | Variable 'uc1' has variable-width type. |
+| test.cpp:40:21:40:23 | sc1 | Variable 'sc1' has variable-width type. |
+| test.cpp:42:13:42:14 | i1 | Variable 'i1' has variable-width type. |
+| test.cpp:43:22:43:24 | ui1 | Variable 'ui1' has variable-width type. |
+| test.cpp:44:18:44:19 | u1 | Variable 'u1' has variable-width type. |
+| test.cpp:45:20:45:22 | si1 | Variable 'si1' has variable-width type. |
+| test.cpp:46:16:46:17 | s1 | Variable 's1' has variable-width type. |
+| test.cpp:48:15:48:17 | sh1 | Variable 'sh1' has variable-width type. |
+| test.cpp:49:24:49:27 | ush1 | Variable 'ush1' has variable-width type. |
+| test.cpp:50:22:50:25 | ssh1 | Variable 'ssh1' has variable-width type. |
+| test.cpp:52:14:52:15 | l1 | Variable 'l1' has variable-width type. |
+| test.cpp:53:23:53:25 | ul1 | Variable 'ul1' has variable-width type. |
+| test.cpp:54:21:54:23 | sl1 | Variable 'sl1' has variable-width type. |
+| test.cpp:57:26:57:28 | uc2 | Variable 'uc2' has variable-width type. |
+| test.cpp:58:24:58:26 | sc2 | Variable 'sc2' has variable-width type. |
+| test.cpp:60:16:60:17 | i2 | Variable 'i2' has variable-width type. |
+| test.cpp:61:25:61:27 | ui2 | Variable 'ui2' has variable-width type. |
+| test.cpp:62:21:62:22 | u2 | Variable 'u2' has variable-width type. |
+| test.cpp:63:23:63:25 | si2 | Variable 'si2' has variable-width type. |
+| test.cpp:64:19:64:20 | s2 | Variable 's2' has variable-width type. |
+| test.cpp:66:18:66:20 | sh2 | Variable 'sh2' has variable-width type. |
+| test.cpp:67:27:67:30 | ush2 | Variable 'ush2' has variable-width type. |
+| test.cpp:68:25:68:28 | ssh2 | Variable 'ssh2' has variable-width type. |
+| test.cpp:70:17:70:18 | l2 | Variable 'l2' has variable-width type. |
+| test.cpp:71:26:71:28 | ul2 | Variable 'ul2' has variable-width type. |
+| test.cpp:72:24:72:26 | sl2 | Variable 'sl2' has variable-width type. |
diff --git a/cpp/autosar/test/rules/A3-9-1/VariableWidthPlainCharTypeUsed.expected b/cpp/autosar/test/rules/A3-9-1/VariableWidthPlainCharTypeUsed.expected
@@ -0,0 +1,3 @@
+| test.cpp:4:8:4:8 | c | Variable 'c' has variable-width char type. |
+| test.cpp:38:14:38:15 | c1 | Variable 'c1' has variable-width char type. |
+| test.cpp:56:17:56:18 | c2 | Variable 'c2' has variable-width char type. |
diff --git a/cpp/autosar/test/rules/A3-9-1/VariableWidthPlainCharTypeUsed.qlref b/cpp/autosar/test/rules/A3-9-1/VariableWidthPlainCharTypeUsed.qlref
@@ -0,0 +1 @@
+rules/A3-9-1/VariableWidthPlainCharTypeUsed.ql
diff --git a/cpp/autosar/test/rules/A3-9-1/test.cpp b/cpp/autosar/test/rules/A3-9-1/test.cpp
@@ -3,7 +3,7 @@
 void test_variable_width_type_variables() {
   char c;           // NON_COMPLIANT
   unsigned char uc; // NON_COMPLIANT
-  signed char sc;   // NON_COMPLIANt
+  signed char sc;   // NON_COMPLIANT
 
   int i;           // NON_COMPLIANT
   unsigned int ui; // NON_COMPLIANT
@@ -32,4 +32,42 @@ void test_variable_width_type_variables() {
 
 int main(int argc, char *argv[]) { // COMPLIANT
   // main as an exception
+}
+
+void test_variable_width_type_qualified_variables() {
+  const char c1 = 0;           // NON_COMPLIANT
+  const unsigned char uc1 = 0; // NON_COMPLIANT
+  const signed char sc1 = 0;   // NON_COMPLIANt
+
+  const int i1 = 0;           // NON_COMPLIANT
+  const unsigned int ui1 = 0; // NON_COMPLIANT
+  const unsigned u1 = 0;      // NON_COMPLIANT
+  const signed int si1 = 0;   // NON_COMPLIANT
+  const signed s1 = 0;        // NON_COMPLIANT
+
+  const short sh1 = 0;           // NON_COMPLIANT
+  const unsigned short ush1 = 0; // NON_COMPLIANT
+  const signed short ssh1 = 0;   // NON_COMPLIANT
+
+  const long l1 = 0;           // NON_COMPLIANT
+  const unsigned long ul1 = 0; // NON_COMPLIANT
+  const signed long sl1 = 0;   // NON_COMPLIANT
+
+  volatile char c2;           // NON_COMPLIANT
+  volatile unsigned char uc2; // NON_COMPLIANT
+  volatile signed char sc2;   // NON_COMPLIANt
+
+  volatile int i2;           // NON_COMPLIANT
+  volatile unsigned int ui2; // NON_COMPLIANT
+  volatile unsigned u2;      // NON_COMPLIANT
+  volatile signed int si2;   // NON_COMPLIANT
+  volatile signed s2;        // NON_COMPLIANT
+
+  volatile short sh2;           // NON_COMPLIANT
+  volatile unsigned short ush2; // NON_COMPLIANT
+  volatile signed short ssh2;   // NON_COMPLIANT
+
+  volatile long l2;           // NON_COMPLIANT
+  volatile unsigned long ul2; // NON_COMPLIANT
+  volatile signed long sl2;   // NON_COMPLIANT
 }
diff --git a/cpp/common/src/codingstandards/cpp/Cpp14Literal.qll b/cpp/common/src/codingstandards/cpp/Cpp14Literal.qll
@@ -24,7 +24,7 @@ module Cpp14Literal {
    * Octal literals must always start with the digit `0`.
    */
   class OctalLiteral extends IntegerLiteral {
-    OctalLiteral() { getValueText().regexpMatch("\\s*0[0-7']+[uUlL]*\\s*") }
+    OctalLiteral() { getValueText().regexpMatch("\\s*0[0-7']*[uUlL]*\\s*") }
 
     override string getAPrimaryQlClass() { result = "OctalLiteral" }
   }
diff --git a/cpp/common/src/codingstandards/cpp/Literals.qll b/cpp/common/src/codingstandards/cpp/Literals.qll
@@ -12,3 +12,19 @@ string getTruncatedLiteralText(Literal l) {
     else result = text
   )
 }
+
+class WideStringLiteral extends StringLiteral {
+  WideStringLiteral() { this.getValueText().regexpMatch("(?s)\\s*L\".*") }
+}
+
+class Utf8StringLiteral extends StringLiteral {
+  Utf8StringLiteral() { this.getValueText().regexpMatch("(?s)\\s*u8\".*") }
+}
+
+class Utf16StringLiteral extends StringLiteral {
+  Utf16StringLiteral() { this.getValueText().regexpMatch("(?s)\\s*u\".*") }
+}
+
+class Utf32StringLiteral extends StringLiteral {
+  Utf32StringLiteral() { this.getValueText().regexpMatch("(?s)\\s*U\".*") }
+}
diff --git a/cpp/common/src/codingstandards/cpp/Type.qll b/cpp/common/src/codingstandards/cpp/Type.qll
@@ -22,3 +22,12 @@ class FundamentalType extends BuiltInType {
 class IncompleteType extends Class {
   IncompleteType() { not hasDefinition() }
 }
+
+/**
+ * A type without `const` and `volatile` specifiers.
+ */
+Type stripSpecifiers(Type type) {
+  if type instanceof SpecifiedType
+  then result = stripSpecifiers(type.(SpecifiedType).getBaseType())
+  else result = type
+}
diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/Declarations.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Declarations.qll
@@ -9,6 +9,7 @@ newtype DeclarationsQuery =
   TGlobalSizedOperatorDeleteNotDefinedQuery() or
   TGlobalUnsizedOperatorDeleteNotDefinedQuery() or
   TVariableWidthIntegerTypesUsedQuery() or
+  TVariableWidthPlainCharTypeUsedQuery() or
   TAutoSpecifierNotUsedAppropriatelyInFunctionDefinitionQuery() or
   TAutoSpecifierNotUsedAppropriatelyInVariableDefinitionQuery() or
   TIdentifierDeclarationAndInitializationNotOnSeparateLinesQuery() or
@@ -68,6 +69,15 @@ predicate isDeclarationsQueryMetadata(Query query, string queryId, string ruleId
   ruleId = "A3-9-1" and
   category = "required"
   or
+  query =
+    // `Query` instance for the `variableWidthPlainCharTypeUsed` query
+    DeclarationsPackage::variableWidthPlainCharTypeUsedQuery() and
+  queryId =
+    // `@id` for the `variableWidthPlainCharTypeUsed` query
+    "cpp/autosar/variable-width-plain-char-type-used" and
+  ruleId = "A3-9-1" and
+  category = "required"
+  or
   query =
     // `Query` instance for the `autoSpecifierNotUsedAppropriatelyInFunctionDefinition` query
     DeclarationsPackage::autoSpecifierNotUsedAppropriatelyInFunctionDefinitionQuery() and
@@ -213,6 +223,13 @@ module DeclarationsPackage {
       TQueryCPP(TDeclarationsPackageQuery(TVariableWidthIntegerTypesUsedQuery()))
   }
 
+  Query variableWidthPlainCharTypeUsedQuery() {
+    //autogenerate `Query` type
+    result =
+      // `Query` type for `variableWidthPlainCharTypeUsed` query
+      TQueryCPP(TDeclarationsPackageQuery(TVariableWidthPlainCharTypeUsedQuery()))
+  }
+
   Query autoSpecifierNotUsedAppropriatelyInFunctionDefinitionQuery() {
     //autogenerate `Query` type
     result =
diff --git a/rule_packages/cpp/Declarations.json b/rule_packages/cpp/Declarations.json
@@ -78,12 +78,28 @@
       },
       "queries": [
         {
-          "description": "The basic numerical types of char, int, short, long are not supposed to be used. The specific-length types from <cstdint> header need be used instead.",
+          "description": "The basic numerical types of signed/unsigned char, int, short, long are not supposed to be used. The specific-length types from <cstdint> header need be used instead.",
           "kind": "problem",
           "name": "Use fixed-width integer types instead of basic, variable-width, integer types",
           "precision": "very-high",
           "severity": "error",
           "short_name": "VariableWidthIntegerTypesUsed",
+          "tags": [
+            "correctness",
+            "security",
+            "maintainability"
+          ],
+          "implementation_scope": {
+            "description": "This implementation excludes the plain char type from consideration."
+          }
+        },
+        {
+          "description": "The basic numerical type char is not supposed to be used. The specific-length types from <cstdint> header need be used instead.",
+          "kind": "problem",
+          "name": "Use a fixed-width integer type instead of a char type",
+          "precision": "very-high",
+          "severity": "error",
+          "short_name": "VariableWidthPlainCharTypeUsed",
           "tags": [
             "correctness",
             "security",

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+rules/RULE-7-3/LowercaseCharacterLUsedInLiteralSuffix.ql`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+This test case was added to validate FP report [#319](https://github.com/github/codeql-coding-standards/issues/319) that occurs when this rule is run on a translation unit with language mode c++.`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+semmle-extractor-options:--clang -std=c++14 --edg --diag_error=implicit_func_decl -nostdinc -I../../../../../cpp/common/test/includes/standard-library`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+int x = false; // COMPLIANT - reported as FP in #319`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+`A2-3-1`: ` cpp/autosar/invalid-character-in-string-literal`
	`2`	`+ - Fixes #311. Exclude wide string literals and utf8 string literal.`