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()->getPointeeType(); + QualType ToPointeeTy = To->getAs()->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(From->getAsArrayTypeUnsafe())->getSize() == + cast(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()->getPointeeType(), + To->getAs()->getPointeeType(), + LangOpts, CurrentLevel + 1, IsToConstSoFar); +} + +bool ExceptionAnalyzer::ExceptionInfo::filterByCatch( + const Type *BaseClass, const LangOptions &LangOpts) { llvm::SmallVector 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(), 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 {