isuckatcs updated this revision to Diff 489626.
isuckatcs marked 7 inline comments as done.
isuckatcs added a comment.
Addressed comments
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135495/new/
https://reviews.llvm.org/D135495
Files:
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -101,6 +101,126 @@
}
}
+void throw_catch_pointer_c() noexcept {
+ try {
+ int a = 1;
+ throw &a;
+ } catch(const int *) {}
+}
+
+void throw_catch_pointer_v() noexcept {
+ try {
+ int a = 1;
+ throw &a;
+ } catch(volatile int *) {}
+}
+
+void throw_catch_pointer_cv() noexcept {
+ try {
+ int a = 1;
+ throw &a;
+ } catch(const volatile int *) {}
+}
+
+void throw_catch_multi_ptr_1() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_catch_multi_ptr_1' which should not throw exceptions
+ try {
+ char **p = 0;
+ throw p;
+ } catch (const char **) {
+ }
+}
+
+void throw_catch_multi_ptr_2() noexcept {
+ try {
+ char **p = 0;
+ throw p;
+ } catch (const char *const *) {
+ }
+}
+
+void throw_catch_multi_ptr_3() noexcept {
+ try {
+ char **p = 0;
+ throw p;
+ } catch (volatile char *const *) {
+ }
+}
+
+void throw_catch_multi_ptr_4() noexcept {
+ try {
+ char **p = 0;
+ throw p;
+ } catch (volatile const char *const *) {
+ }
+}
+
+// FIXME: In this case 'a' is convertible to the handler and should be caught
+// but in reality it's thrown. Note that clang doesn't report a warning for
+// this either.
+void throw_catch_multi_ptr_5() noexcept {
+ try {
+ double *a[2][3];
+ throw a;
+ } catch (double *(*)[3]) {
+ }
+}
+
+
+void throw_c_catch_pointer() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_c_catch_pointer' which should not throw exceptions
+ try {
+ int a = 1;
+ const int *p = &a;
+ throw p;
+ } catch(int *) {}
+}
+
+void throw_c_catch_pointer_v() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_c_catch_pointer_v' which should not throw exceptions
+ try {
+ int a = 1;
+ const int *p = &a;
+ throw p;
+ } catch(volatile int *) {}
+}
+
+void throw_v_catch_pointer() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_v_catch_pointer' which should not throw exceptions
+ try {
+ int a = 1;
+ volatile int *p = &a;
+ throw p;
+ } catch(int *) {}
+}
+
+void throw_v_catch_pointer_c() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_v_catch_pointer_c' which should not throw exceptions
+ try {
+ int a = 1;
+ volatile int *p = &a;
+ throw p;
+ } catch(const int *) {}
+}
+
+void throw_cv_catch_pointer_c() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_cv_catch_pointer_c' which should not throw exceptions
+ try {
+ int a = 1;
+ const volatile int *p = &a;
+ throw p;
+ } catch(const int *) {}
+}
+
+void throw_cv_catch_pointer_v() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_cv_catch_pointer_v' which should not throw exceptions
+ try {
+ int a = 1;
+ const volatile int *p = &a;
+ throw p;
+ } catch(volatile int *) {}
+}
+
class base {};
class derived: public base {};
@@ -112,6 +232,24 @@
}
}
+void throw_derived_catch_base_ptr_c() noexcept {
+ try {
+ derived d;
+ throw &d;
+ } catch(const base *) {
+ }
+}
+
+void throw_derived_catch_base_ptr() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ptr' which should not throw exceptions
+ try {
+ derived d;
+ const derived *p = &d;
+ throw p;
+ } catch(base *) {
+ }
+}
+
void try_nested_try(int n) noexcept {
// CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'try_nested_try' which should not throw exceptions
try {
Index: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
+++ clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
@@ -80,7 +80,7 @@
/// possible to catch multiple exception types by one 'catch' if they
/// are a subclass of the 'catch'ed exception type.
/// Returns 'true' if some exceptions were filtered, otherwise 'false'.
- bool filterByCatch(const Type *BaseClass);
+ bool filterByCatch(const Type *BaseClass, const LangOptions &LangOpts);
/// Filter the set of thrown exception type against a set of ignored
/// types that shall not be considered in the exception analysis.
Index: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -56,11 +56,118 @@
[BaseClass](const CXXRecordDecl *Cur) { return Cur != BaseClass; });
}
-bool ExceptionAnalyzer::ExceptionInfo::filterByCatch(const Type *BaseClass) {
+// Checks if From is convertible to To according to the current LangOpts.
+static bool isMultiLevelConvertiblePointer(QualType From, QualType To,
+ LangOptions LangOpts,
+ unsigned CurrentLevel = 0,
+ bool IsToConstSoFar = false) {
+ if (CurrentLevel == 0) {
+ assert(From->isPointerType() && "From is not a pointer type");
+ assert(To->isPointerType() && "To is not a pointer type");
+
+ QualType FromPointeeTy = From->getAs<PointerType>()->getPointeeType();
+ QualType ToPointeeTy = To->getAs<PointerType>()->getPointeeType();
+
+ if (FromPointeeTy->isArrayType() && ToPointeeTy->isArrayType()) {
+ FromPointeeTy = FromPointeeTy->getAsArrayTypeUnsafe()->getElementType();
+ ToPointeeTy = ToPointeeTy->getAsArrayTypeUnsafe()->getElementType();
+ }
+
+ // If the pointer is not a multi-level pointer, perform
+ // conversion checks.
+ if (!FromPointeeTy->isPointerType() || !ToPointeeTy->isPointerType()) {
+ const Type *FromPointeeUQTy =
+ FromPointeeTy->getUnqualifiedDesugaredType();
+ const Type *ToPointeeUQTy = ToPointeeTy->getUnqualifiedDesugaredType();
+
+ Qualifiers FromQuals = FromPointeeTy.getQualifiers();
+ Qualifiers ToQuals = ToPointeeTy.getQualifiers();
+
+ return (FromPointeeUQTy == ToPointeeUQTy ||
+ isBaseOf(FromPointeeUQTy, ToPointeeUQTy)) &&
+ ToQuals.isStrictSupersetOf(FromQuals);
+ }
+
+ return isMultiLevelConvertiblePointer(FromPointeeTy, ToPointeeTy, LangOpts,
+ CurrentLevel + 1, true);
+ }
+
+ bool Convertible = true;
+
+ if (From->isArrayType() && To->isArrayType()) {
+
+ if (LangOpts.CPlusPlus20 || LangOpts.CPlusPlus2b) {
+ // At every level that array type is involved in, at least
+ // one array type has unknown bound, or both array types
+ // have same size. (since C++20)
+ if (From->isConstantArrayType() && To->isConstantArrayType())
+ Convertible &=
+ cast<ConstantArrayType>(From->getAsArrayTypeUnsafe())->getSize() ==
+ cast<ConstantArrayType>(To->getAsArrayTypeUnsafe())->getSize();
+
+ // If there is an array type of unknown bound at some level
+ // (other than level zero) of From, there is an array type of
+ // unknown bound in the same level of To; (since C++20)
+ if (From->isIncompleteArrayType())
+ Convertible &= To->isIncompleteArrayType();
+
+ // ... [or there is an array type of known bound in From and
+ // an array type of unknown bound in To (since C++20)] then
+ // there must be a 'const' at every single level (other than
+ // level zero) of To up until the current level.
+ if (From->isConstantArrayType() && To->isIncompleteArrayType())
+ Convertible &= IsToConstSoFar;
+ }
+
+ From = From->getAsArrayTypeUnsafe()->getElementType();
+ To = To->getAsArrayTypeUnsafe()->getElementType();
+ }
+
+ // If at the current level To is more cv-qualified than From [...],
+ // then there must be a 'const' at every single level (other than level zero)
+ // of To up until the current level
+ if (To.getQualifiers().isStrictSupersetOf(From.getQualifiers()))
+ Convertible &= IsToConstSoFar;
+
+ // If the pointers don't have the same amount of levels, they are not
+ // convertible to each other.
+ if (!From->isPointerType() || !To->isPointerType())
+ return Convertible && From->getCanonicalTypeUnqualified() ==
+ To->getCanonicalTypeUnqualified();
+
+ // Note that in the C programming language, const/volatile can be added to the
+ // first level only.
+ bool CanBeCVQualified = LangOpts.CPlusPlus || CurrentLevel == 1;
+
+ // If the current level (other than level zero) in From is 'const' qualified,
+ // the same level in To must also be 'const' qualified.
+ if (From.isConstQualified())
+ Convertible &= To.isConstQualified() && CanBeCVQualified;
+
+ // If the current level (other than level zero) in From is 'volatile'
+ // qualified, the same level in To must also be 'volatile' qualified.
+ if (From.isVolatileQualified())
+ Convertible &= To.isVolatileQualified() && CanBeCVQualified;
+
+ IsToConstSoFar &= To.isConstQualified();
+ return Convertible && isMultiLevelConvertiblePointer(
+ From->getAs<PointerType>()->getPointeeType(),
+ To->getAs<PointerType>()->getPointeeType(),
+ LangOpts, CurrentLevel + 1, IsToConstSoFar);
+}
+
+bool ExceptionAnalyzer::ExceptionInfo::filterByCatch(
+ const Type *BaseClass, const LangOptions &LangOpts) {
llvm::SmallVector<const Type *, 8> TypesToDelete;
for (const Type *T : ThrownExceptions) {
if (T == BaseClass || isBaseOf(T, BaseClass))
TypesToDelete.push_back(T);
+ else if (T->isPointerType() && BaseClass->isPointerType()) {
+ if (isMultiLevelConvertiblePointer(QualType(T, 0), QualType(BaseClass, 0),
+ LangOpts)) {
+ TypesToDelete.push_back(T);
+ }
+ }
}
for (const Type *T : TypesToDelete)
@@ -186,11 +293,14 @@
->getUnqualifiedDesugaredType();
}
+ const auto &LangOpts =
+ Catch->getExceptionDecl()->getASTContext().getLangOpts();
+
// If the caught exception will catch multiple previously potential
// thrown types (because it's sensitive to inheritance) the throwing
// situation changes. First of all filter the exception types and
// analyze if the baseclass-exception is rethrown.
- if (Uncaught.filterByCatch(CaughtType)) {
+ if (Uncaught.filterByCatch(CaughtType, LangOpts)) {
ExceptionInfo::Throwables CaughtExceptions;
CaughtExceptions.insert(CaughtType);
ExceptionInfo Rethrown = throwsException(Catch->getHandlerBlock(),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
