summaryrefslogtreecommitdiffstats
path: root/clang/lib/Sema/SemaExprCXX.cpp
diff options
context:
space:
mode:
authorNico Weber <nicolasweber@gmx.de>2016-01-15 21:45:31 +0000
committerNico Weber <nicolasweber@gmx.de>2016-01-15 21:45:31 +0000
commit5a9259caa9b78d098ace55703db69beadacdfcce (patch)
tree55224854401d1238e30fba380483c43ed9a63ee5 /clang/lib/Sema/SemaExprCXX.cpp
parent851da71c8fec4ab7d5a3461d486ddbf83fcc91ec (diff)
downloadbcm5719-llvm-5a9259caa9b78d098ace55703db69beadacdfcce.tar.gz
bcm5719-llvm-5a9259caa9b78d098ace55703db69beadacdfcce.zip
Make -Wdelete-non-virtual-dtor warn on explicit `a->~A()` dtor calls too.
-Wdelete-non-virtual-dtor warns if A is a type with virtual functions but without virtual dtor has its constructor called via `delete a`. This makes the warning also fire if the dtor is called via `a->~A()`. This would've found a security bug in Chromium at compile time. Fixes PR26137. To fix the warning, add a virtual destructor, make the class final, or remove its other virtual methods. If you want to silence the warning, there's also a fixit that shows how: test.cc:12:3: warning: destructor called on 'B' ... [-Wdelete-non-virtual-dtor] b->~B(); ^ test.cc:12:6: note: qualify call to silence this warning b->~B(); ^ B:: http://reviews.llvm.org/D16206 llvm-svn: 257939
Diffstat (limited to 'clang/lib/Sema/SemaExprCXX.cpp')
-rw-r--r--clang/lib/Sema/SemaExprCXX.cpp67
1 files changed, 43 insertions, 24 deletions
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 38fbea18d79..5517994c484 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -2765,30 +2765,10 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
return ExprError();
}
- // C++ [expr.delete]p3:
- // In the first alternative (delete object), if the static type of the
- // object to be deleted is different from its dynamic type, the static
- // type shall be a base class of the dynamic type of the object to be
- // deleted and the static type shall have a virtual destructor or the
- // behavior is undefined.
- //
- // Note: a final class cannot be derived from, no issue there
- if (PointeeRD->isPolymorphic() && !PointeeRD->hasAttr<FinalAttr>()) {
- CXXDestructorDecl *dtor = PointeeRD->getDestructor();
- if (dtor && !dtor->isVirtual()) {
- if (PointeeRD->isAbstract()) {
- // If the class is abstract, we warn by default, because we're
- // sure the code has undefined behavior.
- Diag(StartLoc, diag::warn_delete_abstract_non_virtual_dtor)
- << PointeeElem;
- } else if (!ArrayForm) {
- // Otherwise, if this is not an array delete, it's a bit suspect,
- // but not necessarily wrong.
- Diag(StartLoc, diag::warn_delete_non_virtual_dtor) << PointeeElem;
- }
- }
- }
-
+ CheckVirtualDtorCall(PointeeRD->getDestructor(), StartLoc,
+ /*IsDelete=*/true, /*CallCanBeVirtual=*/true,
+ /*WarnOnNonAbstractTypes=*/!ArrayForm,
+ SourceLocation());
}
if (!OperatorDelete)
@@ -2817,6 +2797,45 @@ Sema::ActOnCXXDelete(SourceLocation StartLoc, bool UseGlobal,
return Result;
}
+void Sema::CheckVirtualDtorCall(CXXDestructorDecl *dtor, SourceLocation Loc,
+ bool IsDelete, bool CallCanBeVirtual,
+ bool WarnOnNonAbstractTypes,
+ SourceLocation DtorLoc) {
+ if (!dtor || dtor->isVirtual() || !CallCanBeVirtual)
+ return;
+
+ // C++ [expr.delete]p3:
+ // In the first alternative (delete object), if the static type of the
+ // object to be deleted is different from its dynamic type, the static
+ // type shall be a base class of the dynamic type of the object to be
+ // deleted and the static type shall have a virtual destructor or the
+ // behavior is undefined.
+ //
+ const CXXRecordDecl *PointeeRD = dtor->getParent();
+ // Note: a final class cannot be derived from, no issue there
+ if (!PointeeRD->isPolymorphic() || PointeeRD->hasAttr<FinalAttr>())
+ return;
+
+ QualType ClassType = dtor->getThisType(Context)->getPointeeType();
+ if (PointeeRD->isAbstract()) {
+ // If the class is abstract, we warn by default, because we're
+ // sure the code has undefined behavior.
+ Diag(Loc, diag::warn_delete_abstract_non_virtual_dtor) << (IsDelete ? 0 : 1)
+ << ClassType;
+ } else if (WarnOnNonAbstractTypes) {
+ // Otherwise, if this is not an array delete, it's a bit suspect,
+ // but not necessarily wrong.
+ Diag(Loc, diag::warn_delete_non_virtual_dtor) << (IsDelete ? 0 : 1)
+ << ClassType;
+ }
+ if (!IsDelete) {
+ std::string TypeStr;
+ ClassType.getAsStringInternal(TypeStr, getPrintingPolicy());
+ Diag(DtorLoc, diag::note_delete_non_virtual)
+ << FixItHint::CreateInsertion(DtorLoc, TypeStr + "::");
+ }
+}
+
/// \brief Check the use of the given variable as a C++ condition in an if,
/// while, do-while, or switch statement.
ExprResult Sema::CheckConditionVariable(VarDecl *ConditionVar,
OpenPOWER on IntegriCloud