[clang-tools-extra] r249899 - Add a new checker that tests whether a throw expression throws by value, and whether a catch statement catches by reference.
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 9 13:42:44 PDT 2015
Author: aaronballman
Date: Fri Oct 9 15:42:44 2015
New Revision: 249899
URL: http://llvm.org/viewvc/llvm-project?rev=249899&view=rev
Log:
Add a new checker that tests whether a throw expression throws by value, and whether a catch statement catches by reference.
Patch by Tobias Langner!
Added:
clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
clang-tools-extra/trunk/test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=249899&r1=249898&r2=249899&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Fri Oct 9 15:42:44 2015
@@ -17,6 +17,7 @@ add_clang_library(clangTidyMiscModule
SizeofContainerCheck.cpp
StaticAssertCheck.cpp
SwappedArgumentsCheck.cpp
+ ThrowByValueCatchByReferenceCheck.cpp
UndelegatedConstructor.cpp
UnusedAliasDeclsCheck.cpp
UnusedParametersCheck.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=249899&r1=249898&r2=249899&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Fri Oct 9 15:42:44 2015
@@ -25,6 +25,7 @@
#include "SizeofContainerCheck.h"
#include "StaticAssertCheck.h"
#include "SwappedArgumentsCheck.h"
+#include "ThrowByValueCatchByReferenceCheck.h"
#include "UndelegatedConstructor.h"
#include "UniqueptrResetReleaseCheck.h"
#include "UnusedAliasDeclsCheck.h"
@@ -66,6 +67,8 @@ public:
"misc-static-assert");
CheckFactories.registerCheck<SwappedArgumentsCheck>(
"misc-swapped-arguments");
+ CheckFactories.registerCheck<ThrowByValueCatchByReferenceCheck>(
+ "misc-throw-by-value-catch-by-reference");
CheckFactories.registerCheck<UndelegatedConstructorCheck>(
"misc-undelegated-constructor");
CheckFactories.registerCheck<UniqueptrResetReleaseCheck>(
Added: clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp?rev=249899&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp Fri Oct 9 15:42:44 2015
@@ -0,0 +1,159 @@
+//===--- ThrowByValueCatchByReferenceCheck.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 "ThrowByValueCatchByReferenceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/AST/OperationKinds.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+ThrowByValueCatchByReferenceCheck::ThrowByValueCatchByReferenceCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ CheckAnonymousTemporaries(Options.get("CheckThrowTemporaries", true)) {}
+
+void ThrowByValueCatchByReferenceCheck::registerMatchers(MatchFinder *Finder) {
+ // This is a C++ only check thus we register the matchers only for C++
+ if (!getLangOpts().CPlusPlus)
+ return;
+
+ Finder->addMatcher(cxxThrowExpr().bind("throw"), this);
+ Finder->addMatcher(cxxCatchStmt().bind("catch"), this);
+}
+
+void ThrowByValueCatchByReferenceCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "CheckThrowTemporaries", true);
+}
+
+void ThrowByValueCatchByReferenceCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ diagnoseThrowLocations(Result.Nodes.getNodeAs<CXXThrowExpr>("throw"));
+ diagnoseCatchLocations(Result.Nodes.getNodeAs<CXXCatchStmt>("catch"),
+ *Result.Context);
+}
+
+bool ThrowByValueCatchByReferenceCheck::isFunctionParameter(
+ const DeclRefExpr *declRefExpr) {
+ return isa<ParmVarDecl>(declRefExpr->getDecl());
+}
+
+bool ThrowByValueCatchByReferenceCheck::isCatchVariable(
+ const DeclRefExpr *declRefExpr) {
+ auto *valueDecl = declRefExpr->getDecl();
+ if (auto *varDecl = dyn_cast<VarDecl>(valueDecl))
+ return varDecl->isExceptionVariable();
+ return false;
+}
+
+bool ThrowByValueCatchByReferenceCheck::isFunctionOrCatchVar(
+ const DeclRefExpr *declRefExpr) {
+ return isFunctionParameter(declRefExpr) || isCatchVariable(declRefExpr);
+}
+
+void ThrowByValueCatchByReferenceCheck::diagnoseThrowLocations(
+ const CXXThrowExpr *throwExpr) {
+ if (!throwExpr)
+ return;
+ auto *subExpr = throwExpr->getSubExpr();
+ if (!subExpr)
+ return;
+ auto qualType = subExpr->getType();
+ if (qualType->isPointerType()) {
+ // The code is throwing a pointer.
+ // In case it is strng literal, it is safe and we return.
+ auto *inner = subExpr->IgnoreParenImpCasts();
+ if (isa<StringLiteral>(inner))
+ return;
+ // If it's a variable from a catch statement, we return as well.
+ auto *declRef = dyn_cast<DeclRefExpr>(inner);
+ if (declRef && isCatchVariable(declRef)) {
+ return;
+ }
+ diag(subExpr->getLocStart(), "throw expression throws a pointer; it should "
+ "throw a non-pointer value instead");
+ }
+ // If the throw statement does not throw by pointer then it throws by value
+ // which is ok.
+ // There are addition checks that emit diagnosis messages if the thrown value
+ // is not an RValue. See:
+ // https://www.securecoding.cert.org/confluence/display/cplusplus/ERR09-CPP.+Throw+anonymous+temporaries
+ // This behavior can be influenced by an option.
+
+ // If we encounter a CXXThrowExpr, we move through all casts until you either
+ // encounter a DeclRefExpr or a CXXConstructExpr.
+ // If it's a DeclRefExpr, we emit a message if the referenced variable is not
+ // a catch variable or function parameter.
+ // When encountering a CopyOrMoveConstructor: emit message if after casts,
+ // the expression is a LValue
+ if (CheckAnonymousTemporaries) {
+ bool emit = false;
+ auto *currentSubExpr = subExpr->IgnoreImpCasts();
+ const DeclRefExpr *variableReference =
+ dyn_cast<DeclRefExpr>(currentSubExpr);
+ const CXXConstructExpr *constructorCall =
+ dyn_cast<CXXConstructExpr>(currentSubExpr);
+ // If we have a DeclRefExpr, we flag for emitting a diagnosis message in
+ // case the referenced variable is neither a function parameter nor a
+ // variable declared in the catch statement.
+ if (variableReference)
+ emit = !isFunctionOrCatchVar(variableReference);
+ else if (constructorCall &&
+ constructorCall->getConstructor()->isCopyOrMoveConstructor()) {
+ // If we have a copy / move construction, we emit a diagnosis message if
+ // the object that we copy construct from is neither a function parameter
+ // nor a variable declared in a catch statement
+ auto argIter =
+ constructorCall
+ ->arg_begin(); // there's only one for copy constructors
+ auto *currentSubExpr = (*argIter)->IgnoreImpCasts();
+ if (currentSubExpr->isLValue()) {
+ if (auto *tmp = dyn_cast<DeclRefExpr>(currentSubExpr))
+ emit = !isFunctionOrCatchVar(tmp);
+ else if (isa<CallExpr>(currentSubExpr))
+ emit = true;
+ }
+ }
+ if (emit)
+ diag(subExpr->getLocStart(),
+ "throw expression should throw anonymous temporary values instead");
+ }
+}
+
+void ThrowByValueCatchByReferenceCheck::diagnoseCatchLocations(
+ const CXXCatchStmt *catchStmt, ASTContext &context) {
+ const char *diagMsgCatchReference = "catch handler catches a pointer value; "
+ "should throw a non-pointer value and "
+ "catch by reference instead";
+ if (!catchStmt)
+ return;
+ auto caughtType = catchStmt->getCaughtType();
+ if (caughtType.isNull())
+ return;
+ auto *varDecl = catchStmt->getExceptionDecl();
+ if (const auto *PT = caughtType.getCanonicalType()->getAs<PointerType>()) {
+ // We do not diagnose when catching pointer to strings since we also allow
+ // throwing string literals.
+ if (!PT->getPointeeType()->isAnyCharacterType())
+ diag(varDecl->getLocStart(), diagMsgCatchReference);
+ } else if (!caughtType->isReferenceType()) {
+ // If it's not a pointer and not a reference then it must be thrown "by
+ // value". In this case we should emit a diagnosis message unless the type
+ // is trivial.
+ if (!caughtType.isTrivialType(context))
+ diag(varDecl->getLocStart(), diagMsgCatchReference);
+ }
+}
+
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h?rev=249899&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h Fri Oct 9 15:42:44 2015
@@ -0,0 +1,49 @@
+//===--- ThrowByValueCatchByReferenceCheck.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_THROW_BY_VALUE_CATCH_BY_REFERENCE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_THROW_BY_VALUE_CATCH_BY_REFERENCE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+///\brief checks for locations that do not throw by value
+// or catch by reference.
+// The check is C++ only. It checks that all throw locations
+// throw by value and not by pointer. Additionally it
+// contains an option ("CheckThrowTemporaries", default value "true") that
+// checks that thrown objects are anonymous temporaries. It is also
+// acceptable for this check to throw string literals.
+// This test checks that exceptions are caught by reference
+// and not by value or pointer. It will not warn when catching
+// pointer to char, wchar_t, char16_t or char32_t. This is
+// due to not warning on throwing string literals.
+class ThrowByValueCatchByReferenceCheck : public ClangTidyCheck {
+public:
+ ThrowByValueCatchByReferenceCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ void diagnoseThrowLocations(const CXXThrowExpr *throwExpr);
+ void diagnoseCatchLocations(const CXXCatchStmt *catchStmt,
+ ASTContext &context);
+ bool isFunctionParameter(const DeclRefExpr *declRefExpr);
+ bool isCatchVariable(const DeclRefExpr *declRefExpr);
+ bool isFunctionOrCatchVar(const DeclRefExpr *declRefExpr);
+ const bool CheckAnonymousTemporaries;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_THROW_BY_VALUE_CATCH_BY_REFERENCE_H
Added: clang-tools-extra/trunk/test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp?rev=249899&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-throw-by-value-catch-by-reference.cpp Fri Oct 9 15:42:44 2015
@@ -0,0 +1,156 @@
+// RUN: %python %S/check_clang_tidy.py %s misc-throw-by-value-catch-by-reference %t
+
+
+class logic_error {
+public:
+ logic_error(const char *message) {}
+};
+
+typedef logic_error *logic_ptr;
+typedef logic_ptr logic_double_typedef;
+
+int lastException;
+
+template <class T> struct remove_reference { typedef T type; };
+template <class T> struct remove_reference<T &> { typedef T type; };
+template <class T> struct remove_reference<T &&> { typedef T type; };
+
+template <typename T> typename remove_reference<T>::type &&move(T &&arg) {
+ return static_cast<typename remove_reference<T>::type &&>(arg);
+}
+
+logic_error CreateException() { return logic_error("created"); }
+
+void testThrowFunc() {
+ throw new logic_error("by pointer");
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression throws a pointer; it should throw a non-pointer value instead [misc-throw-by-value-catch-by-reference]
+ logic_ptr tmp = new logic_error("by pointer");
+ throw tmp;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+ // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: throw expression throws a pointer; it should throw a non-pointer value instead [misc-throw-by-value-catch-by-reference]
+ throw logic_error("by value");
+ auto *literal = "test";
+ throw logic_error(literal);
+ throw "test string literal";
+ throw L"throw wide string literal";
+ const char *characters = 0;
+ throw characters;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+ // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: throw expression throws a pointer; it should throw a non-pointer value instead [misc-throw-by-value-catch-by-reference]
+ logic_error lvalue("lvalue");
+ throw lvalue;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+
+ throw move(lvalue);
+ int &ex = lastException;
+ throw ex;
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+ throw CreateException();
+}
+
+void throwReferenceFunc(logic_error &ref) { throw ref; }
+
+void catchByPointer() {
+ try {
+ testThrowFunc();
+ } catch (logic_error *e) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference]
+ }
+}
+
+void catchByValue() {
+ try {
+ testThrowFunc();
+ } catch (logic_error e) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference]
+ }
+}
+
+void catchByReference() {
+ try {
+ testThrowFunc();
+ } catch (logic_error &e) {
+ }
+}
+
+void catchByConstReference() {
+ try {
+ testThrowFunc();
+ } catch (const logic_error &e) {
+ }
+}
+
+void catchTypedef() {
+ try {
+ testThrowFunc();
+ } catch (logic_ptr) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference]
+ }
+}
+
+void catchAll() {
+ try {
+ testThrowFunc();
+ } catch (...) {
+ }
+}
+
+void catchLiteral() {
+ try {
+ testThrowFunc();
+ } catch (const char *) {
+ } catch (const wchar_t *) {
+ // disabled for now until it is clear
+ // how to enable them in the test
+ //} catch (const char16_t*) {
+ //} catch (const char32_t*) {
+ }
+}
+
+// catching fundamentals should not warn
+void catchFundamental() {
+ try {
+ testThrowFunc();
+ } catch (int) {
+ } catch (double) {
+ } catch (unsigned long) {
+ }
+}
+
+struct TrivialType {
+ double x;
+ double y;
+};
+
+void catchTrivial() {
+ try {
+ testThrowFunc();
+ } catch (TrivialType) {
+ }
+}
+
+typedef logic_error &fine;
+void additionalTests() {
+ try {
+ } catch (int i) { // ok
+ throw i; // ok
+ } catch (fine e) { // ok
+ throw e; // ok
+ } catch (logic_error *e) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches a pointer value; should throw a non-pointer value and catch by reference instead [misc-throw-by-value-catch-by-reference]
+ throw e; // ok, despite throwing a pointer
+ } catch (...) { // ok
+ throw; // ok
+ }
+}
+
+struct S {};
+
+S &returnByReference();
+S returnByValue();
+
+void f() {
+ throw returnByReference(); // Should diagnose
+ // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: throw expression should throw anonymous temporary values instead [misc-throw-by-value-catch-by-reference]
+ throw returnByValue(); // Should not diagnose
+}
More information about the cfe-commits
mailing list