[clang-tidy] Add llvm-derefencing-dyn-cast check

[njames93]

Summary

Add a check that aims to prevent dereferencing casts that may return nullptr.

[njames93]

I had thought about merging this into the prefer-isa-or-dyn-cast in conditionals check, however there just isn't enough cross-over between the 2 and the names wouldn't really match up with what each part of the check does.
There could be merit in merging them both into a new check and removing the old one, however I feel that could cause disruption for downstream users.

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

 set(LLVM_LINK_COMPONENTS
   FrontendOpenMP
   Support
   )
 
 add_clang_library(clangTidyLLVMModule
+  DereferencingDynCastCheck.cpp
   HeaderGuardCheck.cpp
   IncludeOrderCheck.cpp
   LLVMTidyModule.cpp
   PreferIsaOrDynCastInConditionalsCheck.cpp
   PreferRegisterOverUnsignedCheck.cpp
   TwineLocalCheck.cpp
 
   LINK_LIBS

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

+//===--- DereferencingDynCastCheck.h - clang-tidy ---------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_DEREFERENCINGDYNCASTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_DEREFERENCINGDYNCASTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace llvm_check {
+
+/// Finds cases where the result of a dyn_cast is dereferenced. This will
+/// segfault if the dyn_cast fails.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/llvm/dereferencing-dyn-cast.html
+class DereferencingDynCastCheck : public ClangTidyCheck {
+public:
+  DereferencingDynCastCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    // Require C++14 just because llvm requires it.
+    return LangOpts.CPlusPlus14;
+  }
+  llvm::Optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+};
+
+} // namespace llvm_check
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_DEREFERENCINGDYNCASTCHECK_H

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

+//===--- DereferencingDynCastCheck.cpp - clang-tidy -----------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "DereferencingDynCastCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace llvm_check {
+
+namespace {
+AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); }
+AST_MATCHER(UnaryOperator, isDereference) {
+  return Node.getOpcode() == UO_Deref;
+}
+} // namespace
+
+static constexpr char DynCastID[] = "dyn_cast";
+static constexpr char CalleeID[] = "callee";
+static constexpr char UnaryID[] = "Unary";
+static constexpr char MemberID[] = "Member";
+
+void DereferencingDynCastCheck::registerMatchers(MatchFinder *Finder) {
+  auto DynCastCallMatcher = callExpr(
+      unless(isMacroID()), unless(cxxMemberCallExpr()), argumentCountIs(1),
+      callee(functionDecl(hasAnyName("dyn_cast", "dyn_cast_or_null",
+                                     "cast_or_null", "dyn_cast_if_present",
+                                     "cast_if_present"))
+                 .bind(DynCastID)),
+      callee(declRefExpr().bind(CalleeID)));
+  Finder->addMatcher(
+      unaryOperator(isDereference(),
+                    hasUnaryOperand(ignoringParens(DynCastCallMatcher)))
+          .bind(UnaryID),
+      this);
+  Finder->addMatcher(
+      memberExpr(hasObjectExpression(ignoringParens(DynCastCallMatcher)),
+                 isArrow())
+          .bind(MemberID),
+      this);
+}
+
+void DereferencingDynCastCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *DynCastCallee = Result.Nodes.getNodeAs<FunctionDecl>(DynCastID);
+  assert(DynCastCallee->getDeclName().isIdentifier());
+  SourceLocation ErrorLoc;
+  if (const auto *Unary = Result.Nodes.getNodeAs<UnaryOperator>(UnaryID)) {
+    ErrorLoc = Unary->getOperatorLoc();
+  } else {
+    const auto *Member = Result.Nodes.getNodeAs<MemberExpr>(MemberID);
+    assert(Member);
+    ErrorLoc = Member->getOperatorLoc();
+  }
+
+  bool EmitFix = DynCastCallee->getName() == "dyn_cast";
+  auto D = diag(ErrorLoc,
+                "dereferencing the result of '%0' will segfault if the "
+                "conversion fails%select{|; did you mean to call `cast`?}1")
+           << DynCastCallee->getName() << EmitFix;
+
+  assert(DynCastCallee->getDeclName().isIdentifier());
+
+  if (EmitFix)
+    D << FixItHint::CreateReplacement(
+        Result.Nodes.getNodeAs<DeclRefExpr>(CalleeID)->getLocation(), "cast");
+}
+
+} // namespace llvm_check
+} // namespace tidy
+} // namespace clang

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

 //===--- LLVMTidyModule.cpp - clang-tidy ----------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../readability/ElseAfterReturnCheck.h"
 #include "../readability/NamespaceCommentCheck.h"
 #include "../readability/QualifiedAutoCheck.h"
+#include "DereferencingDynCastCheck.h"
 #include "HeaderGuardCheck.h"
 #include "IncludeOrderCheck.h"
 #include "PreferIsaOrDynCastInConditionalsCheck.h"
 #include "PreferRegisterOverUnsignedCheck.h"
 #include "TwineLocalCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace llvm_check {
 
 class LLVMModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<DereferencingDynCastCheck>(
+        "llvm-dereferencing-dyn-cast");
     CheckFactories.registerCheck<readability::ElseAfterReturnCheck>(
         "llvm-else-after-return");
     CheckFactories.registerCheck<LLVMHeaderGuardCheck>("llvm-header-guard");
     CheckFactories.registerCheck<IncludeOrderCheck>("llvm-include-order");
     CheckFactories.registerCheck<readability::NamespaceCommentCheck>(
         "llvm-namespace-comment");
     CheckFactories.registerCheck<PreferIsaOrDynCastInConditionalsCheck>(
         "llvm-prefer-isa-or-dyn-cast-in-conditionals");

clang-tools-extra/docs/ReleaseNotes.rst

 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
 New checks
 ^^^^^^^^^^
 
+- New :doc:`llvm-dereferencing-dyn-cast
+  <clang-tidy/checks/llvm/dereferencing-dyn-cast>` check.
+
+  Finds cases where the result of a ``dyn_cast<>`` is dereferenced. This will 
+  segfault if the ``dyn_cast<>`` fails.
+
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Removed checks
 ^^^^^^^^^^^^^^

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `google-runtime-operator <google/runtime-operator.html>`_,
    `google-upgrade-googletest-case <google/upgrade-googletest-case.html>`_, "Yes"
    `hicpp-avoid-goto <hicpp/avoid-goto.html>`_,
    `hicpp-exception-baseclass <hicpp/exception-baseclass.html>`_,
    `hicpp-multiway-paths-covered <hicpp/multiway-paths-covered.html>`_,
    `hicpp-no-assembler <hicpp/no-assembler.html>`_,
    `hicpp-signed-bitwise <hicpp/signed-bitwise.html>`_,
    `linuxkernel-must-use-errs <linuxkernel/must-use-errs.html>`_,
+   `llvm-dereferencing-dyn-cast <llvm/dereferencing-dyn-cast.html>`_, "Yes"
    `llvm-header-guard <llvm/header-guard.html>`_,
    `llvm-include-order <llvm/include-order.html>`_, "Yes"
    `llvm-namespace-comment <llvm/namespace-comment.html>`_,
    `llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm/prefer-isa-or-dyn-cast-in-conditionals.html>`_, "Yes"
    `llvm-prefer-register-over-unsigned <llvm/prefer-register-over-unsigned.html>`_, "Yes"
    `llvm-twine-local <llvm/twine-local.html>`_, "Yes"
    `llvmlibc-callee-namespace <llvmlibc/callee-namespace.html>`_,
    `llvmlibc-implementation-in-namespace <llvmlibc/implementation-in-namespace.html>`_,

clang-tools-extra/docs/clang-tidy/checks/llvm/dereferencing-dyn-cast.rst

+.. title:: clang-tidy - llvm-dereferencing-dyn-cast
+
+llvm-dereferencing-dyn-cast
+===========================
+
+Finds cases where the result of a ``dyn_cast<>`` is dereferenced. This will 
+segfault if the ``dyn_cast<>`` fails.
+
+If the call was to ``dyn_cast``, It will suggest replacing with ``cast``, For 
+the cases when the call is for r`(dyn_)?cast_(or_null|if_present)` no fix will
+be suggested.
+
+.. code-block::c++
+
+  auto X = dyn_cast<Foo>(Bar)->getX();
+  auto &FRef = *dyn_cast<Foo>(Bar);
+  // Replaced with.
+  auto X = cast<Foo>(Bar)->getX();
+  auto &FRef = *cast<Foo>(Bar);
+
+  // Warn with no replacement.
+  auto X = dyn_cast_or_null<Foo>(Bar)->getX();
+  auto X = dyn_cast_if_present<Foo>(Bar)->getX();
+  auto X = cast_or_null<Foo>(Bar)->getX();
+  auto X = cast_if_present<Foo>(Bar)->getX();

clang-tools-extra/test/clang-tidy/checkers/llvm/dereferencing-dyn-cast.cpp

+// RUN: %check_clang_tidy %s -std=c++14-or-later llvm-dereferencing-dyn-cast %t
+
+namespace llvm {
+template <class X, class Y> X *cast(Y *);
+template <class X, class Y> X *dyn_cast(Y *);
+template <class X, class Y> X *dyn_cast_or_null(Y *);
+template <class X, class Y> X *dyn_cast_if_present(Y *);
+template <class X, class Y> X *cast_or_null(Y *);
+template <class X, class Y> X *cast_if_present(Y *);
+} // namespace llvm
+
+using namespace llvm;
+
+struct Baz;
+struct Foo {
+  int getX() const;
+};
+
+void test(Baz *Bar) {
+  // CHECK-MESSAGES: :[[@LINE+3]]:29: warning: dereferencing the result of 'dyn_cast' will segfault if the conversion fails; did you mean to call `cast`? [llvm-dereferencing-dyn-cast]
+  // CHECK-MESSAGES: :[[@LINE+3]]:15: warning: dereferencing the result of 'dyn_cast' will segfault if the conversion fails; did you mean to call `cast`? [llvm-dereferencing-dyn-cast]
+  // CHECK-MESSAGES: :[[@LINE+3]]:31: warning: dereferencing the result of 'dyn_cast' will segfault if the conversion fails; did you mean to call `cast`? [llvm-dereferencing-dyn-cast]
+  int X = dyn_cast<Foo>(Bar)->getX();
+  Foo &FRef = *dyn_cast<Foo>(Bar);
+  X = llvm::dyn_cast<Foo>(Bar)->getX();
+  //      CHECK-FIXES: int X = cast<Foo>(Bar)->getX();
+  // CHECK-FIXES-NEXT: Foo &FRef = *cast<Foo>(Bar);
+  // CHECK-FIXES-NEXT: X = llvm::cast<Foo>(Bar)->getX();
+
+  // Warn with no replacement.
+  // CHECK-MESSAGES: :[[@LINE+4]]:33: warning: dereferencing the result of 'dyn_cast_or_null' will segfault if the conversion fails [llvm-dereferencing-dyn-cast]
+  // CHECK-MESSAGES: :[[@LINE+4]]:36: warning: dereferencing the result of 'dyn_cast_if_present' will segfault if the conversion fails [llvm-dereferencing-dyn-cast]
+  // CHECK-MESSAGES: :[[@LINE+4]]:29: warning: dereferencing the result of 'cast_or_null' will segfault if the conversion fails [llvm-dereferencing-dyn-cast]
+  // CHECK-MESSAGES: :[[@LINE+4]]:32: warning: dereferencing the result of 'cast_if_present' will segfault if the conversion fails [llvm-dereferencing-dyn-cast]
+  X = dyn_cast_or_null<Foo>(Bar)->getX();
+  X = dyn_cast_if_present<Foo>(Bar)->getX();
+  X = cast_or_null<Foo>(Bar)->getX();
+  X = cast_if_present<Foo>(Bar)->getX();
+}