• 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 #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.
UnwrapType(const Type * type)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 
IsGtestTestFixture(const CXXRecordDecl * decl)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 ;-)
FixItRemovalForVirtual(const SourceManager & manager,const LangOptions & lang_opts,const CXXMethodDecl * method)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 
IsPodOrTemplateType(const CXXRecordDecl & record)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.
GetLateParsedFunctionDecls(TranslationUnitDecl * decl)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 
GetAutoReplacementTypeAsString(QualType type)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 
FindBadConstructsConsumer(CompilerInstance & instance,const Options & options)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 
Traverse(ASTContext & context)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 
TraverseDecl(Decl * decl)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 
VisitTagDecl(clang::TagDecl * tag_decl)196 bool FindBadConstructsConsumer::VisitTagDecl(clang::TagDecl* tag_decl) {
197   if (tag_decl->isCompleteDefinition())
198     CheckTag(tag_decl);
199   return true;
200 }
201 
VisitTemplateSpecializationType(TemplateSpecializationType * spec)202 bool FindBadConstructsConsumer::VisitTemplateSpecializationType(
203     TemplateSpecializationType* spec) {
204   if (ipc_visitor_) ipc_visitor_->VisitTemplateSpecializationType(spec);
205   return true;
206 }
207 
VisitCallExpr(CallExpr * call_expr)208 bool FindBadConstructsConsumer::VisitCallExpr(CallExpr* call_expr) {
209   if (ipc_visitor_) ipc_visitor_->VisitCallExpr(call_expr);
210   return true;
211 }
212 
VisitVarDecl(clang::VarDecl * var_decl)213 bool FindBadConstructsConsumer::VisitVarDecl(clang::VarDecl* var_decl) {
214   CheckVarDecl(var_decl);
215   return true;
216 }
217 
CheckChromeClass(SourceLocation record_location,CXXRecordDecl * record)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 
CheckChromeEnum(SourceLocation enum_location,EnumDecl * enum_decl)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 
CheckCtorDtorWeight(SourceLocation record_location,CXXRecordDecl * record)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 
InTestingNamespace(const Decl * record)424 bool FindBadConstructsConsumer::InTestingNamespace(const Decl* record) {
425   return GetNamespace(record).find("testing") != std::string::npos;
426 }
427 
IsMethodInBannedOrTestingNamespace(const CXXMethodDecl * method)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
ReportIfSpellingLocNotIgnored(SourceLocation loc,unsigned diagnostic_id)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.
CheckVirtualMethods(SourceLocation record_location,CXXRecordDecl * record,bool warn_on_inline_bodies)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.
CheckVirtualSpecifiers(const CXXMethodDecl * method)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 
CheckVirtualBodies(const CXXMethodDecl * method)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 
CountType(const Type * type,int * trivial_member,int * non_trivial_member,int * templated_non_trivial_member)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
CheckRecordForRefcountIssue(const CXXRecordDecl * record,SourceLocation & loc)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).
IsRefCounted(const CXXBaseSpecifier * base,CXXBasePath & path)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
HasPublicDtorCallback(const CXXBaseSpecifier * base,CXXBasePath & path,void * user_data)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.
PrintInheritanceChain(const CXXBasePath & path)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 
DiagnosticForIssue(RefcountIssue issue)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<>).
CheckRefCountedDtors(SourceLocation record_location,CXXRecordDecl * record)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.
CheckWeakPtrFactoryMembers(SourceLocation record_location,CXXRecordDecl * record)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
ParseFunctionTemplates(TranslationUnitDecl * decl)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 
CheckVarDecl(clang::VarDecl * var_decl)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