Blink GC plugin: disallow GC derived types as part-object fields.

BUG=334149
R=ager@chromium.org
NOTRY=true

Review URL: https://codereview.chromium.org/292743006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271880 0039d316-1c4b-4281-b951-d872f2087c98

tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp

@@ -68,10 +68,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[] =
@@ -445,11 +448,22 @@ 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,
+    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) {}
@@ -482,7 +496,7 @@ 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 {
@@ -491,7 +505,14 @@ class CheckFieldsVisitor : public RecursiveEdgeVisitor {
       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;
     }
 
@@ -504,7 +525,8 @@ class CheckFieldsVisitor : public RecursiveEdgeVisitor {
       if (Parent()->IsOwnPtr() ||
           Parent()->IsRawPtrClass() ||
           (stack_allocated_host_ && Parent()->IsRawPtr())) {
-        invalid_fields_.push_back(std::make_pair(current_, Parent()));
+        invalid_fields_.push_back(std::make_pair(
+            current_, InvalidSmartPtr(Parent())));
         return;
       }
 
@@ -512,12 +534,23 @@ class CheckFieldsVisitor : public RecursiveEdgeVisitor {
     }
 
     if (Parent()->IsRawPtr() || Parent()->IsRefPtr() || Parent()->IsOwnPtr()) {
-      invalid_fields_.push_back(std::make_pair(current_, Parent()));
+      invalid_fields_.push_back(std::make_pair(
+          current_, InvalidSmartPtr(Parent())));
       return;
     }
   }
 
  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_;
@@ -595,10 +628,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(
@@ -717,12 +752,13 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     }
 
     if (info->IsGCDerived()) {
-      CheckLeftMostDerived(info);
 
-      CheckDispatch(info);
-
-      if (CXXMethodDecl* newop = info->DeclaresNewOperator())
-        ReportClassOverridesNew(info, newop);
+      if (!info->IsGCMixin()) {
+        CheckLeftMostDerived(info);
+        CheckDispatch(info);
+        if (CXXMethodDecl* newop = info->DeclaresNewOperator())
+          ReportClassOverridesNew(info, newop);
+      }
 
       {
         CheckGCRootsVisitor visitor;
@@ -1149,17 +1185,23 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     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) {
+        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);
     }
   }
 
@@ -1383,6 +1425,7 @@ class BlinkGCPluginConsumer : public ASTConsumer {
   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_;

tools/clang/blink_gc_plugin/tests/part_object_to_gc_derived_class.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.
+
+#include "part_object_to_gc_derived_class.h"
+
+namespace WebCore {
+
+void B::trace(Visitor* visitor)
+{
+    visitor->trace(m_a);
+}
+
+}

tools/clang/blink_gc_plugin/tests/part_object_to_gc_derived_class.h

+// 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.
+
+#ifndef PART_OBJECT_TO_GC_DERIVED_CLASS_H_
+#define PART_OBJECT_TO_GC_DERIVED_CLASS_H_
+
+#include "heap/stubs.h"
+
+namespace WebCore {
+
+class A : public GarbageCollected<A> { };
+
+class B : public GarbageCollected<B> {
+public:
+    void trace(Visitor*);
+private:
+    A m_a;
+};
+
+}
+
+#endif

tools/clang/blink_gc_plugin/tests/part_object_to_gc_derived_class.txt

+In file included from part_object_to_gc_derived_class.cpp:5:
+./part_object_to_gc_derived_class.h:14:1: warning: [blink-gc] Class 'B' contains invalid fields.
+class B : public GarbageCollected<B> {
+^
+./part_object_to_gc_derived_class.h:18:5: note: [blink-gc] Part-object field 'm_a' to a GC derived class declared here:
+    A m_a;
+    ^
+1 warning generated.