[clang-tools-extra] r264759 - [clang-tidy] Add check to detect dangling references in value handlers.
Samuel Benzaquen via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 29 11:02:27 PDT 2016
Author: sbenza
Date: Tue Mar 29 13:02:26 2016
New Revision: 264759
URL: http://llvm.org/viewvc/llvm-project?rev=264759&view=rev
Log:
[clang-tidy] Add check to detect dangling references in value handlers.
Summary:
Add check misc-dangling-handle to detect dangling references in value
handlers like std::experimental::string_view.
It provides a configuration option to specify other handle types that
should also be checked.
Right now it detects:
- Construction from temporaries.
- Assignment from temporaries.
- Return statements from temporaries or locals.
- Insertion into containers from temporaries.
Reviewers: alexfh
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D17811
Added:
clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-dangling-handle.rst
clang-tools-extra/trunk/test/clang-tidy/misc-dangling-handle.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=264759&r1=264758&r2=264759&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Tue Mar 29 13:02:26 2016
@@ -5,6 +5,7 @@ add_clang_library(clangTidyMiscModule
AssertSideEffectCheck.cpp
AssignOperatorSignatureCheck.cpp
BoolPointerImplicitConversionCheck.cpp
+ DanglingHandleCheck.cpp
DefinitionsInHeadersCheck.cpp
ForwardDeclarationNamespaceCheck.cpp
InaccurateEraseCheck.cpp
Added: clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp?rev=264759&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.cpp Tue Mar 29 13:02:26 2016
@@ -0,0 +1,185 @@
+//===--- DanglingHandleCheck.cpp - clang-tidy------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "DanglingHandleCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+namespace {
+
+static const char HandleClassesDelimiter[] = ";";
+
+std::vector<std::string> parseClasses(StringRef Option) {
+ SmallVector<StringRef, 4> Classes;
+ Option.split(Classes, HandleClassesDelimiter);
+ std::vector<std::string> Result;
+ for (StringRef &Class : Classes) {
+ Class = Class.trim();
+ if (!Class.empty())
+ Result.push_back(Class);
+ }
+ return Result;
+}
+
+ast_matchers::internal::BindableMatcher<Stmt> handleFrom(
+ ast_matchers::internal::Matcher<RecordDecl> IsAHandle,
+ ast_matchers::internal::Matcher<Expr> Arg) {
+ return cxxConstructExpr(hasDeclaration(cxxMethodDecl(ofClass(IsAHandle))),
+ hasArgument(0, Arg));
+}
+
+ast_matchers::internal::Matcher<Stmt> handleFromTemporaryValue(
+ ast_matchers::internal::Matcher<RecordDecl> IsAHandle) {
+ // If a ternary operator returns a temporary value, then both branches hold a
+ // temporary value. If one of them is not a temporary then it must be copied
+ // into one to satisfy the type of the operator.
+ const auto TemporaryTernary =
+ conditionalOperator(hasTrueExpression(cxxBindTemporaryExpr()),
+ hasFalseExpression(cxxBindTemporaryExpr()));
+
+ return handleFrom(IsAHandle, anyOf(cxxBindTemporaryExpr(), TemporaryTernary));
+}
+
+ast_matchers::internal::Matcher<RecordDecl> isASequence() {
+ return hasAnyName("::std::deque", "::std::forward_list", "::std::list",
+ "::std::vector");
+}
+
+ast_matchers::internal::Matcher<RecordDecl> isASet() {
+ return hasAnyName("::std::set", "::std::multiset", "::std::unordered_set",
+ "::std::unordered_multiset");
+}
+
+ast_matchers::internal::Matcher<RecordDecl> isAMap() {
+ return hasAnyName("::std::map", "::std::multimap", "::std::unordered_map",
+ "::std::unordered_multimap");
+}
+
+ast_matchers::internal::BindableMatcher<Stmt>
+makeContainerMatcher(ast_matchers::internal::Matcher<RecordDecl> IsAHandle) {
+ // This matcher could be expanded to detect:
+ // - Constructors: eg. vector<string_view>(3, string("A"));
+ // - emplace*(): This requires a different logic to determine that
+ // the conversion will happen inside the container.
+ // - map's insert: This requires detecting that the pair conversion triggers
+ // the bug. A little more complicated than what we have now.
+ return callExpr(
+ hasAnyArgument(handleFromTemporaryValue(IsAHandle)),
+ anyOf(
+ // For sequences: assign, push_back, resize.
+ cxxMemberCallExpr(
+ callee(functionDecl(hasAnyName("assign", "push_back", "resize"))),
+ on(expr(hasType(recordDecl(isASequence()))))),
+ // For sequences and sets: insert.
+ cxxMemberCallExpr(
+ callee(functionDecl(hasName("insert"))),
+ on(expr(hasType(recordDecl(anyOf(isASequence(), isASet())))))),
+ // For maps: operator[].
+ cxxOperatorCallExpr(callee(cxxMethodDecl(ofClass(isAMap()))),
+ hasOverloadedOperatorName("[]"))));
+}
+
+} // anonymous namespace
+
+DanglingHandleCheck::DanglingHandleCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ HandleClasses(parseClasses(Options.get(
+ "HandleClasses",
+ "std::basic_string_view;std::experimental::basic_string_view"))),
+ IsAHandle(cxxRecordDecl(hasAnyName(std::vector<StringRef>(
+ HandleClasses.begin(), HandleClasses.end())))
+ .bind("handle")) {}
+
+void DanglingHandleCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "HandleClasses",
+ llvm::join(HandleClasses.begin(), HandleClasses.end(),
+ HandleClassesDelimiter));
+}
+
+void DanglingHandleCheck::registerMatchersForVariables(MatchFinder* Finder) {
+ const auto ConvertedHandle = handleFromTemporaryValue(IsAHandle);
+
+ // Find 'Handle foo(ReturnsAValue());'
+ Finder->addMatcher(
+ varDecl(hasType(cxxRecordDecl(IsAHandle)),
+ hasInitializer(
+ exprWithCleanups(has(ConvertedHandle)).bind("bad_stmt"))),
+ this);
+
+ // Find 'Handle foo = ReturnsAValue();'
+ Finder->addMatcher(
+ varDecl(hasType(cxxRecordDecl(IsAHandle)), unless(parmVarDecl()),
+ hasInitializer(
+ exprWithCleanups(has(handleFrom(IsAHandle, ConvertedHandle)))
+ .bind("bad_stmt"))),
+ this);
+ // Find 'foo = ReturnsAValue(); // foo is Handle'
+ Finder->addMatcher(
+ cxxOperatorCallExpr(callee(cxxMethodDecl(ofClass(IsAHandle))),
+ hasOverloadedOperatorName("="),
+ hasArgument(1, ConvertedHandle))
+ .bind("bad_stmt"),
+ this);
+
+ // Container insertions that will dangle.
+ Finder->addMatcher(makeContainerMatcher(IsAHandle).bind("bad_stmt"), this);
+}
+
+void DanglingHandleCheck::registerMatchersForReturn(MatchFinder* Finder) {
+ // Return a local.
+ Finder->addMatcher(
+ returnStmt(
+ // The AST contains two constructor calls:
+ // 1. Value to Handle conversion.
+ // 2. Handle copy construction.
+ // We have to match both.
+ has(handleFrom(
+ IsAHandle,
+ handleFrom(IsAHandle, declRefExpr(to(varDecl(
+ // Is function scope ...
+ hasAutomaticStorageDuration(),
+ // ... and it is a local array or Value.
+ anyOf(hasType(arrayType()),
+ hasType(recordDecl(
+ unless(IsAHandle)))))))))),
+ // Temporary fix for false positives inside lambdas.
+ unless(hasAncestor(lambdaExpr())))
+ .bind("bad_stmt"),
+ this);
+
+ // Return a temporary.
+ Finder->addMatcher(
+ returnStmt(has(exprWithCleanups(has(handleFrom(
+ IsAHandle, handleFromTemporaryValue(IsAHandle))))))
+ .bind("bad_stmt"),
+ this);
+}
+
+void DanglingHandleCheck::registerMatchers(MatchFinder* Finder) {
+ registerMatchersForVariables(Finder);
+ registerMatchersForReturn(Finder);
+}
+
+void DanglingHandleCheck::check(const MatchFinder::MatchResult& Result) {
+ auto *Handle = Result.Nodes.getNodeAs<CXXRecordDecl>("handle");
+ diag(Result.Nodes.getNodeAs<Stmt>("bad_stmt")->getLocStart(),
+ "%0 outlives its value")
+ << Handle->getQualifiedNameAsString();
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.h?rev=264759&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/DanglingHandleCheck.h Tue Mar 29 13:02:26 2016
@@ -0,0 +1,43 @@
+//===--- DanglingHandleCheck.h - clang-tidy----------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DANGLING_HANDLE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DANGLING_HANDLE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Detect dangling references in value handlers like
+/// std::experimental::string_view.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-dangling-handle.html
+class DanglingHandleCheck : public ClangTidyCheck {
+public:
+ DanglingHandleCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ void registerMatchersForVariables(ast_matchers::MatchFinder *Finder);
+ void registerMatchersForReturn(ast_matchers::MatchFinder *Finder);
+
+ const std::vector<std::string> HandleClasses;
+ const ast_matchers::internal::Matcher<RecordDecl> IsAHandle;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DANGLING_HANDLE_H
Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=264759&r1=264758&r2=264759&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Tue Mar 29 13:02:26 2016
@@ -14,6 +14,7 @@
#include "AssertSideEffectCheck.h"
#include "AssignOperatorSignatureCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
+#include "DanglingHandleCheck.h"
#include "DefinitionsInHeadersCheck.h"
#include "ForwardDeclarationNamespaceCheck.h"
#include "InaccurateEraseCheck.h"
@@ -54,6 +55,8 @@ public:
"misc-assign-operator-signature");
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
"misc-bool-pointer-implicit-conversion");
+ CheckFactories.registerCheck<DanglingHandleCheck>(
+ "misc-dangling-handle");
CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
"misc-definitions-in-headers");
CheckFactories.registerCheck<ForwardDeclarationNamespaceCheck>(
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=264759&r1=264758&r2=264759&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Tue Mar 29 13:02:26 2016
@@ -50,6 +50,7 @@ Clang-Tidy Checks
misc-assert-side-effect
misc-assign-operator-signature
misc-bool-pointer-implicit-conversion
+ misc-dangling-handle
misc-definitions-in-headers
misc-forward-declaration-namespace
misc-inaccurate-erase
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-dangling-handle.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-dangling-handle.rst?rev=264759&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-dangling-handle.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-dangling-handle.rst Tue Mar 29 13:02:26 2016
@@ -0,0 +1,34 @@
+.. title:: clang-tidy - misc-dangling-handle
+
+misc-dangling-handle
+====================
+
+Detect dangling references in value handlers like
+`std::experimental::string_view`.
+These dangling references can come from constructing handles from temporary
+values, where the temporary is destroyed soon after the handle is created.
+
+By default only `std::experimental::basic_string_view` is considered.
+This list can be modified by passing a ; separated list of class names using
+the HandleClasses option.
+
+Examples:
+
+.. code-block:: c++
+
+ string_view View = string(); // View will dangle.
+ string A;
+ View = A + "A"; // still dangle.
+
+ vector<string_view> V;
+ V.push_back(string()); // V[0] is dangling.
+ V.resize(3, string()); // V[1] and V[2] will also dangle.
+
+ string_view f() {
+ // All these return values will dangle.
+ return string();
+ string S;
+ return S;
+ char Array[10]{};
+ return Array;
+ }
Added: clang-tools-extra/trunk/test/clang-tidy/misc-dangling-handle.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-dangling-handle.cpp?rev=264759&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-dangling-handle.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-dangling-handle.cpp Tue Mar 29 13:02:26 2016
@@ -0,0 +1,191 @@
+// RUN: %check_clang_tidy %s misc-dangling-handle %t -- \
+// RUN: -config="{CheckOptions: \
+// RUN: [{key: misc-dangling-handle.HandleClasses, \
+// RUN: value: 'std::basic_string_view; ::llvm::StringRef;'}]}" \
+// RUN: -- -std=c++11
+
+namespace std {
+
+template <typename T>
+class vector {
+ public:
+ using const_iterator = const T*;
+ using iterator = T*;
+ using size_type = int;
+
+ void assign(size_type count, const T& value);
+ iterator insert(const_iterator pos, const T& value);
+ iterator insert(const_iterator pos, T&& value);
+ iterator insert(const_iterator pos, size_type count, const T& value);
+ void push_back(const T&);
+ void push_back(T&&);
+ void resize(size_type count, const T& value);
+};
+
+template <typename, typename>
+class pair {};
+
+template <typename T>
+class set {
+ public:
+ using const_iterator = const T*;
+ using iterator = T*;
+
+ std::pair<iterator, bool> insert(const T& value);
+ std::pair<iterator, bool> insert(T&& value);
+ iterator insert(const_iterator hint, const T& value);
+ iterator insert(const_iterator hint, T&& value);
+};
+
+template <typename Key, typename Value>
+class map {
+ public:
+ using value_type = pair<Key, Value>;
+ value_type& operator[](const Key& key);
+ value_type& operator[](Key&& key);
+};
+
+class basic_string {
+ public:
+ basic_string();
+ basic_string(const char*);
+ ~basic_string();
+};
+
+typedef basic_string string;
+
+class basic_string_view {
+ public:
+ basic_string_view(const char*);
+ basic_string_view(const basic_string&);
+};
+
+typedef basic_string_view string_view;
+
+} // namespace std
+
+namespace llvm {
+
+class StringRef {
+ public:
+ StringRef();
+ StringRef(const char*);
+ StringRef(const std::string&);
+};
+
+} // namespace llvm
+
+std::string ReturnsAString();
+
+void Positives() {
+ std::string_view view1 = std::string();
+ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: std::basic_string_view outlives its value [misc-dangling-handle]
+
+ std::string_view view_2 = ReturnsAString();
+ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: std::basic_string_view outlives
+
+ view1 = std::string();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives
+
+ const std::string& str_ref = "";
+ std::string_view view3 = true ? "A" : str_ref;
+ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: std::basic_string_view outlives
+ view3 = true ? "A" : str_ref;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives
+
+ std::string_view view4(ReturnsAString());
+ // CHECK-MESSAGES: [[@LINE-1]]:20: warning: std::basic_string_view outlives
+}
+
+void OtherTypes() {
+ llvm::StringRef ref = std::string();
+ // CHECK-MESSAGES: [[@LINE-1]]:19: warning: llvm::StringRef outlives its value
+}
+
+const char static_array[] = "A";
+std::string_view ReturnStatements(int i, std::string value_arg,
+ const std::string &ref_arg) {
+ const char array[] = "A";
+ const char* ptr = "A";
+ std::string s;
+ static std::string ss;
+ switch (i) {
+ // Bad cases
+ case 0:
+ return array; // refers to local
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: std::basic_string_view outliv
+ case 1:
+ return s; // refers to local
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: std::basic_string_view outliv
+ case 2:
+ return std::string(); // refers to temporary
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: std::basic_string_view outliv
+ case 3:
+ return value_arg; // refers to by-value arg
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: std::basic_string_view outliv
+
+ // Ok cases
+ case 100:
+ return ss; // refers to static
+ case 101:
+ return static_array; // refers to static
+ case 102:
+ return ptr; // pointer is ok
+ case 103:
+ return ref_arg; // refers to by-ref arg
+ }
+
+ struct S {
+ std::string_view view() { return value; }
+ std::string value;
+ };
+
+ (void)[&]()->std::string_view {
+ // This should not warn. The string is bound by reference.
+ return s;
+ };
+ (void)[=]() -> std::string_view {
+ // This should not warn. The reference is valid as long as the lambda.
+ return s;
+ };
+ (void)[=]() -> std::string_view {
+ // FIXME: This one should warn. We are returning a reference to a local
+ // lambda variable.
+ std::string local;
+ return local;
+ };
+ return "";
+}
+
+void Containers() {
+ std::vector<std::string_view> v;
+ v.assign(3, std::string());
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives
+ v.insert(nullptr, std::string());
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives
+ v.insert(nullptr, 3, std::string());
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives
+ v.push_back(std::string());
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives
+ v.resize(3, std::string());
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives
+
+ std::set<std::string_view> s;
+ s.insert(std::string());
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives
+ s.insert(nullptr, std::string());
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives
+
+ std::map<std::string_view, int> m;
+ m[std::string()];
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: std::basic_string_view outlives
+}
+
+void TakesAStringView(std::string_view);
+
+void Negatives(std::string_view default_arg = ReturnsAString()) {
+ std::string str;
+ std::string_view view = str;
+
+ TakesAStringView(std::string());
+}
More information about the cfe-commits
mailing list