[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

llvm-svn: 264759

4 files changed, 232 insertions, 0 deletions

diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index 821e8e4e2e7..1c200953e00 100644
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -5,6 +5,7 @@ add_clang_library(clangTidyMiscModule
   AssertSideEffectCheck.cpp
   AssignOperatorSignatureCheck.cpp
   BoolPointerImplicitConversionCheck.cpp
+  DanglingHandleCheck.cpp
   DefinitionsInHeadersCheck.cpp
   ForwardDeclarationNamespaceCheck.cpp
   InaccurateEraseCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/misc/DanglingHandleCheck.cpp b/clang-tools-extra/clang-tidy/misc/DanglingHandleCheck.cpp
new file mode 100644
index 00000000000..41335151c49
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/DanglingHandleCheck.cpp
@@ -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
diff --git a/clang-tools-extra/clang-tidy/misc/DanglingHandleCheck.h b/clang-tools-extra/clang-tidy/misc/DanglingHandleCheck.h
new file mode 100644
index 00000000000..48797a41d83
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/DanglingHandleCheck.h
@@ -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
diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
index b6d88e45619..2da30b7c58e 100644
--- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -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>(