[clang] e1fdec8 - [analyzer] Add std::string checker
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 25 02:15:55 PDT 2021
Author: Balazs Benics
Date: 2021-10-25T11:15:40+02:00
New Revision: e1fdec875ff13504057fa01227458c9afa08222f
URL: https://github.com/llvm/llvm-project/commit/e1fdec875ff13504057fa01227458c9afa08222f
DIFF: https://github.com/llvm/llvm-project/commit/e1fdec875ff13504057fa01227458c9afa08222f.diff
LOG: [analyzer] Add std::string checker
This patch adds a checker checking `std::string` operations.
At first, it only checks the `std::string` single `const char *`
constructor for nullness.
If It might be `null`, it will constrain it to non-null and place a note
tag there.
Reviewed By: martong
Differential Revision: https://reviews.llvm.org/D111247
Added:
clang/lib/StaticAnalyzer/Checkers/StringChecker.cpp
clang/test/Analysis/std-string.cpp
Modified:
clang/docs/analyzer/checkers.rst
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
clang/test/Analysis/Inputs/system-header-simulator-cxx.h
clang/test/Analysis/diagnostics/explicit-suppression.cpp
Removed:
################################################################################
diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 267ff4437a64f..62eeb16d10dfa 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -313,6 +313,22 @@ cplusplus.SelfAssignment (C++)
""""""""""""""""""""""""""""""
Checks C++ copy and move assignment operators for self assignment.
+.. _cplusplus-StringChecker:
+
+cplusplus.StringChecker (C++)
+"""""""""""""""""""""""""""""
+Checks std::string operations.
+
+.. code-block:: cpp
+
+ #include <string>
+
+ 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
index aac111982b235..bd21d7778f931 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -603,6 +603,10 @@ def SmartPtrModeling: Checker<"SmartPtrModeling">,
]>,
Hidden;
+def StringChecker: Checker<"StringChecker">,
+ HelpText<"Checks C++ std::string bugs">,
+ Documentation<HasDocumentation>;
+
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
index e7d424ae91504..3e85fadef0a2a 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -105,6 +105,7 @@ add_clang_library(clangStaticAnalyzerCheckers
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
index 0000000000000..56b9cdb95c384
--- /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<check::PreCall> {
+ 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<FunctionDecl>(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<Loc>();
+
+ // 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<PathSensitiveBugReport>(
+ 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<StringChecker>();
+}
+
+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
index ff64c5b63e3c8..8633a8beadbff 100644
--- a/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -564,9 +564,38 @@ namespace std {
template <typename CharT>
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 <class InputIt>
+ 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<CharT> ilist,
+ const Allocator &alloc = Allocator());
+ template <class T>
+ 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 @@ namespace std {
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
index 0ef01771e58bc..b98d0260b0965 100644
--- a/clang/test/Analysis/diagnostics/explicit-suppression.cpp
+++ b/clang/test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@ class C {
void testCopyNull(C *I, C *E) {
std::copy(I, E, (C *)0);
#ifndef SUPPRESSED
- // expected-warning at ../Inputs/system-header-simulator-cxx.h:709 {{Called C++ object pointer is null}}
+ // expected-warning at ../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
index 0000000000000..4755018b9546a
--- /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 at -1 2 {{Assuming 'p' is null}}
+ // expected-note at -2 2 {{Taking true branch}}
+ clang_analyzer_eval(p == 0); // expected-warning {{TRUE}} expected-note {{TRUE}}
+ std::string x(p);
+ // expected-warning at -1 {{The parameter must not be null}}
+ // expected-note at -2 {{The parameter must not be null}}
+ clang_analyzer_warnIfReached(); // no-warning
+ }
+}
+
+void null_constant_parameter() {
+ std::string x((char *)0);
+ // expected-warning at -1 {{The parameter must not be null}}
+ // expected-note at -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 at -1 {{Attempt to free released memory}}
+ // expected-note at -2 {{Attempt to free released memory}}
+}
+
+void ctor_no_notetag_symbol_already_constrained(const char *p) {
+ // expected-note at +2 + {{Assuming 'p' is non-null}}
+ // expected-note at +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 at -1 {{Attempt to free released memory}}
+ // expected-note at -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 at -1 {{Attempt to free released memory}}
+ // expected-note at -2 {{Attempt to free released memory}}
+}
More information about the cfe-commits
mailing list