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 LangOptions &LangOpts); /// 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,20 +46,26 @@ return *this; } -static 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) { +bool ExceptionAnalyzer::ExceptionInfo::filterByCatch( + const Type *BaseClass, const LangOptions &LangOpts) { llvm::SmallVector TypesToDelete; for (const Type *T : ThrownExceptions) { - if (T == BaseClass || isBaseOf(T, BaseClass)) + if (T->isPointerType() && BaseClass->isPointerType()) { + + SplitQualType TPointeeTy = T->getPointeeType().split(); + SplitQualType BasePointeeTy = BaseClass->getPointeeType().split(); + + if (!TPointeeTy.Ty->isPointerType() && + (TPointeeTy.Ty == BasePointeeTy.Ty || + ASTContext::isBaseOf(TPointeeTy.Ty, BasePointeeTy.Ty)) && + (BasePointeeTy.Quals.isStrictSupersetOf(TPointeeTy.Quals) || + BasePointeeTy.Quals == TPointeeTy.Quals)) { + TypesToDelete.push_back(T); + } else if (ASTContext::isQualificationConvertiblePointer( + QualType(T, 0), QualType(BaseClass, 0), LangOpts)) { + TypesToDelete.push_back(T); + } + } else if (T == BaseClass || ASTContext::isBaseOf(T, BaseClass)) TypesToDelete.push_back(T); } @@ -186,11 +192,14 @@ ->getUnqualifiedDesugaredType(); } + const auto &LangOpts = + Catch->getExceptionDecl()->getASTContext().getLangOpts(); + // 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, LangOpts)) { ExceptionInfo::Throwables CaughtExceptions; CaughtExceptions.insert(CaughtType); ExceptionInfo Rethrown = throwsException(Catch->getHandlerBlock(), 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/include/clang/AST/ASTContext.h =================================================================== --- clang/include/clang/AST/ASTContext.h +++ clang/include/clang/AST/ASTContext.h @@ -2825,6 +2825,19 @@ bool propertyTypesAreCompatible(QualType, QualType); bool typesAreBlockPointerCompatible(QualType, QualType); + static bool isBaseOf(const Type *DerivedType, const Type *BaseType); + + // 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. + static bool isQualificationConvertiblePointer(QualType From, QualType To, + LangOptions LangOpts, + unsigned CurrentLevel = 0, + bool IsToConstSoFar = false); + bool isObjCIdType(QualType T) const { if (const auto *ET = dyn_cast(T)) T = ET->getNamedType(); Index: clang/lib/AST/ASTContext.cpp =================================================================== --- clang/lib/AST/ASTContext.cpp +++ clang/lib/AST/ASTContext.cpp @@ -13456,3 +13456,125 @@ CUIDHash = llvm::utohexstr(llvm::MD5Hash(LangOpts.CUID), /*LowerCase=*/true); return CUIDHash; } + +static QualType getPointeeOrArrayElementQualType(QualType T) { + if (T->isAnyPointerType()) + return T->getPointeeType(); + + if (T->isArrayType()) + return T->getAsArrayTypeUnsafe()->getElementType(); + + return T; +} + +bool ASTContext::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 ASTContext::isQualificationConvertiblePointer(QualType From, QualType To, + LangOptions LangOpts, + unsigned CurrentLevel, + bool IsToConstSoFar) { + if (CurrentLevel == 0) { + assert(To->isPointerType() || To->isMemberPointerType() && "The type we want to convert to is not a pointer type."); + + if(!To->isPointerType() && !To->isMemberPointerType()) + return false; + + if (!From->canDecayToPointerType() && !From->isPointerType() && !From->isMemberPointerType()) + return false; + + if (From->isMemberPointerType() != To->isMemberPointerType()) + return false; + + if (From->isMemberPointerType() && To->isMemberPointerType()) { + const auto *FromMember = cast(From); + const auto *ToMember = cast(To); + + if (!isBaseOf(ToMember->Class, FromMember->Class) && FromMember->Class != ToMember->Class) + return false; + + From = FromMember->PointeeType; + To = ToMember->PointeeType; + } else { + From = getPointeeOrArrayElementQualType(From); + To = getPointeeOrArrayElementQualType(To); + } + + return isQualificationConvertiblePointer(From, To, + LangOpts, CurrentLevel + 1, true); + } + + bool Convertible = true; + + if (From->isArrayType() && To->isArrayType()) { + if (From->isConstantArrayType() && To->isConstantArrayType()) { + Convertible &= + cast(From->getAsArrayTypeUnsafe())->getSize() == + cast(To->getAsArrayTypeUnsafe())->getSize(); + } else if (LangOpts.CPlusPlus20) { + // If there is an array type of unknown bound at some level + // (other than level zero) of From, there is an array type of + // unknown bound in the same level of To; (since C++20) + if (From->isIncompleteArrayType()) + Convertible &= To->isIncompleteArrayType(); + + // ... [or there is an array type of known bound in From and + // an array type of unknown bound in To (since C++20)] then + // there must be a 'const' at every single level (other than + // level zero) of To up until the current level. + if (From->isConstantArrayType() && To->isIncompleteArrayType()) + Convertible &= IsToConstSoFar; + } else { + return false; + } + + From = From->getAsArrayTypeUnsafe()->getElementType(); + To = To->getAsArrayTypeUnsafe()->getElementType(); + } + + // 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; + + // If the pointers don't have the same amount of levels, they are not + // convertible to each other. + if (!From->isPointerType() || !To->isPointerType()) { + CanQualType FromCanTy = From->getCanonicalTypeUnqualified(); + CanQualType ToCanTy = To->getCanonicalTypeUnqualified(); + + return Convertible && + (MoreCVQualified || From.getQualifiers() == To.getQualifiers()) && + (FromCanTy == ToCanTy || (CurrentLevel == 1 && isBaseOf(FromCanTy->getTypePtr(), ToCanTy->getTypePtr()))); + } + + // Note that in the C programming language, const/volatile can be added to the + // first level only. + bool CanBeCVQualified = LangOpts.CPlusPlus || CurrentLevel == 1; + + // If the current level (other than level zero) in From is 'const' qualified, + // the same level in To must also be 'const' qualified. + if (From.isConstQualified()) + Convertible &= To.isConstQualified() && CanBeCVQualified; + + // If the current level (other than level zero) in From is 'volatile' + // qualified, the same level in To must also be 'volatile' qualified. + if (From.isVolatileQualified()) + Convertible &= To.isVolatileQualified() && CanBeCVQualified; + + IsToConstSoFar &= To.isConstQualified(); + return Convertible && isQualificationConvertiblePointer( + From->getAs()->getPointeeType(), + To->getAs()->getPointeeType(), + LangOpts, CurrentLevel + 1, IsToConstSoFar); +} Index: clang/unittests/AST/CMakeLists.txt =================================================================== --- clang/unittests/AST/CMakeLists.txt +++ clang/unittests/AST/CMakeLists.txt @@ -34,6 +34,7 @@ StructuralEquivalenceTest.cpp TemplateNameTest.cpp TypePrinterTest.cpp + PointerConversionTest.cpp ) clang_target_link_libraries(ASTTests Index: clang/unittests/AST/PointerConversionTest.cpp =================================================================== --- /dev/null +++ clang/unittests/AST/PointerConversionTest.cpp @@ -0,0 +1,285 @@ +//===- unittests/AST/PointerConversionTest.cpp - Pointer conversion tests -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file contains tests for ASTContext::isQualificationConvertiblePointer(), +// which checks if one pointer can be converted to the other. +// +//===----------------------------------------------------------------------===// + +#include "gtest/gtest.h" + +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Tooling/Tooling.h" +#include "clang/Frontend/FrontendActions.h" + +namespace clang { + +auto DeclMatcher = ast_matchers::declaratorDecl(ast_matchers::unless(ast_matchers::parmVarDecl())).bind("id"); + +class TypeExtractor : public ast_matchers::MatchFinder::MatchCallback { + int MatchCount = 0; + +public: + QualType T1, T2; + + void run(const ast_matchers::MatchFinder::MatchResult &Result) override { + if (const auto *VD = Result.Nodes.getNodeAs("id")) { + if (MatchCount == 0) + T1 = VD->getType(); + else { + assert(VD->hasInit() && "Second declaration must be initialized because of compiler error messages."); + T2 = VD->getType(); + } + + ++MatchCount; + } else if (const auto *FD = + Result.Nodes.getNodeAs("id")) { + if (MatchCount == 0) + T1 = FD->getType(); + else + T2 = FD->getType(); + + ++MatchCount; + } + } +}; + +void prepareArgAndLangOptionsFromStd(LangStandard Std, std::string &Arg, LangOptions &Opts) { + Arg = "-std="; + Arg += Std.getName(); + + std::vector Tmp; + LangOptions::setLangDefaults(Opts, Std.Language, {}, Tmp, + Std.getLangKind(Std.getName())); +} + +bool CheckIfCompiles(llvm::StringRef Source, LangStandard Std) { + std::string StdArg; + LangOptions Opts; + prepareArgAndLangOptionsFromStd(Std, StdArg, Opts); + + return tooling::runToolOnCodeWithArgs(tooling::newFrontendActionFactory().get()->create(), Source, {StdArg}); +} + +bool CheckConvertible(llvm::StringRef Source, LangStandard Std) { + TypeExtractor Extractor; + ast_matchers::MatchFinder Finder; + + Finder.addMatcher(DeclMatcher, &Extractor); + std::unique_ptr Factory( + tooling::newFrontendActionFactory(&Finder)); + + std::string StdArg; + LangOptions Opts; + prepareArgAndLangOptionsFromStd(Std, StdArg, Opts); + + tooling::runToolOnCodeWithArgs(Factory->create(), Source, {StdArg}); + + return ASTContext::isQualificationConvertiblePointer(Extractor.T1, + Extractor.T2, Opts); +} + +#define TEST_CONVERSION(ASSERTION, SNIPPET, LANG) { const char* Snippet = SNIPPET; \ + ASSERTION(CheckConvertible(Snippet, LANG)); \ + ASSERTION(CheckIfCompiles(Snippet, LANG)); } + +const LangStandard & StdCXX98 = LangStandard::getLangStandardForKind(LangStandard::lang_cxx98); +const LangStandard & StdCXX17 = LangStandard::getLangStandardForKind(LangStandard::lang_cxx17); +const LangStandard & StdCXX20 = LangStandard::getLangStandardForKind(LangStandard::lang_cxx20); +const LangStandard & StdCXX2b = LangStandard::getLangStandardForKind(LangStandard::lang_cxx2b); + + +TEST(QualificationConversion, builtin) { + TEST_CONVERSION(ASSERT_FALSE, R"cpp(char **p; const char **p1 = p;)cpp", StdCXX98); +} + +TEST(QualificationConversion, builtin2) { + TEST_CONVERSION(ASSERT_TRUE, R"cpp(char **p; const char* const * p2 = p;)cpp", StdCXX98); +} + +TEST(QualificationConversion, builtin3) { + TEST_CONVERSION(ASSERT_TRUE, R"cpp(char **p; volatile char * const * p3 = p;)cpp", StdCXX98); +} + +TEST(QualificationConversion, builtin4) { + TEST_CONVERSION(ASSERT_TRUE, R"cpp(char **p; volatile const char* const* p4 = p;)cpp", StdCXX98); +} + +TEST(QualificationConversion, builtin5) { + TEST_CONVERSION(ASSERT_TRUE, R"cpp(double *a[2][3]; double const * const (*ap)[3] = a;)cpp", StdCXX98); +} + +TEST(QualificationConversion, builtin6) { + TEST_CONVERSION(ASSERT_FALSE,R"cpp(double *a[2][3]; double * const (*ap1)[] = a;)cpp", StdCXX17); + TEST_CONVERSION(ASSERT_TRUE,R"cpp(double *a[2][3]; double * const (*ap1)[] = a;)cpp", StdCXX20); + TEST_CONVERSION(ASSERT_TRUE,R"cpp(double *a[2][3]; double * const (*ap1)[] = a;)cpp", StdCXX2b); +} + +TEST(QualificationConversion, builtin7) { + TEST_CONVERSION(ASSERT_TRUE, R"cpp(int arr[3]; int *p = arr;)cpp", StdCXX98); +} + +TEST(QualificationConversion, builtin8) { + TEST_CONVERSION(ASSERT_FALSE, R"cpp(int arr[3][3]; int **p = arr;)cpp", StdCXX98); + TEST_CONVERSION(ASSERT_TRUE, R"cpp(int arr[3][3]; int (*p)[3] = arr;)cpp", StdCXX98); + + TEST_CONVERSION(ASSERT_FALSE, R"cpp(int arr[3][3]; int (*p)[2] = arr;)cpp",StdCXX98); + + TEST_CONVERSION(ASSERT_FALSE, R"cpp(int arr[3][3]; int (*p)[] = arr;)cpp",StdCXX98); + TEST_CONVERSION(ASSERT_TRUE, R"cpp(int arr[3][3]; int (*p)[] = arr;)cpp",StdCXX20); +} + +TEST(QualificationConversion, builtin9) { + TEST_CONVERSION(ASSERT_TRUE, + R"cpp(void foo(); void (*ptr)() = foo;)cpp", + StdCXX98); + + TEST_CONVERSION(ASSERT_TRUE, + R"cpp(void foo(); void (* const ptr)() = foo;)cpp", + StdCXX98); + + TEST_CONVERSION(ASSERT_TRUE, + R"cpp(void foo(); void (* volatile ptr)() = foo;)cpp", + StdCXX98); + + TEST_CONVERSION(ASSERT_TRUE, + R"cpp(void foo(); void (* const volatile ptr)() = foo;)cpp", + StdCXX98); +} + +TEST(QualificationConversion, builtin10) { + TEST_CONVERSION(ASSERT_FALSE, + R"cpp(const int* p; int* p2 = p;)cpp", + StdCXX98); + + TEST_CONVERSION(ASSERT_TRUE, + R"cpp(const int* p; const int* p2 = p;)cpp", + StdCXX98); +} + +TEST(QualificationConversion, derived) { + TEST_CONVERSION(ASSERT_TRUE, + R"cpp( + class A {}; + + A **p; + volatile const A* const* p4 = p; + )cpp", + StdCXX98); + + TEST_CONVERSION(ASSERT_FALSE, + R"cpp( + class A {}; + class B : public A {}; + + B **p; + volatile const A* const* p4 = p; + )cpp", + StdCXX98); + + TEST_CONVERSION(ASSERT_FALSE, + R"cpp( + class A {}; + class B : public A {}; + + A **p; + volatile const B* const* p4 = p; + )cpp", + StdCXX98); +} + +TEST(QualificationConversion, member) { + TEST_CONVERSION(ASSERT_TRUE, + R"cpp( + class A {}; + + char * A::*p; + volatile const char* const A::* p4 = p; + )cpp", + StdCXX98); + + TEST_CONVERSION(ASSERT_TRUE, + R"cpp( + class A {}; + class B : public A {}; + + char * B::*p; + volatile const char* const B::* p4 = p; + )cpp", + StdCXX98); + + TEST_CONVERSION(ASSERT_TRUE, + R"cpp( + class A {}; + class B : public A {}; + + int A::*p; + int B::* p2 = p; + )cpp", + StdCXX98); + + TEST_CONVERSION(ASSERT_FALSE, + R"cpp( + class A {}; + class B : public A {}; + + int B::*p; + int A::* p2 = p; + )cpp", + StdCXX98); + + TEST_CONVERSION(ASSERT_TRUE, + R"cpp( + class A {}; + class B : public A {}; + + char * A::*p; + volatile const char* const B::* p4 = p; + )cpp", + StdCXX98); + + TEST_CONVERSION(ASSERT_FALSE, + R"cpp( + class A {}; + class B : public A {}; + + char * B::*p; + volatile const char* const A::* p4 = p; + )cpp", + StdCXX98); +} + +TEST(QualificationConversion, function) { + TEST_CONVERSION(ASSERT_TRUE,R"cpp(void foo(int, char); void (*pfoo)(int, char) = foo;)cpp", StdCXX98); + TEST_CONVERSION(ASSERT_FALSE,R"cpp(void foo(int, char); void (*pfoo)() = foo;)cpp", StdCXX98); +} + +TEST(QualificationConversion, memberFunction) { + TEST_CONVERSION(ASSERT_TRUE, + R"cpp( + class A {}; + class B : public A {}; + + void (A::*p)(int, char); + void (B::*p2)(int, char) = p; + )cpp", + StdCXX98); + + TEST_CONVERSION(ASSERT_FALSE, + R"cpp( + class A {}; + class B : public A {}; + + void (B::*p)(int, char); + void (A::*p2)(int, char) = p; + )cpp", + StdCXX98); +} + +} // namespace clang