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