Commit 547e6732 authored by Anton Bikineev's avatar Anton Bikineev Committed by Commit Bot

blink_gc_plugin: Add check for WeakPtrs to GCed objects

This check was missing.

Bug: 522357
Change-Id: I6dfa88e07f30c0c6c3a4f09378c75554f3b3fc91
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2130988Reviewed-by: default avatarMichael Lippautz <mlippautz@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarNico Weber <thakis@chromium.org>
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755391}
parent 1ca36693
...@@ -111,7 +111,7 @@ void CheckFieldsVisitor::AtValue(Value* edge) { ...@@ -111,7 +111,7 @@ void CheckFieldsVisitor::AtValue(Value* edge) {
if (!Parent() || !edge->value()->IsGCAllocated()) if (!Parent() || !edge->value()->IsGCAllocated())
return; return;
// Disallow unique_ptr<T>, RefPtr<T>. // Disallow unique_ptr<T>, scoped_refptr<T>, WeakPtr<T>.
if (Parent()->IsUniquePtr() || Parent()->IsRefPtr()) { if (Parent()->IsUniquePtr() || Parent()->IsRefPtr()) {
invalid_fields_.push_back(std::make_pair( invalid_fields_.push_back(std::make_pair(
current_, InvalidSmartPtr(Parent()))); current_, InvalidSmartPtr(Parent())));
...@@ -133,7 +133,8 @@ void CheckFieldsVisitor::AtCollection(Collection* edge) { ...@@ -133,7 +133,8 @@ void CheckFieldsVisitor::AtCollection(Collection* edge) {
CheckFieldsVisitor::Error CheckFieldsVisitor::InvalidSmartPtr(Edge* ptr) { CheckFieldsVisitor::Error CheckFieldsVisitor::InvalidSmartPtr(Edge* ptr) {
if (ptr->IsRefPtr()) if (ptr->IsRefPtr())
return kRefPtrToGCManaged; return ptr->Kind() == Edge::kStrong ? kRefPtrToGCManaged
: kWeakPtrToGCManaged;
if (ptr->IsUniquePtr()) if (ptr->IsUniquePtr())
return kUniquePtrToGCManaged; return kUniquePtrToGCManaged;
llvm_unreachable("Unknown smart pointer kind"); llvm_unreachable("Unknown smart pointer kind");
......
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
class FieldPoint; class FieldPoint;
// This visitor checks that the fields of a class are "well formed". // This visitor checks that the fields of a class are "well formed".
// - unique_ptr and RefPtr must not point to a GC derived type. // - unique_ptr, scoped_refptr and WeakPtr must not point to a GC derived type.
// - Part objects must not be a GC derived type. // - Part objects must not be a GC derived type.
// - An on-heap class must never contain GC roots. // - An on-heap class must never contain GC roots.
// - Only stack-allocated types may point to stack-allocated types. // - Only stack-allocated types may point to stack-allocated types.
...@@ -23,6 +23,7 @@ class CheckFieldsVisitor : public RecursiveEdgeVisitor { ...@@ -23,6 +23,7 @@ class CheckFieldsVisitor : public RecursiveEdgeVisitor {
enum Error { enum Error {
kRawPtrToGCManaged, kRawPtrToGCManaged,
kRefPtrToGCManaged, kRefPtrToGCManaged,
kWeakPtrToGCManaged,
kReferencePtrToGCManaged, kReferencePtrToGCManaged,
kUniquePtrToGCManaged, kUniquePtrToGCManaged,
kMemberToGCUnmanaged, kMemberToGCUnmanaged,
......
...@@ -61,6 +61,12 @@ class Config { ...@@ -61,6 +61,12 @@ class Config {
static bool IsRefPtr(llvm::StringRef name) { return name == "scoped_refptr"; } static bool IsRefPtr(llvm::StringRef name) { return name == "scoped_refptr"; }
static bool IsWeakPtr(llvm::StringRef name) { return name == "WeakPtr"; }
static bool IsRefOrWeakPtr(llvm::StringRef name) {
return IsRefPtr(name) || IsWeakPtr(name);
}
static bool IsUniquePtr(llvm::StringRef name) { static bool IsUniquePtr(llvm::StringRef name) {
return name == "unique_ptr"; return name == "unique_ptr";
} }
......
...@@ -49,6 +49,9 @@ const char kRawPtrToGCManagedClassNote[] = ...@@ -49,6 +49,9 @@ const char kRawPtrToGCManagedClassNote[] =
const char kRefPtrToGCManagedClassNote[] = const char kRefPtrToGCManagedClassNote[] =
"[blink-gc] scoped_refptr field %0 to a GC managed class declared here:"; "[blink-gc] scoped_refptr field %0 to a GC managed class declared here:";
const char kWeakPtrToGCManagedClassNote[] =
"[blink-gc] WeakPtr field %0 to a GC managed class declared here:";
const char kReferencePtrToGCManagedClassNote[] = const char kReferencePtrToGCManagedClassNote[] =
"[blink-gc] Reference pointer field %0 to a GC managed class" "[blink-gc] Reference pointer field %0 to a GC managed class"
" declared here:"; " declared here:";
...@@ -221,6 +224,8 @@ DiagnosticsReporter::DiagnosticsReporter( ...@@ -221,6 +224,8 @@ DiagnosticsReporter::DiagnosticsReporter(
DiagnosticsEngine::Note, kRawPtrToGCManagedClassNote); DiagnosticsEngine::Note, kRawPtrToGCManagedClassNote);
diag_ref_ptr_to_gc_managed_class_note_ = diagnostic_.getCustomDiagID( diag_ref_ptr_to_gc_managed_class_note_ = diagnostic_.getCustomDiagID(
DiagnosticsEngine::Note, kRefPtrToGCManagedClassNote); DiagnosticsEngine::Note, kRefPtrToGCManagedClassNote);
diag_weak_ptr_to_gc_managed_class_note_ = diagnostic_.getCustomDiagID(
DiagnosticsEngine::Note, kWeakPtrToGCManagedClassNote);
diag_reference_ptr_to_gc_managed_class_note_ = diagnostic_.getCustomDiagID( diag_reference_ptr_to_gc_managed_class_note_ = diagnostic_.getCustomDiagID(
DiagnosticsEngine::Note, kReferencePtrToGCManagedClassNote); DiagnosticsEngine::Note, kReferencePtrToGCManagedClassNote);
diag_unique_ptr_to_gc_managed_class_note_ = diagnostic_.getCustomDiagID( diag_unique_ptr_to_gc_managed_class_note_ = diagnostic_.getCustomDiagID(
...@@ -326,6 +331,8 @@ void DiagnosticsReporter::ClassContainsInvalidFields( ...@@ -326,6 +331,8 @@ void DiagnosticsReporter::ClassContainsInvalidFields(
note = diag_raw_ptr_to_gc_managed_class_note_; note = diag_raw_ptr_to_gc_managed_class_note_;
} else if (error.second == CheckFieldsVisitor::kRefPtrToGCManaged) { } else if (error.second == CheckFieldsVisitor::kRefPtrToGCManaged) {
note = diag_ref_ptr_to_gc_managed_class_note_; note = diag_ref_ptr_to_gc_managed_class_note_;
} else if (error.second == CheckFieldsVisitor::kWeakPtrToGCManaged) {
note = diag_weak_ptr_to_gc_managed_class_note_;
} else if (error.second == CheckFieldsVisitor::kReferencePtrToGCManaged) { } else if (error.second == CheckFieldsVisitor::kReferencePtrToGCManaged) {
note = diag_reference_ptr_to_gc_managed_class_note_; note = diag_reference_ptr_to_gc_managed_class_note_;
} else if (error.second == CheckFieldsVisitor::kUniquePtrToGCManaged) { } else if (error.second == CheckFieldsVisitor::kUniquePtrToGCManaged) {
......
...@@ -125,6 +125,7 @@ class DiagnosticsReporter { ...@@ -125,6 +125,7 @@ class DiagnosticsReporter {
unsigned diag_field_should_not_be_traced_note_; unsigned diag_field_should_not_be_traced_note_;
unsigned diag_raw_ptr_to_gc_managed_class_note_; unsigned diag_raw_ptr_to_gc_managed_class_note_;
unsigned diag_ref_ptr_to_gc_managed_class_note_; unsigned diag_ref_ptr_to_gc_managed_class_note_;
unsigned diag_weak_ptr_to_gc_managed_class_note_;
unsigned diag_reference_ptr_to_gc_managed_class_note_; unsigned diag_reference_ptr_to_gc_managed_class_note_;
unsigned diag_own_ptr_to_gc_managed_class_note_; unsigned diag_own_ptr_to_gc_managed_class_note_;
unsigned diag_unique_ptr_to_gc_managed_class_note_; unsigned diag_unique_ptr_to_gc_managed_class_note_;
......
...@@ -161,14 +161,17 @@ class RawPtr : public PtrEdge { ...@@ -161,14 +161,17 @@ class RawPtr : public PtrEdge {
class RefPtr : public PtrEdge { class RefPtr : public PtrEdge {
public: public:
explicit RefPtr(Edge* ptr) : PtrEdge(ptr) { } RefPtr(Edge* ptr, LivenessKind kind) : PtrEdge(ptr), kind_(kind) {}
bool IsRefPtr() override { return true; } bool IsRefPtr() override { return true; }
LivenessKind Kind() override { return kStrong; } LivenessKind Kind() override { return kind_; }
bool NeedsFinalization() override { return true; } bool NeedsFinalization() override { return true; }
TracingStatus NeedsTracing(NeedsTracingOption) override { TracingStatus NeedsTracing(NeedsTracingOption) override {
return TracingStatus::Illegal(); return TracingStatus::Illegal();
} }
void Accept(EdgeVisitor* visitor) override { visitor->VisitRefPtr(this); } void Accept(EdgeVisitor* visitor) override { visitor->VisitRefPtr(this); }
private:
LivenessKind kind_;
}; };
class UniquePtr : public PtrEdge { class UniquePtr : public PtrEdge {
......
...@@ -656,9 +656,10 @@ Edge* RecordInfo::CreateEdge(const Type* type) { ...@@ -656,9 +656,10 @@ Edge* RecordInfo::CreateEdge(const Type* type) {
TemplateArgs args; TemplateArgs args;
if (Config::IsRefPtr(info->name()) && info->GetTemplateArgs(1, &args)) { if (Config::IsRefOrWeakPtr(info->name()) && info->GetTemplateArgs(1, &args)) {
if (Edge* ptr = CreateEdge(args[0])) if (Edge* ptr = CreateEdge(args[0]))
return new RefPtr(ptr); return new RefPtr(
ptr, Config::IsRefPtr(info->name()) ? Edge::kStrong : Edge::kWeak);
return 0; return 0;
} }
......
...@@ -32,6 +32,13 @@ public: ...@@ -32,6 +32,13 @@ public:
T* operator->() { return 0; } T* operator->() { return 0; }
}; };
template<typename T> class WeakPtr {
public:
~WeakPtr() { }
operator T*() const { return 0; }
T* operator->() { return 0; }
};
class DefaultAllocator { class DefaultAllocator {
public: public:
static const bool isGarbageCollected = false; static const bool isGarbageCollected = false;
......
...@@ -15,6 +15,7 @@ class PartObject { ...@@ -15,6 +15,7 @@ class PartObject {
DISALLOW_NEW(); DISALLOW_NEW();
private: private:
scoped_refptr<HeapObject> m_obj; scoped_refptr<HeapObject> m_obj;
WeakPtr<HeapObject> m_obj2;
}; };
class HeapObject : public GarbageCollected<HeapObject> { class HeapObject : public GarbageCollected<HeapObject> {
......
...@@ -5,10 +5,13 @@ class PartObject { ...@@ -5,10 +5,13 @@ class PartObject {
./ref_ptr_to_gc_managed_class.h:17:5: note: [blink-gc] scoped_refptr field 'm_obj' to a GC managed class declared here: ./ref_ptr_to_gc_managed_class.h:17:5: note: [blink-gc] scoped_refptr field 'm_obj' to a GC managed class declared here:
scoped_refptr<HeapObject> m_obj; scoped_refptr<HeapObject> m_obj;
^ ^
./ref_ptr_to_gc_managed_class.h:20:1: warning: [blink-gc] Class 'HeapObject' contains invalid fields. ./ref_ptr_to_gc_managed_class.h:18:5: note: [blink-gc] WeakPtr field 'm_obj2' to a GC managed class declared here:
WeakPtr<HeapObject> m_obj2;
^
./ref_ptr_to_gc_managed_class.h:21:1: warning: [blink-gc] Class 'HeapObject' contains invalid fields.
class HeapObject : public GarbageCollected<HeapObject> { class HeapObject : public GarbageCollected<HeapObject> {
^ ^
./ref_ptr_to_gc_managed_class.h:26:3: note: [blink-gc] scoped_refptr field 'm_objs' to a GC managed class declared here: ./ref_ptr_to_gc_managed_class.h:27:3: note: [blink-gc] scoped_refptr field 'm_objs' to a GC managed class declared here:
Vector<scoped_refptr<HeapObject>> m_objs; Vector<scoped_refptr<HeapObject>> m_objs;
^ ^
2 warnings generated. 2 warnings generated.
...@@ -23,7 +23,7 @@ class DerivedHeapObject2 : public HeapObject { ...@@ -23,7 +23,7 @@ class DerivedHeapObject2 : public HeapObject {
./stack_allocated.h:43:3: warning: [blink-gc] Garbage collected class 'DerivedHeapObject2' is not permitted to override its new operator. ./stack_allocated.h:43:3: warning: [blink-gc] Garbage collected class 'DerivedHeapObject2' is not permitted to override its new operator.
STACK_ALLOCATED(); STACK_ALLOCATED();
^ ^
./heap/stubs.h:182:5: note: expanded from macro 'STACK_ALLOCATED' ./heap/stubs.h:189:5: note: expanded from macro 'STACK_ALLOCATED'
__attribute__((annotate("blink_stack_allocated"))) \ __attribute__((annotate("blink_stack_allocated"))) \
^ ^
stack_allocated.cpp:12:1: warning: [blink-gc] Class 'AnonStackObject' contains invalid fields. stack_allocated.cpp:12:1: warning: [blink-gc] Class 'AnonStackObject' contains invalid fields.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment