baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: alexfh, aaron.ballman, lebedev.ri.
baloghadamsoftware added a project: clang-tools-extra.
Herald added subscribers: gamesh411, Szelethus, rnkovacs.
Herald added a project: clang.
Catching trivial objects by value is not dangerous but may be inefficient if
they are too large. This patch adds an option `WarnOnLargeObject` to the
checker to also warn if such an object is caught by value. An object is
considered as "large" if its size is greater than `MaxSize` which is another
option. Default value is the machine word of the architecture (size of the type
`size_t`).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D61851
Files:
clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
test/clang-tidy/misc-throw-by-value-catch-by-reference-warn-on-large-object.cpp
Index: test/clang-tidy/misc-throw-by-value-catch-by-reference-warn-on-large-object.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-throw-by-value-catch-by-reference-warn-on-large-object.cpp
@@ -0,0 +1,36 @@
+// RUN: %check_clang_tidy %s misc-throw-by-value-catch-by-reference %t -- -config="{CheckOptions: [{key: misc-throw-by-value-catch-by-reference.WarnOnLargeObject, value: 1}]}" -- -std=c++11 -fcxx-exceptions
+
+struct SmallType {
+ long n;
+};
+
+struct LargeType {
+ long v[255];
+};
+
+void testThrowFunc() {
+ throw SmallType();
+ throw LargeType();
+}
+
+void catchSmall() {
+ try {
+ testThrowFunc();
+ } catch (SmallType s) {
+ }
+}
+
+void catchLarge() {
+ try {
+ testThrowFunc();
+ } catch (LargeType l) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: catch handler catches by value; should catch by reference instead [misc-throw-by-value-catch-by-reference]
+ }
+}
+
+void catchByReference() {
+ try {
+ testThrowFunc();
+ } catch (LargeType &l) {
+ }
+}
Index: docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
===================================================================
--- docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
+++ docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
@@ -32,3 +32,15 @@
<https://www.securecoding.cert.org/confluence/display/cplusplus/ERR09-CPP.+Throw+anonymous+temporaries>`_.
Default is `1`.
+.. option:: WarnOnLargeObject
+
+ Also warns for any large trivial object caught by value. Catching a large
+ object by value is not dangerous but affects the perofrmance negatively. The
+ maximum size of an object allowed to be caught without warning can be set
+ using option `MaxSize`
+ Default is `0`.
+
+.. option:: MaxSize
+
+ Determines the maximum size of an object allowed to be caught without
+ warning. Only applicable if `WarnOnLargeObject` is set to `1`.
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -179,6 +179,11 @@
but either don't specify it or the clause is specified but with the kind
other than ``none``, and suggests to use the ``default(none)`` clause.
+- New options `WarnOnLargeObject` and `MaxSize` for check
+ :doc:`misc-throw-by-value-catch-by-reference
+ <clang-tidy/misc-throw-by-value-catch-by-reference.rst>` check to warn
+ on any large trivial object caught by value.
+
Improvements to clang-include-fixer
-----------------------------------
Index: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
===================================================================
--- clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
+++ clang-tidy/misc/ThrowByValueCatchByReferenceCheck.h
@@ -41,6 +41,8 @@
bool isCatchVariable(const DeclRefExpr *declRefExpr);
bool isFunctionOrCatchVar(const DeclRefExpr *declRefExpr);
const bool CheckAnonymousTemporaries;
+ const bool WarnOnLargeObject;
+ uint64_t MaxSize; // No `const` bacause we have to set it in two steps.
};
} // namespace misc
Index: clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
===================================================================
--- clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
+++ clang-tidy/misc/ThrowByValueCatchByReferenceCheck.cpp
@@ -20,7 +20,10 @@
ThrowByValueCatchByReferenceCheck::ThrowByValueCatchByReferenceCheck(
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- CheckAnonymousTemporaries(Options.get("CheckThrowTemporaries", true)) {}
+ CheckAnonymousTemporaries(Options.get("CheckThrowTemporaries", true)),
+ WarnOnLargeObject(Options.get("WarnOnLargeObject", false)),
+ // Cannot access `ASTContext` from here so set it to an extremal value
+ MaxSize(Options.get("MaxSize", std::numeric_limits<uint64_t>::max())) {}
void ThrowByValueCatchByReferenceCheck::registerMatchers(MatchFinder *Finder) {
// This is a C++ only check thus we register the matchers only for C++
@@ -150,8 +153,19 @@
// If it's not a pointer and not a reference then it must be caught "by
// value". In this case we should emit a diagnosis message unless the type
// is trivial.
- if (!caughtType.isTrivialType(context))
+ if (!caughtType.isTrivialType(context)) {
diag(varDecl->getBeginLoc(), diagMsgCatchReference);
+ } else if (WarnOnLargeObject) {
+ // If the type is trivial, then catching it by reference is not dangerous.
+ // However, catching large objects by value decreases the performance.
+
+ // We can now access `ASTContext` so if `MaxSize` is an extremal value
+ // then set it to the size of `size_t`.
+ if (MaxSize == std::numeric_limits<uint64_t>::max())
+ MaxSize = context.getTypeSize(context.getSizeType());
+ if (context.getTypeSize(caughtType) > MaxSize)
+ diag(varDecl->getBeginLoc(), diagMsgCatchReference);
+ }
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
