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

Reply via email to