[analyzer] Warn if the size of the array in `new[]` is undefined

This patch introduces a new checker, called NewArraySize checker,
which detects if the expression that yields the element count of
the array in new[], results in an Undefined value.

Differential Revision: https://reviews.llvm.org/D131299

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 623a520..2903b3d 100644 (file)
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -245,6 +245,22 @@ Check for uninitialized values being returned to the caller.
    return x; // warn
  }
 
+.. _core-uninitialized-NewArraySize:
+
+core.uninitialized.NewArraySize (C++)
+"""""""""""""""""""""""""""""""""""""
+
+Check if the element count in new[] is garbage or undefined.
+
+.. code-block:: cpp
+
+  void test() {
+    int n;
+    int *arr = new int[n]; // warn: Element count in new[] is a garbage value
+    delete[] arr;
+  }
+
+
 .. _cplusplus-checkers:
 
 

diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index cf8cec3..3dd2c7c 100644 (file)
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -437,6 +437,10 @@ def ReturnUndefChecker : Checker<"UndefReturn">,
   HelpText<"Check for uninitialized values being returned to the caller">,
   Documentation<HasDocumentation>;
 
+def UndefinedNewArraySizeChecker : Checker<"NewArraySize">,
+  HelpText<"Check if the size of the array in a new[] expression is undefined">,
+  Documentation<HasDocumentation>;
+
 } // end "core.uninitialized"
 
 //===----------------------------------------------------------------------===//

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index 50a27a2..42a51b4 100644 (file)
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -1033,6 +1033,18 @@ public:
     return getOriginExpr()->getNumPlacementArgs() + getNumImplicitArgs();
   }
 
+  bool isArray() const { return getOriginExpr()->isArray(); }
+
+  Optional<const clang::Expr *> getArraySizeExpr() const {
+    return getOriginExpr()->getArraySize();
+  }
+
+  SVal getArraySizeVal() const {
+    assert(isArray() && "The allocator call doesn't allocate and array!");
+
+    return getState()->getSVal(*getArraySizeExpr(), getLocationContext());
+  }
+
   const Expr *getArgExpr(unsigned Index) const override {
     // The first argument of an allocator call is the size of the allocation.
     if (Index < getNumImplicitArgs())

diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 84886b9..c6be8fe 100644 (file)
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -120,6 +120,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   UndefResultChecker.cpp
   UndefinedArraySubscriptChecker.cpp
   UndefinedAssignmentChecker.cpp
+  UndefinedNewArraySizeChecker.cpp
   UninitializedObject/UninitializedObjectChecker.cpp
   UninitializedObject/UninitializedPointee.cpp
   UnixAPIChecker.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index b281d9b..5613b8f 100644 (file)
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1733,6 +1733,10 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
   // Fill the region with the initialization value.
   State = State->bindDefaultInitial(RetVal, Init, LCtx);
 
+  // If Size is somehow undefined at this point, this line prevents a crash.
+  if (Size.isUndef())
+    Size = UnknownVal();
+
   // Set the region's extent.
   State = setDynamicExtent(State, RetVal.getAsRegion(),
                            Size.castAs<DefinedOrUnknownSVal>(), svalBuilder);

diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefinedNewArraySizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefinedNewArraySizeChecker.cpp
new file mode 100644 (file)
index 0000000..f053ee8
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/UndefinedNewArraySizeChecker.cpp
@@ -0,0 +1,80 @@
+//===--- UndefinedNewArraySizeChecker.cpp -----------------------*- C++ -*--==//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This defines UndefinedNewArraySizeChecker, a builtin check in ExprEngine
+// that checks if the size of the array in a new[] expression is undefined.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class UndefinedNewArraySizeChecker : public Checker<check::PreCall> {
+
+private:
+  BugType BT{this, "Undefined array element count in new[]",
+             categories::LogicError};
+
+public:
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+  void HandleUndefinedArrayElementCount(CheckerContext &C, SVal ArgVal,
+                                        const Expr *Init,
+                                        SourceRange Range) const;
+};
+} // namespace
+
+void UndefinedNewArraySizeChecker::checkPreCall(const CallEvent &Call,
+                                                CheckerContext &C) const {
+  if (const auto *AC = dyn_cast<CXXAllocatorCall>(&Call)) {
+    if (!AC->isArray())
+      return;
+
+    auto *SizeEx = *AC->getArraySizeExpr();
+    auto SizeVal = AC->getArraySizeVal();
+
+    if (SizeVal.isUndef())
+      HandleUndefinedArrayElementCount(C, SizeVal, SizeEx,
+                                       SizeEx->getSourceRange());
+  }
+}
+
+void UndefinedNewArraySizeChecker::HandleUndefinedArrayElementCount(
+    CheckerContext &C, SVal ArgVal, const Expr *Init, SourceRange Range) const {
+
+  if (ExplodedNode *N = C.generateErrorNode()) {
+
+    SmallString<100> buf;
+    llvm::raw_svector_ostream os(buf);
+
+    os << "Element count in new[] is a garbage value";
+
+    auto R = std::make_unique<PathSensitiveBugReport>(BT, os.str(), N);
+    R->markInteresting(ArgVal);
+    R->addRange(Range);
+    bugreporter::trackExpressionValue(N, Init, *R);
+
+    C.emitReport(std::move(R));
+  }
+}
+
+void ento::registerUndefinedNewArraySizeChecker(CheckerManager &mgr) {
+  mgr.registerChecker<UndefinedNewArraySizeChecker>();
+}
+
+bool ento::shouldRegisterUndefinedNewArraySizeChecker(
+    const CheckerManager &mgr) {
+  return mgr.getLangOpts().CPlusPlus;
+}

diff --git a/clang/test/Analysis/Issue56873.cpp b/clang/test/Analysis/Issue56873.cpp
index 36fe5ff..081fc28 100644 (file)
--- a/clang/test/Analysis/Issue56873.cpp
+++ b/clang/test/Analysis/Issue56873.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify %s
 
 void clang_analyzer_warnIfReached();
 

diff --git a/clang/test/Analysis/undefined-new-element.cpp b/clang/test/Analysis/undefined-new-element.cpp
new file mode 100644 (file)
index 0000000..87d8155
--- /dev/null
+++ b/clang/test/Analysis/undefined-new-element.cpp
@@ -0,0 +1,69 @@
+// RUN: %clang_analyze_cc1 %s -analyzer-checker=core.uninitialized.NewArraySize -analyzer-output=text -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void checkUndefinedElmenetCountValue() {
+  int n;
+  // expected-note@-1{{'n' declared without an initial value}}
+
+  int *arr = new int[n]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}
+
+void checkUndefinedElmenetCountMultiDimensionalValue() {
+  int n;
+  // expected-note@-1{{'n' declared without an initial value}}
+
+  auto *arr = new int[n][5]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}
+
+void checkUndefinedElmenetCountReference() {
+  int n;
+  // expected-note@-1{{'n' declared without an initial value}}
+
+  int &ref = n;
+  // expected-note@-1{{'ref' initialized here}}
+
+  int *arr = new int[ref]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}
+
+void checkUndefinedElmenetCountMultiDimensionalReference() {
+  int n;
+  // expected-note@-1{{'n' declared without an initial value}}
+
+  int &ref = n;
+  // expected-note@-1{{'ref' initialized here}}
+
+  auto *arr = new int[ref][5]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}
+
+int foo() {
+  int n;
+
+  return n;
+}
+
+void checkUndefinedElmenetCountFunction() {
+  int *arr = new int[foo()]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}
+
+void checkUndefinedElmenetCountMultiDimensionalFunction() {
+  auto *arr = new int[foo()][5]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}
+
+void *malloc(size_t);
+
+void checkUndefinedPlacementElementCount() {
+  int n;
+  // expected-note@-1{{'n' declared without an initial value}}
+  
+  void *buffer = malloc(sizeof(std::string) * 10);
+  std::string *p =
+      ::new (buffer) std::string[n]; // expected-warning{{Element count in new[] is a garbage value}}
+  // expected-note@-1{{Element count in new[] is a garbage value}}
+}