1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 2 // Use of this source code is governed by a BSD-style license that can be 3 // found in the LICENSE file. 4 5 #include "FindBadConstructsConsumer.h" 6 7 #include "clang/Frontend/CompilerInstance.h" 8 #include "clang/AST/Attr.h" 9 #include "clang/Lex/Lexer.h" 10 #include "clang/Sema/Sema.h" 11 #include "llvm/Support/raw_ostream.h" 12 13 using namespace clang; 14 15 namespace chrome_checker { 16 17 namespace { 18 19 const char kMethodRequiresOverride[] = 20 "[chromium-style] Overriding method must be marked with 'override' or " 21 "'final'."; 22 const char kRedundantVirtualSpecifier[] = 23 "[chromium-style] %0 is redundant; %1 implies %0."; 24 // http://llvm.org/bugs/show_bug.cgi?id=21051 has been filed to make this a 25 // Clang warning. 26 const char kBaseMethodVirtualAndFinal[] = 27 "[chromium-style] The virtual method does not override anything and is " 28 "final; consider making it non-virtual."; 29 const char kNoExplicitDtor[] = 30 "[chromium-style] Classes that are ref-counted should have explicit " 31 "destructors that are declared protected or private."; 32 const char kPublicDtor[] = 33 "[chromium-style] Classes that are ref-counted should have " 34 "destructors that are declared protected or private."; 35 const char kProtectedNonVirtualDtor[] = 36 "[chromium-style] Classes that are ref-counted and have non-private " 37 "destructors should declare their destructor virtual."; 38 const char kWeakPtrFactoryOrder[] = 39 "[chromium-style] WeakPtrFactory members which refer to their outer class " 40 "must be the last member in the outer class definition."; 41 const char kBadLastEnumValue[] = 42 "[chromium-style] _LAST/Last constants of enum types must have the maximal " 43 "value for any constant of that type."; 44 const char kAutoDeducedToAPointerType[] = 45 "[chromium-style] auto variable type must not deduce to a raw pointer " 46 "type."; 47 const char kNoteInheritance[] = "[chromium-style] %0 inherits from %1 here"; 48 const char kNoteImplicitDtor[] = 49 "[chromium-style] No explicit destructor for %0 defined"; 50 const char kNotePublicDtor[] = 51 "[chromium-style] Public destructor declared here"; 52 const char kNoteProtectedNonVirtualDtor[] = 53 "[chromium-style] Protected non-virtual destructor declared here"; 54 55 // Returns the underlying Type for |type| by expanding typedefs and removing 56 // any namespace qualifiers. This is similar to desugaring, except that for 57 // ElaboratedTypes, desugar will unwrap too much. 58 const Type* UnwrapType(const Type* type) { 59 if (const ElaboratedType* elaborated = dyn_cast<ElaboratedType>(type)) 60 return UnwrapType(elaborated->getNamedType().getTypePtr()); 61 if (const TypedefType* typedefed = dyn_cast<TypedefType>(type)) 62 return UnwrapType(typedefed->desugar().getTypePtr()); 63 return type; 64 } 65 66 bool IsGtestTestFixture(const CXXRecordDecl* decl) { 67 return decl->getQualifiedNameAsString() == "testing::Test"; 68 } 69 70 // Generates a fixit hint to remove the 'virtual' keyword. 71 // Unfortunately, there doesn't seem to be a good way to determine the source 72 // location of the 'virtual' keyword. It's available in Declarator, but that 73 // isn't accessible from the AST. So instead, make an educated guess that the 74 // first token is probably the virtual keyword. Strictly speaking, this doesn't 75 // have to be true, but it probably will be. 76 // TODO(dcheng): Add a warning to force virtual to always appear first ;-) 77 FixItHint FixItRemovalForVirtual(const SourceManager& manager, 78 const LangOptions& lang_opts, 79 const CXXMethodDecl* method) { 80 SourceRange range(method->getLocStart()); 81 // Get the spelling loc just in case it was expanded from a macro. 82 SourceRange spelling_range(manager.getSpellingLoc(range.getBegin())); 83 // Sanity check that the text looks like virtual. 84 StringRef text = clang::Lexer::getSourceText( 85 CharSourceRange::getTokenRange(spelling_range), manager, lang_opts); 86 if (text.trim() != "virtual") 87 return FixItHint(); 88 return FixItHint::CreateRemoval(range); 89 } 90 91 bool IsPodOrTemplateType(const CXXRecordDecl& record) { 92 return record.isPOD() || 93 record.getDescribedClassTemplate() || 94 record.getTemplateSpecializationKind() || 95 record.isDependentType(); 96 } 97 98 // Use a local RAV implementation to simply collect all FunctionDecls marked for 99 // late template parsing. This happens with the flag -fdelayed-template-parsing, 100 // which is on by default in MSVC-compatible mode. 101 std::set<FunctionDecl*> GetLateParsedFunctionDecls(TranslationUnitDecl* decl) { 102 struct Visitor : public RecursiveASTVisitor<Visitor> { 103 bool VisitFunctionDecl(FunctionDecl* function_decl) { 104 if (function_decl->isLateTemplateParsed()) 105 late_parsed_decls.insert(function_decl); 106 return true; 107 } 108 109 std::set<FunctionDecl*> late_parsed_decls; 110 } v; 111 v.TraverseDecl(decl); 112 return v.late_parsed_decls; 113 } 114 115 std::string GetAutoReplacementTypeAsString(QualType type) { 116 QualType non_reference_type = type.getNonReferenceType(); 117 if (!non_reference_type->isPointerType()) 118 return "auto"; 119 120 std::string result = 121 GetAutoReplacementTypeAsString(non_reference_type->getPointeeType()); 122 result += "*"; 123 if (non_reference_type.isLocalConstQualified()) 124 result += " const"; 125 if (non_reference_type.isLocalVolatileQualified()) 126 result += " volatile"; 127 if (type->isReferenceType() && !non_reference_type.isLocalConstQualified()) { 128 if (type->isLValueReferenceType()) 129 result += "&"; 130 else if (type->isRValueReferenceType()) 131 result += "&&"; 132 } 133 return result; 134 } 135 136 } // namespace 137 138 FindBadConstructsConsumer::FindBadConstructsConsumer(CompilerInstance& instance, 139 const Options& options) 140 : ChromeClassTester(instance, options) { 141 if (options.check_ipc) { 142 ipc_visitor_.reset(new CheckIPCVisitor(instance)); 143 } 144 145 // Messages for virtual method specifiers. 146 diag_method_requires_override_ = 147 diagnostic().getCustomDiagID(getErrorLevel(), kMethodRequiresOverride); 148 diag_redundant_virtual_specifier_ = 149 diagnostic().getCustomDiagID(getErrorLevel(), kRedundantVirtualSpecifier); 150 diag_base_method_virtual_and_final_ = 151 diagnostic().getCustomDiagID(getErrorLevel(), kBaseMethodVirtualAndFinal); 152 153 // Messages for destructors. 154 diag_no_explicit_dtor_ = 155 diagnostic().getCustomDiagID(getErrorLevel(), kNoExplicitDtor); 156 diag_public_dtor_ = 157 diagnostic().getCustomDiagID(getErrorLevel(), kPublicDtor); 158 diag_protected_non_virtual_dtor_ = 159 diagnostic().getCustomDiagID(getErrorLevel(), kProtectedNonVirtualDtor); 160 161 // Miscellaneous messages. 162 diag_weak_ptr_factory_order_ = 163 diagnostic().getCustomDiagID(getErrorLevel(), kWeakPtrFactoryOrder); 164 diag_bad_enum_last_value_ = 165 diagnostic().getCustomDiagID(getErrorLevel(), kBadLastEnumValue); 166 diag_auto_deduced_to_a_pointer_type_ = 167 diagnostic().getCustomDiagID(getErrorLevel(), kAutoDeducedToAPointerType); 168 169 // Registers notes to make it easier to interpret warnings. 170 diag_note_inheritance_ = 171 diagnostic().getCustomDiagID(DiagnosticsEngine::Note, kNoteInheritance); 172 diag_note_implicit_dtor_ = 173 diagnostic().getCustomDiagID(DiagnosticsEngine::Note, kNoteImplicitDtor); 174 diag_note_public_dtor_ = 175 diagnostic().getCustomDiagID(DiagnosticsEngine::Note, kNotePublicDtor); 176 diag_note_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID( 177 DiagnosticsEngine::Note, kNoteProtectedNonVirtualDtor); 178 } 179 180 void FindBadConstructsConsumer::Traverse(ASTContext& context) { 181 if (ipc_visitor_) { 182 ipc_visitor_->set_context(&context); 183 ParseFunctionTemplates(context.getTranslationUnitDecl()); 184 } 185 RecursiveASTVisitor::TraverseDecl(context.getTranslationUnitDecl()); 186 if (ipc_visitor_) ipc_visitor_->set_context(nullptr); 187 } 188 189 bool FindBadConstructsConsumer::TraverseDecl(Decl* decl) { 190 if (ipc_visitor_) ipc_visitor_->BeginDecl(decl); 191 bool result = RecursiveASTVisitor::TraverseDecl(decl); 192 if (ipc_visitor_) ipc_visitor_->EndDecl(); 193 return result; 194 } 195 196 bool FindBadConstructsConsumer::VisitTagDecl(clang::TagDecl* tag_decl) { 197 if (tag_decl->isCompleteDefinition()) 198 CheckTag(tag_decl); 199 return true; 200 } 201 202 bool FindBadConstructsConsumer::VisitTemplateSpecializationType( 203 TemplateSpecializationType* spec) { 204 if (ipc_visitor_) ipc_visitor_->VisitTemplateSpecializationType(spec); 205 return true; 206 } 207 208 bool FindBadConstructsConsumer::VisitCallExpr(CallExpr* call_expr) { 209 if (ipc_visitor_) ipc_visitor_->VisitCallExpr(call_expr); 210 return true; 211 } 212 213 bool FindBadConstructsConsumer::VisitVarDecl(clang::VarDecl* var_decl) { 214 CheckVarDecl(var_decl); 215 return true; 216 } 217 218 void FindBadConstructsConsumer::CheckChromeClass(SourceLocation record_location, 219 CXXRecordDecl* record) { 220 bool implementation_file = InImplementationFile(record_location); 221 222 if (!implementation_file) { 223 // Only check for "heavy" constructors/destructors in header files; 224 // within implementation files, there is no performance cost. 225 226 // If this is a POD or a class template or a type dependent on a 227 // templated class, assume there's no ctor/dtor/virtual method 228 // optimization that we should do. 229 if (!IsPodOrTemplateType(*record)) 230 CheckCtorDtorWeight(record_location, record); 231 } 232 233 bool warn_on_inline_bodies = !implementation_file; 234 // Check that all virtual methods are annotated with override or final. 235 // Note this could also apply to templates, but for some reason Clang 236 // does not always see the "override", so we get false positives. 237 // See http://llvm.org/bugs/show_bug.cgi?id=18440 and 238 // http://llvm.org/bugs/show_bug.cgi?id=21942 239 if (!IsPodOrTemplateType(*record)) 240 CheckVirtualMethods(record_location, record, warn_on_inline_bodies); 241 242 CheckRefCountedDtors(record_location, record); 243 244 CheckWeakPtrFactoryMembers(record_location, record); 245 } 246 247 void FindBadConstructsConsumer::CheckChromeEnum(SourceLocation enum_location, 248 EnumDecl* enum_decl) { 249 if (!options_.check_enum_last_value) 250 return; 251 252 bool got_one = false; 253 bool is_signed = false; 254 llvm::APSInt max_so_far; 255 EnumDecl::enumerator_iterator iter; 256 for (iter = enum_decl->enumerator_begin(); 257 iter != enum_decl->enumerator_end(); 258 ++iter) { 259 llvm::APSInt current_value = iter->getInitVal(); 260 if (!got_one) { 261 max_so_far = current_value; 262 is_signed = current_value.isSigned(); 263 got_one = true; 264 } else { 265 if (is_signed != current_value.isSigned()) { 266 // This only happens in some cases when compiling C (not C++) files, 267 // so it is OK to bail out here. 268 return; 269 } 270 if (current_value > max_so_far) 271 max_so_far = current_value; 272 } 273 } 274 for (iter = enum_decl->enumerator_begin(); 275 iter != enum_decl->enumerator_end(); 276 ++iter) { 277 std::string name = iter->getNameAsString(); 278 if (((name.size() > 4 && name.compare(name.size() - 4, 4, "Last") == 0) || 279 (name.size() > 5 && name.compare(name.size() - 5, 5, "_LAST") == 0)) && 280 iter->getInitVal() < max_so_far) { 281 diagnostic().Report(iter->getLocation(), diag_bad_enum_last_value_); 282 } 283 } 284 } 285 286 void FindBadConstructsConsumer::CheckCtorDtorWeight( 287 SourceLocation record_location, 288 CXXRecordDecl* record) { 289 // We don't handle anonymous structs. If this record doesn't have a 290 // name, it's of the form: 291 // 292 // struct { 293 // ... 294 // } name_; 295 if (record->getIdentifier() == NULL) 296 return; 297 298 // We don't handle unions. 299 if (record->isUnion()) 300 return; 301 302 // Skip records that derive from ignored base classes. 303 if (HasIgnoredBases(record)) 304 return; 305 306 // Count the number of templated base classes as a feature of whether the 307 // destructor can be inlined. 308 int templated_base_classes = 0; 309 for (CXXRecordDecl::base_class_const_iterator it = record->bases_begin(); 310 it != record->bases_end(); 311 ++it) { 312 if (it->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() == 313 TypeLoc::TemplateSpecialization) { 314 ++templated_base_classes; 315 } 316 } 317 318 // Count the number of trivial and non-trivial member variables. 319 int trivial_member = 0; 320 int non_trivial_member = 0; 321 int templated_non_trivial_member = 0; 322 for (RecordDecl::field_iterator it = record->field_begin(); 323 it != record->field_end(); 324 ++it) { 325 CountType(it->getType().getTypePtr(), 326 &trivial_member, 327 &non_trivial_member, 328 &templated_non_trivial_member); 329 } 330 331 // Check to see if we need to ban inlined/synthesized constructors. Note 332 // that the cutoffs here are kind of arbitrary. Scores over 10 break. 333 int dtor_score = 0; 334 // Deriving from a templated base class shouldn't be enough to trigger 335 // the ctor warning, but if you do *anything* else, it should. 336 // 337 // TODO(erg): This is motivated by templated base classes that don't have 338 // any data members. Somehow detect when templated base classes have data 339 // members and treat them differently. 340 dtor_score += templated_base_classes * 9; 341 // Instantiating a template is an insta-hit. 342 dtor_score += templated_non_trivial_member * 10; 343 // The fourth normal class member should trigger the warning. 344 dtor_score += non_trivial_member * 3; 345 346 int ctor_score = dtor_score; 347 // You should be able to have 9 ints before we warn you. 348 ctor_score += trivial_member; 349 350 if (ctor_score >= 10) { 351 if (!record->hasUserDeclaredConstructor()) { 352 emitWarning(record_location, 353 "Complex class/struct needs an explicit out-of-line " 354 "constructor."); 355 } else { 356 // Iterate across all the constructors in this file and yell if we 357 // find one that tries to be inline. 358 for (CXXRecordDecl::ctor_iterator it = record->ctor_begin(); 359 it != record->ctor_end(); 360 ++it) { 361 // The current check is buggy. An implicit copy constructor does not 362 // have an inline body, so this check never fires for classes with a 363 // user-declared out-of-line constructor. 364 if (it->hasInlineBody()) { 365 if (it->isCopyConstructor() && 366 !record->hasUserDeclaredCopyConstructor()) { 367 // In general, implicit constructors are generated on demand. But 368 // in the Windows component build, dllexport causes instantiation of 369 // the copy constructor which means that this fires on many more 370 // classes. For now, suppress this on dllexported classes. 371 // (This does mean that windows component builds will not emit this 372 // warning in some cases where it is emitted in other configs, but 373 // that's the better tradeoff at this point). 374 // TODO(dcheng): With the RecursiveASTVisitor, these warnings might 375 // be emitted on other platforms too, reevaluate if we want to keep 376 // surpressing this then http://crbug.com/467288 377 if (!record->hasAttr<DLLExportAttr>()) 378 emitWarning(record_location, 379 "Complex class/struct needs an explicit out-of-line " 380 "copy constructor."); 381 } else { 382 // See the comment in the previous branch about copy constructors. 383 // This does the same for implicit move constructors. 384 bool is_likely_compiler_generated_dllexport_move_ctor = 385 it->isMoveConstructor() && 386 !record->hasUserDeclaredMoveConstructor() && 387 record->hasAttr<DLLExportAttr>(); 388 if (!is_likely_compiler_generated_dllexport_move_ctor) 389 emitWarning(it->getInnerLocStart(), 390 "Complex constructor has an inlined body."); 391 } 392 } else if (it->isInlined() && !it->isInlineSpecified() && 393 !it->isDeleted() && (!it->isCopyOrMoveConstructor() || 394 it->isExplicitlyDefaulted())) { 395 // isInlined() is a more reliable check than hasInlineBody(), but 396 // unfortunately, it results in warnings for implicit copy/move 397 // constructors in the previously mentioned situation. To preserve 398 // compatibility with existing Chromium code, only warn if it's an 399 // explicitly defaulted copy or move constructor. 400 emitWarning(it->getInnerLocStart(), 401 "Complex constructor has an inlined body."); 402 } 403 } 404 } 405 } 406 407 // The destructor side is equivalent except that we don't check for 408 // trivial members; 20 ints don't need a destructor. 409 if (dtor_score >= 10 && !record->hasTrivialDestructor()) { 410 if (!record->hasUserDeclaredDestructor()) { 411 emitWarning(record_location, 412 "Complex class/struct needs an explicit out-of-line " 413 "destructor."); 414 } else if (CXXDestructorDecl* dtor = record->getDestructor()) { 415 if (dtor->isInlined() && !dtor->isInlineSpecified() && 416 !dtor->isDeleted()) { 417 emitWarning(dtor->getInnerLocStart(), 418 "Complex destructor has an inline body."); 419 } 420 } 421 } 422 } 423 424 bool FindBadConstructsConsumer::InTestingNamespace(const Decl* record) { 425 return GetNamespace(record).find("testing") != std::string::npos; 426 } 427 428 bool FindBadConstructsConsumer::IsMethodInBannedOrTestingNamespace( 429 const CXXMethodDecl* method) { 430 if (InBannedNamespace(method)) 431 return true; 432 for (CXXMethodDecl::method_iterator i = method->begin_overridden_methods(); 433 i != method->end_overridden_methods(); 434 ++i) { 435 const CXXMethodDecl* overridden = *i; 436 if (IsMethodInBannedOrTestingNamespace(overridden) || 437 // Provide an exception for ::testing::Test. gtest itself uses some 438 // magic to try to make sure SetUp()/TearDown() aren't capitalized 439 // incorrectly, but having the plugin enforce override is also nice. 440 (InTestingNamespace(overridden) && 441 !IsGtestTestFixture(overridden->getParent()))) { 442 return true; 443 } 444 } 445 446 return false; 447 } 448 449 SuppressibleDiagnosticBuilder 450 FindBadConstructsConsumer::ReportIfSpellingLocNotIgnored( 451 SourceLocation loc, 452 unsigned diagnostic_id) { 453 return SuppressibleDiagnosticBuilder( 454 &diagnostic(), loc, diagnostic_id, 455 InBannedDirectory(instance().getSourceManager().getSpellingLoc(loc))); 456 } 457 458 // Checks that virtual methods are correctly annotated, and have no body in a 459 // header file. 460 void FindBadConstructsConsumer::CheckVirtualMethods( 461 SourceLocation record_location, 462 CXXRecordDecl* record, 463 bool warn_on_inline_bodies) { 464 // Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a 465 // trick to get around that. If a class has member variables whose types are 466 // in the "testing" namespace (which is how gmock works behind the scenes), 467 // there's a really high chance we won't care about these errors 468 for (CXXRecordDecl::field_iterator it = record->field_begin(); 469 it != record->field_end(); 470 ++it) { 471 CXXRecordDecl* record_type = it->getTypeSourceInfo() 472 ->getTypeLoc() 473 .getTypePtr() 474 ->getAsCXXRecordDecl(); 475 if (record_type) { 476 if (InTestingNamespace(record_type)) { 477 return; 478 } 479 } 480 } 481 482 for (CXXRecordDecl::method_iterator it = record->method_begin(); 483 it != record->method_end(); 484 ++it) { 485 if (it->isCopyAssignmentOperator() || isa<CXXConstructorDecl>(*it)) { 486 // Ignore constructors and assignment operators. 487 } else if (isa<CXXDestructorDecl>(*it) && 488 !record->hasUserDeclaredDestructor()) { 489 // Ignore non-user-declared destructors. 490 } else if (!it->isVirtual()) { 491 continue; 492 } else { 493 CheckVirtualSpecifiers(*it); 494 if (warn_on_inline_bodies) 495 CheckVirtualBodies(*it); 496 } 497 } 498 } 499 500 // Makes sure that virtual methods use the most appropriate specifier. If a 501 // virtual method overrides a method from a base class, only the override 502 // specifier should be used. If the method should not be overridden by derived 503 // classes, only the final specifier should be used. 504 void FindBadConstructsConsumer::CheckVirtualSpecifiers( 505 const CXXMethodDecl* method) { 506 bool is_override = method->size_overridden_methods() > 0; 507 bool has_virtual = method->isVirtualAsWritten(); 508 OverrideAttr* override_attr = method->getAttr<OverrideAttr>(); 509 FinalAttr* final_attr = method->getAttr<FinalAttr>(); 510 511 if (IsMethodInBannedOrTestingNamespace(method)) 512 return; 513 514 SourceManager& manager = instance().getSourceManager(); 515 const LangOptions& lang_opts = instance().getLangOpts(); 516 517 // Complain if a method is annotated virtual && (override || final). 518 if (has_virtual && (override_attr || final_attr)) { 519 // ... but only if virtual does not originate in a macro from a banned file. 520 // Note this is just an educated guess: the assumption here is that any 521 // macro for declaring methods will probably be at the start of the method's 522 // source range. 523 ReportIfSpellingLocNotIgnored(method->getLocStart(), 524 diag_redundant_virtual_specifier_) 525 << "'virtual'" 526 << (override_attr ? static_cast<Attr*>(override_attr) : final_attr) 527 << FixItRemovalForVirtual(manager, lang_opts, method); 528 } 529 530 // Complain if a method is an override and is not annotated with override or 531 // final. 532 if (is_override && !override_attr && !final_attr) { 533 SourceRange range = method->getSourceRange(); 534 SourceLocation loc; 535 if (method->hasInlineBody()) { 536 loc = method->getBody()->getSourceRange().getBegin(); 537 } else { 538 loc = Lexer::getLocForEndOfToken(manager.getSpellingLoc(range.getEnd()), 539 0, manager, lang_opts); 540 // The original code used the ending source loc of TypeSourceInfo's 541 // TypeLoc. Unfortunately, this breaks down in the presence of attributes. 542 // Attributes often appear at the end of a TypeLoc, e.g. 543 // virtual ULONG __stdcall AddRef() 544 // has a TypeSourceInfo that looks something like: 545 // ULONG AddRef() __attribute(stdcall) 546 // so a fix-it insertion would be generated to insert 'override' after 547 // __stdcall in the code as written. 548 // While using the spelling loc of the CXXMethodDecl fixes attribute 549 // handling, it breaks handling of "= 0" and similar constructs.. To work 550 // around this, scan backwards in the source text for a '=' or ')' token 551 // and adjust the location as needed... 552 for (SourceLocation l = loc.getLocWithOffset(-1); 553 l != manager.getLocForStartOfFile(manager.getFileID(loc)); 554 l = l.getLocWithOffset(-1)) { 555 l = Lexer::GetBeginningOfToken(l, manager, lang_opts); 556 Token token; 557 // getRawToken() returns *true* on failure. In that case, just give up 558 // and don't bother generating a possibly incorrect fix-it. 559 if (Lexer::getRawToken(l, token, manager, lang_opts, true)) { 560 loc = SourceLocation(); 561 break; 562 } 563 if (token.is(tok::r_paren)) { 564 break; 565 } else if (token.is(tok::equal)) { 566 loc = l; 567 break; 568 } 569 } 570 } 571 // Again, only emit the warning if it doesn't originate from a macro in 572 // a system header. 573 if (loc.isValid()) { 574 ReportIfSpellingLocNotIgnored(loc, diag_method_requires_override_) 575 << FixItHint::CreateInsertion(loc, " override"); 576 } else { 577 ReportIfSpellingLocNotIgnored(range.getBegin(), 578 diag_method_requires_override_); 579 } 580 } 581 582 if (final_attr && override_attr) { 583 ReportIfSpellingLocNotIgnored(override_attr->getLocation(), 584 diag_redundant_virtual_specifier_) 585 << override_attr << final_attr 586 << FixItHint::CreateRemoval(override_attr->getRange()); 587 } 588 589 if (final_attr && !is_override) { 590 ReportIfSpellingLocNotIgnored(method->getLocStart(), 591 diag_base_method_virtual_and_final_) 592 << FixItRemovalForVirtual(manager, lang_opts, method) 593 << FixItHint::CreateRemoval(final_attr->getRange()); 594 } 595 } 596 597 void FindBadConstructsConsumer::CheckVirtualBodies( 598 const CXXMethodDecl* method) { 599 // Virtual methods should not have inline definitions beyond "{}". This 600 // only matters for header files. 601 if (method->hasBody() && method->hasInlineBody()) { 602 if (CompoundStmt* cs = dyn_cast<CompoundStmt>(method->getBody())) { 603 if (cs->size()) { 604 SourceLocation loc = cs->getLBracLoc(); 605 // CR_BEGIN_MSG_MAP_EX and BEGIN_SAFE_MSG_MAP_EX try to be compatible 606 // to BEGIN_MSG_MAP(_EX). So even though they are in chrome code, 607 // we can't easily fix them, so explicitly whitelist them here. 608 bool emit = true; 609 if (loc.isMacroID()) { 610 SourceManager& manager = instance().getSourceManager(); 611 if (InBannedDirectory(manager.getSpellingLoc(loc))) 612 emit = false; 613 else { 614 StringRef name = Lexer::getImmediateMacroName( 615 loc, manager, instance().getLangOpts()); 616 if (name == "CR_BEGIN_MSG_MAP_EX" || 617 name == "BEGIN_SAFE_MSG_MAP_EX") 618 emit = false; 619 } 620 } 621 if (emit) 622 emitWarning(loc, 623 "virtual methods with non-empty bodies shouldn't be " 624 "declared inline."); 625 } 626 } 627 } 628 } 629 630 void FindBadConstructsConsumer::CountType(const Type* type, 631 int* trivial_member, 632 int* non_trivial_member, 633 int* templated_non_trivial_member) { 634 switch (type->getTypeClass()) { 635 case Type::Record: { 636 auto* record_decl = type->getAsCXXRecordDecl(); 637 // Simplifying; the whole class isn't trivial if the dtor is, but 638 // we use this as a signal about complexity. 639 // Note that if a record doesn't have a definition, it doesn't matter how 640 // it's counted, since the translation unit will fail to build. In that 641 // case, just count it as a trivial member to avoid emitting warnings that 642 // might be spurious. 643 if (!record_decl->hasDefinition() || record_decl->hasTrivialDestructor()) 644 (*trivial_member)++; 645 else 646 (*non_trivial_member)++; 647 break; 648 } 649 case Type::TemplateSpecialization: { 650 TemplateName name = 651 dyn_cast<TemplateSpecializationType>(type)->getTemplateName(); 652 bool whitelisted_template = false; 653 654 // HACK: I'm at a loss about how to get the syntax checker to get 655 // whether a template is externed or not. For the first pass here, 656 // just do simple string comparisons. 657 if (TemplateDecl* decl = name.getAsTemplateDecl()) { 658 std::string base_name = decl->getNameAsString(); 659 if (base_name == "basic_string") 660 whitelisted_template = true; 661 } 662 663 if (whitelisted_template) 664 (*non_trivial_member)++; 665 else 666 (*templated_non_trivial_member)++; 667 break; 668 } 669 case Type::Elaborated: { 670 CountType(dyn_cast<ElaboratedType>(type)->getNamedType().getTypePtr(), 671 trivial_member, 672 non_trivial_member, 673 templated_non_trivial_member); 674 break; 675 } 676 case Type::Typedef: { 677 while (const TypedefType* TT = dyn_cast<TypedefType>(type)) { 678 if (auto* decl = TT->getDecl()) { 679 const std::string name = decl->getNameAsString(); 680 auto* context = decl->getDeclContext(); 681 if (name == "atomic_int" && context->isStdNamespace()) { 682 (*trivial_member)++; 683 return; 684 } 685 type = decl->getUnderlyingType().getTypePtr(); 686 } 687 } 688 CountType(type, 689 trivial_member, 690 non_trivial_member, 691 templated_non_trivial_member); 692 break; 693 } 694 default: { 695 // Stupid assumption: anything we see that isn't the above is a POD 696 // or reference type. 697 (*trivial_member)++; 698 break; 699 } 700 } 701 } 702 703 // Check |record| for issues that are problematic for ref-counted types. 704 // Note that |record| may not be a ref-counted type, but a base class for 705 // a type that is. 706 // If there are issues, update |loc| with the SourceLocation of the issue 707 // and returns appropriately, or returns None if there are no issues. 708 // static 709 FindBadConstructsConsumer::RefcountIssue 710 FindBadConstructsConsumer::CheckRecordForRefcountIssue( 711 const CXXRecordDecl* record, 712 SourceLocation& loc) { 713 if (!record->hasUserDeclaredDestructor()) { 714 loc = record->getLocation(); 715 return ImplicitDestructor; 716 } 717 718 if (CXXDestructorDecl* dtor = record->getDestructor()) { 719 if (dtor->getAccess() == AS_public) { 720 loc = dtor->getInnerLocStart(); 721 return PublicDestructor; 722 } 723 } 724 725 return None; 726 } 727 728 // Returns true if |base| specifies one of the Chromium reference counted 729 // classes (base::RefCounted / base::RefCountedThreadSafe). 730 bool FindBadConstructsConsumer::IsRefCounted( 731 const CXXBaseSpecifier* base, 732 CXXBasePath& path) { 733 FindBadConstructsConsumer* self = this; 734 const TemplateSpecializationType* base_type = 735 dyn_cast<TemplateSpecializationType>( 736 UnwrapType(base->getType().getTypePtr())); 737 if (!base_type) { 738 // Base-most definition is not a template, so this cannot derive from 739 // base::RefCounted. However, it may still be possible to use with a 740 // scoped_refptr<> and support ref-counting, so this is not a perfect 741 // guarantee of safety. 742 return false; 743 } 744 745 TemplateName name = base_type->getTemplateName(); 746 if (TemplateDecl* decl = name.getAsTemplateDecl()) { 747 std::string base_name = decl->getNameAsString(); 748 749 // Check for both base::RefCounted and base::RefCountedThreadSafe. 750 if (base_name.compare(0, 10, "RefCounted") == 0 && 751 self->GetNamespace(decl) == "base") { 752 return true; 753 } 754 } 755 756 return false; 757 } 758 759 // Returns true if |base| specifies a class that has a public destructor, 760 // either explicitly or implicitly. 761 // static 762 bool FindBadConstructsConsumer::HasPublicDtorCallback( 763 const CXXBaseSpecifier* base, 764 CXXBasePath& path, 765 void* user_data) { 766 // Only examine paths that have public inheritance, as they are the 767 // only ones which will result in the destructor potentially being 768 // exposed. This check is largely redundant, as Chromium code should be 769 // exclusively using public inheritance. 770 if (path.Access != AS_public) 771 return false; 772 773 CXXRecordDecl* record = 774 dyn_cast<CXXRecordDecl>(base->getType()->getAs<RecordType>()->getDecl()); 775 SourceLocation unused; 776 return None != CheckRecordForRefcountIssue(record, unused); 777 } 778 779 // Outputs a C++ inheritance chain as a diagnostic aid. 780 void FindBadConstructsConsumer::PrintInheritanceChain(const CXXBasePath& path) { 781 for (CXXBasePath::const_iterator it = path.begin(); it != path.end(); ++it) { 782 diagnostic().Report(it->Base->getLocStart(), diag_note_inheritance_) 783 << it->Class << it->Base->getType(); 784 } 785 } 786 787 unsigned FindBadConstructsConsumer::DiagnosticForIssue(RefcountIssue issue) { 788 switch (issue) { 789 case ImplicitDestructor: 790 return diag_no_explicit_dtor_; 791 case PublicDestructor: 792 return diag_public_dtor_; 793 case None: 794 assert(false && "Do not call DiagnosticForIssue with issue None"); 795 return 0; 796 } 797 assert(false); 798 return 0; 799 } 800 801 // Check |record| to determine if it has any problematic refcounting 802 // issues and, if so, print them as warnings/errors based on the current 803 // value of getErrorLevel(). 804 // 805 // If |record| is a C++ class, and if it inherits from one of the Chromium 806 // ref-counting classes (base::RefCounted / base::RefCountedThreadSafe), 807 // ensure that there are no public destructors in the class hierarchy. This 808 // is to guard against accidentally stack-allocating a RefCounted class or 809 // sticking it in a non-ref-counted container (like std::unique_ptr<>). 810 void FindBadConstructsConsumer::CheckRefCountedDtors( 811 SourceLocation record_location, 812 CXXRecordDecl* record) { 813 // Skip anonymous structs. 814 if (record->getIdentifier() == NULL) 815 return; 816 817 // Determine if the current type is even ref-counted. 818 CXXBasePaths refcounted_path; 819 if (!record->lookupInBases( 820 [this](const CXXBaseSpecifier* base, CXXBasePath& path) { 821 return IsRefCounted(base, path); 822 }, 823 refcounted_path)) { 824 return; // Class does not derive from a ref-counted base class. 825 } 826 827 // Easy check: Check to see if the current type is problematic. 828 SourceLocation loc; 829 RefcountIssue issue = CheckRecordForRefcountIssue(record, loc); 830 if (issue != None) { 831 diagnostic().Report(loc, DiagnosticForIssue(issue)); 832 PrintInheritanceChain(refcounted_path.front()); 833 return; 834 } 835 if (CXXDestructorDecl* dtor = 836 refcounted_path.begin()->back().Class->getDestructor()) { 837 if (dtor->getAccess() == AS_protected && !dtor->isVirtual()) { 838 loc = dtor->getInnerLocStart(); 839 diagnostic().Report(loc, diag_protected_non_virtual_dtor_); 840 return; 841 } 842 } 843 844 // Long check: Check all possible base classes for problematic 845 // destructors. This checks for situations involving multiple 846 // inheritance, where the ref-counted class may be implementing an 847 // interface that has a public or implicit destructor. 848 // 849 // struct SomeInterface { 850 // virtual void DoFoo(); 851 // }; 852 // 853 // struct RefCountedInterface 854 // : public base::RefCounted<RefCountedInterface>, 855 // public SomeInterface { 856 // private: 857 // friend class base::Refcounted<RefCountedInterface>; 858 // virtual ~RefCountedInterface() {} 859 // }; 860 // 861 // While RefCountedInterface is "safe", in that its destructor is 862 // private, it's possible to do the following "unsafe" code: 863 // scoped_refptr<RefCountedInterface> some_class( 864 // new RefCountedInterface); 865 // // Calls SomeInterface::~SomeInterface(), which is unsafe. 866 // delete static_cast<SomeInterface*>(some_class.get()); 867 if (!options_.check_base_classes) 868 return; 869 870 // Find all public destructors. This will record the class hierarchy 871 // that leads to the public destructor in |dtor_paths|. 872 CXXBasePaths dtor_paths; 873 if (!record->lookupInBases( 874 [](const CXXBaseSpecifier* base, CXXBasePath& path) { 875 // TODO(thakis): Inline HasPublicDtorCallback() after clang roll. 876 return HasPublicDtorCallback(base, path, nullptr); 877 }, 878 dtor_paths)) { 879 return; 880 } 881 882 for (CXXBasePaths::const_paths_iterator it = dtor_paths.begin(); 883 it != dtor_paths.end(); 884 ++it) { 885 // The record with the problem will always be the last record 886 // in the path, since it is the record that stopped the search. 887 const CXXRecordDecl* problem_record = dyn_cast<CXXRecordDecl>( 888 it->back().Base->getType()->getAs<RecordType>()->getDecl()); 889 890 issue = CheckRecordForRefcountIssue(problem_record, loc); 891 892 if (issue == ImplicitDestructor) { 893 diagnostic().Report(record_location, diag_no_explicit_dtor_); 894 PrintInheritanceChain(refcounted_path.front()); 895 diagnostic().Report(loc, diag_note_implicit_dtor_) << problem_record; 896 PrintInheritanceChain(*it); 897 } else if (issue == PublicDestructor) { 898 diagnostic().Report(record_location, diag_public_dtor_); 899 PrintInheritanceChain(refcounted_path.front()); 900 diagnostic().Report(loc, diag_note_public_dtor_); 901 PrintInheritanceChain(*it); 902 } 903 } 904 } 905 906 // Check for any problems with WeakPtrFactory class members. This currently 907 // only checks that any WeakPtrFactory<T> member of T appears as the last 908 // data member in T. We could consider checking for bad uses of 909 // WeakPtrFactory to refer to other data members, but that would require 910 // looking at the initializer list in constructors to see what the factory 911 // points to. 912 // Note, if we later add other unrelated checks of data members, we should 913 // consider collapsing them in to one loop to avoid iterating over the data 914 // members more than once. 915 void FindBadConstructsConsumer::CheckWeakPtrFactoryMembers( 916 SourceLocation record_location, 917 CXXRecordDecl* record) { 918 // Skip anonymous structs. 919 if (record->getIdentifier() == NULL) 920 return; 921 922 // Iterate through members of the class. 923 RecordDecl::field_iterator iter(record->field_begin()), 924 the_end(record->field_end()); 925 SourceLocation weak_ptr_factory_location; // Invalid initially. 926 for (; iter != the_end; ++iter) { 927 const TemplateSpecializationType* template_spec_type = 928 iter->getType().getTypePtr()->getAs<TemplateSpecializationType>(); 929 bool param_is_weak_ptr_factory_to_self = false; 930 if (template_spec_type) { 931 const TemplateDecl* template_decl = 932 template_spec_type->getTemplateName().getAsTemplateDecl(); 933 if (template_decl && template_spec_type->getNumArgs() == 1) { 934 if (template_decl->getNameAsString().compare("WeakPtrFactory") == 0 && 935 GetNamespace(template_decl) == "base") { 936 // Only consider WeakPtrFactory members which are specialized for the 937 // owning class. 938 const TemplateArgument& arg = template_spec_type->getArg(0); 939 if (arg.getAsType().getTypePtr()->getAsCXXRecordDecl() == 940 record->getTypeForDecl()->getAsCXXRecordDecl()) { 941 if (!weak_ptr_factory_location.isValid()) { 942 // Save the first matching WeakPtrFactory member for the 943 // diagnostic. 944 weak_ptr_factory_location = iter->getLocation(); 945 } 946 param_is_weak_ptr_factory_to_self = true; 947 } 948 } 949 } 950 } 951 // If we've already seen a WeakPtrFactory<OwningType> and this param is not 952 // one of those, it means there is at least one member after a factory. 953 if (weak_ptr_factory_location.isValid() && 954 !param_is_weak_ptr_factory_to_self) { 955 diagnostic().Report(weak_ptr_factory_location, 956 diag_weak_ptr_factory_order_); 957 } 958 } 959 } 960 961 // Copied from BlinkGCPlugin, see crrev.com/1135333007 962 void FindBadConstructsConsumer::ParseFunctionTemplates( 963 TranslationUnitDecl* decl) { 964 if (!instance().getLangOpts().DelayedTemplateParsing) 965 return; // Nothing to do. 966 967 std::set<FunctionDecl*> late_parsed_decls = GetLateParsedFunctionDecls(decl); 968 clang::Sema& sema = instance().getSema(); 969 970 for (const FunctionDecl* fd : late_parsed_decls) { 971 assert(fd->isLateTemplateParsed()); 972 973 if (instance().getSourceManager().isInSystemHeader( 974 instance().getSourceManager().getSpellingLoc(fd->getLocation()))) 975 continue; 976 977 // Parse and build AST for yet-uninstantiated template functions. 978 clang::LateParsedTemplate* lpt = sema.LateParsedTemplateMap[fd].get(); 979 sema.LateTemplateParser(sema.OpaqueParser, *lpt); 980 } 981 } 982 983 void FindBadConstructsConsumer::CheckVarDecl(clang::VarDecl* var_decl) { 984 if (!options_.check_auto_raw_pointer) 985 return; 986 987 // Check whether auto deduces to a raw pointer. 988 QualType non_reference_type = var_decl->getType().getNonReferenceType(); 989 // We might have a case where the type is written as auto*, but the actual 990 // type is deduced to be an int**. For that reason, keep going down the 991 // pointee type until we get an 'auto' or a non-pointer type. 992 for (;;) { 993 const clang::AutoType* auto_type = 994 non_reference_type->getAs<clang::AutoType>(); 995 if (auto_type) { 996 if (auto_type->isDeduced()) { 997 QualType deduced_type = auto_type->getDeducedType(); 998 if (!deduced_type.isNull() && deduced_type->isPointerType() && 999 !deduced_type->isFunctionPointerType()) { 1000 // Check if we should even be considering this type (note that there 1001 // should be fewer auto types than banned namespace/directory types, 1002 // so check this last. 1003 if (!InBannedNamespace(var_decl) && 1004 !InBannedDirectory(var_decl->getLocStart())) { 1005 // The range starts from |var_decl|'s loc start, which is the 1006 // beginning of the full expression defining this |var_decl|. It 1007 // ends, however, where this |var_decl|'s type loc ends, since 1008 // that's the end of the type of |var_decl|. 1009 // Note that the beginning source location of type loc omits cv 1010 // qualifiers, which is why it's not a good candidate to use for the 1011 // start of the range. 1012 clang::SourceRange range( 1013 var_decl->getLocStart(), 1014 var_decl->getTypeSourceInfo()->getTypeLoc().getLocEnd()); 1015 ReportIfSpellingLocNotIgnored(range.getBegin(), 1016 diag_auto_deduced_to_a_pointer_type_) 1017 << FixItHint::CreateReplacement( 1018 range, 1019 GetAutoReplacementTypeAsString(var_decl->getType())); 1020 } 1021 } 1022 } 1023 } else if (non_reference_type->isPointerType()) { 1024 non_reference_type = non_reference_type->getPointeeType(); 1025 continue; 1026 } 1027 break; 1028 } 1029 } 1030 1031 } // namespace chrome_checker 1032