Fix FP reported in 424

change_notes/2024-02-13-fix-fp-a15-4-4.md

@@ -0,0 +1,2 @@
+-`A15-4-4` - `MissingNoExcept.ql`:
+    - Fix FP reported in #424. Exclude functions calling `std::string::reserve` or `std::string::append` that may throw even if their signatures don't specify it.

cpp/autosar/test/rules/A15-4-4/test.cpp

@@ -43,4 +43,17 @@ void test_swap_wrapper() noexcept {
   int a = 0;
   int b = 1;
   swap_wrapper(&a, &b);
+}
+
+#include <stdexcept>
+#include <string>
+
+std::string test_fp_reported_in_424(
+    const std::string &s1,
+    const std::string &s2) { // COMPLIANT - `reserve` and `append` may throw.
+  std::string s3;
+  s3.reserve(s1.size() + s2.size());
+  s3.append(s1.c_str(), s1.size());
+  s3.append(s2.c_str(), s2.size());
+  return s3;
 }

cpp/common/src/codingstandards/cpp/exceptions/ExceptionFlow.qll

@@ -5,6 +5,7 @@
 import cpp
 import codingstandards.cpp.standardlibrary.Exceptions
 import codingstandards.cpp.exceptions.ExceptionSpecifications
+import codingstandards.cpp.exceptions.ExceptionFlowCustomizations
 import ThirdPartyExceptions
 
 /*
@@ -271,72 +272,6 @@ ExceptionType getAFunctionThrownType(Function f, ThrowingExpr throwingExpr) {
   )
 }
 
-/** A `ThrowingExpr` which is the origin of a exceptions in the program. */
-abstract class OriginThrowingExpr extends ThrowingExpr { }
-
-/** An expression which directly throws. */
-class DirectThrowExprThrowingExpr extends DirectThrowExpr, OriginThrowingExpr {
-  override ExceptionType getAnExceptionType() { result = getExceptionType() }
-}
-
-/** An `typeid` expression which may throw `std::bad_typeid`. */
-class TypeIdThrowingExpr extends TypeidOperator, OriginThrowingExpr {
-  override ExceptionType getAnExceptionType() { result instanceof StdBadTypeId }
-}
-
-/** An `new[]` expression which may throw `std::bad_array_new_length`. */
-class NewThrowingExpr extends NewArrayExpr, OriginThrowingExpr {
-  NewThrowingExpr() {
-    // If the extent is known to be below 0 at runtime
-    getExtent().getValue().toInt() < 0
-    or
-    // initializer has more elements than the array size
-    getExtent().getValue().toInt() < getInitializer().(ArrayAggregateLiteral).getArraySize()
-  }
-
-  override ExceptionType getAnExceptionType() { result instanceof StdBadArrayNewLength }
-}
-
-/** A `ReThrowExpr` which throws a previously caught exception. */
-class ReThrowExprThrowingExpr extends ReThrowExpr, ThrowingExpr {
-  predicate rethrows(CatchBlock cb, ExceptionType et, ThrowingExpr te) {
-    // Find the nearest CatchBlock
-    cb = getNearestCatch(this.getEnclosingStmt()) and
-    // Find an `ExceptionType` which is caught by this catch block, and `ThrowingExpr` which throws that exception type
-    catches(cb, te, et)
-  }
-
-  override ExceptionType getAnExceptionType() { rethrows(_, result, _) }
-
-  CatchBlock getCatchBlock() { rethrows(result, _, _) }
-}
-
-/** An expression which calls a function which may throw an exception. */
-class FunctionCallThrowingExpr extends FunctionCall, ThrowingExpr {
-  override ExceptionType getAnExceptionType() {
-    exists(Function target |
-      target = getTarget() and
-      result = getAFunctionThrownType(target, _) and
-      // [expect.spec] states that throwing an exception type that is prohibited
-      // by the specification will result in the program terminating, unless
-      // a custom `unexpected_handler` is registered that throws an exception type
-      // which is compatible with the dynamic exception specification, or the
-      // dynamic exception specification lists `std::bad_exception`, in which case
-      // a `std::bad_exception` is thrown.
-      // As dynamic exception specifications and the `unexpected_handler` are both
-      // deprecated in C++14 and removed in C++17, we assume a default
-      // `std::unexpected` handler that calls `std::terminate` and therefore
-      // do not propagate such exceptions to the call sites for the function.
-      not (
-        hasDynamicExceptionSpecification(target) and
-        not result = getAHandledExceptionType(target.getAThrownType())
-        or
-        isNoExceptTrue(target)
-      )
-    )
-  }
-}
-
 module ExceptionPathGraph {
   /**
    * A function for which we want path information.

cpp/common/src/codingstandards/cpp/exceptions/ExceptionFlowCustomizations.qll

@@ -0,0 +1,113 @@
+/*
+ * A library customize models that model the flow of exceptions through the program.
+ */
+
+import cpp
+private import codingstandards.cpp.exceptions.ExceptionFlow
+
+/** A `ThrowingExpr` which is the origin of a exceptions in the program. */
+abstract class OriginThrowingExpr extends ThrowingExpr { }
+
+/**
+ * A `FunctionCall` to an external function without an exception specification that *
+ *  may throw an exception.
+ */
+abstract class ExternalUnderspecifiedFunctionCallThrowingExpr extends FunctionCall, ThrowingExpr { }
+
+/**
+ * An extensible predicate that describes functions that when called may throw an exception.
+ */
+extensible predicate throwingFunctionModel(
+  string functionNamespaceQualifier, string functionTypeQualifier, string functionName,
+  string exceptionNamespaceQualifier, string exceptionType
+);
+
+/**
+ * A `FunctionCall` that may throw an exception of type `ExceptionType` as provded by
+ * the extensible predicate `throwingFunctionModel`.
+ */
+private class ExternalFunctionCallThrowingExpr extends FunctionCall, ThrowingExpr {
+  ExceptionType exceptionType;
+
+  ExternalFunctionCallThrowingExpr() {
+    exists(
+      string functionNamespaceQualifier, string functionTypeQualifier, string functionName,
+      string exceptionNamespaceQualifier, string exceptionTypeSpec
+    |
+      throwingFunctionModel(functionNamespaceQualifier, functionTypeQualifier, functionName,
+        exceptionNamespaceQualifier, exceptionTypeSpec) and
+      this.getTarget()
+          .hasQualifiedName(functionNamespaceQualifier, functionTypeQualifier, functionName) and
+      exceptionType.(Class).hasQualifiedName(exceptionNamespaceQualifier, exceptionTypeSpec)
+    )
+  }
+
+  override ExceptionType getAnExceptionType() { result = exceptionType }
+}
+
+/** An expression which directly throws. */
+class DirectThrowExprThrowingExpr extends DirectThrowExpr, OriginThrowingExpr {
+  override ExceptionType getAnExceptionType() { result = getExceptionType() }
+}
+
+/** A `ReThrowExpr` which throws a previously caught exception. */
+class ReThrowExprThrowingExpr extends ReThrowExpr, ThrowingExpr {
+  predicate rethrows(CatchBlock cb, ExceptionType et, ThrowingExpr te) {
+    // Find the nearest CatchBlock
+    cb = getNearestCatch(this.getEnclosingStmt()) and
+    // Find an `ExceptionType` which is caught by this catch block, and `ThrowingExpr` which throws that exception type
+    catches(cb, te, et)
+  }
+
+  override ExceptionType getAnExceptionType() { rethrows(_, result, _) }
+
+  CatchBlock getCatchBlock() { rethrows(result, _, _) }
+}
+
+/** An expression which calls a function which may throw an exception. */
+class FunctionCallThrowingExpr extends FunctionCall, ThrowingExpr {
+  override ExceptionType getAnExceptionType() {
+    exists(Function target |
+      target = getTarget() and
+      result = getAFunctionThrownType(target, _) and
+      // [expect.spec] states that throwing an exception type that is prohibited
+      // by the specification will result in the program terminating, unless
+      // a custom `unexpected_handler` is registered that throws an exception type
+      // which is compatible with the dynamic exception specification, or the
+      // dynamic exception specification lists `std::bad_exception`, in which case
+      // a `std::bad_exception` is thrown.
+      // As dynamic exception specifications and the `unexpected_handler` are both
+      // deprecated in C++14 and removed in C++17, we assume a default
+      // `std::unexpected` handler that calls `std::terminate` and therefore
+      // do not propagate such exceptions to the call sites for the function.
+      not (
+        hasDynamicExceptionSpecification(target) and
+        not result = getAHandledExceptionType(target.getAThrownType())
+        or
+        isNoExceptTrue(target)
+      )
+    )
+    or
+    result = this.(ExternalUnderspecifiedFunctionCallThrowingExpr).getAnExceptionType()
+    or
+    result = this.(ExternalFunctionCallThrowingExpr).getAnExceptionType()
+  }
+}
+
+/** An `typeid` expression which may throw `std::bad_typeid`. */
+private class TypeIdThrowingExpr extends TypeidOperator, OriginThrowingExpr {
+  override ExceptionType getAnExceptionType() { result instanceof StdBadTypeId }
+}
+
+/** An `new[]` expression which may throw `std::bad_array_new_length`. */
+private class NewThrowingExpr extends NewArrayExpr, OriginThrowingExpr {
+  NewThrowingExpr() {
+    // If the extent is known to be below 0 at runtime
+    getExtent().getValue().toInt() < 0
+    or
+    // initializer has more elements than the array size
+    getExtent().getValue().toInt() < getInitializer().(ArrayAggregateLiteral).getArraySize()
+  }
+
+  override ExceptionType getAnExceptionType() { result instanceof StdBadArrayNewLength }
+}

cpp/common/src/ext/stdc++.model.yml

@@ -0,0 +1,7 @@
+extensions:
+  - addsTo:
+      pack: codeql/common-cpp-coding-standards
+      extensible: throwingFunctionModel
+    data:
+      - ["std", "basic_string", "append", "std", "out_of_range"] 
+      - ["std", "basic_string", "reserve", "std", "length_error"] 

cpp/common/src/qlpack.yml

@@ -3,3 +3,5 @@ version: 2.22.0-dev
 license: MIT
 dependencies:
   codeql/cpp-all: 0.9.3
+dataExtensions:
+  - ext/*.model.yml

cpp/common/test/includes/standard-library/stdexcept.h

@@ -28,5 +28,7 @@ class nested_exception {
 template <typename T> [[noreturn]] void throw_with_nested(T &&t);
 template <typename E> void rethrow_if_nested(E const &e);
 
+class length_error : public logic_error{};
+class out_of_range: public logic_error{};
 } // namespace std
 #endif // _GHLIBCPP_STDEXCEPT

cpp/common/test/includes/standard-library/string

@@ -166,6 +166,8 @@ public:
   int compare(const charT *s) const;
   int compare(size_type pos1, size_type n1, const charT *s) const;
   int compare(size_type pos1, size_type n1, const charT *s, size_type n2) const;
+
+  void reserve(size_type new_cap = 0);
 };
 
 template <class charT, class traits, class Allocator>