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 retarded 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 type = TT->getDecl()->getUnderlyingType().getTypePtr();
679 }
680 CountType(type,
681 trivial_member,
682 non_trivial_member,
683 templated_non_trivial_member);
684 break;
685 }
686 default: {
687 // Stupid assumption: anything we see that isn't the above is a POD
688 // or reference type.
689 (*trivial_member)++;
690 break;
691 }
692 }
693 }
694
695 // Check |record| for issues that are problematic for ref-counted types.
696 // Note that |record| may not be a ref-counted type, but a base class for
697 // a type that is.
698 // If there are issues, update |loc| with the SourceLocation of the issue
699 // and returns appropriately, or returns None if there are no issues.
700 // static
701 FindBadConstructsConsumer::RefcountIssue
CheckRecordForRefcountIssue(const CXXRecordDecl * record,SourceLocation & loc)702 FindBadConstructsConsumer::CheckRecordForRefcountIssue(
703 const CXXRecordDecl* record,
704 SourceLocation& loc) {
705 if (!record->hasUserDeclaredDestructor()) {
706 loc = record->getLocation();
707 return ImplicitDestructor;
708 }
709
710 if (CXXDestructorDecl* dtor = record->getDestructor()) {
711 if (dtor->getAccess() == AS_public) {
712 loc = dtor->getInnerLocStart();
713 return PublicDestructor;
714 }
715 }
716
717 return None;
718 }
719
720 // Returns true if |base| specifies one of the Chromium reference counted
721 // classes (base::RefCounted / base::RefCountedThreadSafe).
IsRefCounted(const CXXBaseSpecifier * base,CXXBasePath & path)722 bool FindBadConstructsConsumer::IsRefCounted(
723 const CXXBaseSpecifier* base,
724 CXXBasePath& path) {
725 FindBadConstructsConsumer* self = this;
726 const TemplateSpecializationType* base_type =
727 dyn_cast<TemplateSpecializationType>(
728 UnwrapType(base->getType().getTypePtr()));
729 if (!base_type) {
730 // Base-most definition is not a template, so this cannot derive from
731 // base::RefCounted. However, it may still be possible to use with a
732 // scoped_refptr<> and support ref-counting, so this is not a perfect
733 // guarantee of safety.
734 return false;
735 }
736
737 TemplateName name = base_type->getTemplateName();
738 if (TemplateDecl* decl = name.getAsTemplateDecl()) {
739 std::string base_name = decl->getNameAsString();
740
741 // Check for both base::RefCounted and base::RefCountedThreadSafe.
742 if (base_name.compare(0, 10, "RefCounted") == 0 &&
743 self->GetNamespace(decl) == "base") {
744 return true;
745 }
746 }
747
748 return false;
749 }
750
751 // Returns true if |base| specifies a class that has a public destructor,
752 // either explicitly or implicitly.
753 // static
HasPublicDtorCallback(const CXXBaseSpecifier * base,CXXBasePath & path,void * user_data)754 bool FindBadConstructsConsumer::HasPublicDtorCallback(
755 const CXXBaseSpecifier* base,
756 CXXBasePath& path,
757 void* user_data) {
758 // Only examine paths that have public inheritance, as they are the
759 // only ones which will result in the destructor potentially being
760 // exposed. This check is largely redundant, as Chromium code should be
761 // exclusively using public inheritance.
762 if (path.Access != AS_public)
763 return false;
764
765 CXXRecordDecl* record =
766 dyn_cast<CXXRecordDecl>(base->getType()->getAs<RecordType>()->getDecl());
767 SourceLocation unused;
768 return None != CheckRecordForRefcountIssue(record, unused);
769 }
770
771 // Outputs a C++ inheritance chain as a diagnostic aid.
PrintInheritanceChain(const CXXBasePath & path)772 void FindBadConstructsConsumer::PrintInheritanceChain(const CXXBasePath& path) {
773 for (CXXBasePath::const_iterator it = path.begin(); it != path.end(); ++it) {
774 diagnostic().Report(it->Base->getLocStart(), diag_note_inheritance_)
775 << it->Class << it->Base->getType();
776 }
777 }
778
DiagnosticForIssue(RefcountIssue issue)779 unsigned FindBadConstructsConsumer::DiagnosticForIssue(RefcountIssue issue) {
780 switch (issue) {
781 case ImplicitDestructor:
782 return diag_no_explicit_dtor_;
783 case PublicDestructor:
784 return diag_public_dtor_;
785 case None:
786 assert(false && "Do not call DiagnosticForIssue with issue None");
787 return 0;
788 }
789 assert(false);
790 return 0;
791 }
792
793 // Check |record| to determine if it has any problematic refcounting
794 // issues and, if so, print them as warnings/errors based on the current
795 // value of getErrorLevel().
796 //
797 // If |record| is a C++ class, and if it inherits from one of the Chromium
798 // ref-counting classes (base::RefCounted / base::RefCountedThreadSafe),
799 // ensure that there are no public destructors in the class hierarchy. This
800 // is to guard against accidentally stack-allocating a RefCounted class or
801 // sticking it in a non-ref-counted container (like std::unique_ptr<>).
CheckRefCountedDtors(SourceLocation record_location,CXXRecordDecl * record)802 void FindBadConstructsConsumer::CheckRefCountedDtors(
803 SourceLocation record_location,
804 CXXRecordDecl* record) {
805 // Skip anonymous structs.
806 if (record->getIdentifier() == NULL)
807 return;
808
809 // Determine if the current type is even ref-counted.
810 CXXBasePaths refcounted_path;
811 if (!record->lookupInBases(
812 [this](const CXXBaseSpecifier* base, CXXBasePath& path) {
813 return IsRefCounted(base, path);
814 },
815 refcounted_path)) {
816 return; // Class does not derive from a ref-counted base class.
817 }
818
819 // Easy check: Check to see if the current type is problematic.
820 SourceLocation loc;
821 RefcountIssue issue = CheckRecordForRefcountIssue(record, loc);
822 if (issue != None) {
823 diagnostic().Report(loc, DiagnosticForIssue(issue));
824 PrintInheritanceChain(refcounted_path.front());
825 return;
826 }
827 if (CXXDestructorDecl* dtor =
828 refcounted_path.begin()->back().Class->getDestructor()) {
829 if (dtor->getAccess() == AS_protected && !dtor->isVirtual()) {
830 loc = dtor->getInnerLocStart();
831 diagnostic().Report(loc, diag_protected_non_virtual_dtor_);
832 return;
833 }
834 }
835
836 // Long check: Check all possible base classes for problematic
837 // destructors. This checks for situations involving multiple
838 // inheritance, where the ref-counted class may be implementing an
839 // interface that has a public or implicit destructor.
840 //
841 // struct SomeInterface {
842 // virtual void DoFoo();
843 // };
844 //
845 // struct RefCountedInterface
846 // : public base::RefCounted<RefCountedInterface>,
847 // public SomeInterface {
848 // private:
849 // friend class base::Refcounted<RefCountedInterface>;
850 // virtual ~RefCountedInterface() {}
851 // };
852 //
853 // While RefCountedInterface is "safe", in that its destructor is
854 // private, it's possible to do the following "unsafe" code:
855 // scoped_refptr<RefCountedInterface> some_class(
856 // new RefCountedInterface);
857 // // Calls SomeInterface::~SomeInterface(), which is unsafe.
858 // delete static_cast<SomeInterface*>(some_class.get());
859 if (!options_.check_base_classes)
860 return;
861
862 // Find all public destructors. This will record the class hierarchy
863 // that leads to the public destructor in |dtor_paths|.
864 CXXBasePaths dtor_paths;
865 if (!record->lookupInBases(
866 [](const CXXBaseSpecifier* base, CXXBasePath& path) {
867 // TODO(thakis): Inline HasPublicDtorCallback() after clang roll.
868 return HasPublicDtorCallback(base, path, nullptr);
869 },
870 dtor_paths)) {
871 return;
872 }
873
874 for (CXXBasePaths::const_paths_iterator it = dtor_paths.begin();
875 it != dtor_paths.end();
876 ++it) {
877 // The record with the problem will always be the last record
878 // in the path, since it is the record that stopped the search.
879 const CXXRecordDecl* problem_record = dyn_cast<CXXRecordDecl>(
880 it->back().Base->getType()->getAs<RecordType>()->getDecl());
881
882 issue = CheckRecordForRefcountIssue(problem_record, loc);
883
884 if (issue == ImplicitDestructor) {
885 diagnostic().Report(record_location, diag_no_explicit_dtor_);
886 PrintInheritanceChain(refcounted_path.front());
887 diagnostic().Report(loc, diag_note_implicit_dtor_) << problem_record;
888 PrintInheritanceChain(*it);
889 } else if (issue == PublicDestructor) {
890 diagnostic().Report(record_location, diag_public_dtor_);
891 PrintInheritanceChain(refcounted_path.front());
892 diagnostic().Report(loc, diag_note_public_dtor_);
893 PrintInheritanceChain(*it);
894 }
895 }
896 }
897
898 // Check for any problems with WeakPtrFactory class members. This currently
899 // only checks that any WeakPtrFactory<T> member of T appears as the last
900 // data member in T. We could consider checking for bad uses of
901 // WeakPtrFactory to refer to other data members, but that would require
902 // looking at the initializer list in constructors to see what the factory
903 // points to.
904 // Note, if we later add other unrelated checks of data members, we should
905 // consider collapsing them in to one loop to avoid iterating over the data
906 // members more than once.
CheckWeakPtrFactoryMembers(SourceLocation record_location,CXXRecordDecl * record)907 void FindBadConstructsConsumer::CheckWeakPtrFactoryMembers(
908 SourceLocation record_location,
909 CXXRecordDecl* record) {
910 // Skip anonymous structs.
911 if (record->getIdentifier() == NULL)
912 return;
913
914 // Iterate through members of the class.
915 RecordDecl::field_iterator iter(record->field_begin()),
916 the_end(record->field_end());
917 SourceLocation weak_ptr_factory_location; // Invalid initially.
918 for (; iter != the_end; ++iter) {
919 const TemplateSpecializationType* template_spec_type =
920 iter->getType().getTypePtr()->getAs<TemplateSpecializationType>();
921 bool param_is_weak_ptr_factory_to_self = false;
922 if (template_spec_type) {
923 const TemplateDecl* template_decl =
924 template_spec_type->getTemplateName().getAsTemplateDecl();
925 if (template_decl && template_spec_type->getNumArgs() == 1) {
926 if (template_decl->getNameAsString().compare("WeakPtrFactory") == 0 &&
927 GetNamespace(template_decl) == "base") {
928 // Only consider WeakPtrFactory members which are specialized for the
929 // owning class.
930 const TemplateArgument& arg = template_spec_type->getArg(0);
931 if (arg.getAsType().getTypePtr()->getAsCXXRecordDecl() ==
932 record->getTypeForDecl()->getAsCXXRecordDecl()) {
933 if (!weak_ptr_factory_location.isValid()) {
934 // Save the first matching WeakPtrFactory member for the
935 // diagnostic.
936 weak_ptr_factory_location = iter->getLocation();
937 }
938 param_is_weak_ptr_factory_to_self = true;
939 }
940 }
941 }
942 }
943 // If we've already seen a WeakPtrFactory<OwningType> and this param is not
944 // one of those, it means there is at least one member after a factory.
945 if (weak_ptr_factory_location.isValid() &&
946 !param_is_weak_ptr_factory_to_self) {
947 diagnostic().Report(weak_ptr_factory_location,
948 diag_weak_ptr_factory_order_);
949 }
950 }
951 }
952
953 // Copied from BlinkGCPlugin, see crrev.com/1135333007
ParseFunctionTemplates(TranslationUnitDecl * decl)954 void FindBadConstructsConsumer::ParseFunctionTemplates(
955 TranslationUnitDecl* decl) {
956 if (!instance().getLangOpts().DelayedTemplateParsing)
957 return; // Nothing to do.
958
959 std::set<FunctionDecl*> late_parsed_decls = GetLateParsedFunctionDecls(decl);
960 clang::Sema& sema = instance().getSema();
961
962 for (const FunctionDecl* fd : late_parsed_decls) {
963 assert(fd->isLateTemplateParsed());
964
965 if (instance().getSourceManager().isInSystemHeader(
966 instance().getSourceManager().getSpellingLoc(fd->getLocation())))
967 continue;
968
969 // Parse and build AST for yet-uninstantiated template functions.
970 clang::LateParsedTemplate* lpt = sema.LateParsedTemplateMap[fd];
971 sema.LateTemplateParser(sema.OpaqueParser, *lpt);
972 }
973 }
974
CheckVarDecl(clang::VarDecl * var_decl)975 void FindBadConstructsConsumer::CheckVarDecl(clang::VarDecl* var_decl) {
976 if (!options_.check_auto_raw_pointer)
977 return;
978
979 // Check whether auto deduces to a raw pointer.
980 QualType non_reference_type = var_decl->getType().getNonReferenceType();
981 // We might have a case where the type is written as auto*, but the actual
982 // type is deduced to be an int**. For that reason, keep going down the
983 // pointee type until we get an 'auto' or a non-pointer type.
984 for (;;) {
985 const clang::AutoType* auto_type =
986 non_reference_type->getAs<clang::AutoType>();
987 if (auto_type) {
988 if (auto_type->isDeduced()) {
989 QualType deduced_type = auto_type->getDeducedType();
990 if (!deduced_type.isNull() && deduced_type->isPointerType() &&
991 !deduced_type->isFunctionPointerType()) {
992 // Check if we should even be considering this type (note that there
993 // should be fewer auto types than banned namespace/directory types,
994 // so check this last.
995 if (!InBannedNamespace(var_decl) &&
996 !InBannedDirectory(var_decl->getLocStart())) {
997 // The range starts from |var_decl|'s loc start, which is the
998 // beginning of the full expression defining this |var_decl|. It
999 // ends, however, where this |var_decl|'s type loc ends, since
1000 // that's the end of the type of |var_decl|.
1001 // Note that the beginning source location of type loc omits cv
1002 // qualifiers, which is why it's not a good candidate to use for the
1003 // start of the range.
1004 clang::SourceRange range(
1005 var_decl->getLocStart(),
1006 var_decl->getTypeSourceInfo()->getTypeLoc().getLocEnd());
1007 ReportIfSpellingLocNotIgnored(range.getBegin(),
1008 diag_auto_deduced_to_a_pointer_type_)
1009 << FixItHint::CreateReplacement(
1010 range,
1011 GetAutoReplacementTypeAsString(var_decl->getType()));
1012 }
1013 }
1014 }
1015 } else if (non_reference_type->isPointerType()) {
1016 non_reference_type = non_reference_type->getPointeeType();
1017 continue;
1018 }
1019 break;
1020 }
1021 }
1022
1023 } // namespace chrome_checker
1024