Upstream version 10.39.225.0
[platform/framework/web/crosswalk.git] / src / tools / clang / blink_gc_plugin / BlinkGCPlugin.cpp
index b6d7ddb..4830a45 100644 (file)
@@ -23,13 +23,18 @@ using std::string;
 
 namespace {
 
+const char kClassMustLeftMostlyDeriveGC[] =
+    "[blink-gc] Class %0 must derive its GC base in the left-most position.";
+
 const char kClassRequiresTraceMethod[] =
-    "[blink-gc] Class %0 requires a trace method"
-    " because it contains fields that require tracing.";
+    "[blink-gc] Class %0 requires a trace method.";
 
 const char kBaseRequiresTracing[] =
     "[blink-gc] Base class %0 of derived class %1 requires tracing.";
 
+const char kBaseRequiresTracingNote[] =
+    "[blink-gc] Untraced base class %0 declared here:";
+
 const char kFieldsRequireTracing[] =
     "[blink-gc] Class %0 has untraced fields that require tracing.";
 
@@ -45,6 +50,9 @@ const char kClassContainsGCRoot[] =
 const char kClassRequiresFinalization[] =
     "[blink-gc] Class %0 requires finalization.";
 
+const char kClassDoesNotRequireFinalization[] =
+    "[blink-gc] Class %0 may not require finalization.";
+
 const char kFinalizerAccessesFinalizedField[] =
     "[blink-gc] Finalizer %0 accesses potentially finalized field %1.";
 
@@ -63,10 +71,13 @@ const char kStackAllocatedFieldNote[] =
 const char kMemberInUnmanagedClassNote[] =
     "[blink-gc] Member field %0 in unmanaged class declared here:";
 
-const char kPartObjectContainsGCRoot[] =
+const char kPartObjectToGCDerivedClassNote[] =
+    "[blink-gc] Part-object field %0 to a GC derived class declared here:";
+
+const char kPartObjectContainsGCRootNote[] =
     "[blink-gc] Field %0 with embedded GC root in %1 declared here:";
 
-const char kFieldContainsGCRoot[] =
+const char kFieldContainsGCRootNote[] =
     "[blink-gc] Field %0 defining a GC root declared here:";
 
 const char kOverriddenNonVirtualTrace[] =
@@ -113,10 +124,32 @@ const char kDerivesNonStackAllocated[] =
     "[blink-gc] Stack-allocated class %0 derives class %1"
     " which is not stack allocated.";
 
+const char kClassOverridesNew[] =
+    "[blink-gc] Garbage collected class %0"
+    " is not permitted to override its new operator.";
+
+const char kClassDeclaresPureVirtualTrace[] =
+    "[blink-gc] Garbage collected class %0"
+    " is not permitted to declare a pure-virtual trace method.";
+
+const char kLeftMostBaseMustBePolymorphic[] =
+    "[blink-gc] Left-most base class %0 of derived class %1"
+    " must be polymorphic.";
+
+const char kBaseClassMustDeclareVirtualTrace[] =
+    "[blink-gc] Left-most base class %0 of derived class %1"
+    " must define a virtual trace method.";
+
 struct BlinkGCPluginOptions {
-  BlinkGCPluginOptions() : enable_oilpan(false), dump_graph(false) {}
+  BlinkGCPluginOptions()
+    : enable_oilpan(false)
+    , dump_graph(false)
+    , warn_raw_ptr(false)
+    , warn_unneeded_finalizer(false) {}
   bool enable_oilpan;
   bool dump_graph;
+  bool warn_raw_ptr;
+  bool warn_unneeded_finalizer;
   std::set<std::string> ignored_classes;
   std::set<std::string> checked_namespaces;
   std::vector<std::string> ignored_directories;
@@ -291,67 +324,187 @@ class CheckTraceVisitor : public RecursiveASTVisitor<CheckTraceVisitor> {
   CheckTraceVisitor(CXXMethodDecl* trace, RecordInfo* info)
       : trace_(trace), info_(info) {}
 
-  // Allow recursive traversal by using VisitMemberExpr.
   bool VisitMemberExpr(MemberExpr* member) {
-    // If this member expression references a field decl, mark it as traced.
-    if (FieldDecl* field = dyn_cast<FieldDecl>(member->getMemberDecl())) {
-      if (IsTemplateInstantiation(info_->record())) {
-        // Pointer equality on fields does not work for template instantiations.
-        // The trace method refers to fields of the template definition which
-        // are different from the instantiated fields that need to be traced.
-        const string& name = field->getNameAsString();
+    // 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);
+    }
+    return true;
+  }
+
+  bool VisitCallExpr(CallExpr* call) {
+    // In weak callbacks we don't check calls (see VisitMemberExpr).
+    if (IsWeakCallback())
+      return true;
+
+    Expr* callee = call->getCallee();
+
+    // Trace calls from a templated derived class result in a
+    // DependentScopeMemberExpr because the concrete trace call depends on the
+    // instantiation of any shared template parameters. In this case the call is
+    // "unresolved" and we resort to comparing the syntactic type names.
+    if (CXXDependentScopeMemberExpr* expr =
+        dyn_cast<CXXDependentScopeMemberExpr>(callee)) {
+      CheckCXXDependentScopeMemberExpr(call, expr);
+      return true;
+    }
+
+    // A tracing call will have either a |visitor| or a |m_field| argument.
+    // A registerWeakMembers call will have a |this| argument.
+    if (call->getNumArgs() != 1)
+      return true;
+    Expr* arg = call->getArg(0);
+
+    if (UnresolvedMemberExpr* expr = dyn_cast<UnresolvedMemberExpr>(callee)) {
+      // 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->first->getNameAsString() == name) {
-            MarkTraced(it);
-            break;
-          }
+          if (it->second.edge()->IsWeakMember())
+            it->second.MarkTraced();
         }
-      } else {
-        RecordInfo::Fields::iterator it = info_->GetFields().find(field);
-        if (it != info_->GetFields().end())
-          MarkTraced(it);
       }
-      return true;
-    }
 
-    // If this is a weak callback function we only check field tracing.
-    if (IsWeakCallback())
+      QualType base = expr->getBaseType();
+      if (!base->isPointerType())
+        return true;
+      CXXRecordDecl* decl = base->getPointeeType()->getAsCXXRecordDecl();
+      if (decl)
+        CheckTraceFieldCall(expr->getMemberName().getAsString(), decl, arg);
       return true;
+    }
 
-    // For method calls, check tracing of bases and other special GC methods.
-    if (CXXMethodDecl* fn = dyn_cast<CXXMethodDecl>(member->getMemberDecl())) {
-      const string& name = fn->getNameAsString();
-      // Check weak callbacks.
-      if (name == kRegisterWeakMembersName) {
-        if (fn->isTemplateInstantiation()) {
-          const TemplateArgumentList& args =
-              *fn->getTemplateSpecializationInfo()->TemplateArguments;
-          // The second template argument is the callback method.
-          if (args.size() > 1 &&
-              args[1].getKind() == TemplateArgument::Declaration) {
-            if (FunctionDecl* callback =
-                    dyn_cast<FunctionDecl>(args[1].getAsDecl())) {
-              if (callback->hasBody()) {
-                CheckTraceVisitor nested_visitor(info_);
-                nested_visitor.TraverseStmt(callback->getBody());
-              }
-            }
-          }
-        }
+    if (CXXMemberCallExpr* expr = dyn_cast<CXXMemberCallExpr>(call)) {
+      if (CheckTraceFieldCall(expr) || CheckRegisterWeakMembers(expr))
         return true;
+    }
+
+    CheckTraceBaseCall(call);
+    return true;
+  }
+
+ private:
+
+  CXXRecordDecl* GetDependentTemplatedDecl(CXXDependentScopeMemberExpr* expr) {
+    NestedNameSpecifier* qual = expr->getQualifier();
+    if (!qual)
+      return 0;
+
+    const Type* type = qual->getAsType();
+    if (!type)
+      return 0;
+
+    const TemplateSpecializationType* tmpl_type =
+        type->getAs<TemplateSpecializationType>();
+    if (!tmpl_type)
+      return 0;
+
+    TemplateDecl* tmpl_decl = tmpl_type->getTemplateName().getAsTemplateDecl();
+    if (!tmpl_decl)
+      return 0;
+
+    return dyn_cast<CXXRecordDecl>(tmpl_decl->getTemplatedDecl());
+  }
+
+  void CheckCXXDependentScopeMemberExpr(CallExpr* call,
+                                        CXXDependentScopeMemberExpr* expr) {
+    string fn_name = expr->getMember().getAsString();
+    CXXRecordDecl* tmpl = GetDependentTemplatedDecl(expr);
+    if (!tmpl)
+      return;
+
+    // Check for Super<T>::trace(visitor)
+    if (call->getNumArgs() == 1 && fn_name == trace_->getName()) {
+      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());
+    }
+  }
+
+  bool CheckTraceBaseCall(CallExpr* call) {
+    MemberExpr* callee = dyn_cast<MemberExpr>(call->getCallee());
+    if (!callee)
+      return false;
+
+    FunctionDecl* fn = dyn_cast<FunctionDecl>(callee->getMemberDecl());
+    if (!fn || !Config::IsTraceMethod(fn))
+      return false;
 
-      // TODO: It is possible to have multiple bases, where one must be traced
-      // using a traceAfterDispatch. In such a case we should also check that
-      // the mixin does not add a vtable.
-      if (Config::IsTraceMethod(fn) && member->hasQualifier()) {
-        if (const Type* type = member->getQualifier()->getAsType()) {
-          if (CXXRecordDecl* decl = type->getAsCXXRecordDecl()) {
-            RecordInfo::Bases::iterator it = info_->GetBases().find(decl);
-            if (it != info_->GetBases().end())
-              it->second.MarkTraced();
+    // 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 (fn->getName() != trace_->getName())
+      return false;
+
+    CXXRecordDecl* decl = 0;
+    if (callee && callee->hasQualifier()) {
+      if (const Type* type = callee->getQualifier()->getAsType())
+        decl = type->getAsCXXRecordDecl();
+    }
+    if (!decl)
+      return false;
+
+    RecordInfo::Bases::iterator it = info_->GetBases().find(decl);
+    if (it != info_->GetBases().end()) {
+      it->second.MarkTraced();
+    }
+
+    return true;
+  }
+
+  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()))
+      return false;
+
+    FindFieldVisitor finder;
+    finder.TraverseStmt(arg);
+    if (finder.field())
+      FoundField(finder.field());
+
+    return true;
+  }
+
+  bool CheckRegisterWeakMembers(CXXMemberCallExpr* call) {
+    CXXMethodDecl* fn = call->getMethodDecl();
+    if (fn->getName() != kRegisterWeakMembersName)
+      return false;
+
+    if (fn->isTemplateInstantiation()) {
+      const TemplateArgumentList& args =
+          *fn->getTemplateSpecializationInfo()->TemplateArguments;
+      // The second template argument is the callback method.
+      if (args.size() > 1 &&
+          args[1].getKind() == TemplateArgument::Declaration) {
+        if (FunctionDecl* callback =
+            dyn_cast<FunctionDecl>(args[1].getAsDecl())) {
+          if (callback->hasBody()) {
+            CheckTraceVisitor nested_visitor(info_);
+            nested_visitor.TraverseStmt(callback->getBody());
           }
         }
       }
@@ -359,7 +512,24 @@ class CheckTraceVisitor : public RecursiveASTVisitor<CheckTraceVisitor> {
     return true;
   }
 
- private:
+  class FindFieldVisitor : public RecursiveASTVisitor<FindFieldVisitor> {
+   public:
+    FindFieldVisitor() : member_(0), field_(0) {}
+    MemberExpr* member() const { return member_; }
+    FieldDecl* field() const { return field_; }
+    bool TraverseMemberExpr(MemberExpr* member) {
+      if (FieldDecl* field = dyn_cast<FieldDecl>(member->getMemberDecl())) {
+        member_ = member;
+        field_ = field;
+        return false;
+      }
+      return true;
+    }
+   private:
+    MemberExpr* member_;
+    FieldDecl* field_;
+  };
+
   // Nested checking for weak callbacks.
   CheckTraceVisitor(RecordInfo* info) : trace_(0), info_(info) {}
 
@@ -372,6 +542,27 @@ class CheckTraceVisitor : public RecursiveASTVisitor<CheckTraceVisitor> {
     it->second.MarkTraced();
   }
 
+  void FoundField(FieldDecl* field) {
+    if (IsTemplateInstantiation(info_->record())) {
+      // Pointer equality on fields does not work for template instantiations.
+      // The trace method refers to fields of the template definition which
+      // are different from the instantiated fields that need to be traced.
+      const string& name = field->getNameAsString();
+      for (RecordInfo::Fields::iterator it = info_->GetFields().begin();
+           it != info_->GetFields().end();
+           ++it) {
+        if (it->first->getNameAsString() == name) {
+          MarkTraced(it);
+          break;
+        }
+      }
+    } else {
+      RecordInfo::Fields::iterator it = info_->GetFields().find(field);
+      if (it != info_->GetFields().end())
+        MarkTraced(it);
+    }
+  }
+
   CXXMethodDecl* trace_;
   RecordInfo* info_;
 };
@@ -429,10 +620,23 @@ class CheckGCRootsVisitor : public RecursiveEdgeVisitor {
 
 // This visitor checks that the fields of a class are "well formed".
 // - OwnPtr, RefPtr and RawPtr must not point to a GC derived types.
+// - Part objects must not be GC derived types.
 // - An on-heap class must never contain GC roots.
+// - Only stack-allocated types may point to stack-allocated types.
 class CheckFieldsVisitor : public RecursiveEdgeVisitor {
  public:
-  typedef std::vector<std::pair<FieldPoint*, Edge*> > Errors;
+
+  enum Error {
+    kRawPtrToGCManaged,
+    kRawPtrToGCManagedWarning,
+    kRefPtrToGCManaged,
+    kOwnPtrToGCManaged,
+    kMemberInUnmanaged,
+    kPtrFromHeapToStack,
+    kGCDerivedPartObject
+  };
+
+  typedef std::vector<std::pair<FieldPoint*, Error> > Errors;
 
   CheckFieldsVisitor(const BlinkGCPluginOptions& options)
       : options_(options), current_(0), stack_allocated_host_(false) {}
@@ -455,7 +659,7 @@ class CheckFieldsVisitor : public RecursiveEdgeVisitor {
     return !invalid_fields_.empty();
   }
 
-  void VisitMember(Member* edge) override {
+  void AtMember(Member* edge) override {
     if (managed_host_)
       return;
     // A member is allowed to appear in the context of a root.
@@ -465,37 +669,69 @@ class CheckFieldsVisitor : public RecursiveEdgeVisitor {
       if ((*it)->Kind() == Edge::kRoot)
         return;
     }
-    invalid_fields_.push_back(std::make_pair(current_, edge));
+    invalid_fields_.push_back(std::make_pair(current_, kMemberInUnmanaged));
   }
 
-  void VisitValue(Value* edge) override {
+  void AtValue(Value* edge) override {
     // TODO: what should we do to check unions?
     if (edge->value()->record()->isUnion())
       return;
 
     if (!stack_allocated_host_ && edge->value()->IsStackAllocated()) {
-      invalid_fields_.push_back(std::make_pair(current_, edge));
+      invalid_fields_.push_back(std::make_pair(current_, kPtrFromHeapToStack));
+      return;
+    }
+
+    if (!Parent() &&
+        edge->value()->IsGCDerived() &&
+        !edge->value()->IsGCMixin()) {
+      invalid_fields_.push_back(std::make_pair(current_, kGCDerivedPartObject));
       return;
     }
 
     if (!Parent() || !edge->value()->IsGCAllocated())
       return;
 
-    if (Parent()->IsOwnPtr() ||
-        (stack_allocated_host_ && Parent()->IsRawPtr())) {
-      invalid_fields_.push_back(std::make_pair(current_, Parent()));
+    // In transition mode, disallow  OwnPtr<T>, RawPtr<T> to GC allocated T's,
+    // also disallow T* in stack-allocated types.
+    if (options_.enable_oilpan) {
+      if (Parent()->IsOwnPtr() ||
+          Parent()->IsRawPtrClass() ||
+          (stack_allocated_host_ && Parent()->IsRawPtr())) {
+        invalid_fields_.push_back(std::make_pair(
+            current_, InvalidSmartPtr(Parent())));
+        return;
+      }
+      if (options_.warn_raw_ptr && Parent()->IsRawPtr()) {
+        invalid_fields_.push_back(std::make_pair(
+            current_, kRawPtrToGCManagedWarning));
+      }
       return;
     }
 
-    // Don't check raw and ref pointers in transition mode.
-    if (options_.enable_oilpan)
+    if (Parent()->IsRawPtr() || Parent()->IsRefPtr() || Parent()->IsOwnPtr()) {
+      invalid_fields_.push_back(std::make_pair(
+          current_, InvalidSmartPtr(Parent())));
       return;
+    }
+  }
 
-    if (Parent()->IsRawPtr() || Parent()->IsRefPtr())
-      invalid_fields_.push_back(std::make_pair(current_, Parent()));
+  void AtCollection(Collection* edge) override {
+    if (edge->on_heap() && Parent() && Parent()->IsOwnPtr())
+      invalid_fields_.push_back(std::make_pair(current_, kOwnPtrToGCManaged));
   }
 
  private:
+  Error InvalidSmartPtr(Edge* ptr) {
+    if (ptr->IsRawPtr())
+      return kRawPtrToGCManaged;
+    if (ptr->IsRefPtr())
+      return kRefPtrToGCManaged;
+    if (ptr->IsOwnPtr())
+      return kOwnPtrToGCManaged;
+    assert(false && "Unknown smart pointer kind");
+  }
+
   const BlinkGCPluginOptions& options_;
   FieldPoint* current_;
   bool stack_allocated_host_;
@@ -503,6 +739,28 @@ class CheckFieldsVisitor : public RecursiveEdgeVisitor {
   Errors invalid_fields_;
 };
 
+class EmptyStmtVisitor
+    : public RecursiveASTVisitor<EmptyStmtVisitor> {
+public:
+  static bool isEmpty(Stmt* stmt) {
+    EmptyStmtVisitor visitor;
+    visitor.TraverseStmt(stmt);
+    return visitor.empty_;
+  }
+
+  bool WalkUpFromCompoundStmt(CompoundStmt* stmt) {
+    empty_ = stmt->body_empty();
+    return false;
+  }
+  bool VisitStmt(Stmt*) {
+    empty_ = false;
+    return false;
+  }
+private:
+  EmptyStmtVisitor() : empty_(true) {}
+  bool empty_;
+};
+
 // Main class containing checks for various invariants of the Blink
 // garbage collection infrastructure.
 class BlinkGCPluginConsumer : public ASTConsumer {
@@ -514,15 +772,16 @@ class BlinkGCPluginConsumer : public ASTConsumer {
         options_(options),
         json_(0) {
 
-    // Only check structures in the blink, WebCore and WebKit namespaces.
+    // Only check structures in the blink and WebKit namespaces.
     options_.checked_namespaces.insert("blink");
-    options_.checked_namespaces.insert("WebCore");
     options_.checked_namespaces.insert("WebKit");
 
     // Ignore GC implementation files.
     options_.ignored_directories.push_back("/heap/");
 
     // Register warning/error messages.
+    diag_class_must_left_mostly_derive_gc_ = diagnostic_.getCustomDiagID(
+        getErrorLevel(), kClassMustLeftMostlyDeriveGC);
     diag_class_requires_trace_method_ =
         diagnostic_.getCustomDiagID(getErrorLevel(), kClassRequiresTraceMethod);
     diag_base_requires_tracing_ =
@@ -531,10 +790,14 @@ class BlinkGCPluginConsumer : public ASTConsumer {
         diagnostic_.getCustomDiagID(getErrorLevel(), kFieldsRequireTracing);
     diag_class_contains_invalid_fields_ = diagnostic_.getCustomDiagID(
         getErrorLevel(), kClassContainsInvalidFields);
+    diag_class_contains_invalid_fields_warning_ = diagnostic_.getCustomDiagID(
+        DiagnosticsEngine::Warning, kClassContainsInvalidFields);
     diag_class_contains_gc_root_ =
         diagnostic_.getCustomDiagID(getErrorLevel(), kClassContainsGCRoot);
     diag_class_requires_finalization_ = diagnostic_.getCustomDiagID(
         getErrorLevel(), kClassRequiresFinalization);
+    diag_class_does_not_require_finalization_ = diagnostic_.getCustomDiagID(
+        DiagnosticsEngine::Warning, kClassDoesNotRequireFinalization);
     diag_finalizer_accesses_finalized_field_ = diagnostic_.getCustomDiagID(
         getErrorLevel(), kFinalizerAccessesFinalizedField);
     diag_overridden_non_virtual_trace_ = diagnostic_.getCustomDiagID(
@@ -551,8 +814,18 @@ class BlinkGCPluginConsumer : public ASTConsumer {
         diagnostic_.getCustomDiagID(getErrorLevel(), kMissingFinalizeDispatch);
     diag_derives_non_stack_allocated_ =
         diagnostic_.getCustomDiagID(getErrorLevel(), kDerivesNonStackAllocated);
+    diag_class_overrides_new_ =
+        diagnostic_.getCustomDiagID(getErrorLevel(), kClassOverridesNew);
+    diag_class_declares_pure_virtual_trace_ = diagnostic_.getCustomDiagID(
+        getErrorLevel(), kClassDeclaresPureVirtualTrace);
+    diag_left_most_base_must_be_polymorphic_ = diagnostic_.getCustomDiagID(
+        getErrorLevel(), kLeftMostBaseMustBePolymorphic);
+    diag_base_class_must_declare_virtual_trace_ = diagnostic_.getCustomDiagID(
+        getErrorLevel(), kBaseClassMustDeclareVirtualTrace);
 
     // Register note messages.
+    diag_base_requires_tracing_note_ = diagnostic_.getCustomDiagID(
+        DiagnosticsEngine::Note, kBaseRequiresTracingNote);
     diag_field_requires_tracing_note_ = diagnostic_.getCustomDiagID(
         DiagnosticsEngine::Note, kFieldRequiresTracingNote);
     diag_raw_ptr_to_gc_managed_class_note_ = diagnostic_.getCustomDiagID(
@@ -565,10 +838,12 @@ class BlinkGCPluginConsumer : public ASTConsumer {
         DiagnosticsEngine::Note, kStackAllocatedFieldNote);
     diag_member_in_unmanaged_class_note_ = diagnostic_.getCustomDiagID(
         DiagnosticsEngine::Note, kMemberInUnmanagedClassNote);
+    diag_part_object_to_gc_derived_class_note_ = diagnostic_.getCustomDiagID(
+        DiagnosticsEngine::Note, kPartObjectToGCDerivedClassNote);
     diag_part_object_contains_gc_root_note_ = diagnostic_.getCustomDiagID(
-        DiagnosticsEngine::Note, kPartObjectContainsGCRoot);
+        DiagnosticsEngine::Note, kPartObjectContainsGCRootNote);
     diag_field_contains_gc_root_note_ = diagnostic_.getCustomDiagID(
-        DiagnosticsEngine::Note, kFieldContainsGCRoot);
+        DiagnosticsEngine::Note, kFieldContainsGCRootNote);
     diag_finalized_field_note_ = diagnostic_.getCustomDiagID(
         DiagnosticsEngine::Note, kFinalizedFieldNote);
     diag_user_declared_destructor_note_ = diagnostic_.getCustomDiagID(
@@ -590,7 +865,7 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     visitor.TraverseDecl(context.getTranslationUnitDecl());
 
     if (options_.dump_graph) {
-      string err;
+      std::error_code err;
       // TODO: Make createDefaultOutputFile or a shorter createOutputFile work.
       json_ = JsonWriter::from(instance_.createOutputFile(
           "",                                      // OutputPath
@@ -603,7 +878,7 @@ class BlinkGCPluginConsumer : public ASTConsumer {
           false,                                   // CreateMissingDirectories
           0,                                       // ResultPathName
           0));                                     // TempPathName
-      if (err.empty() && json_) {
+      if (!err && json_) {
         json_->OpenList();
       } else {
         json_ = 0;
@@ -673,8 +948,19 @@ class BlinkGCPluginConsumer : public ASTConsumer {
       }
     }
 
-    if (info->RequiresTraceMethod() && !info->GetTraceMethod())
+    if (CXXMethodDecl* trace = info->GetTraceMethod()) {
+      if (trace->isPure())
+        ReportClassDeclaresPureVirtualTrace(info, trace);
+    } else if (info->RequiresTraceMethod()) {
       ReportClassRequiresTraceMethod(info);
+    }
+
+    // Check polymorphic classes that are GC-derived or have a trace method.
+    if (info->record()->hasDefinition() && info->record()->isPolymorphic()) {
+      CXXMethodDecl* trace = info->GetTraceMethod();
+      if (trace || info->IsGCDerived())
+        CheckPolymorphicClass(info, trace);
+    }
 
     {
       CheckFieldsVisitor visitor(options_);
@@ -683,19 +969,156 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     }
 
     if (info->IsGCDerived()) {
-      CheckDispatch(info);
 
-      CheckGCRootsVisitor visitor;
-      if (visitor.ContainsGCRoots(info))
-        ReportClassContainsGCRoots(info, &visitor.gc_roots());
+      if (!info->IsGCMixin()) {
+        CheckLeftMostDerived(info);
+        CheckDispatch(info);
+        if (CXXMethodDecl* newop = info->DeclaresNewOperator())
+          ReportClassOverridesNew(info, newop);
+      }
+
+      {
+        CheckGCRootsVisitor visitor;
+        if (visitor.ContainsGCRoots(info))
+          ReportClassContainsGCRoots(info, &visitor.gc_roots());
+      }
 
       if (info->NeedsFinalization())
         CheckFinalization(info);
+
+      if (options_.warn_unneeded_finalizer && info->IsGCFinalized())
+        CheckUnneededFinalization(info);
     }
 
     DumpClass(info);
   }
 
+  CXXRecordDecl* GetDependentTemplatedDecl(const Type& type) {
+    const TemplateSpecializationType* tmpl_type =
+        type.getAs<TemplateSpecializationType>();
+    if (!tmpl_type)
+      return 0;
+
+    TemplateDecl* tmpl_decl = tmpl_type->getTemplateName().getAsTemplateDecl();
+    if (!tmpl_decl)
+      return 0;
+
+    return dyn_cast<CXXRecordDecl>(tmpl_decl->getTemplatedDecl());
+  }
+
+  // The GC infrastructure assumes that if the vtable of a polymorphic
+  // base-class is not initialized for a given object (ie, it is partially
+  // initialized) then the object does not need to be traced. Thus, we must
+  // ensure that any polymorphic class with a trace method does not have any
+  // tractable fields that are initialized before we are sure that the vtable
+  // and the trace method are both defined.  There are two cases that need to
+  // hold to satisfy that assumption:
+  //
+  // 1. If trace is virtual, then it must be defined in the left-most base.
+  // This ensures that if the vtable is initialized then it contains a pointer
+  // to the trace method.
+  //
+  // 2. If trace is non-virtual, then the trace method is defined and we must
+  // ensure that the left-most base defines a vtable. This ensures that the
+  // first thing to be initialized when constructing the object is the vtable
+  // itself.
+  void CheckPolymorphicClass(RecordInfo* info, CXXMethodDecl* trace) {
+    CXXRecordDecl* left_most = info->record();
+    CXXRecordDecl::base_class_iterator it = left_most->bases_begin();
+    CXXRecordDecl* left_most_base = 0;
+    while (it != left_most->bases_end()) {
+      left_most_base = it->getType()->getAsCXXRecordDecl();
+      if (!left_most_base && it->getType()->isDependentType())
+        left_most_base = GetDependentTemplatedDecl(*it->getType());
+
+      // TODO: Find a way to correctly check actual instantiations
+      // for dependent types. The escape below will be hit, eg, when
+      // we have a primary template with no definition and
+      // specializations for each case (such as SupplementBase) in
+      // which case we don't succeed in checking the required
+      // properties.
+      if (!left_most_base || !left_most_base->hasDefinition())
+        return;
+
+      StringRef name = left_most_base->getName();
+      // We know GCMixin base defines virtual trace.
+      if (Config::IsGCMixinBase(name))
+        return;
+
+      // Stop with the left-most prior to a safe polymorphic base (a safe base
+      // is non-polymorphic and contains no fields).
+      if (Config::IsSafePolymorphicBase(name))
+        break;
+
+      left_most = left_most_base;
+      it = left_most->bases_begin();
+    }
+
+    if (RecordInfo* left_most_info = cache_.Lookup(left_most)) {
+
+      // Check condition (1):
+      if (trace && trace->isVirtual()) {
+        if (CXXMethodDecl* trace = left_most_info->GetTraceMethod()) {
+          if (trace->isVirtual())
+            return;
+        }
+        ReportBaseClassMustDeclareVirtualTrace(info, left_most);
+        return;
+      }
+
+      // Check condition (2):
+      if (DeclaresVirtualMethods(left_most))
+        return;
+      if (left_most_base) {
+        ++it; // Get the base next to the "safe polymorphic base"
+        if (it != left_most->bases_end()) {
+          if (CXXRecordDecl* next_base = it->getType()->getAsCXXRecordDecl()) {
+            if (CXXRecordDecl* next_left_most = GetLeftMostBase(next_base)) {
+              if (DeclaresVirtualMethods(next_left_most))
+                return;
+              ReportLeftMostBaseMustBePolymorphic(info, next_left_most);
+              return;
+            }
+          }
+        }
+      }
+      ReportLeftMostBaseMustBePolymorphic(info, left_most);
+    }
+  }
+
+  CXXRecordDecl* GetLeftMostBase(CXXRecordDecl* left_most) {
+    CXXRecordDecl::base_class_iterator it = left_most->bases_begin();
+    while (it != left_most->bases_end()) {
+      if (it->getType()->isDependentType())
+        left_most = GetDependentTemplatedDecl(*it->getType());
+      else
+        left_most = it->getType()->getAsCXXRecordDecl();
+      if (!left_most || !left_most->hasDefinition())
+        return 0;
+      it = left_most->bases_begin();
+    }
+    return left_most;
+  }
+
+  bool DeclaresVirtualMethods(CXXRecordDecl* decl) {
+    CXXRecordDecl::method_iterator it = decl->method_begin();
+    for (; it != decl->method_end(); ++it)
+      if (it->isVirtual() && !it->isPure())
+        return true;
+    return false;
+  }
+
+  void CheckLeftMostDerived(RecordInfo* info) {
+    CXXRecordDecl* left_most = info->record();
+    CXXRecordDecl::base_class_iterator it = left_most->bases_begin();
+    while (it != left_most->bases_end()) {
+      left_most = it->getType()->getAsCXXRecordDecl();
+      it = left_most->bases_begin();
+    }
+    if (!Config::IsGCBase(left_most->getName()))
+      ReportClassMustLeftMostlyDeriveGC(info);
+  }
+
   void CheckDispatch(RecordInfo* info) {
     bool finalized = info->IsGCFinalized();
     CXXMethodDecl* trace_dispatch = info->GetTraceDispatchMethod();
@@ -789,6 +1212,32 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     }
   }
 
+  void CheckUnneededFinalization(RecordInfo* info) {
+    if (!HasNonEmptyFinalizer(info))
+      ReportClassDoesNotRequireFinalization(info);
+  }
+
+  bool HasNonEmptyFinalizer(RecordInfo* info) {
+    CXXDestructorDecl* dtor = info->record()->getDestructor();
+    if (dtor && dtor->isUserProvided()) {
+      if (!dtor->hasBody() || !EmptyStmtVisitor::isEmpty(dtor->getBody()))
+        return true;
+    }
+    for (RecordInfo::Bases::iterator it = info->GetBases().begin();
+         it != info->GetBases().end();
+         ++it) {
+      if (HasNonEmptyFinalizer(it->second.info()))
+        return true;
+    }
+    for (RecordInfo::Fields::iterator it = info->GetFields().begin();
+         it != info->GetFields().end();
+         ++it) {
+      if (it->second.edge()->NeedsFinalization())
+        return true;
+    }
+    return false;
+  }
+
   // This is the main entry for tracing method definitions.
   void CheckTracingMethod(CXXMethodDecl* method) {
     RecordInfo* parent = cache_.Lookup(method->getParent());
@@ -827,16 +1276,12 @@ class BlinkGCPluginConsumer : public ASTConsumer {
   void CheckTraceMethod(RecordInfo* parent,
                         CXXMethodDecl* trace,
                         bool isTraceAfterDispatch) {
-    // A non-virtual trace method must not override another trace.
-    if (!isTraceAfterDispatch && !trace->isVirtual()) {
+    // A trace method must not override any non-virtual trace methods.
+    if (!isTraceAfterDispatch) {
       for (RecordInfo::Bases::iterator it = parent->GetBases().begin();
            it != parent->GetBases().end();
            ++it) {
         RecordInfo* base = it->second.info();
-        // We allow mixin bases to contain a non-virtual trace since it will
-        // never be used for dispatching.
-        if (base->IsGCMixin())
-          continue;
         if (CXXMethodDecl* other = base->InheritsNonVirtualTrace())
           ReportOverriddenNonVirtualTrace(parent, trace, other);
       }
@@ -886,6 +1331,14 @@ class BlinkGCPluginConsumer : public ASTConsumer {
         json_->Write("lbl", lbl);
         json_->Write("kind", kind);
         json_->Write("loc", loc);
+        json_->Write("ptr",
+                     !Parent() ? "val" :
+                     Parent()->IsRawPtr() ? "raw" :
+                     Parent()->IsRefPtr() ? "ref" :
+                     Parent()->IsOwnPtr() ? "own" :
+                     (Parent()->IsMember() ||
+                      Parent()->IsWeakMember()) ? "mem" :
+                     "val");
         json_->CloseObject();
       }
 
@@ -997,15 +1450,15 @@ class BlinkGCPluginConsumer : public ASTConsumer {
   bool InCheckedNamespace(RecordInfo* info) {
     if (!info)
       return false;
-    DeclContext* context = info->record()->getDeclContext();
-    if (context->isRecord())
-      return InCheckedNamespace(cache_.Lookup(context));
-    if (context->isNamespace()) {
-      const NamespaceDecl* decl = dyn_cast<NamespaceDecl>(context);
-      if (decl->isAnonymousNamespace())
-        return false;
-      return options_.checked_namespaces.find(decl->getNameAsString()) !=
-          options_.checked_namespaces.end();
+    for (DeclContext* context = info->record()->getDeclContext();
+         !context->isTranslationUnit();
+         context = context->getParent()) {
+      if (NamespaceDecl* decl = dyn_cast<NamespaceDecl>(context)) {
+        if (options_.checked_namespaces.find(decl->getNameAsString()) !=
+            options_.checked_namespaces.end()) {
+          return true;
+        }
+      }
     }
     return false;
   }
@@ -1023,12 +1476,28 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     return true;
   }
 
+  void ReportClassMustLeftMostlyDeriveGC(RecordInfo* info) {
+    SourceLocation loc = info->record()->getInnerLocStart();
+    SourceManager& manager = instance_.getSourceManager();
+    FullSourceLoc full_loc(loc, manager);
+    diagnostic_.Report(full_loc, diag_class_must_left_mostly_derive_gc_)
+        << info->record();
+  }
+
   void ReportClassRequiresTraceMethod(RecordInfo* info) {
     SourceLocation loc = info->record()->getInnerLocStart();
     SourceManager& manager = instance_.getSourceManager();
     FullSourceLoc full_loc(loc, manager);
     diagnostic_.Report(full_loc, diag_class_requires_trace_method_)
         << info->record();
+
+    for (RecordInfo::Bases::iterator it = info->GetBases().begin();
+         it != info->GetBases().end();
+         ++it) {
+      if (it->second.NeedsTracing().IsNeeded())
+        NoteBaseRequiresTracing(&it->second);
+    }
+
     for (RecordInfo::Fields::iterator it = info->GetFields().begin();
          it != info->GetFields().end();
          ++it) {
@@ -1066,22 +1535,38 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     SourceLocation loc = info->record()->getLocStart();
     SourceManager& manager = instance_.getSourceManager();
     FullSourceLoc full_loc(loc, manager);
-    diagnostic_.Report(full_loc, diag_class_contains_invalid_fields_)
+    bool only_warnings = options_.warn_raw_ptr;
+    for (CheckFieldsVisitor::Errors::iterator it = errors->begin();
+         only_warnings && it != errors->end();
+         ++it) {
+      if (it->second != CheckFieldsVisitor::kRawPtrToGCManagedWarning)
+        only_warnings = false;
+    }
+    diagnostic_.Report(full_loc, only_warnings ?
+                       diag_class_contains_invalid_fields_warning_ :
+                       diag_class_contains_invalid_fields_)
         << info->record();
     for (CheckFieldsVisitor::Errors::iterator it = errors->begin();
          it != errors->end();
          ++it) {
-      if (it->second->IsRawPtr()) {
-        NoteField(it->first, diag_raw_ptr_to_gc_managed_class_note_);
-      } else if (it->second->IsRefPtr()) {
-        NoteField(it->first, diag_ref_ptr_to_gc_managed_class_note_);
-      } else if (it->second->IsOwnPtr()) {
-        NoteField(it->first, diag_own_ptr_to_gc_managed_class_note_);
-      } else if (it->second->IsMember()) {
-        NoteField(it->first, diag_member_in_unmanaged_class_note_);
-      } else if (it->second->IsValue()) {
-        NoteField(it->first, diag_stack_allocated_field_note_);
+      unsigned error;
+      if (it->second == CheckFieldsVisitor::kRawPtrToGCManaged ||
+          it->second == CheckFieldsVisitor::kRawPtrToGCManagedWarning) {
+        error = diag_raw_ptr_to_gc_managed_class_note_;
+      } else if (it->second == CheckFieldsVisitor::kRefPtrToGCManaged) {
+        error = diag_ref_ptr_to_gc_managed_class_note_;
+      } else if (it->second == CheckFieldsVisitor::kOwnPtrToGCManaged) {
+        error = diag_own_ptr_to_gc_managed_class_note_;
+      } else if (it->second == CheckFieldsVisitor::kMemberInUnmanaged) {
+        error = diag_member_in_unmanaged_class_note_;
+      } else if (it->second == CheckFieldsVisitor::kPtrFromHeapToStack) {
+        error = diag_stack_allocated_field_note_;
+      } else if (it->second == CheckFieldsVisitor::kGCDerivedPartObject) {
+        error = diag_part_object_to_gc_derived_class_note_;
+      } else {
+        assert(false && "Unknown field error");
       }
+      NoteField(it->first, error);
     }
   }
 
@@ -1128,6 +1613,14 @@ class BlinkGCPluginConsumer : public ASTConsumer {
         << info->record();
   }
 
+  void ReportClassDoesNotRequireFinalization(RecordInfo* info) {
+    SourceLocation loc = info->record()->getInnerLocStart();
+    SourceManager& manager = instance_.getSourceManager();
+    FullSourceLoc full_loc(loc, manager);
+    diagnostic_.Report(full_loc, diag_class_does_not_require_finalization_)
+        << info->record();
+  }
+
   void ReportOverriddenNonVirtualTrace(RecordInfo* info,
                                        CXXMethodDecl* trace,
                                        CXXMethodDecl* overridden) {
@@ -1191,6 +1684,40 @@ class BlinkGCPluginConsumer : public ASTConsumer {
         << info->record() << base->info()->record();
   }
 
+  void ReportClassOverridesNew(RecordInfo* info, CXXMethodDecl* newop) {
+    SourceLocation loc = newop->getLocStart();
+    SourceManager& manager = instance_.getSourceManager();
+    FullSourceLoc full_loc(loc, manager);
+    diagnostic_.Report(full_loc, diag_class_overrides_new_) << info->record();
+  }
+
+  void ReportClassDeclaresPureVirtualTrace(RecordInfo* info,
+                                           CXXMethodDecl* trace) {
+    SourceLocation loc = trace->getLocStart();
+    SourceManager& manager = instance_.getSourceManager();
+    FullSourceLoc full_loc(loc, manager);
+    diagnostic_.Report(full_loc, diag_class_declares_pure_virtual_trace_)
+        << info->record();
+  }
+
+  void ReportLeftMostBaseMustBePolymorphic(RecordInfo* derived,
+                                           CXXRecordDecl* base) {
+    SourceLocation loc = base->getLocStart();
+    SourceManager& manager = instance_.getSourceManager();
+    FullSourceLoc full_loc(loc, manager);
+    diagnostic_.Report(full_loc, diag_left_most_base_must_be_polymorphic_)
+        << base << derived->record();
+  }
+
+  void ReportBaseClassMustDeclareVirtualTrace(RecordInfo* derived,
+                                              CXXRecordDecl* base) {
+    SourceLocation loc = base->getLocStart();
+    SourceManager& manager = instance_.getSourceManager();
+    FullSourceLoc full_loc(loc, manager);
+    diagnostic_.Report(full_loc, diag_base_class_must_declare_virtual_trace_)
+        << base << derived->record();
+  }
+
   void NoteManualDispatchMethod(CXXMethodDecl* dispatch) {
     SourceLocation loc = dispatch->getLocStart();
     SourceManager& manager = instance_.getSourceManager();
@@ -1198,6 +1725,14 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     diagnostic_.Report(full_loc, diag_manual_dispatch_method_note_) << dispatch;
   }
 
+  void NoteBaseRequiresTracing(BasePoint* base) {
+    SourceLocation loc = base->spec().getLocStart();
+    SourceManager& manager = instance_.getSourceManager();
+    FullSourceLoc full_loc(loc, manager);
+    diagnostic_.Report(full_loc, diag_base_requires_tracing_note_)
+        << base->info()->record();
+  }
+
   void NoteFieldRequiresTracing(RecordInfo* holder, FieldDecl* field) {
     NoteField(field, diag_field_requires_tracing_note_);
   }
@@ -1256,12 +1791,15 @@ class BlinkGCPluginConsumer : public ASTConsumer {
         << overridden;
   }
 
+  unsigned diag_class_must_left_mostly_derive_gc_;
   unsigned diag_class_requires_trace_method_;
   unsigned diag_base_requires_tracing_;
   unsigned diag_fields_require_tracing_;
   unsigned diag_class_contains_invalid_fields_;
+  unsigned diag_class_contains_invalid_fields_warning_;
   unsigned diag_class_contains_gc_root_;
   unsigned diag_class_requires_finalization_;
+  unsigned diag_class_does_not_require_finalization_;
   unsigned diag_finalizer_accesses_finalized_field_;
   unsigned diag_overridden_non_virtual_trace_;
   unsigned diag_missing_trace_dispatch_method_;
@@ -1270,13 +1808,19 @@ class BlinkGCPluginConsumer : public ASTConsumer {
   unsigned diag_missing_trace_dispatch_;
   unsigned diag_missing_finalize_dispatch_;
   unsigned diag_derives_non_stack_allocated_;
+  unsigned diag_class_overrides_new_;
+  unsigned diag_class_declares_pure_virtual_trace_;
+  unsigned diag_left_most_base_must_be_polymorphic_;
+  unsigned diag_base_class_must_declare_virtual_trace_;
 
+  unsigned diag_base_requires_tracing_note_;
   unsigned diag_field_requires_tracing_note_;
   unsigned diag_raw_ptr_to_gc_managed_class_note_;
   unsigned diag_ref_ptr_to_gc_managed_class_note_;
   unsigned diag_own_ptr_to_gc_managed_class_note_;
   unsigned diag_stack_allocated_field_note_;
   unsigned diag_member_in_unmanaged_class_note_;
+  unsigned diag_part_object_to_gc_derived_class_note_;
   unsigned diag_part_object_contains_gc_root_note_;
   unsigned diag_field_contains_gc_root_note_;
   unsigned diag_finalized_field_note_;
@@ -1300,9 +1844,10 @@ class BlinkGCPluginAction : public PluginASTAction {
 
  protected:
   // Overridden from PluginASTAction:
-  virtual ASTConsumer* CreateASTConsumer(CompilerInstance& instance,
-                                         llvm::StringRef ref) {
-    return new BlinkGCPluginConsumer(instance, options_);
+  virtual std::unique_ptr<ASTConsumer> CreateASTConsumer(
+      CompilerInstance& instance,
+      llvm::StringRef ref) {
+    return llvm::make_unique<BlinkGCPluginConsumer>(instance, options_);
   }
 
   virtual bool ParseArgs(const CompilerInstance& instance,
@@ -1314,6 +1859,10 @@ class BlinkGCPluginAction : public PluginASTAction {
         options_.enable_oilpan = true;
       } else if (args[i] == "dump-graph") {
         options_.dump_graph = true;
+      } else if (args[i] == "warn-raw-ptr") {
+        options_.warn_raw_ptr = true;
+      } else if (args[i] == "warn-unneeded-finalizer") {
+        options_.warn_unneeded_finalizer = true;
       } else {
         parsed = false;
         llvm::errs() << "Unknown blink-gc-plugin argument: " << args[i] << "\n";