Chromium roll (https://codereview.chromium.org/976353002)

tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // This clang plugin checks various invariants of the Blink garbage
 // collection infrastructure.
 //
 // Errors are described at:
 // http://www.chromium.org/developers/blink-gc-plugin-errors
 
@@ skipping 297 matching lines @@
   bool dispatched_to_receiver() { return dispatched_to_receiver_; }
 
   bool VisitMemberExpr(MemberExpr* member) {
     if (CXXMethodDecl* fn = dyn_cast<CXXMethodDecl>(member->getMemberDecl())) {
       if (fn->getParent() == receiver_->record())
         dispatched_to_receiver_ = true;
     }
     return true;
   }
 
+  bool VisitUnresolvedMemberExpr(UnresolvedMemberExpr* member) {
+    for (Decl* decl : member->decls()) {
+      if (CXXMethodDecl* method = dyn_cast<CXXMethodDecl>(decl)) {
+        if (method->getParent() == receiver_->record() &&
+            Config::GetTraceMethodType(method) ==
+            Config::TRACE_AFTER_DISPATCH_METHOD) {
+          dispatched_to_receiver_ = true;
+          return true;
+        }
+      }
+    }
+    return true;
+  }
+
  private:
   RecordInfo* receiver_;
   bool dispatched_to_receiver_;
 };
 
 // This visitor checks a tracing method by traversing its body.
 // - A member field is considered traced if it is referenced in the body.
 // - A base is traced if a base-qualified call to a trace method is found.
 class CheckTraceVisitor : public RecursiveASTVisitor<CheckTraceVisitor> {
  public:
-  CheckTraceVisitor(CXXMethodDecl* trace, RecordInfo* info)
-      : trace_(trace), info_(info), delegates_to_traceimpl_(false) {}
+  CheckTraceVisitor(CXXMethodDecl* trace, RecordInfo* info, RecordCache* cache)
+      : trace_(trace),
+        info_(info),
+        cache_(cache),
+        delegates_to_traceimpl_(false) {
+  }
 
   bool delegates_to_traceimpl() const { return delegates_to_traceimpl_; }
 
   bool VisitMemberExpr(MemberExpr* member) {
     // In weak callbacks, consider any occurrence as a correct usage.
     // TODO: We really want to require that isAlive is checked on manually
     // processed weak fields.
     if (IsWeakCallback()) {
       if (FieldDecl* field = dyn_cast<FieldDecl>(member->getMemberDecl()))
         FoundField(field);
@@ skipping 23 matching lines @@
     if (call->getNumArgs() != 1)
       return true;
     Expr* arg = call->getArg(0);
 
     if (UnresolvedMemberExpr* expr = dyn_cast<UnresolvedMemberExpr>(callee)) {
       // This could be a trace call of a base class, as explained in the
       // comments of CheckTraceBaseCall().
       if (CheckTraceBaseCall(call))
         return true;
 
-      // If we find a call to registerWeakMembers which is unresolved we
-      // unsoundly consider all weak members as traced.
-      // TODO: Find out how to validate weak member tracing for unresolved call.
-      if (expr->getMemberName().getAsString() == kRegisterWeakMembersName) {
-        for (RecordInfo::Fields::iterator it = info_->GetFields().begin();
-             it != info_->GetFields().end();
-             ++it) {
-          if (it->second.edge()->IsWeakMember())
-            it->second.MarkTraced();
-        }
-      }
+      if (expr->getMemberName().getAsString() == kRegisterWeakMembersName)
+        MarkAllWeakMembersTraced();
 
       QualType base = expr->getBaseType();
       if (!base->isPointerType())
         return true;
       CXXRecordDecl* decl = base->getPointeeType()->getAsCXXRecordDecl();
       if (decl)
         CheckTraceFieldCall(expr->getMemberName().getAsString(), decl, arg);
       if (Config::IsTraceImplName(expr->getMemberName().getAsString()))
         delegates_to_traceimpl_ = true;
       return true;
     }
 
     if (CXXMemberCallExpr* expr = dyn_cast<CXXMemberCallExpr>(call)) {
       if (CheckTraceFieldCall(expr) || CheckRegisterWeakMembers(expr))
         return true;
 
       if (Config::IsTraceImplName(expr->getMethodDecl()->getNameAsString())) {
         delegates_to_traceimpl_ = true;
         return true;
       }
     }
 
     CheckTraceBaseCall(call);
     return true;
   }
 
  private:
+  bool IsTraceCallName(const std::string& name) {
+    if (trace_->getName() == kTraceImplName)
+      return name == kTraceName;
+    if (trace_->getName() == kTraceAfterDispatchImplName)
+      return name == kTraceAfterDispatchName;
+    // Currently, a manually dispatched class cannot have mixin bases (having
+    // one would add a vtable which we explicitly check against). This means
+    // that we can only make calls to a trace method of the same name. Revisit
+    // this if our mixin/vtable assumption changes.
+    return name == trace_->getName();
+  }
+
   CXXRecordDecl* GetDependentTemplatedDecl(CXXDependentScopeMemberExpr* expr) {
     NestedNameSpecifier* qual = expr->getQualifier();
     if (!qual)
       return 0;
 
     const Type* type = qual->getAsType();
     if (!type)
       return 0;
 
     return RecordInfo::GetDependentTemplatedDecl(*type);
   }
 
   void CheckCXXDependentScopeMemberExpr(CallExpr* call,
                                         CXXDependentScopeMemberExpr* expr) {
     string fn_name = expr->getMember().getAsString();
 
-    // Check for VisitorDispatcher::trace(field)
+    // Check for VisitorDispatcher::trace(field) and
+    // VisitorDispatcher::registerWeakMembers.
     if (!expr->isImplicitAccess()) {
       if (clang::DeclRefExpr* base_decl =
               clang::dyn_cast<clang::DeclRefExpr>(expr->getBase())) {
-        if (Config::IsVisitorDispatcherType(base_decl->getType()) &&
-            call->getNumArgs() == 1 && fn_name == kTraceName) {
-          FindFieldVisitor finder;
-          finder.TraverseStmt(call->getArg(0));
-          if (finder.field())
-            FoundField(finder.field());
+        if (Config::IsVisitorDispatcherType(base_decl->getType())) {
+          if (call->getNumArgs() == 1 && fn_name == kTraceName) {
+            FindFieldVisitor finder;
+            finder.TraverseStmt(call->getArg(0));
+            if (finder.field())
+              FoundField(finder.field());
 
-          return;
+            return;
+          } else if (call->getNumArgs() == 1 &&
+                     fn_name == kRegisterWeakMembersName) {
+            MarkAllWeakMembersTraced();
+          }
         }
       }
     }
 
     CXXRecordDecl* tmpl = GetDependentTemplatedDecl(expr);
     if (!tmpl)
       return;
 
     // Check for Super<T>::trace(visitor)
-    if (call->getNumArgs() == 1 && fn_name == trace_->getName()) {
+    if (call->getNumArgs() == 1 && IsTraceCallName(fn_name)) {
       RecordInfo::Bases::iterator it = info_->GetBases().begin();
       for (; it != info_->GetBases().end(); ++it) {
         if (it->first->getName() == tmpl->getName())
           it->second.MarkTraced();
       }
-      return;
     }
 
     // Check for TraceIfNeeded<T>::trace(visitor, &field)
     if (call->getNumArgs() == 2 && fn_name == kTraceName &&
         tmpl->getName() == kTraceIfNeededName) {
       FindFieldVisitor finder;
       finder.TraverseStmt(call->getArg(1));
       if (finder.field())
         FoundField(finder.field());
     }
@@ skipping 52 matching lines @@
       if (call->getNumArgs() != 1)
         return false;
       DeclRefExpr* arg = dyn_cast<DeclRefExpr>(call->getArg(0));
       if (!arg || arg->getNameInfo().getAsString() != kVisitorVarName)
         return false;
 
       callee_record = trace_decl->getParent();
       func_name = callee->getMemberName().getAsString();
     }
 
-    if (trace_->getName() == kTraceImplName) {
-      if (func_name != kTraceName)
-        return false;
-    } else if (trace_->getName() == kTraceAfterDispatchImplName) {
-      if (func_name != kTraceAfterDispatchName)
-        return false;
-    } else {
-      // Currently, a manually dispatched class cannot have mixin bases (having
-      // one would add a vtable which we explicitly check against). This means
-      // that we can only make calls to a trace method of the same name. Revisit
-      // this if our mixin/vtable assumption changes.
-      if (func_name != trace_->getName())
-        return false;
-    }
-
     if (!callee_record)
       return false;
-    RecordInfo::Bases::iterator iter = info_->GetBases().find(callee_record);
-    if (iter == info_->GetBases().end())
+
+    if (!IsTraceCallName(func_name))
       return false;
 
-    iter->second.MarkTraced();
-    return true;
+    for (auto& base : info_->GetBases()) {
+      // We want to deal with omitted trace() function in an intermediary
+      // class in the class hierarchy, e.g.:
+      //     class A : public GarbageCollected<A> { trace() { ... } };
+      //     class B : public A { /* No trace(); have nothing to trace. */ };
+      //     class C : public B { trace() { B::trace(visitor); } }
+      // where, B::trace() is actually A::trace(), and in some cases we get
+      // A as |callee_record| instead of B. We somehow need to mark B as
+      // traced if we find A::trace() call.
+      //
+      // To solve this, here we keep going up the class hierarchy as long as
+      // they are not required to have a trace method. The implementation is
+      // a simple DFS, where |base_records| represents the set of base classes
+      // we need to visit.
+
+      std::vector<CXXRecordDecl*> base_records;
+      base_records.push_back(base.first);
+
+      while (!base_records.empty()) {
+        CXXRecordDecl* base_record = base_records.back();
+        base_records.pop_back();
+
+        if (base_record == callee_record) {
+          // If we find a matching trace method, pretend the user has written
+          // a correct trace() method of the base; in the example above, we
+          // find A::trace() here and mark B as correctly traced.
+          base.second.MarkTraced();
+          return true;
+        }
+
+        if (RecordInfo* base_info = cache_->Lookup(base_record)) {
+          if (!base_info->RequiresTraceMethod()) {
+            // If this base class is not required to have a trace method, then
+            // the actual trace method may be defined in an ancestor.
+            for (auto& inner_base : base_info->GetBases())
+              base_records.push_back(inner_base.first);
+          }
+        }
+      }
+    }
+
+    return false;
   }
 
   bool CheckTraceFieldCall(CXXMemberCallExpr* call) {
     return CheckTraceFieldCall(call->getMethodDecl()->getNameAsString(),
                                call->getRecordDecl(),
                                call->getArg(0));
   }
 
   bool CheckTraceFieldCall(string name, CXXRecordDecl* callee, Expr* arg) {
     if (name != kTraceName || !Config::IsVisitor(callee->getName()))
@@ skipping 42 matching lines @@
         return false;
       }
       return true;
     }
    private:
     MemberExpr* member_;
     FieldDecl* field_;
   };
 
   // Nested checking for weak callbacks.
-  CheckTraceVisitor(RecordInfo* info) : trace_(0), info_(info) {}
+  CheckTraceVisitor(RecordInfo* info)
+      : trace_(nullptr), info_(info), cache_(nullptr) {}
 
   bool IsWeakCallback() { return !trace_; }
 
   void MarkTraced(RecordInfo::Fields::iterator it) {
     // In a weak callback we can't mark strong fields as traced.
     if (IsWeakCallback() && !it->second.edge()->IsWeakMember())
       return;
     it->second.MarkTraced();
   }
 
@@ skipping 11 matching lines @@
           break;
         }
       }
     } else {
       RecordInfo::Fields::iterator it = info_->GetFields().find(field);
       if (it != info_->GetFields().end())
         MarkTraced(it);
     }
   }
 
+  void MarkAllWeakMembersTraced() {
+    // If we find a call to registerWeakMembers which is unresolved we
+    // unsoundly consider all weak members as traced.
+    // TODO: Find out how to validate weak member tracing for unresolved call.
+    for (auto& field : info_->GetFields()) {
+      if (field.second.edge()->IsWeakMember())
+        field.second.MarkTraced();
+    }
+  }
+
   CXXMethodDecl* trace_;
   RecordInfo* info_;
+  RecordCache* cache_;
   bool delegates_to_traceimpl_;
 };
 
 // This visitor checks that the fields of a class and the fields of
 // its part objects don't define GC roots.
 class CheckGCRootsVisitor : public RecursiveEdgeVisitor {
  public:
   typedef std::vector<FieldPoint*> RootPath;
   typedef std::set<RecordInfo*> VisitingSet;
   typedef std::vector<RootPath> Errors;
@@ skipping 406 matching lines @@
       if (visitor.ContainsInvalidFields(info))
         ReportClassContainsInvalidFields(info, &visitor.invalid_fields());
     }
 
     if (info->IsGCDerived()) {
 
       if (!info->IsGCMixin()) {
         CheckLeftMostDerived(info);
         CheckDispatch(info);
         if (CXXMethodDecl* newop = info->DeclaresNewOperator())
-          ReportClassOverridesNew(info, newop);
+          if (!Config::IsIgnoreAnnotated(newop))
+            ReportClassOverridesNew(info, newop);
         if (info->IsGCMixinInstance()) {
           // Require that declared GCMixin implementations
           // also provide a trace() override.
           if (info->DeclaresGCMixinMethods()
               && !info->DeclaresLocalTraceMethod())
             ReportClassMustDeclareGCMixinTraceMethod(info);
         }
       }
 
       {
@@ skipping 275 matching lines @@
       }
       return;
     }
 
     CheckTraceOrDispatchMethod(parent, method);
   }
 
   // Determine what type of tracing method this is (dispatch or trace).
   void CheckTraceOrDispatchMethod(RecordInfo* parent, CXXMethodDecl* method) {
     Config::TraceMethodType trace_type = Config::GetTraceMethodType(method);
-    if (trace_type != Config::TRACE_METHOD ||
+    if (trace_type == Config::TRACE_AFTER_DISPATCH_METHOD ||
+        trace_type == Config::TRACE_AFTER_DISPATCH_IMPL_METHOD ||
         !parent->GetTraceDispatchMethod()) {
       CheckTraceMethod(parent, method, trace_type);
     }
     // Dispatch methods are checked when we identify subclasses.
   }
 
   // Check an actual trace method.
   void CheckTraceMethod(RecordInfo* parent,
                         CXXMethodDecl* trace,
                         Config::TraceMethodType trace_type) {
     // A trace method must not override any non-virtual trace methods.
     if (trace_type == Config::TRACE_METHOD) {
       for (RecordInfo::Bases::iterator it = parent->GetBases().begin();
            it != parent->GetBases().end();
            ++it) {
         RecordInfo* base = it->second.info();
         if (CXXMethodDecl* other = base->InheritsNonVirtualTrace())
           ReportOverriddenNonVirtualTrace(parent, trace, other);
       }
     }
 
-    CheckTraceVisitor visitor(trace, parent);
+    CheckTraceVisitor visitor(trace, parent, &cache_);
     visitor.TraverseCXXMethodDecl(trace);
 
     // Skip reporting if this trace method is a just delegate to
     // traceImpl (or traceAfterDispatchImpl) method. We will report on
     // CheckTraceMethod on traceImpl method.
     if (visitor.delegates_to_traceimpl())
       return;
 
     for (RecordInfo::Bases::iterator it = parent->GetBases().begin();
          it != parent->GetBases().end();
@@ skipping 589 matching lines @@
 
  private:
   BlinkGCPluginOptions options_;
 };
 
 }  // namespace
 
 static FrontendPluginRegistry::Add<BlinkGCPluginAction> X(
     "blink-gc-plugin",
     "Check Blink GC invariants");