Author: Hana Dusíková Date: 2024-07-22T20:49:25+02:00 New Revision: 52dd4dbb1ad30ed057c532148cc95f34efe34aa0
URL: https://github.com/llvm/llvm-project/commit/52dd4dbb1ad30ed057c532148cc95f34efe34aa0 DIFF: https://github.com/llvm/llvm-project/commit/52dd4dbb1ad30ed057c532148cc95f34efe34aa0.diff LOG: [clang-tidy] `bugprone-exception-escape` didn't detech catching of an exception with pointer type by `void *` exception handler (#99773) As in title, code which checks eligibility of exceptions with pointer types to be handled by exception handler of type `void *` disallowed this case. It was working like this: ```c++ if (isStandardPointerConvertible(ExceptionCanTy, HandlerCanTy) && isUnambiguousPublicBaseClass( ExceptionCanTy->getTypePtr()->getPointeeType().getTypePtr(), HandlerCanTy->getTypePtr()->getPointeeType().getTypePtr())) { ``` but in `isUnambiguousPublicBaseClass` there was code which looked for definitions: ```c++ bool isUnambiguousPublicBaseClass(const Type *DerivedType, const Type *BaseType) { const auto *DerivedClass = DerivedType->getCanonicalTypeUnqualified()->getAsCXXRecordDecl(); const auto *BaseClass = BaseType->getCanonicalTypeUnqualified()->getAsCXXRecordDecl(); if (!DerivedClass || !BaseClass) return false; ``` This code disallowed usage of `void *` type which was already correctly detected in `isStandardPointerConvertible`. AFAIK this seems like misinterpretation of specification: > 14.4 Handling an exception > a standard [pointer conversion](https://eel.is/c++draft/conv.ptr) not involving conversions to pointers to private or protected or ambiguous classes (https://eel.is/c++draft/except.handle#3.3.1) and > 7.3.12 Pointer conversions > ... If B is an inaccessible ([[class.access]](https://eel.is/c++draft/class.access)) or ambiguous ([[class.member.lookup]](https://eel.is/c++draft/class.member.lookup)) base class of D, a program that necessitates this conversion is ill-formed[.](https://eel.is/c++draft/conv.ptr#3.sentence-2) ... (https://eel.is/c++draft/conv.ptr#3) 14.4 is carving out private, protected, and ambiguous base classes, but they are already carved out in 7.3.12 and implemented in `isStandardPointerConvertible` --------- Co-authored-by: Piotr Zegar <m...@piotrzegar.pl> Added: Modified: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp index 6ae46e2b1262a..9bfb7e2677533 100644 --- a/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp +++ b/clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp @@ -141,7 +141,10 @@ bool isStandardPointerConvertible(QualType From, QualType To) { if (RD->isCompleteDefinition() && isBaseOf(From->getPointeeType().getTypePtr(), To->getPointeeType().getTypePtr())) { - return true; + // If B is an inaccessible or ambiguous base class of D, a program + // that necessitates this conversion is ill-formed + return isUnambiguousPublicBaseClass(From->getPointeeType().getTypePtr(), + To->getPointeeType().getTypePtr()); } } @@ -375,10 +378,7 @@ bool ExceptionAnalyzer::ExceptionInfo::filterByCatch( isPointerOrPointerToMember(ExceptionCanTy->getTypePtr())) { // A standard pointer conversion not involving conversions to pointers to // private or protected or ambiguous classes ... - if (isStandardPointerConvertible(ExceptionCanTy, HandlerCanTy) && - isUnambiguousPublicBaseClass( - ExceptionCanTy->getTypePtr()->getPointeeType().getTypePtr(), - HandlerCanTy->getTypePtr()->getPointeeType().getTypePtr())) { + if (isStandardPointerConvertible(ExceptionCanTy, HandlerCanTy)) { TypesToDelete.push_back(ExceptionTy); } // A function pointer conversion ... diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 70fc91fc0f6d1..0b2c04c23761c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -247,6 +247,10 @@ Changes in existing checks where source is already a ``void``` pointer, making middle ``void`` pointer casts bug-free. +- Improved :doc:`bugprone-exception-escape + <clang-tidy/checks/bugprone/exception-escape>` check to correctly detect exception + handler of type ``CV void *`` as catching all ``CV`` compatible pointer types. + - Improved :doc:`bugprone-forwarding-reference-overload <clang-tidy/checks/bugprone/forwarding-reference-overload>` check to ignore deleted constructors which won't hide other overloads. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp index f5e74df1621ce..26c443b139629 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp @@ -756,3 +756,21 @@ struct test_implicit_throw { }; }} + +void pointer_exception_can_not_escape_with_const_void_handler() noexcept { + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'pointer_exception_can_not_escape_with_const_void_handler' which should not throw exceptions + const int value = 42; + try { + throw &value; + } catch (const void *) { + } +} + +void pointer_exception_can_not_escape_with_void_handler() noexcept { + // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'pointer_exception_can_not_escape_with_void_handler' which should not throw exceptions + int value = 42; + try { + throw &value; + } catch (void *) { + } +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
