• Home
  • Line#
  • Scopes#
  • Navigate#
  • Raw
  • Download
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 // This file defines a bunch of recurring problems in the Chromium C++ code.
6 //
7 // Checks that are implemented:
8 // - Constructors/Destructors should not be inlined if they are of a complex
9 //   class type.
10 // - Missing "virtual" keywords on methods that should be virtual.
11 // - Non-annotated overriding virtual methods.
12 // - Virtual methods with nonempty implementations in their headers.
13 // - Classes that derive from base::RefCounted / base::RefCountedThreadSafe
14 //   should have protected or private destructors.
15 // - WeakPtrFactory members that refer to their outer class should be the last
16 //   member.
17 
18 #include "clang/AST/ASTConsumer.h"
19 #include "clang/AST/AST.h"
20 #include "clang/AST/Attr.h"
21 #include "clang/AST/CXXInheritance.h"
22 #include "clang/AST/TypeLoc.h"
23 #include "clang/Basic/SourceManager.h"
24 #include "clang/Frontend/CompilerInstance.h"
25 #include "clang/Frontend/FrontendPluginRegistry.h"
26 #include "clang/Lex/Lexer.h"
27 #include "llvm/Support/raw_ostream.h"
28 
29 #include "ChromeClassTester.h"
30 
31 using namespace clang;
32 
33 namespace {
34 
35 const char kMethodRequiresOverride[] =
36     "[chromium-style] Overriding method must be marked with OVERRIDE.";
37 const char kMethodRequiresVirtual[] =
38     "[chromium-style] Overriding method must have \"virtual\" keyword.";
39 const char kNoExplicitDtor[] =
40     "[chromium-style] Classes that are ref-counted should have explicit "
41     "destructors that are declared protected or private.";
42 const char kPublicDtor[] =
43     "[chromium-style] Classes that are ref-counted should have "
44     "destructors that are declared protected or private.";
45 const char kProtectedNonVirtualDtor[] =
46     "[chromium-style] Classes that are ref-counted and have non-private "
47     "destructors should declare their destructor virtual.";
48 const char kWeakPtrFactoryOrder[] =
49     "[chromium-style] WeakPtrFactory members which refer to their outer class "
50     "must be the last member in the outer class definition.";
51 const char kNoteInheritance[] =
52     "[chromium-style] %0 inherits from %1 here";
53 const char kNoteImplicitDtor[] =
54     "[chromium-style] No explicit destructor for %0 defined";
55 const char kNotePublicDtor[] =
56     "[chromium-style] Public destructor declared here";
57 const char kNoteProtectedNonVirtualDtor[] =
58     "[chromium-style] Protected non-virtual destructor declared here";
59 
TypeHasNonTrivialDtor(const Type * type)60 bool TypeHasNonTrivialDtor(const Type* type) {
61   if (const CXXRecordDecl* cxx_r = type->getPointeeCXXRecordDecl())
62     return cxx_r->hasTrivialDestructor();
63 
64   return false;
65 }
66 
67 // Returns the underlying Type for |type| by expanding typedefs and removing
68 // any namespace qualifiers. This is similar to desugaring, except that for
69 // ElaboratedTypes, desugar will unwrap too much.
UnwrapType(const Type * type)70 const Type* UnwrapType(const Type* type) {
71   if (const ElaboratedType* elaborated = dyn_cast<ElaboratedType>(type))
72     return UnwrapType(elaborated->getNamedType().getTypePtr());
73   if (const TypedefType* typedefed = dyn_cast<TypedefType>(type))
74     return UnwrapType(typedefed->desugar().getTypePtr());
75   return type;
76 }
77 
78 struct FindBadConstructsOptions {
FindBadConstructsOptions__anon3856888b0111::FindBadConstructsOptions79   FindBadConstructsOptions() : check_base_classes(false),
80                                check_virtuals_in_implementations(true),
81                                check_url_directory(false),
82                                check_weak_ptr_factory_order(false) {
83   }
84   bool check_base_classes;
85   bool check_virtuals_in_implementations;
86   bool check_url_directory;
87   bool check_weak_ptr_factory_order;
88 };
89 
90 // Searches for constructs that we know we don't want in the Chromium code base.
91 class FindBadConstructsConsumer : public ChromeClassTester {
92  public:
FindBadConstructsConsumer(CompilerInstance & instance,const FindBadConstructsOptions & options)93   FindBadConstructsConsumer(CompilerInstance& instance,
94                             const FindBadConstructsOptions& options)
95       : ChromeClassTester(instance, options.check_url_directory),
96         options_(options) {
97     // Register warning/error messages.
98     diag_method_requires_override_ = diagnostic().getCustomDiagID(
99         getErrorLevel(), kMethodRequiresOverride);
100     diag_method_requires_virtual_ = diagnostic().getCustomDiagID(
101         getErrorLevel(), kMethodRequiresVirtual);
102     diag_no_explicit_dtor_ = diagnostic().getCustomDiagID(
103         getErrorLevel(), kNoExplicitDtor);
104     diag_public_dtor_ = diagnostic().getCustomDiagID(
105         getErrorLevel(), kPublicDtor);
106     diag_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
107         getErrorLevel(), kProtectedNonVirtualDtor);
108     diag_weak_ptr_factory_order_ = diagnostic().getCustomDiagID(
109         getErrorLevel(), kWeakPtrFactoryOrder);
110 
111     // Registers notes to make it easier to interpret warnings.
112     diag_note_inheritance_ = diagnostic().getCustomDiagID(
113         DiagnosticsEngine::Note, kNoteInheritance);
114     diag_note_implicit_dtor_ = diagnostic().getCustomDiagID(
115         DiagnosticsEngine::Note, kNoteImplicitDtor);
116     diag_note_public_dtor_ = diagnostic().getCustomDiagID(
117         DiagnosticsEngine::Note, kNotePublicDtor);
118     diag_note_protected_non_virtual_dtor_ = diagnostic().getCustomDiagID(
119         DiagnosticsEngine::Note, kNoteProtectedNonVirtualDtor);
120   }
121 
CheckChromeClass(SourceLocation record_location,CXXRecordDecl * record)122   virtual void CheckChromeClass(SourceLocation record_location,
123                                 CXXRecordDecl* record) {
124     bool implementation_file = InImplementationFile(record_location);
125 
126     if (!implementation_file) {
127       // Only check for "heavy" constructors/destructors in header files;
128       // within implementation files, there is no performance cost.
129       CheckCtorDtorWeight(record_location, record);
130     }
131 
132     if (!implementation_file || options_.check_virtuals_in_implementations) {
133       bool warn_on_inline_bodies = !implementation_file;
134 
135       // Check that all virtual methods are marked accordingly with both
136       // virtual and OVERRIDE.
137       CheckVirtualMethods(record_location, record, warn_on_inline_bodies);
138     }
139 
140     CheckRefCountedDtors(record_location, record);
141 
142     if (options_.check_weak_ptr_factory_order)
143       CheckWeakPtrFactoryMembers(record_location, record);
144   }
145 
146  private:
147   // The type of problematic ref-counting pattern that was encountered.
148   enum RefcountIssue {
149     None,
150     ImplicitDestructor,
151     PublicDestructor
152   };
153 
154   FindBadConstructsOptions options_;
155 
156   unsigned diag_method_requires_override_;
157   unsigned diag_method_requires_virtual_;
158   unsigned diag_no_explicit_dtor_;
159   unsigned diag_public_dtor_;
160   unsigned diag_protected_non_virtual_dtor_;
161   unsigned diag_weak_ptr_factory_order_;
162   unsigned diag_note_inheritance_;
163   unsigned diag_note_implicit_dtor_;
164   unsigned diag_note_public_dtor_;
165   unsigned diag_note_protected_non_virtual_dtor_;
166 
167   // Prints errors if the constructor/destructor weight is too heavy.
CheckCtorDtorWeight(SourceLocation record_location,CXXRecordDecl * record)168   void CheckCtorDtorWeight(SourceLocation record_location,
169                            CXXRecordDecl* record) {
170     // We don't handle anonymous structs. If this record doesn't have a
171     // name, it's of the form:
172     //
173     // struct {
174     //   ...
175     // } name_;
176     if (record->getIdentifier() == NULL)
177       return;
178 
179     // Count the number of templated base classes as a feature of whether the
180     // destructor can be inlined.
181     int templated_base_classes = 0;
182     for (CXXRecordDecl::base_class_const_iterator it = record->bases_begin();
183          it != record->bases_end(); ++it) {
184       if (it->getTypeSourceInfo()->getTypeLoc().getTypeLocClass() ==
185           TypeLoc::TemplateSpecialization) {
186         ++templated_base_classes;
187       }
188     }
189 
190     // Count the number of trivial and non-trivial member variables.
191     int trivial_member = 0;
192     int non_trivial_member = 0;
193     int templated_non_trivial_member = 0;
194     for (RecordDecl::field_iterator it = record->field_begin();
195          it != record->field_end(); ++it) {
196       CountType(it->getType().getTypePtr(),
197                 &trivial_member,
198                 &non_trivial_member,
199                 &templated_non_trivial_member);
200     }
201 
202     // Check to see if we need to ban inlined/synthesized constructors. Note
203     // that the cutoffs here are kind of arbitrary. Scores over 10 break.
204     int dtor_score = 0;
205     // Deriving from a templated base class shouldn't be enough to trigger
206     // the ctor warning, but if you do *anything* else, it should.
207     //
208     // TODO(erg): This is motivated by templated base classes that don't have
209     // any data members. Somehow detect when templated base classes have data
210     // members and treat them differently.
211     dtor_score += templated_base_classes * 9;
212     // Instantiating a template is an insta-hit.
213     dtor_score += templated_non_trivial_member * 10;
214     // The fourth normal class member should trigger the warning.
215     dtor_score += non_trivial_member * 3;
216 
217     int ctor_score = dtor_score;
218     // You should be able to have 9 ints before we warn you.
219     ctor_score += trivial_member;
220 
221     if (ctor_score >= 10) {
222       if (!record->hasUserDeclaredConstructor()) {
223         emitWarning(record_location,
224                     "Complex class/struct needs an explicit out-of-line "
225                     "constructor.");
226       } else {
227         // Iterate across all the constructors in this file and yell if we
228         // find one that tries to be inline.
229         for (CXXRecordDecl::ctor_iterator it = record->ctor_begin();
230              it != record->ctor_end(); ++it) {
231           if (it->hasInlineBody()) {
232             if (it->isCopyConstructor() &&
233                 !record->hasUserDeclaredCopyConstructor()) {
234               emitWarning(record_location,
235                           "Complex class/struct needs an explicit out-of-line "
236                           "copy constructor.");
237             } else {
238               emitWarning(it->getInnerLocStart(),
239                           "Complex constructor has an inlined body.");
240             }
241           }
242         }
243       }
244     }
245 
246     // The destructor side is equivalent except that we don't check for
247     // trivial members; 20 ints don't need a destructor.
248     if (dtor_score >= 10 && !record->hasTrivialDestructor()) {
249       if (!record->hasUserDeclaredDestructor()) {
250         emitWarning(
251             record_location,
252             "Complex class/struct needs an explicit out-of-line "
253             "destructor.");
254       } else if (CXXDestructorDecl* dtor = record->getDestructor()) {
255         if (dtor->hasInlineBody()) {
256           emitWarning(dtor->getInnerLocStart(),
257                       "Complex destructor has an inline body.");
258         }
259       }
260     }
261   }
262 
CheckVirtualMethod(const CXXMethodDecl * method,bool warn_on_inline_bodies)263   void CheckVirtualMethod(const CXXMethodDecl* method,
264                           bool warn_on_inline_bodies) {
265     if (!method->isVirtual())
266       return;
267 
268     if (!method->isVirtualAsWritten()) {
269       SourceLocation loc = method->getTypeSpecStartLoc();
270       if (isa<CXXDestructorDecl>(method))
271         loc = method->getInnerLocStart();
272       SourceManager& manager = instance().getSourceManager();
273       FullSourceLoc full_loc(loc, manager);
274       SourceLocation spelling_loc = manager.getSpellingLoc(loc);
275       diagnostic().Report(full_loc, diag_method_requires_virtual_)
276           << FixItHint::CreateInsertion(spelling_loc, "virtual ");
277     }
278 
279     // Virtual methods should not have inline definitions beyond "{}". This
280     // only matters for header files.
281     if (warn_on_inline_bodies && method->hasBody() &&
282         method->hasInlineBody()) {
283       if (CompoundStmt* cs = dyn_cast<CompoundStmt>(method->getBody())) {
284         if (cs->size()) {
285           emitWarning(
286               cs->getLBracLoc(),
287               "virtual methods with non-empty bodies shouldn't be "
288               "declared inline.");
289         }
290       }
291     }
292   }
293 
InTestingNamespace(const Decl * record)294   bool InTestingNamespace(const Decl* record) {
295     return GetNamespace(record).find("testing") != std::string::npos;
296   }
297 
IsMethodInBannedOrTestingNamespace(const CXXMethodDecl * method)298   bool IsMethodInBannedOrTestingNamespace(const CXXMethodDecl* method) {
299     if (InBannedNamespace(method))
300       return true;
301     for (CXXMethodDecl::method_iterator i = method->begin_overridden_methods();
302          i != method->end_overridden_methods();
303          ++i) {
304       const CXXMethodDecl* overridden = *i;
305       if (IsMethodInBannedOrTestingNamespace(overridden) ||
306           InTestingNamespace(overridden)) {
307         return true;
308       }
309     }
310 
311     return false;
312   }
313 
CheckOverriddenMethod(const CXXMethodDecl * method)314   void CheckOverriddenMethod(const CXXMethodDecl* method) {
315     if (!method->size_overridden_methods() || method->getAttr<OverrideAttr>())
316       return;
317 
318     if (isa<CXXDestructorDecl>(method) || method->isPure())
319       return;
320 
321     if (IsMethodInBannedOrTestingNamespace(method))
322       return;
323 
324     SourceManager& manager = instance().getSourceManager();
325     SourceRange type_info_range =
326         method->getTypeSourceInfo()->getTypeLoc().getSourceRange();
327     FullSourceLoc loc(type_info_range.getBegin(), manager);
328 
329     // Build the FixIt insertion point after the end of the method definition,
330     // including any const-qualifiers and attributes, and before the opening
331     // of the l-curly-brace (if inline) or the semi-color (if a declaration).
332     SourceLocation spelling_end =
333         manager.getSpellingLoc(type_info_range.getEnd());
334     if (spelling_end.isValid()) {
335       SourceLocation token_end = Lexer::getLocForEndOfToken(
336           spelling_end, 0, manager, LangOptions());
337       diagnostic().Report(token_end, diag_method_requires_override_)
338           << FixItHint::CreateInsertion(token_end, " OVERRIDE");
339     } else {
340       diagnostic().Report(loc, diag_method_requires_override_);
341     }
342   }
343 
344   // Makes sure there is a "virtual" keyword on virtual methods.
345   //
346   // Gmock objects trigger these for each MOCK_BLAH() macro used. So we have a
347   // trick to get around that. If a class has member variables whose types are
348   // in the "testing" namespace (which is how gmock works behind the scenes),
349   // there's a really high chance we won't care about these errors
CheckVirtualMethods(SourceLocation record_location,CXXRecordDecl * record,bool warn_on_inline_bodies)350   void CheckVirtualMethods(SourceLocation record_location,
351                            CXXRecordDecl* record,
352                            bool warn_on_inline_bodies) {
353     for (CXXRecordDecl::field_iterator it = record->field_begin();
354          it != record->field_end(); ++it) {
355       CXXRecordDecl* record_type =
356           it->getTypeSourceInfo()->getTypeLoc().getTypePtr()->
357           getAsCXXRecordDecl();
358       if (record_type) {
359         if (InTestingNamespace(record_type)) {
360           return;
361         }
362       }
363     }
364 
365     for (CXXRecordDecl::method_iterator it = record->method_begin();
366          it != record->method_end(); ++it) {
367       if (it->isCopyAssignmentOperator() || isa<CXXConstructorDecl>(*it)) {
368         // Ignore constructors and assignment operators.
369       } else if (isa<CXXDestructorDecl>(*it) &&
370           !record->hasUserDeclaredDestructor()) {
371         // Ignore non-user-declared destructors.
372       } else {
373         CheckVirtualMethod(*it, warn_on_inline_bodies);
374         CheckOverriddenMethod(*it);
375       }
376     }
377   }
378 
CountType(const Type * type,int * trivial_member,int * non_trivial_member,int * templated_non_trivial_member)379   void CountType(const Type* type,
380                  int* trivial_member,
381                  int* non_trivial_member,
382                  int* templated_non_trivial_member) {
383     switch (type->getTypeClass()) {
384       case Type::Record: {
385         // Simplifying; the whole class isn't trivial if the dtor is, but
386         // we use this as a signal about complexity.
387         if (TypeHasNonTrivialDtor(type))
388           (*trivial_member)++;
389         else
390           (*non_trivial_member)++;
391         break;
392       }
393       case Type::TemplateSpecialization: {
394         TemplateName name =
395             dyn_cast<TemplateSpecializationType>(type)->getTemplateName();
396         bool whitelisted_template = false;
397 
398         // HACK: I'm at a loss about how to get the syntax checker to get
399         // whether a template is exterened or not. For the first pass here,
400         // just do retarded string comparisons.
401         if (TemplateDecl* decl = name.getAsTemplateDecl()) {
402           std::string base_name = decl->getNameAsString();
403           if (base_name == "basic_string")
404             whitelisted_template = true;
405         }
406 
407         if (whitelisted_template)
408           (*non_trivial_member)++;
409         else
410           (*templated_non_trivial_member)++;
411         break;
412       }
413       case Type::Elaborated: {
414         CountType(
415             dyn_cast<ElaboratedType>(type)->getNamedType().getTypePtr(),
416             trivial_member, non_trivial_member, templated_non_trivial_member);
417         break;
418       }
419       case Type::Typedef: {
420         while (const TypedefType* TT = dyn_cast<TypedefType>(type)) {
421           type = TT->getDecl()->getUnderlyingType().getTypePtr();
422         }
423         CountType(type, trivial_member, non_trivial_member,
424                   templated_non_trivial_member);
425         break;
426       }
427       default: {
428         // Stupid assumption: anything we see that isn't the above is one of
429         // the 20 integer types.
430         (*trivial_member)++;
431         break;
432       }
433     }
434   }
435 
436   // Check |record| for issues that are problematic for ref-counted types.
437   // Note that |record| may not be a ref-counted type, but a base class for
438   // a type that is.
439   // If there are issues, update |loc| with the SourceLocation of the issue
440   // and returns appropriately, or returns None if there are no issues.
CheckRecordForRefcountIssue(const CXXRecordDecl * record,SourceLocation & loc)441   static RefcountIssue CheckRecordForRefcountIssue(
442       const CXXRecordDecl* record,
443       SourceLocation &loc) {
444     if (!record->hasUserDeclaredDestructor()) {
445       loc = record->getLocation();
446       return ImplicitDestructor;
447     }
448 
449     if (CXXDestructorDecl* dtor = record->getDestructor()) {
450       if (dtor->getAccess() == AS_public) {
451         loc = dtor->getInnerLocStart();
452         return PublicDestructor;
453       }
454     }
455 
456     return None;
457   }
458 
459   // Adds either a warning or error, based on the current handling of
460   // -Werror.
getErrorLevel()461   DiagnosticsEngine::Level getErrorLevel() {
462     return diagnostic().getWarningsAsErrors() ?
463         DiagnosticsEngine::Error : DiagnosticsEngine::Warning;
464   }
465 
466   // Returns true if |base| specifies one of the Chromium reference counted
467   // classes (base::RefCounted / base::RefCountedThreadSafe).
IsRefCountedCallback(const CXXBaseSpecifier * base,CXXBasePath & path,void * user_data)468   static bool IsRefCountedCallback(const CXXBaseSpecifier* base,
469                                    CXXBasePath& path,
470                                    void* user_data) {
471     FindBadConstructsConsumer* self =
472         static_cast<FindBadConstructsConsumer*>(user_data);
473 
474     const TemplateSpecializationType* base_type =
475         dyn_cast<TemplateSpecializationType>(
476             UnwrapType(base->getType().getTypePtr()));
477     if (!base_type) {
478       // Base-most definition is not a template, so this cannot derive from
479       // base::RefCounted. However, it may still be possible to use with a
480       // scoped_refptr<> and support ref-counting, so this is not a perfect
481       // guarantee of safety.
482       return false;
483     }
484 
485     TemplateName name = base_type->getTemplateName();
486     if (TemplateDecl* decl = name.getAsTemplateDecl()) {
487       std::string base_name = decl->getNameAsString();
488 
489       // Check for both base::RefCounted and base::RefCountedThreadSafe.
490       if (base_name.compare(0, 10, "RefCounted") == 0 &&
491           self->GetNamespace(decl) == "base") {
492         return true;
493       }
494     }
495 
496     return false;
497   }
498 
499   // Returns true if |base| specifies a class that has a public destructor,
500   // either explicitly or implicitly.
HasPublicDtorCallback(const CXXBaseSpecifier * base,CXXBasePath & path,void * user_data)501   static bool HasPublicDtorCallback(const CXXBaseSpecifier* base,
502                                     CXXBasePath& path,
503                                     void* user_data) {
504     // Only examine paths that have public inheritance, as they are the
505     // only ones which will result in the destructor potentially being
506     // exposed. This check is largely redundant, as Chromium code should be
507     // exclusively using public inheritance.
508     if (path.Access != AS_public)
509       return false;
510 
511     CXXRecordDecl* record = dyn_cast<CXXRecordDecl>(
512         base->getType()->getAs<RecordType>()->getDecl());
513     SourceLocation unused;
514     return None != CheckRecordForRefcountIssue(record, unused);
515   }
516 
517   // Outputs a C++ inheritance chain as a diagnostic aid.
PrintInheritanceChain(const CXXBasePath & path)518   void PrintInheritanceChain(const CXXBasePath& path) {
519     for (CXXBasePath::const_iterator it = path.begin(); it != path.end();
520          ++it) {
521       diagnostic().Report(it->Base->getLocStart(), diag_note_inheritance_)
522           << it->Class << it->Base->getType();
523     }
524   }
525 
DiagnosticForIssue(RefcountIssue issue)526   unsigned DiagnosticForIssue(RefcountIssue issue) {
527     switch (issue) {
528       case ImplicitDestructor:
529         return diag_no_explicit_dtor_;
530       case PublicDestructor:
531         return diag_public_dtor_;
532       case None:
533         assert(false && "Do not call DiagnosticForIssue with issue None");
534         return 0;
535     }
536     assert(false);
537     return 0;
538   }
539 
540   // Check |record| to determine if it has any problematic refcounting
541   // issues and, if so, print them as warnings/errors based on the current
542   // value of getErrorLevel().
543   //
544   // If |record| is a C++ class, and if it inherits from one of the Chromium
545   // ref-counting classes (base::RefCounted / base::RefCountedThreadSafe),
546   // ensure that there are no public destructors in the class hierarchy. This
547   // is to guard against accidentally stack-allocating a RefCounted class or
548   // sticking it in a non-ref-counted container (like scoped_ptr<>).
CheckRefCountedDtors(SourceLocation record_location,CXXRecordDecl * record)549   void CheckRefCountedDtors(SourceLocation record_location,
550                             CXXRecordDecl* record) {
551     // Skip anonymous structs.
552     if (record->getIdentifier() == NULL)
553       return;
554 
555     // Determine if the current type is even ref-counted.
556     CXXBasePaths refcounted_path;
557     if (!record->lookupInBases(
558             &FindBadConstructsConsumer::IsRefCountedCallback, this,
559             refcounted_path)) {
560       return;  // Class does not derive from a ref-counted base class.
561     }
562 
563     // Easy check: Check to see if the current type is problematic.
564     SourceLocation loc;
565     RefcountIssue issue = CheckRecordForRefcountIssue(record, loc);
566     if (issue != None) {
567       diagnostic().Report(loc, DiagnosticForIssue(issue));
568       PrintInheritanceChain(refcounted_path.front());
569       return;
570     }
571     if (CXXDestructorDecl* dtor =
572         refcounted_path.begin()->back().Class->getDestructor()) {
573       if (dtor->getAccess() == AS_protected &&
574           !dtor->isVirtual()) {
575         loc = dtor->getInnerLocStart();
576         diagnostic().Report(loc, diag_protected_non_virtual_dtor_);
577         return;
578       }
579     }
580 
581     // Long check: Check all possible base classes for problematic
582     // destructors. This checks for situations involving multiple
583     // inheritance, where the ref-counted class may be implementing an
584     // interface that has a public or implicit destructor.
585     //
586     // struct SomeInterface {
587     //   virtual void DoFoo();
588     // };
589     //
590     // struct RefCountedInterface
591     //    : public base::RefCounted<RefCountedInterface>,
592     //      public SomeInterface {
593     //  private:
594     //   friend class base::Refcounted<RefCountedInterface>;
595     //   virtual ~RefCountedInterface() {}
596     // };
597     //
598     // While RefCountedInterface is "safe", in that its destructor is
599     // private, it's possible to do the following "unsafe" code:
600     //   scoped_refptr<RefCountedInterface> some_class(
601     //       new RefCountedInterface);
602     //   // Calls SomeInterface::~SomeInterface(), which is unsafe.
603     //   delete static_cast<SomeInterface*>(some_class.get());
604     if (!options_.check_base_classes)
605       return;
606 
607     // Find all public destructors. This will record the class hierarchy
608     // that leads to the public destructor in |dtor_paths|.
609     CXXBasePaths dtor_paths;
610     if (!record->lookupInBases(
611             &FindBadConstructsConsumer::HasPublicDtorCallback, this,
612             dtor_paths)) {
613       return;
614     }
615 
616     for (CXXBasePaths::const_paths_iterator it = dtor_paths.begin();
617          it != dtor_paths.end(); ++it) {
618       // The record with the problem will always be the last record
619       // in the path, since it is the record that stopped the search.
620       const CXXRecordDecl* problem_record = dyn_cast<CXXRecordDecl>(
621           it->back().Base->getType()->getAs<RecordType>()->getDecl());
622 
623       issue = CheckRecordForRefcountIssue(problem_record, loc);
624 
625       if (issue == ImplicitDestructor) {
626         diagnostic().Report(record_location, diag_no_explicit_dtor_);
627         PrintInheritanceChain(refcounted_path.front());
628         diagnostic().Report(loc, diag_note_implicit_dtor_) << problem_record;
629         PrintInheritanceChain(*it);
630       } else if (issue == PublicDestructor) {
631         diagnostic().Report(record_location, diag_public_dtor_);
632         PrintInheritanceChain(refcounted_path.front());
633         diagnostic().Report(loc, diag_note_public_dtor_);
634         PrintInheritanceChain(*it);
635       }
636     }
637   }
638 
639   // Check for any problems with WeakPtrFactory class members. This currently
640   // only checks that any WeakPtrFactory<T> member of T appears as the last
641   // data member in T. We could consider checking for bad uses of
642   // WeakPtrFactory to refer to other data members, but that would require
643   // looking at the initializer list in constructors to see what the factory
644   // points to.
645   // Note, if we later add other unrelated checks of data members, we should
646   // consider collapsing them in to one loop to avoid iterating over the data
647   // members more than once.
CheckWeakPtrFactoryMembers(SourceLocation record_location,CXXRecordDecl * record)648   void CheckWeakPtrFactoryMembers(SourceLocation record_location,
649                                   CXXRecordDecl* record) {
650     // Skip anonymous structs.
651     if (record->getIdentifier() == NULL)
652       return;
653 
654     // Iterate through members of the class.
655     RecordDecl::field_iterator iter(record->field_begin()),
656                                the_end(record->field_end());
657     SourceLocation weak_ptr_factory_location;  // Invalid initially.
658     for (; iter != the_end; ++iter) {
659       // If we enter the loop but have already seen a matching WeakPtrFactory,
660       // it means there is at least one member after the factory.
661       if (weak_ptr_factory_location.isValid()) {
662         diagnostic().Report(weak_ptr_factory_location,
663                             diag_weak_ptr_factory_order_);
664       }
665       const TemplateSpecializationType* template_spec_type =
666           iter->getType().getTypePtr()->getAs<TemplateSpecializationType>();
667       if (template_spec_type) {
668         const TemplateDecl* template_decl =
669             template_spec_type->getTemplateName().getAsTemplateDecl();
670         if (template_decl && template_spec_type->getNumArgs() >= 1) {
671           if (template_decl->getNameAsString().compare("WeakPtrFactory") == 0 &&
672               GetNamespace(template_decl) == "base") {
673             const TemplateArgument& arg = template_spec_type->getArg(0);
674             if (arg.getAsType().getTypePtr()->getAsCXXRecordDecl() ==
675                 record->getTypeForDecl()->getAsCXXRecordDecl()) {
676               weak_ptr_factory_location = iter->getLocation();
677             }
678           }
679         }
680       }
681     }
682   }
683 };
684 
685 
686 class FindBadConstructsAction : public PluginASTAction {
687  public:
FindBadConstructsAction()688   FindBadConstructsAction() {
689   }
690 
691  protected:
692   // Overridden from PluginASTAction:
CreateASTConsumer(CompilerInstance & instance,llvm::StringRef ref)693   virtual ASTConsumer* CreateASTConsumer(CompilerInstance& instance,
694                                          llvm::StringRef ref) {
695     return new FindBadConstructsConsumer(instance, options_);
696   }
697 
ParseArgs(const CompilerInstance & instance,const std::vector<std::string> & args)698   virtual bool ParseArgs(const CompilerInstance& instance,
699                          const std::vector<std::string>& args) {
700     bool parsed = true;
701 
702     for (size_t i = 0; i < args.size() && parsed; ++i) {
703       if (args[i] == "skip-virtuals-in-implementations") {
704         // TODO(rsleevi): Remove this once http://crbug.com/115047 is fixed.
705         options_.check_virtuals_in_implementations = false;
706       } else if (args[i] == "check-base-classes") {
707         // TODO(rsleevi): Remove this once http://crbug.com/123295 is fixed.
708         options_.check_base_classes = true;
709       } else if (args[i] == "check-url-directory") {
710         // TODO(tfarina): Remove this once http://crbug.com/229660 is fixed.
711         options_.check_url_directory = true;
712       } else if (args[i] == "check-weak-ptr-factory-order") {
713         // TODO(dmichael): Remove this once http://crbug.com/303818 is fixed.
714         options_.check_weak_ptr_factory_order = true;
715       } else {
716         parsed = false;
717         llvm::errs() << "Unknown clang plugin argument: " << args[i] << "\n";
718       }
719     }
720 
721     return parsed;
722   }
723 
724  private:
725   FindBadConstructsOptions options_;
726 };
727 
728 }  // namespace
729 
730 static FrontendPluginRegistry::Add<FindBadConstructsAction>
731 X("find-bad-constructs", "Finds bad C++ constructs");
732