diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h --- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h +++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h @@ -78,7 +78,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 ASTContext &Context); /// Filter the set of thrown exception type against a set of ignored /// types that shall not be considered in the exception analysis. diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp --- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp +++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp @@ -44,7 +44,56 @@ return *this; } -static bool isBaseOf(const Type *DerivedType, const Type *BaseType) { +// FIXME: This could be ported to clang later. +namespace { + +bool isUnambiguousPublicBaseClass(const Type *DerivedType, + const Type *BaseType) { + const auto *DerivedClass = + DerivedType->getCanonicalTypeUnqualified()->getAsCXXRecordDecl(); + const auto *BaseClass = + BaseType->getCanonicalTypeUnqualified()->getAsCXXRecordDecl(); + if (!DerivedClass || !BaseClass) + return false; + + CXXBasePaths Paths; + Paths.setOrigin(DerivedClass); + + bool IsPublicBaseClass = false; + DerivedClass->lookupInBases( + [&BaseClass, &IsPublicBaseClass](const CXXBaseSpecifier *BS, + CXXBasePath &) { + if (BS->getType() + ->getCanonicalTypeUnqualified() + ->getAsCXXRecordDecl() == BaseClass && + BS->getAccessSpecifier() == AS_public) { + IsPublicBaseClass = true; + return true; + } + + return false; + }, + Paths); + + return !Paths.isAmbiguous(BaseType->getCanonicalTypeUnqualified()) && + IsPublicBaseClass; +} + +inline bool isPointerOrPointerToMember(const Type *T) { + return T->isPointerType() || T->isMemberPointerType(); +} + +std::optional getPointeeOrArrayElementQualType(QualType T) { + if (T->isAnyPointerType() || T->isMemberPointerType()) + return T->getPointeeType(); + + if (T->isArrayType()) + return T->getAsArrayTypeUnsafe()->getElementType(); + + return std::nullopt; +} + +bool isBaseOf(const Type *DerivedType, const Type *BaseType) { const auto *DerivedClass = DerivedType->getAsCXXRecordDecl(); const auto *BaseClass = BaseType->getAsCXXRecordDecl(); if (!DerivedClass || !BaseClass) @@ -54,11 +103,273 @@ [BaseClass](const CXXRecordDecl *Cur) { return Cur != BaseClass; }); } -bool ExceptionAnalyzer::ExceptionInfo::filterByCatch(const Type *BaseClass) { +// Check if T1 is more or Equally qualified than T2. +bool moreOrEquallyQualified(QualType T1, QualType T2) { + return T1.getQualifiers().isStrictSupersetOf(T2.getQualifiers()) || + T1.getQualifiers() == T2.getQualifiers(); +} + +bool isStandardPointerConvertible(QualType From, QualType To) { + assert((From->isPointerType() || From->isMemberPointerType()) && + (To->isPointerType() || To->isMemberPointerType()) && + "Pointer conversion should be performed on pointer types only."); + + if (!moreOrEquallyQualified(To->getPointeeType(), From->getPointeeType())) + return false; + + // (1) + // A null pointer constant can be converted to a pointer type ... + // The conversion of a null pointer constant to a pointer to cv-qualified type + // is a single conversion, and not the sequence of a pointer conversion + // followed by a qualification conversion. A null pointer constant of integral + // type can be converted to a prvalue of type std::nullptr_t + if (To->isPointerType() && From->isNullPtrType()) + return true; + + // (2) + // A prvalue of type “pointer to cv T”, where T is an object type, can be + // converted to a prvalue of type “pointer to cv void”. + if (To->isVoidPointerType() && From->isObjectPointerType()) + return true; + + // (3) + // A prvalue of type “pointer to cv D”, where D is a complete class type, can + // be converted to a prvalue of type “pointer to cv B”, where B is a base + // class of D. If B is an inaccessible or ambiguous base class of D, a program + // that necessitates this conversion is ill-formed. + if (const auto *RD = From->getPointeeCXXRecordDecl()) { + if (RD->isCompleteDefinition() && + isBaseOf(From->getPointeeType().getTypePtr(), + To->getPointeeType().getTypePtr())) { + return true; + } + } + + return false; +} + +bool isFunctionPointerConvertible(QualType From, QualType To) { + if (!From->isFunctionPointerType() && !From->isFunctionType() && + !From->isMemberFunctionPointerType()) + return false; + + if (!To->isFunctionPointerType() && !To->isMemberFunctionPointerType()) + return false; + + if (To->isFunctionPointerType()) { + if (From->isFunctionPointerType()) + return To->getPointeeType() == From->getPointeeType(); + + if (From->isFunctionType()) + return To->getPointeeType() == From; + + return false; + } + + if (To->isMemberFunctionPointerType()) { + if (!From->isMemberFunctionPointerType()) + return false; + + const auto *FromMember = cast(From); + const auto *ToMember = cast(To); + + // Note: converting Derived::* to Base::* is a different kind of conversion, + // called Pointer-to-member conversion. + return FromMember->getClass() == ToMember->getClass() && + FromMember->getPointeeType() == ToMember->getPointeeType(); + } + + return false; +} + +// Checks if From is qualification convertible to To based on the current +// LangOpts. If From is any array, we perform the array to pointer conversion +// first. The function only performs checks based on C++ rules, which can differ +// from the C rules. +// +// The function should only be called in C++ mode. +bool isQualificationConvertiblePointer(QualType From, QualType To, + LangOptions LangOpts) { + + // [N4659 7.5 (1)] + // A cv-decomposition of a type T is a sequence of cv_i and P_i such that T is + // cv_0 P_0 cv_1 P_1 ... cv_n−1 P_n−1 cv_n U” for n > 0, + // where each cv_i is a set of cv-qualifiers, and each P_i is “pointer to”, + // “pointer to member of class C_i of type”, “array of N_i”, or + // “array of unknown bound of”. + // + // If P_i designates an array, the cv-qualifiers cv_i+1 on the element type + // are also taken as the cv-qualifiers cvi of the array. + // + // The n-tuple of cv-qualifiers after the first one in the longest + // cv-decomposition of T, that is, cv_1, cv_2, ... , cv_n, is called the + // cv-qualification signature of T. + + auto isValidP_i = [](QualType P) { + return P->isPointerType() || P->isMemberPointerType() || + P->isConstantArrayType() || P->isIncompleteArrayType(); + }; + + auto isSameP_i = [](QualType P1, QualType P2) { + if (P1->isPointerType()) + return P2->isPointerType(); + + if (P1->isMemberPointerType()) + return P2->isMemberPointerType() && + P1->getAs()->getClass() == + P2->getAs()->getClass(); + + if (P1->isConstantArrayType()) + return P2->isConstantArrayType() && + cast(P1)->getSize() == + cast(P2)->getSize(); + + if (P1->isIncompleteArrayType()) + return P2->isIncompleteArrayType(); + + return false; + }; + + // (2) + // Two types From and To are similar if they have cv-decompositions with the + // same n such that corresponding P_i components are the same [(added by + // N4849 7.3.5) or one is “array of N_i” and the other is “array of unknown + // bound of”], and the types denoted by U are the same. + // + // (3) + // A prvalue expression of type From can be converted to type To if the + // following conditions are satisfied: + // - From and To are similar + // - For every i > 0, if const is in cv_i of From then const is in cv_i of + // To, and similarly for volatile. + // - [(derived from addition by N4849 7.3.5) If P_i of From is “array of + // unknown bound of”, P_i of To is “array of unknown bound of”.] + // - If the cv_i of From and cv_i of To are different, then const is in every + // cv_k of To for 0 < k < i. + + int I = 0; + bool ConstUntilI = true; + auto SatisfiesCVRules = [&I, &ConstUntilI](const QualType &From, + const QualType &To) { + if (I > 1) { + if (From.getQualifiers() != To.getQualifiers() && !ConstUntilI) + return false; + } + + if (I > 0) { + if (From.isConstQualified() && !To.isConstQualified()) + return false; + + if (From.isVolatileQualified() && !To.isVolatileQualified()) + return false; + + ConstUntilI = To.isConstQualified(); + } + + return true; + }; + + while (isValidP_i(From) && isValidP_i(To)) { + // Remove every sugar. + From = From.getCanonicalType(); + To = To.getCanonicalType(); + + if (!SatisfiesCVRules(From, To)) + return false; + + if (!isSameP_i(From, To)) { + if (LangOpts.CPlusPlus20) { + if (From->isConstantArrayType() && !To->isIncompleteArrayType()) + return false; + + if (From->isIncompleteArrayType() && !To->isIncompleteArrayType()) + return false; + + } else { + return false; + } + } + + ++I; + std::optional FromPointeeOrElem = + getPointeeOrArrayElementQualType(From); + std::optional ToPointeeOrElem = + getPointeeOrArrayElementQualType(To); + + assert(FromPointeeOrElem && + "From pointer or array has no pointee or element!"); + assert(ToPointeeOrElem && "To pointer or array has no pointee or element!"); + + From = *FromPointeeOrElem; + To = *ToPointeeOrElem; + } + + // In this case the length (n) of From and To are not the same. + if (isValidP_i(From) || isValidP_i(To)) + return false; + + // We hit U. + if (!SatisfiesCVRules(From, To)) + return false; + + return From.getTypePtr() == To.getTypePtr(); +} +} // namespace + +bool ExceptionAnalyzer::ExceptionInfo::filterByCatch( + const Type *HandlerTy, const ASTContext &Context) { llvm::SmallVector TypesToDelete; - for (const Type *T : ThrownExceptions) { - if (T == BaseClass || isBaseOf(T, BaseClass)) - TypesToDelete.push_back(T); + for (const Type *ExceptionTy : ThrownExceptions) { + CanQualType ExceptionCanTy = ExceptionTy->getCanonicalTypeUnqualified(); + CanQualType HandlerCanTy = HandlerTy->getCanonicalTypeUnqualified(); + + // The handler is of type cv T or cv T& and E and T are the same type + // (ignoring the top-level cv-qualifiers) ... + if (ExceptionCanTy == HandlerCanTy) { + TypesToDelete.push_back(ExceptionTy); + } + + // The handler is of type cv T or cv T& and T is an unambiguous public base + // class of E ... + else if (isUnambiguousPublicBaseClass(ExceptionCanTy->getTypePtr(), + HandlerCanTy->getTypePtr())) { + TypesToDelete.push_back(ExceptionTy); + } + + if (HandlerCanTy->getTypeClass() == Type::RValueReference || + (HandlerCanTy->getTypeClass() == Type::LValueReference && + !HandlerCanTy->getTypePtr()->getPointeeType().isConstQualified())) + continue; + // The handler is of type cv T or const T& where T is a pointer or + // pointer-to-member type and E is a pointer or pointer-to-member type that + // can be converted to T by one or more of ... + if (isPointerOrPointerToMember(HandlerCanTy->getTypePtr()) && + isPointerOrPointerToMember(ExceptionCanTy->getTypePtr())) { + // A standard pointer conversion not involving conversions to pointers to + // private or protected or ambiguous classes ... + if (isStandardPointerConvertible(ExceptionCanTy, HandlerCanTy) && + isUnambiguousPublicBaseClass( + ExceptionCanTy->getTypePtr()->getPointeeType().getTypePtr(), + HandlerCanTy->getTypePtr()->getPointeeType().getTypePtr())) { + TypesToDelete.push_back(ExceptionTy); + } + // A function pointer conversion ... + else if (isFunctionPointerConvertible(ExceptionCanTy, HandlerCanTy)) { + TypesToDelete.push_back(ExceptionTy); + } + // A a qualification conversion ... + else if (isQualificationConvertiblePointer(ExceptionCanTy, HandlerCanTy, + Context.getLangOpts())) { + TypesToDelete.push_back(ExceptionTy); + } + } + + // The handler is of type cv T or const T& where T is a pointer or + // pointer-to-member type and E is std::nullptr_t. + else if (isPointerOrPointerToMember(HandlerCanTy->getTypePtr()) && + ExceptionCanTy->isNullPtrType()) { + TypesToDelete.push_back(ExceptionTy); + } } for (const Type *T : TypesToDelete) @@ -188,7 +499,8 @@ // 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, Catch->getExceptionDecl()->getASTContext())) { ExceptionInfo::Throwables CaughtExceptions; CaughtExceptions.insert(CaughtType); ExceptionInfo Rethrown = throwsException(Catch->getHandlerBlock(), diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp +++ b/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,265 @@ } } +void throw_derived_alias_catch_base() noexcept { + using alias = derived; + + try { + throw alias(); + } catch(base &) { + } +} + +void throw_derived_catch_base_alias() noexcept { + using alias = base; + + try { + throw derived(); + } catch(alias &) { + } +} + +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 *) { + } +} + +class A {}; +class B : A {}; + +// The following alias hell is deliberately created for testing. +using aliasedA = A; +class C : protected aliasedA {}; + +typedef aliasedA moreAliasedA; +class D : public moreAliasedA {}; + +using moreMoreAliasedA = moreAliasedA; +using aliasedD = D; +class E : public moreMoreAliasedA, public aliasedD {}; + +void throw_derived_catch_base_private() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_private' which should not throw exceptions + try { + B b; + throw b; + } catch(A) { + } +} + +void throw_derived_catch_base_private_ptr() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_private_ptr' which should not throw exceptions + try { + B b; + throw &b; + } catch(A *) { + } +} + +void throw_derived_catch_base_protected() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_protected' which should not throw exceptions + try { + C c; + throw c; + } catch(A) { + } +} + +void throw_derived_catch_base_protected_ptr() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_protected_ptr' which should not throw exceptions + try { + C c; + throw &c; + } catch(A *) { + } +} + +void throw_derived_catch_base_ambiguous() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ambiguous' which should not throw exceptions + try { + E e; + throw e; + } catch(A) { + } +} + +void throw_derived_catch_base_ambiguous_ptr() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ambiguous_ptr' which should not throw exceptions + try { + E e; + throw e; + } catch(A) { + } +} + +void throw_alias_catch_original() noexcept { + using alias = int; + + try { + alias a = 3; + throw a; + } catch (int) { + } +} + +void throw_alias_catch_original_warn() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_alias_catch_original_warn' which should not throw exceptions + using alias = float; + + try { + alias a = 3; + throw a; + } catch (int) { + } +} + +void throw_original_catch_alias() noexcept { + using alias = char; + + try { + char **p = 0; + throw p; + } catch (volatile const alias *const *) { + } +} + +void throw_original_catch_alias_warn() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_original_catch_alias_warn' which should not throw exceptions + using alias = int; + + try { + char **p = 0; + throw p; + } catch (volatile const alias *const *) { + } +} + +void throw_original_catch_alias_2() noexcept { + using alias = const char *const; + + try { + char **p = 0; + throw p; + } catch (volatile alias *) { + } +} + +namespace a { + int foo() { return 0; }; + + void throw_regular_catch_regular() noexcept { + try { + throw &foo; + } catch(int (*)()) { + } + } +} + +namespace b { + inline int foo() { return 0; }; + + void throw_inline_catch_regular() noexcept { + try { + throw &foo; + } catch(int (*)()) { + } + } +} + +namespace c { + inline int foo() noexcept { return 0; }; + + void throw_noexcept_catch_regular() noexcept { + try { + throw &foo; + } catch(int (*)()) { + } + } +} + +struct baseMember { + int *iptr; + virtual void foo(){}; +}; + +struct derivedMember : baseMember { + void foo() override {}; +}; + +void throw_basefn_catch_derivedfn() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_basefn_catch_derivedfn' which should not throw exceptions + try { + throw &baseMember::foo; + } catch(void(derivedMember::*)()) { + } +} + +void throw_basefn_catch_basefn() noexcept { + try { + throw &baseMember::foo; + } catch(void(baseMember::*)()) { + } +} + +void throw_basem_catch_basem_throw() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_basem_catch_basem_throw' which should not throw exceptions + try { + auto ptr = &baseMember::iptr; + throw &ptr; + } catch(const int* baseMember::* const *) { + } +} + +void throw_basem_catch_basem() noexcept { + try { + auto ptr = &baseMember::iptr; + throw &ptr; + } catch(const int* const baseMember::* const *) { + } +} + +void throw_basem_catch_derivedm() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_basem_catch_derivedm' which should not throw exceptions + try { + auto ptr = &baseMember::iptr; + throw &ptr; + } catch(const int* const derivedMember::* const *) { + } +} + +void throw_derivedm_catch_basem() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derivedm_catch_basem' which should not throw exceptions + try { + int *derivedMember::* ptr = &derivedMember::iptr; + throw &ptr; + } catch(const int* const baseMember::* const *) { + } +} + +void throw_original_catch_alias_2_warn() noexcept { + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_original_catch_alias_2_warn' which should not throw exceptions + using alias = const int *const; + + try { + char **p = 0; + throw p; + } catch (volatile alias *) { + } +} + 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 {