Merge branch 'main' into release-automation/bump-version-to-2.27.0-dev

knewbury01 · web-flow · commit 5f54152a3f60 · 2024-04-09T13:00:32.000-04:00
diff --git a/.github/workflows/finalize-release.yml b/.github/workflows/finalize-release.yml
@@ -109,14 +109,14 @@ jobs:
           git switch main
           git pull --ff-only origin main
 
-          git switch -c release-automation/bump-version
+          git switch -c "release-automation/bump-version-to-$NEXT_VERSION"
 
           # We are running the script in the tooling directory with the release directory as the working directory
           ../tooling/scripts/release/bump-version.sh "$NEXT_VERSION"
 
           git add -u .
           git commit -m "Bump version to $NEXT_VERSION"
-          git push --set-upstream origin release-automation/bump-version
+          git push --set-upstream origin "release-automation/bump-version-to-$NEXT_VERSION"
 
-          gh pr create --repo $GITHUB_REPOSITORY --base main --head release-automation/bump-version --body "Bump the version of main to $NEXT_VERSION" --title "Bump version to $NEXT_VERSION"
+          gh pr create --repo $GITHUB_REPOSITORY --base main --head "release-automation/bump-version-to-$NEXT_VERSION" --body "Bump the version of main to $NEXT_VERSION" --title "Bump version to $NEXT_VERSION"
         working-directory: release 
diff --git a/c/misra/src/rules/RULE-6-1/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.ql b/c/misra/src/rules/RULE-6-1/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.ql
@@ -12,22 +12,33 @@
 
 import cpp
 import codingstandards.c.misra
+import codingstandards.cpp.Compiler
 
-predicate isAppropriatePrimitive(Type type) {
-  /* An appropriate primitive types to which a bit-field can be declared. */
-  type instanceof IntType and
+Type getSupportedBitFieldType(Compiler compiler) {
+  compiler instanceof UnsupportedCompiler and
   (
-    type.(IntegralType).isExplicitlySigned() or
-    type.(IntegralType).isExplicitlyUnsigned()
+    result instanceof IntType and
+    (
+      result.(IntegralType).isExplicitlySigned() or
+      result.(IntegralType).isExplicitlyUnsigned()
+    )
+    or
+    result instanceof BoolType
   )
   or
-  type instanceof BoolType
+  (compiler instanceof Gcc or compiler instanceof Clang) and
+  (
+    result instanceof IntegralOrEnumType
+    or
+    result instanceof BoolType
+  )
 }
 
 from BitField bitField
 where
   not isExcluded(bitField,
     BitfieldTypesPackage::bitFieldsShallOnlyBeDeclaredWithAnAppropriateTypeQuery()) and
   /* A violation would neither be an appropriate primitive type nor an appropriate typedef. */
-  not isAppropriatePrimitive(bitField.getType().resolveTypedefs())
-select bitField, "Bit-field " + bitField + " is declared on type " + bitField.getType() + "."
+  not getSupportedBitFieldType(getCompiler(bitField.getFile())) =
+    bitField.getType().resolveTypedefs()
+select bitField, "Bit-field '" + bitField + "' is declared on type '" + bitField.getType() + "'."
diff --git a/c/misra/test/rules/RULE-6-1/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.expected b/c/misra/test/rules/RULE-6-1/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.expected
@@ -1,4 +1,4 @@
-| test.c:6:7:6:8 | x1 | Bit-field x1 is declared on type int. |
-| test.c:10:15:10:16 | x5 | Bit-field x5 is declared on type signed long. |
-| test.c:12:15:12:16 | x6 | Bit-field x6 is declared on type signed char. |
-| test.c:14:14:14:15 | x7 | Bit-field x7 is declared on type Color. |
+| test.c:6:7:6:8 | x1 | Bit-field 'x1' is declared on type 'int'. |
+| test.c:10:15:10:16 | x5 | Bit-field 'x5' is declared on type 'signed long'. |
+| test.c:12:15:12:16 | x6 | Bit-field 'x6' is declared on type 'signed char'. |
+| test.c:14:14:14:15 | x7 | Bit-field 'x7' is declared on type 'Color'. |
diff --git a/c/misra/test/rules/RULE-6-1/clang/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.expected b/c/misra/test/rules/RULE-6-1/clang/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.expected
diff --git a/c/misra/test/rules/RULE-6-1/clang/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.qlref b/c/misra/test/rules/RULE-6-1/clang/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.qlref
@@ -0,0 +1 @@
+rules/RULE-6-1/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.ql
diff --git a/c/misra/test/rules/RULE-6-1/clang/options b/c/misra/test/rules/RULE-6-1/clang/options
@@ -0,0 +1 @@
+semmle-extractor-options:--mimic clang --std=c11 --edg --diag_error=implicit_func_decl -nostdinc -I../../../../common/test/includes/standard-library
diff --git a/c/misra/test/rules/RULE-6-1/clang/test.c b/c/misra/test/rules/RULE-6-1/clang/test.c
@@ -0,0 +1,15 @@
+typedef unsigned int UINT16;
+
+enum Color { R, G, B };
+
+struct SampleStruct {
+  int x1 : 2;          // COMPLIANT
+  unsigned int x2 : 2; // COMPLIANT - explicitly unsigned
+  signed int x3 : 2;   // COMPLIANT - explicitly signed
+  UINT16 x4 : 2;       // COMPLIANT - type alias resolves to a compliant type
+  signed long x5 : 2;  // COMPLIANT
+  signed char x6 : 2;  // COMPLIANT
+  enum Color x7 : 3;   // COMPLIANT
+  //_Atomic(int) x8 : 2;   // NON_COMPLIANT[COMPILER_CHECKED] - atomic types are
+  // not permitted for bit-fields.
+} sample_struct;
diff --git a/c/misra/test/rules/RULE-6-1/gcc/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.expected b/c/misra/test/rules/RULE-6-1/gcc/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.expected
diff --git a/c/misra/test/rules/RULE-6-1/gcc/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.qlref b/c/misra/test/rules/RULE-6-1/gcc/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.qlref
@@ -0,0 +1 @@
+rules/RULE-6-1/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.ql
diff --git a/c/misra/test/rules/RULE-6-1/gcc/options b/c/misra/test/rules/RULE-6-1/gcc/options
@@ -0,0 +1 @@
+semmle-extractor-options:--mimic gcc --std=c11 --edg --diag_error=implicit_func_decl -nostdinc -I../../../../common/test/includes/standard-library
diff --git a/c/misra/test/rules/RULE-6-1/gcc/test.c b/c/misra/test/rules/RULE-6-1/gcc/test.c
@@ -0,0 +1,15 @@
+typedef unsigned int UINT16;
+
+enum Color { R, G, B };
+
+struct SampleStruct {
+  int x1 : 2;          // COMPLIANT
+  unsigned int x2 : 2; // COMPLIANT - explicitly unsigned
+  signed int x3 : 2;   // COMPLIANT - explicitly signed
+  UINT16 x4 : 2;       // COMPLIANT - type alias resolves to a compliant type
+  signed long x5 : 2;  // COMPLIANT
+  signed char x6 : 2;  // COMPLIANT
+  enum Color x7 : 3;   // COMPLIANT
+  //_Atomic(int) x8 : 2;   // NON_COMPLIANT[COMPILER_CHECKED] - atomic types are
+  // not permitted for bit-fields.
+} sample_struct;
diff --git a/c/misra/test/rules/RULE-6-1/options b/c/misra/test/rules/RULE-6-1/options
@@ -0,0 +1 @@
+semmle-extractor-options:--no-clang --std=c11 --edg --diag_error=implicit_func_decl -nostdinc -I../../../../common/test/includes/standard-library
diff --git a/change_notes/2024-03-19-change-alert-a3-3-1.md b/change_notes/2024-03-19-change-alert-a3-3-1.md
@@ -0,0 +1,2 @@
+- `A3-3-1` - `ExternalLinkageNotDeclaredInHeaderFile.ql`:
+  - Adjust the alert message to comply with the style guide.
diff --git a/change_notes/2024-03-22-fix-fp-ctr55-cpp.md b/change_notes/2024-03-22-fix-fp-ctr55-cpp.md
@@ -0,0 +1,2 @@
+- `CTR55-CPP` - `DoNotUseAnAdditiveOperatorOnAnIterator.ql`:
+  - Address reported FP in #374. Improve logic on valid end checks and size checks on iterators.
diff --git a/change_notes/2024-04-26-fix-fp-rule-6-1.md b/change_notes/2024-04-26-fix-fp-rule-6-1.md
@@ -0,0 +1,2 @@
+- `RULE-6-1` - `BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.ql`:
+    - Address FP reported in #318. Add support for implementation specific bitfield types for Clang and Gcc.
diff --git a/cpp/autosar/src/rules/A3-3-1/ExternalLinkageNotDeclaredInHeaderFile.ql b/cpp/autosar/src/rules/A3-3-1/ExternalLinkageNotDeclaredInHeaderFile.ql
@@ -38,4 +38,4 @@ where
   // Main functions are an exception to the rule
   not de.getDeclaration() instanceof MainFunction and
   if de.getDeclaration() instanceof Function then kind = "function" else kind = "object"
-select de, "Externally linked " + kind + " " + de.getName() + " not declared in header file."
+select de, "Externally linked " + kind + " '" + de.getName() + "' not declared in header file."
diff --git a/cpp/autosar/test/rules/A3-3-1/ExternalLinkageNotDeclaredInHeaderFile.expected b/cpp/autosar/test/rules/A3-3-1/ExternalLinkageNotDeclaredInHeaderFile.expected
@@ -1,6 +1,6 @@
-| test.cpp:3:5:3:6 | definition of g1 | Externally linked object g1 not declared in header file. |
-| test.cpp:4:12:4:13 | declaration of g2 | Externally linked object g2 not declared in header file. |
-| test.cpp:10:5:10:6 | definition of l1 | Externally linked object l1 not declared in header file. |
-| test.cpp:11:6:11:7 | definition of f1 | Externally linked function f1 not declared in header file. |
-| test.cpp:22:5:22:5 | definition of f | Externally linked function f not declared in header file. |
-| test.cpp:25:5:25:6 | declaration of f1 | Externally linked function f1 not declared in header file. |
+| test.cpp:3:5:3:6 | definition of g1 | Externally linked object 'g1' not declared in header file. |
+| test.cpp:4:12:4:13 | declaration of g2 | Externally linked object 'g2' not declared in header file. |
+| test.cpp:10:5:10:6 | definition of l1 | Externally linked object 'l1' not declared in header file. |
+| test.cpp:11:6:11:7 | definition of f1 | Externally linked function 'f1' not declared in header file. |
+| test.cpp:22:5:22:5 | definition of f | Externally linked function 'f' not declared in header file. |
+| test.cpp:25:5:25:6 | declaration of f1 | Externally linked function 'f1' not declared in header file. |
diff --git a/cpp/autosar/test/rules/A3-3-1/test.cpp b/cpp/autosar/test/rules/A3-3-1/test.cpp
@@ -35,4 +35,8 @@ namespace n {
 void f5() { // COMPLIANT
   int i = 0;
 }
-} // namespace n
+} // namespace n
+
+const int c = 1;                // COMPLIANT - internal linkage
+const char *const str2 = "foo"; // COMPLIANT - internal linkage
+constexpr int k = 1;            // COMPLIANT - internal linkage
diff --git a/cpp/cert/src/rules/CTR52-CPP/GuaranteeGenericCppLibraryFunctionsDoNotOverflow.ql b/cpp/cert/src/rules/CTR52-CPP/GuaranteeGenericCppLibraryFunctionsDoNotOverflow.ql
@@ -80,16 +80,7 @@ where
     iteratorCreationCall = outputContainer.getAnIteratorFunctionCall() and
     iteratorCreationCall = c.getOutputIteratorSource()
   |
-    // Guarded by a bounds check that ensures our destination is larger than "some" value
-    exists(
-      GuardCondition guard, ContainerAccessWithoutRangeCheck::ContainerSizeCall sizeCall,
-      boolean branch
-    |
-      globalValueNumber(sizeCall.getQualifier()) =
-        globalValueNumber(iteratorCreationCall.getQualifier()) and
-      guard.controls(c.getBasicBlock(), branch) and
-      relOpWithSwapAndNegate(guard, sizeCall, _, Greater(), _, branch)
-    )
+    sizeCompareBoundsChecked(iteratorCreationCall, c)
     or
     // Container created with sufficient size for the input
     exists(ContainerAccessWithoutRangeCheck::ContainerConstructorCall outputIteratorConstructor |
diff --git a/cpp/cert/src/rules/CTR55-CPP/DoNotUseAnAdditiveOperatorOnAnIterator.ql b/cpp/cert/src/rules/CTR55-CPP/DoNotUseAnAdditiveOperatorOnAnIterator.ql
@@ -14,24 +14,81 @@
 import cpp
 import codingstandards.cpp.cert
 import codingstandards.cpp.Iterators
+import semmle.code.cpp.controlflow.Dominance
 
-from ContainerIteratorAccess it
+/**
+ * Models a call to an iterator's `operator+`
+ */
+class AdditionOperatorFunctionCall extends AdditiveOperatorFunctionCall {
+  AdditionOperatorFunctionCall() { this.getTarget().hasName("operator+") }
+}
+
+/**
+ *  There exists a calculation for the reference one passed the end of some container
+ *  An example derivation is:
+ *     `end = begin() + size()`
+ */
+Expr getDerivedReferenceToOnePassedTheEndElement(Expr containerReference) {
+  exists(
+    ContainerAccessWithoutRangeCheck::ContainerSizeCall size,
+    ContainerAccessWithoutRangeCheck::ContainerBeginCall begin, AdditionOperatorFunctionCall calc
+  |
+    result = calc
+  |
+    DataFlow::localFlow(DataFlow::exprNode(size), DataFlow::exprNode(calc.getAChild+())) and
+    DataFlow::localFlow(DataFlow::exprNode(begin), DataFlow::exprNode(calc.getAChild+())) and
+    //make sure its the same container providing its size as giving the begin
+    globalValueNumber(begin.getQualifier()) = globalValueNumber(size.getQualifier()) and
+    containerReference = begin.getQualifier()
+  )
+}
+
+/**
+ * a wrapper predicate for a couple of types of permitted end bounds checks
+ */
+Expr getReferenceToOnePassedTheEndElement(Expr containerReference) {
+  //a container end access - v.end()
+  result instanceof ContainerAccessWithoutRangeCheck::ContainerEndCall and
+  containerReference = result.(FunctionCall).getQualifier()
+  or
+  result = getDerivedReferenceToOnePassedTheEndElement(containerReference)
+}
+
+/**
+ * some guard exists like: `iterator != end`
+ * where a relevant`.end()` call flowed into end
+ */
+predicate isUpperBoundEndCheckedIteratorAccess(IteratorSource source, ContainerIteratorAccess it) {
+  exists(
+    Expr referenceToOnePassedTheEndElement, BasicBlock basicBlockOfIteratorAccess,
+    GuardCondition upperBoundCheck, ContainerIteratorAccess checkedIteratorAccess,
+    Expr containerReferenceFromEndGuard
+  |
+    //sufficient end guard
+    referenceToOnePassedTheEndElement =
+      getReferenceToOnePassedTheEndElement(containerReferenceFromEndGuard) and
+    //guard controls the access
+    upperBoundCheck.controls(basicBlockOfIteratorAccess, _) and
+    basicBlockOfIteratorAccess.contains(it) and
+    //guard is comprised of end check and an iterator access
+    DataFlow::localFlow(DataFlow::exprNode(referenceToOnePassedTheEndElement),
+      DataFlow::exprNode(upperBoundCheck.getChild(_))) and
+    upperBoundCheck.getChild(_) = checkedIteratorAccess and
+    //make sure its the same iterator being checked in the guard as accessed
+    checkedIteratorAccess.getOwningContainer() = it.getOwningContainer() and
+    //if its the end call itself (or its parts), make sure its the same container providing its end as giving the iterator
+    globalValueNumber(containerReferenceFromEndGuard) = globalValueNumber(source.getQualifier()) and
+    // and the guard call we match must be after the assignment call (to avoid valid guards protecting new iterator accesses further down)
+    source.getASuccessor*() = upperBoundCheck
+  )
+}
+
+from ContainerIteratorAccess it, IteratorSource source
 where
   not isExcluded(it, IteratorsPackage::doNotUseAnAdditiveOperatorOnAnIteratorQuery()) and
   it.isAdditiveOperation() and
   not exists(RangeBasedForStmt fs | fs.getUpdate().getAChild*() = it) and
-  // we get the neraby assignment
-  not exists(STLContainer c, FunctionCall nearbyAssigningIteratorCall, FunctionCall guardCall |
-    nearbyAssigningIteratorCall = it.getANearbyAssigningIteratorCall() and
-    // we look for calls to size or end
-    (guardCall = c.getACallToSize() or guardCall = c.getAnIteratorEndFunctionCall()) and
-    // such that the call to size is before this
-    // access
-    guardCall = it.getAPredecessor*() and
-    // and it uses the same qualifier as the one we were just assigned
-    nearbyAssigningIteratorCall.getQualifier().(VariableAccess).getTarget() =
-      guardCall.getQualifier().(VariableAccess).getTarget() and
-    // and the size call we match must be after the assignment call
-    nearbyAssigningIteratorCall.getASuccessor*() = guardCall
-  )
+  source = it.getANearbyAssigningIteratorCall() and
+  not isUpperBoundEndCheckedIteratorAccess(source, it) and
+  not sizeCompareBoundsChecked(source, it)
 select it, "Increment of iterator may overflow since its bounds are not checked."
diff --git a/cpp/cert/test/rules/CTR55-CPP/DoNotUseAnAdditiveOperatorOnAnIterator.expected b/cpp/cert/test/rules/CTR55-CPP/DoNotUseAnAdditiveOperatorOnAnIterator.expected
@@ -1,4 +1,8 @@
 | test.cpp:8:7:8:7 | i | Increment of iterator may overflow since its bounds are not checked. |
 | test.cpp:9:9:9:9 | i | Increment of iterator may overflow since its bounds are not checked. |
 | test.cpp:10:9:10:9 | i | Increment of iterator may overflow since its bounds are not checked. |
-| test.cpp:27:31:27:31 | i | Increment of iterator may overflow since its bounds are not checked. |
+| test.cpp:22:18:22:18 | i | Increment of iterator may overflow since its bounds are not checked. |
+| test.cpp:28:31:28:31 | i | Increment of iterator may overflow since its bounds are not checked. |
+| test.cpp:41:5:41:8 | end2 | Increment of iterator may overflow since its bounds are not checked. |
+| test.cpp:53:42:53:42 | i | Increment of iterator may overflow since its bounds are not checked. |
+| test.cpp:64:15:64:15 | i | Increment of iterator may overflow since its bounds are not checked. |
diff --git a/cpp/cert/test/rules/CTR55-CPP/test.cpp b/cpp/cert/test/rules/CTR55-CPP/test.cpp
@@ -20,10 +20,47 @@ void f1(std::vector<int> &v) {
   }
   for (auto i = v.begin(),
             l = (i + std::min(static_cast<std::vector<int>::size_type>(10),
-                              v.size()));
-       i != l; ++i) { // COMPLIANT
+                              v.size())); // NON_COMPLIANT - technically in the
+                                          // calculation
+       i != l; ++i) {                     // COMPLIANT
   }
 
   for (auto i = v.begin();; ++i) { // NON_COMPLIANT
   }
+}
+
+void test_fp_reported_in_374(std::vector<int> &v) {
+  {
+    auto end = v.end();
+    for (auto i = v.begin(); i != end; ++i) { // COMPLIANT
+    }
+  }
+
+  {
+    auto end2 = v.end();
+    end2++; // NON_COMPLIANT
+    for (auto i = v.begin(); i != end2;
+         ++i) { // NON_COMPLIANT[FALSE_NEGATIVE] - case of invalidations to
+                // check before use expected to be less frequent, can model in
+                // future if need be
+    }
+  }
+}
+
+void test(std::vector<int> &v, std::vector<int> &v2) {
+  {
+    auto end = v2.end();
+    for (auto i = v.begin(); i != end; ++i) { // NON_COMPLIANT - wrong check
+    }
+  }
+}
+
+void test2(std::vector<int> &v) {
+  auto i = v.begin();
+  while (1) {
+    auto i2 = ((i != v.end()) != 0);
+    if (!i2)
+      break;
+    (void)((++i)); // COMPLIANT[FALSE_POSITIVE]
+  }
 }
diff --git a/cpp/common/src/codingstandards/cpp/Compiler.qll b/cpp/common/src/codingstandards/cpp/Compiler.qll
@@ -0,0 +1,39 @@
+/** A module to reason about the compiler used to compile translation units. */
+
+import cpp
+import codingstandards.cpp.Scope
+
+newtype Compiler =
+  Gcc() or
+  Clang() or
+  UnsupportedCompiler()
+
+/** Get the match pattern to detect the compiler being mimicked by the extractor to determine the compiler used to compile a file. */
+string getMimicMatch(Compiler compiler) {
+  result = ["%gcc", "%g++"] and compiler instanceof Gcc
+  or
+  result = ["%clang", "%clang++"] and compiler instanceof Clang
+}
+
+/** Get the compiler used to compile the translation unit the file `f` is part of. */
+Compiler getCompiler(File f) {
+  exists(Compilation compilation, TranslationUnit translationUnit |
+    compilation.getAFileCompiled() = translationUnit and
+    (f = translationUnit or f = translationUnit.getAUserFile())
+  |
+    if exists(int mimicIndex | compilation.getArgument(mimicIndex) = "--mimic")
+    then
+      exists(int mimicIndex |
+        compilation.getArgument(mimicIndex) = "--mimic" and
+        (
+          compilation.getArgument(mimicIndex + 1).matches(getMimicMatch(result))
+          or
+          forall(string match | match = getMimicMatch(_) |
+            not compilation.getArgument(mimicIndex + 1).matches(match)
+          ) and
+          result = UnsupportedCompiler()
+        )
+      )
+    else result = UnsupportedCompiler()
+  )
+}
diff --git a/cpp/common/src/codingstandards/cpp/Iterators.qll b/cpp/common/src/codingstandards/cpp/Iterators.qll
diff --git a/cpp/common/src/codingstandards/cpp/rules/containeraccesswithoutrangecheck/ContainerAccessWithoutRangeCheck.qll b/cpp/common/src/codingstandards/cpp/rules/containeraccesswithoutrangecheck/ContainerAccessWithoutRangeCheck.qll

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+rules/RULE-6-1/BitFieldsShallOnlyBeDeclaredWithAnAppropriateType.ql`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+semmle-extractor-options:--mimic clang --std=c11 --edg --diag_error=implicit_func_decl -nostdinc -I../../../../common/test/includes/standard-library`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+semmle-extractor-options:--mimic gcc --std=c11 --edg --diag_error=implicit_func_decl -nostdinc -I../../../../common/test/includes/standard-library`