diff --git a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt @@ -4,6 +4,7 @@ ) add_clang_library(clangTidyLLVMModule + DereferencingDynCastCheck.cpp HeaderGuardCheck.cpp IncludeOrderCheck.cpp LLVMTidyModule.cpp diff --git a/clang-tools-extra/clang-tidy/llvm/DereferencingDynCastCheck.h b/clang-tools-extra/clang-tidy/llvm/DereferencingDynCastCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/DereferencingDynCastCheck.h @@ -0,0 +1,42 @@ +//===--- 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 getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } +}; + +} // namespace llvm_check +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_DEREFERENCINGDYNCASTCHECK_H diff --git a/clang-tools-extra/clang-tidy/llvm/DereferencingDynCastCheck.cpp b/clang-tools-extra/clang-tidy/llvm/DereferencingDynCastCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/DereferencingDynCastCheck.cpp @@ -0,0 +1,78 @@ +//===--- 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(DynCastID); + assert(DynCastCallee->getDeclName().isIdentifier()); + SourceLocation ErrorLoc; + if (const auto *Unary = Result.Nodes.getNodeAs(UnaryID)) { + ErrorLoc = Unary->getOperatorLoc(); + } else { + const auto *Member = Result.Nodes.getNodeAs(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(CalleeID)->getLocation(), "cast"); +} + +} // namespace llvm_check +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp --- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp @@ -12,6 +12,7 @@ #include "../readability/ElseAfterReturnCheck.h" #include "../readability/NamespaceCommentCheck.h" #include "../readability/QualifiedAutoCheck.h" +#include "DereferencingDynCastCheck.h" #include "HeaderGuardCheck.h" #include "IncludeOrderCheck.h" #include "PreferIsaOrDynCastInConditionalsCheck.h" @@ -25,6 +26,8 @@ class LLVMModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "llvm-dereferencing-dyn-cast"); CheckFactories.registerCheck( "llvm-else-after-return"); CheckFactories.registerCheck("llvm-header-guard"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -99,6 +99,12 @@ New checks ^^^^^^^^^^ +- New :doc:`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 ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -229,6 +229,7 @@ `hicpp-no-assembler `_, `hicpp-signed-bitwise `_, `linuxkernel-must-use-errs `_, + `llvm-dereferencing-dyn-cast `_, "Yes" `llvm-header-guard `_, `llvm-include-order `_, "Yes" `llvm-namespace-comment `_, diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/dereferencing-dyn-cast.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/dereferencing-dyn-cast.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/dereferencing-dyn-cast.rst @@ -0,0 +1,25 @@ +.. 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(Bar)->getX(); + auto &FRef = *dyn_cast(Bar); + // Replaced with. + auto X = cast(Bar)->getX(); + auto &FRef = *cast(Bar); + + // Warn with no replacement. + auto X = dyn_cast_or_null(Bar)->getX(); + auto X = dyn_cast_if_present(Bar)->getX(); + auto X = cast_or_null(Bar)->getX(); + auto X = cast_if_present(Bar)->getX(); diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/dereferencing-dyn-cast.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/dereferencing-dyn-cast.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/dereferencing-dyn-cast.cpp @@ -0,0 +1,39 @@ +// RUN: %check_clang_tidy %s -std=c++14-or-later llvm-dereferencing-dyn-cast %t + +namespace llvm { +template X *cast(Y *); +template X *dyn_cast(Y *); +template X *dyn_cast_or_null(Y *); +template X *dyn_cast_if_present(Y *); +template X *cast_or_null(Y *); +template 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(Bar)->getX(); + Foo &FRef = *dyn_cast(Bar); + X = llvm::dyn_cast(Bar)->getX(); + // CHECK-FIXES: int X = cast(Bar)->getX(); + // CHECK-FIXES-NEXT: Foo &FRef = *cast(Bar); + // CHECK-FIXES-NEXT: X = llvm::cast(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(Bar)->getX(); + X = dyn_cast_if_present(Bar)->getX(); + X = cast_or_null(Bar)->getX(); + X = cast_if_present(Bar)->getX(); +}