Author: David Spickett Date: 2023-02-24T16:37:14Z New Revision: 8ae5e9edcdb394794d8c4d1ee286f1b500aaf826
URL: https://github.com/llvm/llvm-project/commit/8ae5e9edcdb394794d8c4d1ee286f1b500aaf826 DIFF: https://github.com/llvm/llvm-project/commit/8ae5e9edcdb394794d8c4d1ee286f1b500aaf826.diff LOG: Revert "[clang-tidy] handle exceptions properly in `ExceptionAnalyzer`" This reverts commit 6b0cf1e15f8f84e3d4b6f9522287f6460527a7bf. The included test is timing out on Arm/AArch64 bots. Added: Modified: 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 Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp index 251d78e21a330..359e7e9dc46af 100644 --- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp +++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp @@ -44,56 +44,7 @@ ExceptionAnalyzer::ExceptionInfo &ExceptionAnalyzer::ExceptionInfo::merge( return *this; } -// 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(); -} - -QualType getPointeeOrArrayElementQualType(QualType T) { - if (T->isAnyPointerType()) - return T->getPointeeType(); - - if (T->isArrayType()) - return T->getAsArrayTypeUnsafe()->getElementType(); - - return T; -} - -bool isBaseOf(const Type *DerivedType, const Type *BaseType) { +static bool isBaseOf(const Type *DerivedType, const Type *BaseType) { const auto *DerivedClass = DerivedType->getAsCXXRecordDecl(); const auto *BaseClass = BaseType->getAsCXXRecordDecl(); if (!DerivedClass || !BaseClass) @@ -103,264 +54,11 @@ bool isBaseOf(const Type *DerivedType, const Type *BaseType) { [BaseClass](const CXXRecordDecl *Cur) { return Cur != 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<MemberPointerType>(From); - const auto *ToMember = cast<MemberPointerType>(To); - - // Note: converting Derived::* to Base::* is a diff erent 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 diff er -// 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<MemberPointerType>()->getClass() == - P2->getAs<MemberPointerType>()->getClass(); - - if (P1->isConstantArrayType()) - return P2->isConstantArrayType() && - cast<ConstantArrayType>(P1)->getSize() == - cast<ConstantArrayType>(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 diff erent, 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; - From = getPointeeOrArrayElementQualType(From); - To = getPointeeOrArrayElementQualType(To); - } - - // 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) { +bool ExceptionAnalyzer::ExceptionInfo::filterByCatch(const Type *BaseClass) { llvm::SmallVector<const Type *, 8> TypesToDelete; - 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 : ThrownExceptions) { + if (T == BaseClass || isBaseOf(T, BaseClass)) + TypesToDelete.push_back(T); } for (const Type *T : TypesToDelete) @@ -490,8 +188,7 @@ ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException( // 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, Catch->getExceptionDecl()->getASTContext())) { + if (Uncaught.filterByCatch(CaughtType)) { ExceptionInfo::Throwables CaughtExceptions; CaughtExceptions.insert(CaughtType); ExceptionInfo Rethrown = throwsException(Catch->getHandlerBlock(), diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h index fd65284d570ab..2cb988abc1cd8 100644 --- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h +++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h @@ -78,7 +78,7 @@ class ExceptionAnalyzer { /// 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, const ASTContext &Context); + bool filterByCatch(const Type *BaseClass); /// 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/test/clang-tidy/checkers/bugprone/exception-escape.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp index ab288462cfd28..769064d74adc5 100644 --- 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,126 +101,6 @@ void throw_catch_rethrow_the_rest(int n) noexcept { } } -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 {}; @@ -232,229 +112,6 @@ void throw_derived_catch_base() noexcept { } } -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 *) { - } -} - -{ - int foo() { return 0; }; - - void throw_regular_catch_regular() noexcept { - try { - throw &foo; - } catch(int (*)()) { - } - } -} - -{ - inline int foo() { return 0; }; - - void throw_inline_catch_regular() noexcept { - try { - throw &foo; - } catch(int (*)()) { - } - } -} - -{ - inline int foo() noexcept { return 0; }; - - void throw_noexcept_catch_regular() noexcept { - try { - throw &foo; - } catch(int (*)()) { - } - } -} - -struct baseMemberFn { - virtual void foo(){}; -}; - -struct derivedMemberFn : baseMemberFn { - 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 &baseMemberFn::foo; - } catch(void(derivedMemberFn::*)()) { - } -} - -void throw_basefn_catch_basefn() noexcept { - try { - throw &baseMemberFn::foo; - } catch(void(baseMemberFn::*)()) { - } -} - -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 { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
