[clang-tidy] handle exceptions properly in `ExceptionAnalyzer`

[isuckatcs]

Summary

Prior to this patch ExceptionAnalyzer reported many false
positives due to insufficient checking of conversion rules
inside exception handlers.

One false positive example from the previous version:

void foo() noexcept {
  try {
    int a = 1;
    throw &a;
  } catch (const int *) {
  }
}

In function foo() the &a is caught by the handler, but clang-tidy
reports the following warning:

warning: an exception may be thrown in function 'foo' which should not throw exceptions [bugprone-exception-escape]

The standard says the following about exception handling:

[N4849 14.4 Handling an exception]

(3) A handler is a match for an exception object of type E if

• The handler is of type cv T or cv T& and E and T are the same type (ignoring the top-level cv-qualifiers), or
• the handler is of type cv T or cv T& and T is an unambiguous public base class of E, or
• 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
  • a standard pointer conversion (7.3.11) not involving conversions to pointers to private or protected or ambiguous classes
  • a function pointer conversion (7.3.13)
  • a qualification conversion (7.3.5), or
• 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.

This patch is supposed to implement all of these checks.

[isuckatcs]

Updated

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

315

Please don't use auto unless type is spelled in same statement or iterator. Same below.

[isuckatcs]

Addressed inline comment

[njames93]

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

328

This harms readability and it doesn't cover extended qualifiers. I believe Qualifiers::isStrictSuperSet would be a better candidate to use here.

clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp

105

These check not lines are useless and should be removed, same goes below.

[isuckatcs]

Addressed comments and added support for multi-level pointers too.

[isuckatcs]

I've found a strange scenario.

The following conversions are allowed

double *a[2][3];
double const * const (*ap)[3] = a; // OK
double * const (*ap1)[] = a;       // OK since C++20

However if the same conversion is supposed to be performed in a catch() statement, it's not happening and the thrown object is not caught.
See it on godbolt.

Quoteing cppreference:

When an exception is thrown by any statement in compound-statement, the exception object of type E is matched against the types of the formal parameters T of each catch-clause in handler-seq, in the order in which the catch clauses are listed. The exception is a match if any of the following is true:

...
- T is (possibly cv-qualified) U or const U& (since C++14), and U is a pointer or pointer to member type, and E is also a pointer or pointer to member type that is implicitly convertible to U by one or more of
  - a standard pointer conversion other than one to a private, protected, or ambiguous base class
  - **a qualification conversion**
  - a function pointer conversion (since C++17)
...

Checking the quality conversion related part of cppreference lists the examples I quoted before as performable conversions.

[isuckatcs]

ping

[xazax.hun]

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

105

Did you take a look at ASTContext::hasCvrSimilarType? I wonder if it is already doing what you want.

[xazax.hun]

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

105

Oh, maybe it is not, but it might also make sense to take a look at ASTContext::typesAreCompatible.

[xazax.hun]

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

140

If none of the functions I recommended works for you, I still strongly suggest reusing utilities from ASTContext, like UnwrapSimilarTypes and UnwrapSimilarArrayTypes.

[isuckatcs]

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

105

Did you take a look at ASTContext::hasCvrSimilarType? I wonder if it is already doing what you want.

This function compares the types by removing the CVR qualifiers.

Oh, maybe it is not, but it might also make sense to take a look at ASTContext::typesAreCompatible.

This one only compares the canonical types if the language is C++.

if (getLangOpts().CPlusPlus)
    return hasSameType(LHS, RHS);
----------------------------
bool hasSameType(QualType T1, QualType T2) const {
    return getCanonicalType(T1) == getCanonicalType(T2);
  }

140

I didn't check all of the functions inside ASTContext, but here we have very specific rules, we need to check. One utility might help with one check or two, but won't replace the need to go ove all of them. Also I think it's easier to understand which section checks what, if I keep them separated.

In an ealier comment I link to the section on cppreference, which discusses what these rules are. Also there I found one controversial example, that's only detected by MSVC. I wonder if you know why that happens.

[xazax.hun]

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

105

Could you rename the arguments like To, From or Exception, Catch or something to make the direction of the conversion clearer?

140

I see. It is unfortunate that we cannot simply reuse the corresponding functionality from Sema. The code mostly looks good to me, apart from some nits.

156

If this rule only applies to C++20 and above, consider adding a language version check for LangOpts.

175–176

Looking at the documentation of DesugaredType:

This takes off typedefs, typeof's etc. If the outer level of the type is already concrete, it returns it unmodified. This is similar to getting the canonical type, but it doesn't remove all typedefs. For example, it returns "T*" as "T*", (not as "int*"), because the pointer is concrete.

I wonder if we actually want to compare canonical types rather than desugared types.

[xazax.hun]

Also, could you open a bug report about the strange exception behavior on GitHub? Hopefully someone working on conformance can take a look.

[isuckatcs]

Addressed comments

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

140

It is unfortunate that we cannot simply reuse the corresponding functionality from Sema.

It is indeed unfortunate, though I wonder if we want to move this functionality to ASTContext, so that it can be reused outside the ExceptionAnalyzer.

[xazax.hun]

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

126

Here I am also wondering if we actually want canonical types.

140

I am not opposed to that, but I think in that case we want this method to be used in the regular compilation pipeline to make sure it is correct.

[isuckatcs]

• Renamed and moved isMultiLevelConvertiblePointer() to ASTContext
• Added a unit test to test the said function

[xazax.hun]

Since now the patch touches Clang proper, adding one more reviewer.

[xazax.hun]

Overall looks good to me, I wonder if the tests could be less manual though. E.g., instead of asserting true/false, checking if the assignment would compile. This way we can be sure that the method in ASTContext matches the behavior of the compiler (and we get notified when the two diverge). If we could extract the corresponding code from Sema, it would be even better, but I do not insist as that might be a lot of work depending on how it interacts with other conversions.

[erichkeane]

Not a great reviewer here as I'm not particularly sure what is going on, but I 'looked at the clang parts.

clang/include/clang/AST/ASTContext.h

2828 ↗

(On Diff #489940)

Please document what this is doing...

2829 ↗

(On Diff #489940)

Why is this static if you need lang-opts? This should be retrieved by the ASTContext itself.

2831 ↗

(On Diff #489940)

What does this name mean here? It isn't clear to me.

clang/include/clang/AST/Type.h

7364 ↗

(On Diff #489940)

This changes the meaning here, and this is a commonly used thing. Why are you doing this?

clang/lib/AST/ASTContext.cpp

13465 ↗

(On Diff #489940)

Perhaps this should be asserted on!

13466 ↗

(On Diff #489940)

I find myself shocked we don't have something like this already, but what do we mean by 'qualification convertible'? Is that a term of art I'm missing?

13485 ↗

(On Diff #489940)

I would expect at least the 'to' here to assert as well. Passing a 'not pointer' as the 'two' when youre testing 'convertible pointer' is odd and a mistake?

13489 ↗

(On Diff #489940)

debatable whether we can use 'auto' here. I'd lean toward 'no'.

13492 ↗

(On Diff #489940)

I think the recursion here is making this too complicated iwth the "IsConstSoFar" and "Level", I'd probably suggest splitting these tasks up.

[isuckatcs]

Addressed comments
Updated tests

[isuckatcs]

E.g., instead of asserting true/false, checking if the assignment would compile.

This is actually biased. If the result of the compilation is different from the result we get from this function, it can also mean a bug in the compiler.

Take a look at this example on godbolt. The snippet here is only valid from C++20, but GCC compiles it even in C++17.

clang/include/clang/AST/ASTContext.h

2828 ↗

(On Diff #489940)

This was actually documented, but at the definition. My bad.

2829 ↗

(On Diff #489940)

By taking the language options externally, we might be able to produce more descriptive warning messages in the future.
E.g.: This conversion is only valid from C++20, etc.

Also calling this function doesn't depend on ASTContext any other way, so it can be called even if we don't have access to the ASTContext for some reason.

I don't know however if it makes sense to worry about these uses at all.

2831 ↗

(On Diff #489940)

This is documented at the use site of this variable. I couldn't come up with a better name for it.

// If at the current level To is more cv-qualified than From [...],
// then there must be a 'const' at every single level (other than level zero)
// of To up until the current level
bool MoreCVQualified =
    To.getQualifiers().isStrictSupersetOf(From.getQualifiers());
if (MoreCVQualified)
  Convertible &= IsToConstSoFar;

clang/include/clang/AST/Type.h

7364 ↗

(On Diff #489940)

I missed that it changes the meaning. Though I need this use case, so I reverted these changes and created a static global function instead.

clang/lib/AST/ASTContext.cpp

13465 ↗

(On Diff #489940)

I personally don't want to assert it, as it won't crash in C mode, it's just the fact that some rules here are different in C.
E.g. (from cppreference):

char** p = 0;
const char* const * p2 = p; // error in C, OK in C++

(The example above is checked properly but I didn't dig deeper into the other C rules, that's why I said that it shouldn't be called)

13466 ↗

(On Diff #489940)

I didn't come up with this name. It is what this conversion is called by the standard.

It is §7.3.5 in N4849 and you can also find it under the same name on cppreference.

13485 ↗

(On Diff #489940)

Well, technically MemberPointerType is also accepted and it's not derived from PointerType. Though I added an assertion here, but I left the check so that we don't crash in release mode.

13492 ↗

(On Diff #489940)

Splitting those up would just make it more complicated I think. We would need to perform nearly the same checks in both branches, though I keep thinking about it.

[isuckatcs]

This patch got a little bit out of control at the last revision, so I decided to remove every change from clang and add everything to the ExceptionAnalyzer only.
The reason for that is with exceptions we have less conversions to check than we have inside the compiler, which can lead to confusion.

For example:

class A {};
class B : public A {};

int A::* pa;
int B::* pb = pa; <-- valid outside of exception handler, invalid in exception handler

We can have the conversion int B::* pb = pa; because of 7.3.12 Pointer-to-member conversions, which is by standard not performed when an exception needs to be caught.
See godbolt. (MSVC does catch A::* with a B::* handler for some reason, maybe I miss some flag)

For the above reason, sadly we can't test the changes the way you suggested @xazax.hun, like checking if the assigment compiles or not.

[isuckatcs]

• Reverted the changes related to clang itself
• Added more checks for exception handling
• isQualificationConvertiblePointer is now iterative, not recursive
• Added more test cases

[isuckatcs]

applied clang-format

[isuckatcs]

ping

[xazax.hun]

Overall, I like the structure of this patch and the references back to the standard. But every time we compare type pointers, they might compare inequal when one of the types have sugar on it while the other does not. Please review all of those comparisons to see where do we need to get the canonical types instead, and add some tests with type aliases.

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

66

Will this work with typedefs and other sugar or do you need to get the canonical type here? I do not see test coverage for that scenario.

84

What about Type::getPointeeOrArrayElementType?

[isuckatcs]

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

84

I didn't use that for two reasons.

1. It returns a Type instead of a QualType, so the qualifiers are lost.
2. It uses Type::getBaseElementTypeUnsafe() for arrays, which discards all array types for multi-dimensional arrays. E.g.: int[2][3] will become int and I want to take only one step backward, so I want int[2][3] to become int[2].

I was thinking about inserting this function into Type or QualType but because of the different behaviour with arrays I decided to use it as a helper in this source file only instead. I agree that the naming is confusing a bit, but it is literally what the function does.

[isuckatcs]

Addressed comments

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

66

In this specific case it will work because getAsCXXRecordDecl() comes from the CanProxyBase utility class, which already canonicalizes the type, but I agree that it's not intuitive and unreadable.

I also modified the current test cases to have aliases.

[xazax.hun]

Thanks!

[isuckatcs]

Added tests for function pointers

[isuckatcs]

@xazax.hun can we merge this or should we wait for someone else's approval too?

[xazax.hun]

I think at this point it is ok to merge. Any other comments can be addressed in follow-up commits.

[isuckatcs]

fixed failing test

[DavidSpickett]

The test was timing out on Arm/AArch64 bots so I have reverted this change.

I think because there were other test failures on the initial run, that obscured the time out. This is one of the first builds to include this change: https://lab.llvm.org/buildbot/#/builders/188/builds/26480 Note that there is an unrelated failure, and the stdio shows a timeout.

PASS: lit :: shtest-define.py (70393 of 70394)
command timed out: 1200 seconds without output running [b'ninja', b'check-all'], attempting to kill

I confirmed that on the bot itself, it was exception-escape that was holding things up. Local bisect backs that up.

If this is not happening outside of Arm/AArch64 I can help you understand what went wrong (though I will be finishing work for the week shortly, back Monday).

[DavidSpickett]

If it helps, the bot I linked is pretty vanilla. You can find all the details in the buildbot UI but just in case:

cmake -G Ninja ../llvm/llvm -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=True '-DLLVM_LIT_ARGS='"'"'-v'"'"'' -DCMAKE_INSTALL_PREFIX=../stage1.install '-DLLVM_TARGETS_TO_BUILD='"'"'AArch64'"'"'' '-DLLVM_ENABLE_PROJECTS=llvm;clang;clang-tools-extra'

We build with clang 15.0.0 release. Though I guess it's not a compiler related issue.

[isuckatcs]

I'm not sure if I can run clang targetting AArch64 on an X86 machine but I tried it regardless and I get compiler errors on startup, so I can't debug this issue at the moment.
Besided that I don't know why it times out on that buildbot. The build job that is performed on every revision passed without any problems.

[erichkeane]

In D135495#4151238, @isuckatcs wrote:

I'm not sure if I can run clang targetting AArch64 on an X86 machine but I tried it regardless and I get compiler errors on startup, so I can't debug this issue at the moment.
Besided that I don't know why it times out on that buildbot. The build job that is performed on every revision passed without any problems.

As long as you don't need the library (which, if you have a preprocessed file or a test, should absolutely not be necessary), you can just do the -cc1 option of -triple <whatever>, or the non- -cc1 option of -target <whatever>.

Both use basically the same options list, and that bot seems to be "aarch64-unknown-linux-gnu" (see under cmake-stage-1 the llvm host triple).

[isuckatcs]

Fixed the timed out test

[isuckatcs]

As long as you don't need the library (which, if you have a preprocessed file or a test, should absolutely not be necessary), you can just do the -cc1 option of -triple <whatever>, or the non- -cc1 option of -target <whatever>.

Both use basically the same options list, and that bot seems to be "aarch64-unknown-linux-gnu" (see under cmake-stage-1 the llvm host triple)

Thanks for this hint, it helped a lot!

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h

public:
     /// part is unknown and the rest is non-throwing the result will be
     /// unknown.
     ExceptionInfo &merge(const ExceptionInfo &Other);
 
     /// This method is useful in case 'catch' clauses are analyzed as it is
     /// 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.
     /// This includes explicit `std::bad_alloc` ignoring as separate option.
     ExceptionInfo &
     filterIgnoredExceptions(const llvm::StringSet<> &IgnoredTypes,
                             bool IgnoreBadAlloc);
 

clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp

else if (Other.Behaviour == State::Unknown && Behaviour == State::NotThrowing)
     Behaviour = State::Unknown;
 
   ContainsUnknown = ContainsUnknown || Other.ContainsUnknown;
   ThrownExceptions.insert(Other.ThrownExceptions.begin(),
                           Other.ThrownExceptions.end());
   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)
     return false;
 
   return !DerivedClass->forallBases(
       [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)
     ThrownExceptions.erase(T);
 
   reevaluateBehaviour();
   return TypesToDelete.size() > 0;
 }
for (unsigned I = 0; I < Try->getNumHandlers(); ++I) {
                            ->getPointeeType()
                            ->getUnqualifiedDesugaredType();
         }
 
         // If the caught exception will catch multiple previously potential
         // 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(),
                                                    CaughtExceptions, CallStack);
           Results.merge(Rethrown);
         }
       }
     }

clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp

try {
     if (n) throw 1;
     throw 1.1;
   } catch(int &) {
   } catch(...) {
     throw;
   }
 }
 
+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 {};
 
 void throw_derived_catch_base() noexcept {
   // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base' which should not throw exceptions
   try {
     throw derived();
   } catch(base &) {
   }
 }
 
+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 {
     try {
       if (n) throw 1;
       throw 1.1;
     } catch(int &) {
     }