[clang-tidy] Add a little checker for Twine locals in LLVM.

Those often cause use after free bugs and should be generally avoided.
Technically it is safe to have a Twine with >=2 components in a variable
but I don't think it is a good pattern to follow. The almost trivial checker
comes with elaborated fix-it hints that turn the Twine into a std::string
if necessary and otherwise fall back to the original type if the Twine
is created from a single value.

llvm-svn: 212535

clang-tools-extra/clang-tidy/llvm/CMakeLists.txt

@@ -4,6 +4,7 @@ add_clang_library(clangTidyLLVMModule
   IncludeOrderCheck.cpp
   LLVMTidyModule.cpp
   NamespaceCommentCheck.cpp
+  TwineLocalCheck.cpp
 
   LINK_LIBS
   clangAST

clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp

@@ -12,6 +12,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "IncludeOrderCheck.h"
 #include "NamespaceCommentCheck.h"
+#include "TwineLocalCheck.h"
 
 namespace clang {
 namespace tidy {
@@ -24,6 +25,9 @@ public:
     CheckFactories.addCheckFactory(
         "llvm-namespace-comment",
         new ClangTidyCheckFactory<NamespaceCommentCheck>());
+    CheckFactories.addCheckFactory(
+        "llvm-twine-local",
+        new ClangTidyCheckFactory<TwineLocalCheck>());
   }
 };
 

clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.cpp

@@ -0,0 +1,64 @@
+//===--- TwineLocalCheck.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 "TwineLocalCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+TwineLocalCheck::TwineLocalCheck() {}
+
+void TwineLocalCheck::registerMatchers(MatchFinder *Finder) {
+  auto TwineType =
+      qualType(hasDeclaration(recordDecl(hasName("::llvm::Twine"))));
+  Finder->addMatcher(varDecl(hasType(TwineType)).bind("variable"), this);
+}
+
+void TwineLocalCheck::check(const MatchFinder::MatchResult &Result) {
+  const VarDecl *VD = Result.Nodes.getNodeAs<VarDecl>("variable");
+  auto Diag = diag(VD->getLocation(),
+                   "twine variables are prone to use after free bugs");
+
+  // If this VarDecl has an initializer try to fix it.
+  if (VD->hasInit()) {
+    // Peel away implicit constructors and casts so we can see the actual type
+    // of the initializer.
+    const Expr *C = VD->getInit();
+    while (isa<CXXConstructExpr>(C))
+      C = cast<CXXConstructExpr>(C)->getArg(0)->IgnoreParenImpCasts();
+
+    SourceRange TypeRange =
+        VD->getTypeSourceInfo()->getTypeLoc().getSourceRange();
+
+    // A real Twine, turn it into a std::string.
+    if (VD->getType()->getCanonicalTypeUnqualified() ==
+        C->getType()->getCanonicalTypeUnqualified()) {
+      SourceLocation EndLoc = Lexer::getLocForEndOfToken(
+          VD->getInit()->getLocEnd(), 0, *Result.SourceManager,
+          Result.Context->getLangOpts());
+      Diag << FixItHint::CreateReplacement(TypeRange, "std::string")
+           << FixItHint::CreateInsertion(VD->getInit()->getLocStart(), "(")
+           << FixItHint::CreateInsertion(EndLoc, ").str()");
+    } else {
+      // Just an implicit conversion. Insert the real type.
+      Diag << FixItHint::CreateReplacement(
+          TypeRange,
+          C->getType().getAsString(Result.Context->getPrintingPolicy()));
+    }
+  }
+}
+
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/llvm/TwineLocalCheck.h

@@ -0,0 +1,31 @@
+//===--- TwineLocalCheck.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_LLVM_TWINE_LOCAL_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_TWINE_LOCAL_CHECK_H
+
+#include "../ClangTidy.h"
+#include "llvm/Support/Regex.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief Looks for local Twine variables which are prone to use after frees
+/// and should be generally avoided.
+class TwineLocalCheck : public ClangTidyCheck {
+public:
+  TwineLocalCheck();
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_TWINE_LOCAL_CHECK_H

clang-tools-extra/test/clang-tidy/llvm-twine-local.cpp

@@ -0,0 +1,35 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,llvm-twine-local' -fix -- > %t.msg 2>&1
+// RUN: FileCheck -input-file=%t.cpp %s
+// RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s
+
+namespace llvm {
+class Twine {
+public:
+  Twine(const char *);
+  Twine(int);
+  Twine &operator+(const Twine &);
+};
+}
+
+using namespace llvm;
+
+void foo(const Twine &x);
+
+static Twine Moo = Twine("bark") + "bah";
+// CHECK-MASSAGES: twine variables are prone to use after free bugs
+// CHECK-MESSAGES: note: FIX-IT applied suggested code changes
+// CHECK: static std::string Moo = (Twine("bark") + "bah").str();
+
+int main() {
+  const Twine t = Twine("a") + "b" + Twine(42);
+// CHECK-MASSAGES: twine variables are prone to use after free bugs
+// CHECK-MESSAGES: note: FIX-IT applied suggested code changes
+// CHECK: std::string t = (Twine("a") + "b" + Twine(42)).str();
+  foo(Twine("a") + "b");
+
+  Twine Prefix = false ? "__INT_FAST" : "__UINT_FAST";
+// CHECK-MASSAGES: twine variables are prone to use after free bugs
+// CHECK-MESSAGES: note: FIX-IT applied suggested code changes
+// CHECK: const char * Prefix = false ? "__INT_FAST" : "__UINT_FAST";
+}