aaron.ballman created this revision.
aaron.ballman added reviewers: alexfh, rsmith.
aaron.ballman added a subscriber: cfe-commits.
Per [except.handle]p10, the handler for a constructor or destructor
function-try-block cannot refer to a non-static member of the object under
construction. This patch adds a new clang-tidy check that warns the user when
they've hit this undefined behavior.
Due to how infrequent function-try-blocks appear on constructors and
destructors in the wild compared to how often member expressions are
encountered, I felt this was more appropriate as a clang-tidy check than as a
semantic warning. I was concerned with efficiency of checking whether an
arbitrary member expression was referring to the object under
construction/destruction within the function-try-block catch handler scope.
This patch corresponds to the CERT secure coding rule ERR53-CPP
(https://www.securecoding.cert.org/confluence/display/cplusplus/ERR53-CPP.+Do+not+reference+base+classes+or+class+data+members+in+a+constructor+or+destructor+function-try-block+handler)
http://reviews.llvm.org/D12301
Files:
clang-tidy/misc/CDtorCatchHandlerCheck.cpp
clang-tidy/misc/CDtorCatchHandlerCheck.h
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
test/clang-tidy/misc-cdtor-catch-handler.cpp
Index: test/clang-tidy/misc-cdtor-catch-handler.cpp
===================================================================
--- test/clang-tidy/misc-cdtor-catch-handler.cpp
+++ test/clang-tidy/misc-cdtor-catch-handler.cpp
@@ -0,0 +1,92 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-cdtor-catch-handler %t -- -fcxx-exceptions -std=c++14
+
+int FileScope;
+
+struct A {
+ int I;
+ void f();
+ A() try {
+ } catch (...) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: cannot refer to a non-static member from the handler of a constructor function-try-block
+ I = 12;
+ // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: cannot refer to a non-static member from the handler of a constructor function-try-block
+ f();
+
+ FileScope = 12; // ok
+ A a;
+ a.I = 12; // ok
+ }
+};
+
+struct B {
+ int I;
+ void f();
+};
+
+struct C : B {
+ C() try {
+ } catch (...) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: cannot refer to a non-static member from the handler of a constructor function-try-block
+ I = 12;
+ // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: cannot refer to a non-static member from the handler of a constructor function-try-block
+ f();
+ }
+};
+
+struct D {
+ static int I;
+ static void f();
+
+ D() try {
+ } catch (...) {
+ I = 12; // ok
+ f(); // ok
+ }
+};
+int D::I;
+
+struct E {
+ int I;
+ void f();
+ static int J;
+ static void g();
+
+ ~E() try {
+ } catch (...) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: cannot refer to a non-static member from the handler of a destructor function-try-block
+ I = 12;
+ // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: cannot refer to a non-static member from the handler of a destructor function-try-block
+ f();
+
+ J = 12; // ok
+ g(); // ok
+ }
+};
+int E::J;
+
+struct F {
+ static int I;
+ static void f();
+};
+int F::I;
+
+struct G : F {
+ G() try {
+ } catch (...) {
+ I = 12; // ok
+ f(); // ok
+ }
+};
+
+struct H {
+ struct A {};
+ enum {
+ E
+ };
+
+ H() try {
+ } catch (...) {
+ H::A a; // ok
+ int I = E; // ok
+ }
+};
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -14,6 +14,7 @@
#include "AssertSideEffectCheck.h"
#include "AssignOperatorSignatureCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
+#include "CDtorCatchHandlerCheck.h"
#include "InaccurateEraseCheck.h"
#include "InefficientAlgorithmCheck.h"
#include "MacroParenthesesCheck.h"
@@ -43,6 +44,8 @@
"misc-assign-operator-signature");
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
"misc-bool-pointer-implicit-conversion");
+ CheckFactories.registerCheck<CDtorCatchHandlerCheck>(
+ "misc-cdtor-catch-handler");
CheckFactories.registerCheck<InaccurateEraseCheck>(
"misc-inaccurate-erase");
CheckFactories.registerCheck<InefficientAlgorithmCheck>(
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -5,6 +5,7 @@
AssertSideEffectCheck.cpp
AssignOperatorSignatureCheck.cpp
BoolPointerImplicitConversionCheck.cpp
+ CDtorCatchHandlerCheck.cpp
InaccurateEraseCheck.cpp
InefficientAlgorithmCheck.cpp
MacroParenthesesCheck.cpp
Index: clang-tidy/misc/CDtorCatchHandlerCheck.h
===================================================================
--- clang-tidy/misc/CDtorCatchHandlerCheck.h
+++ clang-tidy/misc/CDtorCatchHandlerCheck.h
@@ -0,0 +1,32 @@
+//===--- CDtorCatchHandlerCheck.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_MISC_CDTORCATCHHANDLERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_CDTORCATCHHANDLERCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief The check flags user-defined move constructors that have a
+/// ctor-initializer initializing a member or base class through a copy
+/// constructor instead of a move constructor.
+class CDtorCatchHandlerCheck : public ClangTidyCheck {
+public:
+ CDtorCatchHandlerCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ 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_MISC_CDTORCATCHHANDLERCHECK_H
Index: clang-tidy/misc/CDtorCatchHandlerCheck.cpp
===================================================================
--- clang-tidy/misc/CDtorCatchHandlerCheck.cpp
+++ clang-tidy/misc/CDtorCatchHandlerCheck.cpp
@@ -0,0 +1,42 @@
+//===--- CDtorCatchHandlerCheck.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 "CDtorCatchHandlerCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+void CDtorCatchHandlerCheck::registerMatchers(MatchFinder *Finder) {
+ auto parentIsCDtorTryBlock = hasParent(
+ tryStmt(hasParent(functionDecl(anyOf(constructorDecl(), destructorDecl()))
+ .bind("function"))));
+ Finder->addMatcher(
+ catchStmt(parentIsCDtorTryBlock,
+ forEachDescendant(memberExpr(hasObjectExpression(
+ ignoringImpCasts(thisExpr())))
+ .bind("expr"))),
+ this);
+}
+
+void CDtorCatchHandlerCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *F = Result.Nodes.getNodeAs<FunctionDecl>("function");
+ const auto *ME = Result.Nodes.getNodeAs<MemberExpr>("expr");
+
+ diag(ME->getExprLoc(), "cannot refer to a non-static member from the handler "
+ "of a %0 function-try-block")
+ << (isa<CXXConstructorDecl>(F) ? "constructor" : "destructor");
+}
+
+} // namespace tidy
+} // namespace clang
+
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
