From a46154cb1cd09aa26bc803d8696e6e9283aac6a9 Mon Sep 17 00:00:00 2001 From: isuckatcs <65320245+isuckatcs@users.noreply.github.com> Date: Tue, 23 Aug 2022 09:21:16 +0200 Subject: [PATCH] [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 --- clang/docs/analyzer/checkers.rst | 16 +++++ .../clang/StaticAnalyzer/Checkers/Checkers.td | 4 ++ .../StaticAnalyzer/Core/PathSensitive/CallEvent.h | 12 ++++ clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt | 1 + .../lib/StaticAnalyzer/Checkers/MallocChecker.cpp | 4 ++ .../Checkers/UndefinedNewArraySizeChecker.cpp | 80 ++++++++++++++++++++++ clang/test/Analysis/Issue56873.cpp | 2 +- clang/test/Analysis/undefined-new-element.cpp | 69 +++++++++++++++++++ 8 files changed, 187 insertions(+), 1 deletion(-) create mode 100644 clang/lib/StaticAnalyzer/Checkers/UndefinedNewArraySizeChecker.cpp create mode 100644 clang/test/Analysis/undefined-new-element.cpp diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst index 623a520..2903b3d 100644 --- 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 --- 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; +def UndefinedNewArraySizeChecker : Checker<"NewArraySize">, + HelpText<"Check if the size of the array in a new[] expression is undefined">, + Documentation; + } // 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 --- 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 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 --- 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 --- 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(), svalBuilder); diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefinedNewArraySizeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefinedNewArraySizeChecker.cpp new file mode 100644 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 { + +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(&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(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(); +} + +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 --- 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 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}} +} -- 2.7.4