From 95263797a51384f6605950460ab7b89bcf603c95 Mon Sep 17 00:00:00 2001 From: Javier Alvarez Garcia Date: Wed, 20 Jul 2022 13:52:26 +0200 Subject: [PATCH] nullptr deref fixes --- .../daedalean/DerivedClassesCheck.cpp | 102 +++++++++++------- .../PreprocessingDirectivesCheck.cpp | 6 +- 2 files changed, 67 insertions(+), 41 deletions(-) diff --git a/clang-tools-extra/clang-tidy/daedalean/DerivedClassesCheck.cpp b/clang-tools-extra/clang-tidy/daedalean/DerivedClassesCheck.cpp index ba7c8a42299259..d02efb8500ff77 100644 --- a/clang-tools-extra/clang-tidy/daedalean/DerivedClassesCheck.cpp +++ b/clang-tools-extra/clang-tidy/daedalean/DerivedClassesCheck.cpp @@ -10,9 +10,9 @@ #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include #include #include -#include using namespace clang::ast_matchers; @@ -31,7 +31,10 @@ bool isInterface(const CXXRecordDecl *pDecl) { return false; } - for (const auto & m : pDecl->methods()) { + for (const auto *m : pDecl->methods()) { + if (m == nullptr) { + continue; + } if (llvm::isa(m) || m->isImplicit()) { continue; @@ -49,10 +52,9 @@ bool isFinal(const CXXRecordDecl *pDecl) { if (!Def) return false; return Def->hasAttr(); - } -} +} // namespace void DerivedClassesCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher(cxxRecordDecl(hasDefinition()).bind("x"), this); @@ -62,7 +64,9 @@ void DerivedClassesCheck::check(const MatchFinder::MatchResult &Result) { const auto *MatchedDecl = Result.Nodes.getNodeAs("x"); - if (!MatchedDecl || (MatchedDecl->methods().empty() && MatchedDecl->field_empty() && MatchedDecl->getNumBases() == 0)) { + if (!MatchedDecl || + (MatchedDecl->methods().empty() && MatchedDecl->field_empty() && + MatchedDecl->getNumBases() == 0)) { // Skip empty class return; } @@ -75,27 +79,37 @@ void DerivedClassesCheck::check(const MatchFinder::MatchResult &Result) { const auto dtor = MatchedDecl->getDestructor(); // 1. Each interface MUST have a virtual default destructor. if (!dtor || (dtor->isImplicit() && !dtor->isVirtual())) { - diag(MatchedDecl->getBraceRange().getBegin(), "Interface %0 must have virtual defaulted destructor") << MatchedDecl; + diag(MatchedDecl->getBraceRange().getBegin(), + "Interface %0 must have virtual defaulted destructor") + << MatchedDecl; } else { if (!dtor->isVirtual()) { diag(dtor->getLocation(), "Interface destructor must be virtual"); - diag(dtor->getLocation(), "Make destructor virtual", DiagnosticIDs::Note) << FixItHint::CreateInsertion(dtor->getLocation(), "virtual "); + diag(dtor->getLocation(), "Make destructor virtual", + DiagnosticIDs::Note) + << FixItHint::CreateInsertion(dtor->getLocation(), "virtual "); } if (!dtor->isDefaulted()) { diag(dtor->getLocation(), "Interface destructor must be defaulted"); if (const auto body = dtor->getBody(); body) { - diag(body->getBeginLoc(), "Make destructor default", DiagnosticIDs::Note) << FixItHint::CreateReplacement(body->getSourceRange(), "= default"); + diag(body->getBeginLoc(), "Make destructor default", + DiagnosticIDs::Note) + << FixItHint::CreateReplacement(body->getSourceRange(), + "= default"); } } } } else { - // 7. If a class contains at least one non-pure-virtual method other than destructor it MUST be declared final. + // 7. If a class contains at least one non-pure-virtual method other than + // destructor it MUST be declared final. if (!isFinal(MatchedDecl)) { - diag(MatchedDecl->getLocation(), "Non-interface class %0 must be final") << MatchedDecl; - diag(MatchedDecl->getLocation(), "Make class final", DiagnosticIDs::Note) << FixItHint::CreateInsertion(MatchedDecl->getLocation(), " final"); + diag(MatchedDecl->getLocation(), "Non-interface class %0 must be final") + << MatchedDecl; + diag(MatchedDecl->getLocation(), "Make class final", DiagnosticIDs::Note) + << FixItHint::CreateInsertion(MatchedDecl->getLocation(), " final"); } - for (const auto & m : MatchedDecl->methods()) { + for (const auto &m : MatchedDecl->methods()) { if (m->isImplicit()) { continue; } @@ -109,26 +123,26 @@ void DerivedClassesCheck::check(const MatchFinder::MatchResult &Result) { if (llvm::isa(m) && m->isDefaulted()) { continue; } - // 6. Virtual functions MUST contain exactly one of two specifiers: virtual for new function or final if function overrides method from base class. + // 6. Virtual functions MUST contain exactly one of two specifiers: + // virtual for new function or final if function overrides method from + // base class. if (!m->hasAttr()) { diag(m->getLocation(), "Implemented virtual methods must be final"); if (const auto attr = m->getAttr(); attr) { - diag(m->getLocation(), "Make method final", - DiagnosticIDs::Note) - << FixItHint::CreateReplacement(attr->getLocation(), - " final"); + diag(m->getLocation(), "Make method final", DiagnosticIDs::Note) + << FixItHint::CreateReplacement(attr->getLocation(), " final"); } else { - diag(m->getLocation(), "Make method final", - DiagnosticIDs::Note) - << FixItHint::CreateInsertion(m->getLocation(), - " final"); + diag(m->getLocation(), "Make method final", DiagnosticIDs::Note) + << FixItHint::CreateInsertion(m->getLocation(), " final"); } } } } - // 5. All entity names MUST be unique in all inherited interfaces. If two interfaces share the same methods they MUST extend the common base interface. - for (const auto & m : MatchedDecl->methods()) { + // 5. All entity names MUST be unique in all inherited interfaces. If two + // interfaces share the same methods they MUST extend the common base + // interface. + for (const auto &m : MatchedDecl->methods()) { if (m->isImplicit()) { continue; } @@ -157,11 +171,16 @@ void DerivedClassesCheck::check(const MatchFinder::MatchResult &Result) { } for (const auto base : MatchedDecl->bases()) { - // 2. Inheritance from base class with non-pure-virtual methods other than destructor and/or data members is forbidden. - const CXXRecordDecl * baseRecord = getRecordFromBase(base); + // 2. Inheritance from base class with non-pure-virtual methods other than + // destructor and/or data members is forbidden. + const CXXRecordDecl *baseRecord = getRecordFromBase(base); + if (baseRecord == nullptr) { + continue; + } if (!isInterface(baseRecord)) { - diag(base.getBeginLoc(), "Inheritance from non-interface type is forbidden"); + diag(base.getBeginLoc(), + "Inheritance from non-interface type is forbidden"); } else { // 3. Public inheritance MUST be used to implement interfaces. if (base.getAccessSpecifier() == AS_public) { @@ -176,7 +195,7 @@ void DerivedClassesCheck::check(const MatchFinder::MatchResult &Result) { std::unordered_map> baseReferences; walkBases(MatchedDecl, baseReferences); // 4. Base class MUST be declared virtual if it is in diamond inheritance. - for (const auto [base, derived]: baseReferences) { + for (const auto [base, derived] : baseReferences) { if (derived.size() <= 1) { continue; } @@ -194,39 +213,45 @@ void DerivedClassesCheck::check(const MatchFinder::MatchResult &Result) { } for (const auto d : derived) { - diag(d.base.getBeginLoc(), "Interface " + base + " must be implemented virtually because it's extended by multiple interfaces (" + allRefs.str() + ") with root type " + MatchedDecl->getQualifiedNameAsString()); + diag(d.base.getBeginLoc(), "Interface " + base + + " must be implemented virtually because " + "it's extended by multiple interfaces (" + + allRefs.str() + ") with root type " + + MatchedDecl->getQualifiedNameAsString()); diag(d.base.getBeginLoc(), "Make inheritance virtual", DiagnosticIDs::Note) - << FixItHint::CreateInsertion(d.base.getBeginLoc(),"virtual "); + << FixItHint::CreateInsertion(d.base.getBeginLoc(), "virtual "); } - } - } -const CXXRecordDecl * DerivedClassesCheck::getRecordFromBase(const CXXBaseSpecifier & base) { - if (const CXXRecordDecl * baseRecord = base.getType()->getAsCXXRecordDecl()) { +const CXXRecordDecl * +DerivedClassesCheck::getRecordFromBase(const CXXBaseSpecifier &base) { + if (const CXXRecordDecl *baseRecord = base.getType()->getAsCXXRecordDecl()) { return baseRecord; } - if (const auto spec = llvm::dyn_cast_or_null(base.getType().getTypePtr())) { + if (const auto spec = llvm::dyn_cast_or_null( + base.getType().getTypePtr())) { if (const auto cls = llvm::dyn_cast_or_null( spec->getTemplateName().getAsTemplateDecl())) { return cls->getTemplatedDecl(); } else { - diag(base.getBeginLoc(), - "Unable to get base template spec %0 as record") + diag(base.getBeginLoc(), "Unable to get base template spec %0 as record") << spec->getTemplateName(); return nullptr; } } else { diag(base.getBeginLoc(), - "Unable to get base of type %0 as record or template") << base.getType(); + "Unable to get base of type %0 as record or template") + << base.getType(); return nullptr; } } -void DerivedClassesCheck::walkBases(const CXXRecordDecl *pDecl, std::unordered_map> & references) { +void DerivedClassesCheck::walkBases( + const CXXRecordDecl *pDecl, + std::unordered_map> &references) { const auto ref = pDecl->getQualifiedNameAsString(); for (const auto base : pDecl->bases()) { const auto decl = getRecordFromBase(base); @@ -238,7 +263,6 @@ void DerivedClassesCheck::walkBases(const CXXRecordDecl *pDecl, std::unordered_m } } - } // namespace daedalean } // namespace tidy } // namespace clang diff --git a/clang-tools-extra/clang-tidy/daedalean/PreprocessingDirectivesCheck.cpp b/clang-tools-extra/clang-tidy/daedalean/PreprocessingDirectivesCheck.cpp index 225ddaa3262a27..5cac53e8336b36 100644 --- a/clang-tools-extra/clang-tidy/daedalean/PreprocessingDirectivesCheck.cpp +++ b/clang-tools-extra/clang-tidy/daedalean/PreprocessingDirectivesCheck.cpp @@ -71,8 +71,10 @@ class PreprocessingDirectivesPPCallbacks : public PPCallbacks { virtual void MacroUndefined(const Token &MacroNameTok, const MacroDefinition &MD, const MacroDirective *Undef) override { - Check.diag(Undef->getLocation(), - "Preprocessor directives must not be used"); + if (Undef) { + Check.diag(Undef->getLocation(), + "Preprocessor directives must not be used"); + } } virtual void If(SourceLocation Loc, SourceRange ConditionRange,