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