Skip to content

Commit 4e24765

Browse files
hanickadotyuxuanchen1997
authored andcommitted
[clang-tidy] bugprone-exception-escape didn't detech catching of an exception with pointer type by void * exception handler (#99773)
Summary: 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 <[email protected]> Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251102
1 parent a81d9de commit 4e24765

File tree

3 files changed

+27
-5
lines changed

3 files changed

+27
-5
lines changed

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,10 @@ bool isStandardPointerConvertible(QualType From, QualType To) {
141141
if (RD->isCompleteDefinition() &&
142142
isBaseOf(From->getPointeeType().getTypePtr(),
143143
To->getPointeeType().getTypePtr())) {
144-
return true;
144+
// If B is an inaccessible or ambiguous base class of D, a program
145+
// that necessitates this conversion is ill-formed
146+
return isUnambiguousPublicBaseClass(From->getPointeeType().getTypePtr(),
147+
To->getPointeeType().getTypePtr());
145148
}
146149
}
147150

@@ -375,10 +378,7 @@ bool ExceptionAnalyzer::ExceptionInfo::filterByCatch(
375378
isPointerOrPointerToMember(ExceptionCanTy->getTypePtr())) {
376379
// A standard pointer conversion not involving conversions to pointers to
377380
// private or protected or ambiguous classes ...
378-
if (isStandardPointerConvertible(ExceptionCanTy, HandlerCanTy) &&
379-
isUnambiguousPublicBaseClass(
380-
ExceptionCanTy->getTypePtr()->getPointeeType().getTypePtr(),
381-
HandlerCanTy->getTypePtr()->getPointeeType().getTypePtr())) {
381+
if (isStandardPointerConvertible(ExceptionCanTy, HandlerCanTy)) {
382382
TypesToDelete.push_back(ExceptionTy);
383383
}
384384
// A function pointer conversion ...

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,10 @@ Changes in existing checks
247247
where source is already a ``void``` pointer, making middle ``void`` pointer
248248
casts bug-free.
249249

250+
- Improved :doc:`bugprone-exception-escape
251+
<clang-tidy/checks/bugprone/exception-escape>` check to correctly detect exception
252+
handler of type ``CV void *`` as catching all ``CV`` compatible pointer types.
253+
250254
- Improved :doc:`bugprone-forwarding-reference-overload
251255
<clang-tidy/checks/bugprone/forwarding-reference-overload>`
252256
check to ignore deleted constructors which won't hide other overloads.

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -756,3 +756,21 @@ struct test_implicit_throw {
756756
};
757757

758758
}}
759+
760+
void pointer_exception_can_not_escape_with_const_void_handler() noexcept {
761+
// 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
762+
const int value = 42;
763+
try {
764+
throw &value;
765+
} catch (const void *) {
766+
}
767+
}
768+
769+
void pointer_exception_can_not_escape_with_void_handler() noexcept {
770+
// 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
771+
int value = 42;
772+
try {
773+
throw &value;
774+
} catch (void *) {
775+
}
776+
}

0 commit comments

Comments
 (0)