njames93 created this revision.
njames93 added reviewers: aaron.ballman, JonasToth, LegalizeAdulthood, alexfh.
Herald added subscribers: carlosgalvezp, xazax.hun, mgorny.
Herald added a project: All.
njames93 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Add a check that aims to prevent dereferencing casts that may return nullptr.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D131318
Files:
clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
clang-tools-extra/clang-tidy/llvm/DereferencingDynCastCheck.cpp
clang-tools-extra/clang-tidy/llvm/DereferencingDynCastCheck.h
clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/llvm/dereferencing-dyn-cast.rst
clang-tools-extra/test/clang-tidy/checkers/llvm/dereferencing-dyn-cast.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/llvm/dereferencing-dyn-cast.cpp
===================================================================
--- /dev/null
+++ 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 <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();
+}
Index: clang-tools-extra/docs/clang-tidy/checks/llvm/dereferencing-dyn-cast.rst
===================================================================
--- /dev/null
+++ 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<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();
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -229,6 +229,7 @@
`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>`_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -99,6 +99,12 @@
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
^^^^^^^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ 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<DereferencingDynCastCheck>(
+ "llvm-dereferencing-dyn-cast");
CheckFactories.registerCheck<readability::ElseAfterReturnCheck>(
"llvm-else-after-return");
CheckFactories.registerCheck<LLVMHeaderGuardCheck>("llvm-header-guard");
Index: clang-tools-extra/clang-tidy/llvm/DereferencingDynCastCheck.h
===================================================================
--- /dev/null
+++ 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<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
+};
+
+} // namespace llvm_check
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_DEREFERENCINGDYNCASTCHECK_H
Index: clang-tools-extra/clang-tidy/llvm/DereferencingDynCastCheck.cpp
===================================================================
--- /dev/null
+++ 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<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
Index: clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
@@ -4,6 +4,7 @@
)
add_clang_library(clangTidyLLVMModule
+ DereferencingDynCastCheck.cpp
HeaderGuardCheck.cpp
IncludeOrderCheck.cpp
LLVMTidyModule.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
