diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst --- a/clang/docs/analyzer/checkers.rst +++ b/clang/docs/analyzer/checkers.rst @@ -313,6 +313,22 @@ """""""""""""""""""""""""""""" Checks C++ copy and move assignment operators for self assignment. +.. _cplusplus-StringChecker: + +cplusplus.StringChecker (C++) +""""""""""""""""""""""""""""" +Checks std::string operations. + +.. code-block:: cpp + + #include + + void f(const char *p) { + if (!p) { + std::string msg(p); // warn: p is NULL + } + } + .. _deadcode-checkers: deadcode diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td --- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -603,6 +603,10 @@ ]>, Hidden; +def StringChecker: Checker<"StringChecker">, + HelpText<"Checks C++ std::string bugs">, + Documentation; + def MoveChecker: Checker<"Move">, HelpText<"Find use-after-move bugs in C++">, CheckerOptions<[ diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt --- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -105,6 +105,7 @@ StdLibraryFunctionsChecker.cpp STLAlgorithmModeling.cpp StreamChecker.cpp + StringChecker.cpp Taint.cpp TaintTesterChecker.cpp TestAfterDivZeroChecker.cpp diff --git a/clang/lib/StaticAnalyzer/Checkers/StringChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StringChecker.cpp new file mode 100644 --- /dev/null +++ b/clang/lib/StaticAnalyzer/Checkers/StringChecker.cpp @@ -0,0 +1,101 @@ +//=== StringChecker.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 file implements the modeling of the std::basic_string type. +// This involves checking preconditions of the operations and applying the +// effects of the operations, e.g. their post-conditions. +// +//===----------------------------------------------------------------------===// + +#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" + +using namespace clang; +using namespace ento; + +namespace { +class StringChecker : public Checker { + BugType BT_Null{this, "Dereference of null pointer", categories::LogicError}; + mutable const FunctionDecl *StringConstCharPtrCtor = nullptr; + mutable CanQualType SizeTypeTy; + const CallDescription TwoParamStdStringCtor = { + {"std", "basic_string", "basic_string"}, 2, 2}; + + bool isCharToStringCtor(const CallEvent &Call, const ASTContext &ACtx) const; + +public: + void checkPreCall(const CallEvent &Call, CheckerContext &C) const; +}; + +bool StringChecker::isCharToStringCtor(const CallEvent &Call, + const ASTContext &ACtx) const { + if (!Call.isCalled(TwoParamStdStringCtor)) + return false; + const auto *FD = dyn_cast(Call.getDecl()); + assert(FD); + + // See if we already cached it. + if (StringConstCharPtrCtor && StringConstCharPtrCtor == FD) + return true; + + // Verify that the parameters have the expected types: + // - arg 1: `const CharT *` + // - arg 2: some allocator - which is definately not `size_t`. + const QualType Arg1Ty = Call.getArgExpr(0)->getType().getCanonicalType(); + const QualType Arg2Ty = Call.getArgExpr(1)->getType().getCanonicalType(); + + if (!Arg1Ty->isPointerType()) + return false; + + // It makes sure that we don't select the `string(const char* p, size_t len)` + // overload accidentally. + if (Arg2Ty.getCanonicalType() == ACtx.getSizeType()) + return false; + + StringConstCharPtrCtor = FD; // Cache the decl of the right overload. + return true; +} + +void StringChecker::checkPreCall(const CallEvent &Call, + CheckerContext &C) const { + if (!isCharToStringCtor(Call, C.getASTContext())) + return; + const Loc Param = Call.getArgSVal(0).castAs(); + + // We managed to constrain the parameter to non-null. + ProgramStateRef NotNull, Null; + std::tie(NotNull, Null) = C.getState()->assume(Param); + + if (NotNull) { + const auto Callback = [Param](PathSensitiveBugReport &BR) -> std::string { + return BR.isInteresting(Param) ? "Assuming the pointer is not null." : ""; + }; + + // Emit note only if this operation constrained the pointer to be null. + C.addTransition(NotNull, Null ? C.getNoteTag(Callback) : nullptr); + return; + } + + // We found a path on which the parameter is NULL. + if (ExplodedNode *N = C.generateErrorNode(C.getState())) { + auto R = std::make_unique( + BT_Null, "The parameter must not be null", N); + bugreporter::trackExpressionValue(N, Call.getArgExpr(0), *R); + C.emitReport(std::move(R)); + } +} + +} // end anonymous namespace + +void ento::registerStringChecker(CheckerManager &Mgr) { + Mgr.registerChecker(); +} + +bool ento::shouldRegisterStringChecker(const CheckerManager &) { return true; } diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h --- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h +++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h @@ -564,9 +564,38 @@ template class basic_string { + class Allocator {}; + public: - basic_string(); - basic_string(const CharT *s); + basic_string() : basic_string(Allocator()) {} + explicit basic_string(const Allocator &alloc); + basic_string(size_type count, CharT ch, + const Allocator &alloc = Allocator()); + basic_string(const basic_string &other, + size_type pos, + const Allocator &alloc = Allocator()); + basic_string(const basic_string &other, + size_type pos, size_type count, + const Allocator &alloc = Allocator()); + basic_string(const CharT *s, size_type count, + const Allocator &alloc = Allocator()); + basic_string(const CharT *s, + const Allocator &alloc = Allocator()); + template + basic_string(InputIt first, InputIt last, + const Allocator &alloc = Allocator()); + basic_string(const basic_string &other); + basic_string(const basic_string &other, + const Allocator &alloc); + basic_string(basic_string &&other); + basic_string(basic_string &&other, + const Allocator &alloc); + basic_string(std::initializer_list ilist, + const Allocator &alloc = Allocator()); + template + basic_string(const T &t, size_type pos, size_type n, + const Allocator &alloc = Allocator()); + // basic_string(std::nullptr_t) = delete; ~basic_string(); void clear(); @@ -578,6 +607,9 @@ const CharT *data() const; CharT *data(); + const char *begin() const; + const char *end() const; + basic_string &append(size_type count, CharT ch); basic_string &assign(size_type count, CharT ch); basic_string &erase(size_type index, size_type count); diff --git a/clang/test/Analysis/diagnostics/explicit-suppression.cpp b/clang/test/Analysis/diagnostics/explicit-suppression.cpp --- a/clang/test/Analysis/diagnostics/explicit-suppression.cpp +++ b/clang/test/Analysis/diagnostics/explicit-suppression.cpp @@ -19,6 +19,6 @@ void testCopyNull(C *I, C *E) { std::copy(I, E, (C *)0); #ifndef SUPPRESSED - // expected-warning@../Inputs/system-header-simulator-cxx.h:709 {{Called C++ object pointer is null}} + // expected-warning@../Inputs/system-header-simulator-cxx.h:741 {{Called C++ object pointer is null}} #endif } diff --git a/clang/test/Analysis/std-string.cpp b/clang/test/Analysis/std-string.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/std-string.cpp @@ -0,0 +1,81 @@ +// RUN: %clang_analyze_cc1 -std=c++14 %s -verify \ +// RUN: -analyzer-checker=core,unix.Malloc,debug.ExprInspection \ +// RUN: -analyzer-checker=cplusplus.StringChecker \ +// RUN: -analyzer-config eagerly-assume=false \ +// RUN: -analyzer-output=text + +#include "Inputs/system-header-simulator-cxx.h" + +void clang_analyzer_eval(bool); +void clang_analyzer_warnIfReached(); + +void free(void *ptr); + +void irrelevant_std_string_ctors(const char *p) { + std::string x1; // no-warning + std::string x2(2, 'x'); // no-warning + std::string x3(x1, /*pos=*/2); // no-warning + std::string x4(x1, /*pos=*/2, /*count=*/2); // no-warning + std::string x5(p, /*count=*/(size_t)2); // no-warning + // skip std::string(const char*) + std::string x6(x1.begin(), x1.end()); // no-warning + std::string x7(x1); // no-warning + std::string x8(std::move(x1)); // no-warning + std::string x9({'a', 'b', '\0'}); // no-warning +} + +void null_cstring_parameter(const char *p) { + clang_analyzer_eval(p == 0); // expected-warning {{UNKNOWN}} expected-note {{UNKNOWN}} + if (!p) { + // expected-note@-1 2 {{Assuming 'p' is null}} + // expected-note@-2 2 {{Taking true branch}} + clang_analyzer_eval(p == 0); // expected-warning {{TRUE}} expected-note {{TRUE}} + std::string x(p); + // expected-warning@-1 {{The parameter must not be null}} + // expected-note@-2 {{The parameter must not be null}} + clang_analyzer_warnIfReached(); // no-warning + } +} + +void null_constant_parameter() { + std::string x((char *)0); + // expected-warning@-1 {{The parameter must not be null}} + // expected-note@-2 {{The parameter must not be null}} +} + +void ctor_notetag_on_constraining_symbol(const char *p) { + clang_analyzer_eval(p == 0); // expected-warning {{UNKNOWN}} expected-note {{UNKNOWN}} + std::string x(p); // expected-note {{Assuming the pointer is not null}} + clang_analyzer_eval(p == 0); // expected-warning {{FALSE}} expected-note {{FALSE}} + + free((void *)p); // expected-note {{Memory is released}} + free((void *)p); + // expected-warning@-1 {{Attempt to free released memory}} + // expected-note@-2 {{Attempt to free released memory}} +} + +void ctor_no_notetag_symbol_already_constrained(const char *p) { + // expected-note@+2 + {{Assuming 'p' is non-null}} + // expected-note@+1 + {{Taking false branch}} + if (!p) + return; + + clang_analyzer_eval(p == 0); // expected-warning {{FALSE}} expected-note {{FALSE}} + std::string x(p); // no-note: 'p' is already constrained to be non-null. + clang_analyzer_eval(p == 0); // expected-warning {{FALSE}} expected-note {{FALSE}} + + free((void *)p); // expected-note {{Memory is released}} + free((void *)p); + // expected-warning@-1 {{Attempt to free released memory}} + // expected-note@-2 {{Attempt to free released memory}} +} + +void ctor_no_notetag_if_not_interesting(const char *p1, const char *p2) { + std::string s1(p1); // expected-note {{Assuming the pointer is not null}} + std::string s2(p2); // no-note: s2 is not interesting + + free((void *)p1); // expected-note {{Memory is released}} + free((void *)p1); + // expected-warning@-1 {{Attempt to free released memory}} + // expected-note@-2 {{Attempt to free released memory}} +}