isuckatcs updated this revision to Diff 492115.
isuckatcs added a comment.
- Reverted the changes related to clang itself
- Added more checks for exception handling
- `isQualificationConvertiblePointer` is now iterative, not recursive
- Added more test cases
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,84 @@
}
}
+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 {};
+class C : protected A {};
+class D : public A {};
+class E : public A, public D {};
+
+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 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 ASTContext &Context);
/// 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
@@ -46,7 +46,52 @@
return *this;
}
-static bool isBaseOf(const Type *DerivedType, const Type *BaseType) {
+// FIXME: This could be ported to clang later.
+namespace {
+
+bool isUnunambiguousPublicBaseClass(const Type *DerivedType,
+ const Type *BaseType) {
+ const auto *DerivedClass = DerivedType->getAsCXXRecordDecl();
+ const auto *BaseClass = BaseType->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()->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) {
const auto *DerivedClass = DerivedType->getAsCXXRecordDecl();
const auto *BaseClass = BaseType->getAsCXXRecordDecl();
if (!DerivedClass || !BaseClass)
@@ -56,11 +101,253 @@
[BaseClass](const CXXRecordDecl *Cur) { return Cur != BaseClass; });
}
-bool ExceptionAnalyzer::ExceptionInfo::filterByCatch(const Type *BaseClass) {
+// Check if T1 is more or Equaly qualified than T2.
+bool MoreOrEqualyQualified(QualType T1, QualType T2)
+{
+ return T1.getQualifiers().isStrictSupersetOf(T2.getQualifiers()) || T1.getQualifiers() == T2.getQualifiers();
+}
+
+bool isStandardPointerConvertible(QualType From, QualType To) {
+ assert(From->isPointerType() && To->isPointerType() && "Pointer conversion should be performed on pointer types only.");
+
+ if(!MoreOrEqualyQualified(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 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<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 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;
+ 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) {
llvm::SmallVector<const Type *, 8> 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 (isUnunambiguousPublicBaseClass(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 ...
+ else 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) &&
+ isUnunambiguousPublicBaseClass(
+ 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)
@@ -190,7 +477,7 @@
// 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(),
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
