[clang-tidy][Part3] Add a new module Android and three new checks.

Summary: -- fopen() should include "e" in their mode string. [android-fopen-mode]

Reviewers: chh, alexfh, aaron.ballman, hokein

Reviewed By: hokein

Subscribers: JDevlieghere, srhines, mgorny, xazax.hun

Tags: #clang-tools-extra

Differential Revision: https://reviews.llvm.org/D33747

llvm-svn: 306709

clang-tools-extra/clang-tidy/android/AndroidTidyModule.cpp

@@ -11,6 +11,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "CloexecCreatCheck.h"
+#include "CloexecFopenCheck.h"
 #include "FileOpenFlagCheck.h"
 
 using namespace clang::ast_matchers;
@@ -25,6 +26,7 @@ public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<FileOpenFlagCheck>("android-file-open-flag");
     CheckFactories.registerCheck<CloexecCreatCheck>("android-cloexec-creat");
+    CheckFactories.registerCheck<CloexecFopenCheck>("android-cloexec-fopen");
   }
 };
 

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

@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS support)
 add_clang_library(clangTidyAndroidModule
   AndroidTidyModule.cpp
   CloexecCreatCheck.cpp
+  CloexecFopenCheck.cpp
   FileOpenFlagCheck.cpp
 
   LINK_LIBS

clang-tools-extra/clang-tidy/android/CloexecFopenCheck.cpp

@@ -0,0 +1,74 @@
+//===--- CloexecFopenCheck.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 "CloexecFopenCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Type.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace android {
+
+namespace {
+static const char MODE = 'e';
+
+// Build the replace text. If it's string constant, add 'e' directly in the end
+// of the string. Else, add "e".
+std::string BuildReplaceText(const Expr *Arg, const SourceManager &SM,
+                             const LangOptions &LangOpts) {
+  if (Arg->getLocStart().isMacroID())
+    return (Lexer::getSourceText(
+                CharSourceRange::getTokenRange(Arg->getSourceRange()), SM,
+                LangOpts) +
+            " \"" + Twine(MODE) + "\"")
+        .str();
+
+  StringRef SR = cast<StringLiteral>(Arg->IgnoreParenCasts())->getString();
+  return ("\"" + SR + Twine(MODE) + "\"").str();
+}
+} // namespace
+
+void CloexecFopenCheck::registerMatchers(MatchFinder *Finder) {
+  auto CharPointerType = hasType(pointerType(pointee(isAnyCharacter())));
+
+  Finder->addMatcher(
+      callExpr(callee(functionDecl(isExternC(), returns(asString("FILE *")),
+                                   hasName("fopen"),
+                                   hasParameter(0, CharPointerType),
+                                   hasParameter(1, CharPointerType))
+                          .bind("funcDecl")))
+          .bind("fopenFn"),
+      this);
+}
+
+void CloexecFopenCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedCall = Result.Nodes.getNodeAs<CallExpr>("fopenFn");
+  const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("funcDecl");
+  const Expr *ModeArg = MatchedCall->getArg(1);
+
+  // Check if the 'e' may be in the mode string.
+  const auto *ModeStr = dyn_cast<StringLiteral>(ModeArg->IgnoreParenCasts());
+  if (!ModeStr || (ModeStr->getString().find(MODE) != StringRef::npos))
+    return;
+
+  const std::string &ReplacementText = BuildReplaceText(
+      ModeArg, *Result.SourceManager, Result.Context->getLangOpts());
+
+  diag(ModeArg->getLocStart(), "use %0 mode 'e' to set O_CLOEXEC")
+      << FD
+      << FixItHint::CreateReplacement(ModeArg->getSourceRange(),
+                                      ReplacementText);
+}
+
+} // namespace android
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/android/CloexecFopenCheck.h

@@ -0,0 +1,38 @@
+//===--- CloexecFopenCheck.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_ANDROID_CLOEXEC_FOPEN_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_FOPEN_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace android {
+
+/// fopen() is suggested to include "e" in their mode string; like "re" would be
+/// better than "r".
+///
+/// This check only works when corresponding argument is StringLiteral. No
+/// constant propagation.
+///
+/// http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-fopen.html
+class CloexecFopenCheck : public ClangTidyCheck {
+public:
+  CloexecFopenCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace android
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_FOPEN_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -68,6 +68,11 @@ Improvements to clang-tidy
   Checks if the required file flag ``O_CLOEXEC`` exists in ``open()``,
   ``open64()`` and ``openat()``.
 
+- New `android-cloexec-fopen
+  <http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-fopen.html>`_ check
+
+  Checks if the required mode ``e`` exists in the mode argument of ``fopen()``.
+
 - New `cert-dcl21-cpp
   <http://clang.llvm.org/extra/clang-tidy/checks/cert-dcl21-cpp.html>`_ check
 

clang-tools-extra/docs/clang-tidy/checks/android-cloexec-fopen.rst

@@ -0,0 +1,18 @@
+.. title:: clang-tidy - android-cloexec-fopen
+
+android-cloexec-fopen
+=====================
+
+``fopen()`` should include ``e`` in their mode string; so ``re`` would be
+valid. This is equivalent to having set ``FD_CLOEXEC on`` that descriptor.
+
+Examples:
+
+.. code-block:: c++
+
+  fopen("fn", "r");
+
+  // becomes
+
+  fopen("fn", "re");
+

clang-tools-extra/test/clang-tidy/android-cloexec-fopen.cpp

@@ -0,0 +1,57 @@
+// RUN: %check_clang_tidy %s android-cloexec-fopen %t
+
+#define FILE_OPEN_RO "r"
+
+typedef int FILE;
+
+extern "C" FILE *fopen(const char *filename, const char *mode, ...);
+extern "C" FILE *open(const char *filename, const char *mode, ...);
+
+void f() {
+  fopen("filename", "r");
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use 'fopen' mode 'e' to set O_CLOEXEC [android-cloexec-fopen]
+  // CHECK-FIXES: fopen("filename", "re");
+
+  fopen("filename", FILE_OPEN_RO);
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use 'fopen' mode 'e'
+  // CHECK-FIXES: fopen("filename", FILE_OPEN_RO "e");
+
+  fopen("filename", "er");
+  // CHECK-MESSAGES-NOT: warning:
+  fopen("filename", "re");
+  // CHECK-MESSAGES-NOT: warning:
+  fopen("filename", "e");
+  // CHECK-MESSAGES-NOT: warning:
+  open("filename", "e");
+  // CHECK-MESSAGES-NOT: warning:
+
+  char *str = "r";
+  fopen("filename", str);
+  // CHECK-MESSAGES-NOT: warning:
+  str = "re";
+  fopen("filename", str);
+  // CHECK-MESSAGES-NOT: warning:
+  char arr[2] = "r";
+  fopen("filename", arr);
+  // CHECK-MESSAGES-NOT: warning:
+  char arr2[3] = "re";
+  fopen("filename", arr2);
+  // CHECK-MESSAGES-NOT: warning:
+}
+
+namespace i {
+int *fopen(const char *filename, const char *mode, ...);
+void g() {
+  fopen("filename", "e");
+  // CHECK-MESSAGES-NOT: warning:
+}
+} // namespace i
+
+class C {
+public:
+  int *fopen(const char *filename, const char *mode, ...);
+  void h() {
+    fopen("filename", "e");
+    // CHECK-MESSAGES-NOT: warning:
+  }
+};