diff options
author | Nico Weber <nicolasweber@gmx.de> | 2016-01-15 21:45:31 +0000 |
---|---|---|
committer | Nico Weber <nicolasweber@gmx.de> | 2016-01-15 21:45:31 +0000 |
commit | 5a9259caa9b78d098ace55703db69beadacdfcce (patch) | |
tree | 55224854401d1238e30fba380483c43ed9a63ee5 /clang/lib/Sema/SemaExprCXX.cpp | |
parent | 851da71c8fec4ab7d5a3461d486ddbf83fcc91ec (diff) | |
download | bcm5719-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.cpp | 67 |
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, |