isuckatcs updated this revision to Diff 466312. isuckatcs marked 2 inline comments as done. isuckatcs added a comment.
Addressed comments and added support for multi-level pointers too. 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/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.cpp =================================================================== --- clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp +++ clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp @@ -56,11 +56,108 @@ [BaseClass](const CXXRecordDecl *Cur) { return Cur != BaseClass; }); } +// Checks if T1 is convertible to T2. +static bool isMultiLevelConvertiblePointer(QualType P1, QualType P2, + unsigned CurrentLevel = 0, + bool IsP2ConstSoFar = false) { + if (CurrentLevel == 0) { + assert(P1->isPointerType() && "P1 is not a pointer type"); + assert(P2->isPointerType() && "P2 is not a pointer type"); + + QualType P1PointeeTy = P1->getAs<PointerType>()->getPointeeType(); + QualType P2PointeeTy = P2->getAs<PointerType>()->getPointeeType(); + + if (P1PointeeTy->isArrayType() && P2PointeeTy->isArrayType()) { + P1PointeeTy = P1PointeeTy->getAsArrayTypeUnsafe()->getElementType(); + P2PointeeTy = P2PointeeTy->getAsArrayTypeUnsafe()->getElementType(); + } + + // If the pointer is not a multi-level pointer, perform + // conversion checks. + if (!P1PointeeTy->isPointerType() || !P2PointeeTy->isPointerType()) { + const Type *P1PointeeUQTy = P1PointeeTy->getUnqualifiedDesugaredType(); + const Type *P2PointeeUQTy = P2PointeeTy->getUnqualifiedDesugaredType(); + + Qualifiers P1Quals = P1PointeeTy.getQualifiers(); + Qualifiers P2Quals = P2PointeeTy.getQualifiers(); + + return (P1PointeeUQTy == P2PointeeUQTy || + isBaseOf(P1PointeeUQTy, P2PointeeUQTy)) && + P2Quals.isStrictSupersetOf(P1Quals); + } + + return isMultiLevelConvertiblePointer(P1PointeeTy, P2PointeeTy, + CurrentLevel + 1, true); + } + + bool convertible = true; + + if (P1->isArrayType() && P2->isArrayType()) { + // At every level that array type is involved in, at least + // one array type has unknown bound, or both array types + // have same size. + if (P1->isConstantArrayType() && P2->isConstantArrayType()) + convertible &= + cast<ConstantArrayType>(P1->getAsArrayTypeUnsafe())->getSize() == + cast<ConstantArrayType>(P2->getAsArrayTypeUnsafe())->getSize(); + + // If there is an array type of unknown bound at some level + // (other than level zero) of P1, there is an array type of + // unknown bound in the same level of P2; + if (P1->isIncompleteArrayType()) + convertible &= P2->isIncompleteArrayType(); + + // ... [or there is an array type of known bound in P1 and + // an array type of unknown bound in P2 (since C++20)] then + // there must be a 'const' at every single level (other than + // level zero) of P2 up until the current level. + if (P1->isConstantArrayType() && P2->isIncompleteArrayType()) + convertible &= IsP2ConstSoFar; + + P1 = P1->getAsArrayTypeUnsafe()->getElementType(); + P2 = P2->getAsArrayTypeUnsafe()->getElementType(); + } + + // If at the current level P2 is more cv-qualified than P1 [...], + // then there must be a 'const' at every single level (other than level zero) + // of P2 up until the current level + if (P2.getQualifiers().isStrictSupersetOf(P1.getQualifiers())) + convertible &= IsP2ConstSoFar; + + // If the pointers don't have the same amount of levels, they are not + // convertible to each other. + if (!P1->isPointerType() || !P2->isPointerType()) + return convertible && P1->getUnqualifiedDesugaredType() == + P2->getUnqualifiedDesugaredType(); + + // If the current level (other than level zero) in P1 is 'const' qualified, + // the same level in P2 must also be 'const' qualified. + if (P1.isConstQualified()) + convertible &= P2.isConstQualified(); + + // If the current level (other than level zero) in P1 is 'volatile' qualified, + // the same level in P2 must also be 'volatile' qualified. + if (P1.isVolatileQualified()) + convertible &= P2.isVolatileQualified(); + + IsP2ConstSoFar &= P2.isConstQualified(); + return convertible && isMultiLevelConvertiblePointer( + P1->getAs<PointerType>()->getPointeeType(), + P2->getAs<PointerType>()->getPointeeType(), + CurrentLevel + 1, IsP2ConstSoFar); +} + bool ExceptionAnalyzer::ExceptionInfo::filterByCatch(const Type *BaseClass) { 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))) { + TypesToDelete.push_back(T); + } + } } for (const Type *T : TypesToDelete)
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits