Rule 23.11.1 - UseSmarPtrFactoryFunctions.ql

A new query to report uses of the raw pointer constructors of
the std::unique_ptr and std::shared_ptr classes. [a]

Author: lcartey

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

@@ -23,6 +23,7 @@ class unique_ptr {
   unique_ptr(T *ptr) {}
   unique_ptr(const unique_ptr<T> &t) = delete;
   unique_ptr(unique_ptr<T> &&t) {}
+  unique_ptr(pointer p, Deleter d) noexcept {}
   ~unique_ptr() {}
   T &operator*() const { return *ptr; }
   T *operator->() const noexcept { return ptr; }
@@ -93,8 +94,10 @@ template <typename T> class shared_ptr : public __shared_ptr<T> {
   shared_ptr(T *ptr);
   shared_ptr(const shared_ptr<T> &r) noexcept;
   template <class Y> shared_ptr(const shared_ptr<Y> &r) noexcept;
+  template <class Y> shared_ptr(const shared_ptr<Y> &r, T *p) noexcept;
   shared_ptr(shared_ptr<T> &&r) noexcept;
   template <class Y> shared_ptr(shared_ptr<Y> &&r) noexcept;
+  template <class D> shared_ptr(T *p, D d);
   shared_ptr(unique_ptr<T> &&t) {}
   ~shared_ptr() {}
   T &operator*() const noexcept;

cpp/misra/src/rules/RULE-23-11-1/UseSmartPtrFactoryFunctions.ql

@@ -0,0 +1,32 @@
+/**
+ * @id cpp/misra/use-smart-ptr-factory-functions
+ * @name RULE-23-11-1: The raw pointer constructors of std::shared_ptr and std::unique_ptr should not be used
+ * @description Using raw pointer constructors of std::shared_ptr and std::unique_ptr instead of
+ *              make_shared/make_unique can lead to memory leaks if exceptions occur during
+ *              construction.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity error
+ * @tags external/misra/id/rule-23-11-1
+ *       scope/single-translation-unit
+ *       external/misra/enforcement/decidable
+ *       external/misra/obligation/advisory
+ */
+
+import cpp
+import codingstandards.cpp.misra
+
+from ConstructorCall call, Class smartPtrClass
+where
+  not isExcluded(call, BannedAPIsPackage::useSmartPtrFactoryFunctionsQuery()) and
+  smartPtrClass = call.getTarget().getDeclaringType() and
+  (
+    smartPtrClass.hasQualifiedName("std", "shared_ptr") or
+    smartPtrClass.hasQualifiedName("std", "unique_ptr")
+  ) and
+  call.getNumberOfArguments() >= 1 and
+  exists(Type argType |
+    argType = call.getArgument(0).getType().getUnspecifiedType() and
+    argType instanceof PointerType
+  )
+select call, "Use of raw pointer constructor for 'std::" + smartPtrClass.getSimpleName() + "'."

cpp/misra/test/rules/RULE-23-11-1/UseSmartPtrFactoryFunctions.expected

@@ -0,0 +1,9 @@
+| test.cpp:23:25:23:27 | call to shared_ptr | Use of raw pointer constructor for 'std::shared_ptr'. |
+| test.cpp:28:25:28:27 | call to unique_ptr | Use of raw pointer constructor for 'std::unique_ptr'. |
+| test.cpp:34:3:34:32 | call to shared_ptr | Use of raw pointer constructor for 'std::shared_ptr'. |
+| test.cpp:40:3:40:32 | call to unique_ptr | Use of raw pointer constructor for 'std::unique_ptr'. |
+| test.cpp:46:6:46:32 | call to shared_ptr | Use of raw pointer constructor for 'std::shared_ptr'. |
+| test.cpp:47:6:47:32 | call to shared_ptr | Use of raw pointer constructor for 'std::shared_ptr'. |
+| test.cpp:57:27:57:29 | call to unique_ptr | Use of raw pointer constructor for 'std::unique_ptr'. |
+| test.cpp:67:25:67:31 | call to shared_ptr | Use of raw pointer constructor for 'std::shared_ptr'. |
+| test.cpp:74:39:74:45 | call to unique_ptr | Use of raw pointer constructor for 'std::unique_ptr'. |

cpp/misra/test/rules/RULE-23-11-1/UseSmartPtrFactoryFunctions.qlref

@@ -0,0 +1 @@
+rules/RULE-23-11-1/UseSmartPtrFactoryFunctions.ql

cpp/misra/test/rules/RULE-23-11-1/test.cpp

@@ -0,0 +1,84 @@
+#include <cstdint>
+#include <memory>
+
+struct A {
+  std::int8_t i;
+};
+
+class B {};
+
+void test_make_shared_compliant() {
+  auto p = std::make_shared<A>(); // COMPLIANT
+  std::int8_t *pi = &(p->i);
+  std::shared_ptr<std::int8_t> q(
+      p, pi); // COMPLIANT - aliasing constructor, not taking ownership
+}
+
+void test_make_unique_compliant() {
+  auto p = std::make_unique<A>(); // COMPLIANT
+}
+
+void test_raw_pointer_shared_ptr_non_compliant() {
+  A *l1 = new A();
+  std::shared_ptr<A> l2(l1); // NON_COMPLIANT
+}
+
+void test_raw_pointer_unique_ptr_non_compliant() {
+  A *l1 = new A();
+  std::unique_ptr<A> l2(l1); // NON_COMPLIANT
+}
+
+auto test_exception_safety_issue() {
+  auto *l1 = new A();
+  auto l2 = std::make_unique<A>(); // may throw
+  return std::shared_ptr<A>(l1);   // NON_COMPLIANT - memory leak
+                                   // if make_unique throws
+}
+
+auto test_double_delete_issue(std::unique_ptr<A> p) {
+  auto l1 = p.get();
+  return std::unique_ptr<A>(l1); // NON_COMPLIANT - causes double delete
+}
+
+void f1(std::shared_ptr<A> a, std::shared_ptr<B> b);
+
+void test_function_argument_non_compliant() {
+  f1(std::shared_ptr<A>(new A()),  // NON_COMPLIANT
+     std::shared_ptr<B>(new B())); // NON_COMPLIANT
+}
+
+void test_function_argument_compliant() {
+  f1(std::make_shared<A>(),  // COMPLIANT
+     std::make_shared<B>()); // COMPLIANT
+}
+
+void test_array_raw_pointer_non_compliant() {
+  A *l1 = new A[10];
+  std::unique_ptr<A[]> l2(l1); // NON_COMPLIANT
+}
+
+void test_array_make_unique_compliant() {
+  auto l1 = std::make_unique<A[]>(10); // COMPLIANT
+}
+
+void test_custom_deleter_shared_ptr() {
+  A *l1 = new A();
+  auto l2 = [](A *p) { delete p; };
+  std::shared_ptr<A> l3(l1, l2); // NON_COMPLIANT - still
+                                 // using raw pointer constructor
+}
+
+void test_custom_deleter_unique_ptr() {
+  A *l1 = new A();
+  auto l2 = [](A *p) { delete p; };
+  std::unique_ptr<A, decltype(l2)> l3(l1, l2); // NON_COMPLIANT - still
+                                               // using raw pointer constructor
+}
+
+void test_nullptr_shared_ptr() {
+  std::shared_ptr<A> l1(nullptr); // COMPLIANT - no ownership taken
+}
+
+void test_nullptr_unique_ptr() {
+  std::unique_ptr<A> l1(nullptr); // COMPLIANT - no ownership taken
+}