From 5e1cd18d945edca5bdd823db3a831a5762d2f255 Mon Sep 17 00:00:00 2001 From: "rossberg@chromium.org" Date: Tue, 21 Oct 2014 12:57:19 +0000 Subject: [PATCH] Fix data race with interface caching R=bmeurer@chromium.org BUG=421634 LOG=N Review URL: https://codereview.chromium.org/667703002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24774 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/interface.cc | 99 +++++++++++++++++++++++++++++++++++++----------- src/interface.h | 13 +++---- 2 files changed, 81 insertions(+), 31 deletions(-) diff --git a/src/interface.cc b/src/interface.cc index 62169f597b..d325c0e4d8 100644 --- a/src/interface.cc +++ b/src/interface.cc @@ -6,23 +6,67 @@ #include "src/interface.h" +#include "src/base/lazy-instance.h" + namespace v8 { namespace internal { +// --------------------------------------------------------------------------- +// Initialization. + +struct Interface::ValueCreate { + static void Construct(Interface* ptr) { + ::new (ptr) Interface(VALUE + FROZEN); + } +}; + + +struct Interface::ConstCreate { + static void Construct(Interface* ptr) { + ::new (ptr) Interface(VALUE + CONST + FROZEN); + } +}; + + +namespace { + base::LazyInstance::type value_interface = + LAZY_INSTANCE_INITIALIZER; + + base::LazyInstance::type const_interface = + LAZY_INSTANCE_INITIALIZER; +} + + +Interface* Interface::NewValue() { + return value_interface.Pointer(); // Cached. +} + + +Interface* Interface::NewConst() { + return const_interface.Pointer(); // Cached. +} + + +// --------------------------------------------------------------------------- +// Lookup. + Interface* Interface::Lookup(Handle name, Zone* zone) { DCHECK(IsModule()); ZoneHashMap* map = Chase()->exports_; - if (map == NULL) return NULL; + if (map == nullptr) return nullptr; ZoneAllocationPolicy allocator(zone); - ZoneHashMap::Entry* p = map->Lookup(name.location(), name->Hash(), false, - allocator); - if (p == NULL) return NULL; + ZoneHashMap::Entry* p = + map->Lookup(name.location(), name->Hash(), false, allocator); + if (p == nullptr) return nullptr; DCHECK(*static_cast(p->key) == *name); - DCHECK(p->value != NULL); + DCHECK(p->value != nullptr); return static_cast(p->value); } +// --------------------------------------------------------------------------- +// Addition. + #ifdef DEBUG // Current nesting depth for debug output. class Nesting { @@ -48,9 +92,9 @@ void Interface::DoAdd(const void* name, uint32_t hash, Interface* interface, PrintF("%*s# Adding...\n", Nesting::current(), ""); PrintF("%*sthis = ", Nesting::current(), ""); this->Print(Nesting::current()); - const AstRawString* symbol = static_cast(name); - PrintF("%*s%.*s : ", Nesting::current(), "", symbol->length(), - symbol->raw_data()); + const AstRawString* raw = static_cast(name); + PrintF("%*s%.*s : ", Nesting::current(), "", + raw->length(), raw->raw_data()); interface->Print(Nesting::current()); } #endif @@ -58,7 +102,7 @@ void Interface::DoAdd(const void* name, uint32_t hash, Interface* interface, ZoneHashMap** map = &Chase()->exports_; ZoneAllocationPolicy allocator(zone); - if (*map == NULL) { + if (*map == nullptr) { *map = new(zone->New(sizeof(ZoneHashMap))) ZoneHashMap(ZoneHashMap::PointersMatch, ZoneHashMap::kDefaultHashMapCapacity, allocator); @@ -66,10 +110,10 @@ void Interface::DoAdd(const void* name, uint32_t hash, Interface* interface, ZoneHashMap::Entry* p = (*map)->Lookup(const_cast(name), hash, !IsFrozen(), allocator); - if (p == NULL) { + if (p == nullptr) { // This didn't have name but was frozen already, that's an error. *ok = false; - } else if (p->value == NULL) { + } else if (p->value == nullptr) { p->value = interface; } else { #ifdef DEBUG @@ -88,11 +132,14 @@ void Interface::DoAdd(const void* name, uint32_t hash, Interface* interface, } +// --------------------------------------------------------------------------- +// Unification. + void Interface::Unify(Interface* that, Zone* zone, bool* ok) { if (this->forward_) return this->Chase()->Unify(that, zone, ok); if (that->forward_) return this->Unify(that->Chase(), zone, ok); - DCHECK(this->forward_ == NULL); - DCHECK(that->forward_ == NULL); + DCHECK(this->forward_ == nullptr); + DCHECK(that->forward_ == nullptr); *ok = true; if (this == that) return; @@ -118,7 +165,7 @@ void Interface::Unify(Interface* that, Zone* zone, bool* ok) { #endif // Merge the smaller interface into the larger, for performance. - if (this->exports_ != NULL && (that->exports_ == NULL || + if (this->exports_ != nullptr && (that->exports_ == nullptr || this->exports_->occupancy() >= that->exports_->occupancy())) { this->DoUnify(that, ok, zone); } else { @@ -138,8 +185,8 @@ void Interface::Unify(Interface* that, Zone* zone, bool* ok) { void Interface::DoUnify(Interface* that, bool* ok, Zone* zone) { - DCHECK(this->forward_ == NULL); - DCHECK(that->forward_ == NULL); + DCHECK(this->forward_ == nullptr); + DCHECK(that->forward_ == nullptr); DCHECK(!this->IsValue()); DCHECK(!that->IsValue()); DCHECK(this->index_ == -1); @@ -152,8 +199,8 @@ void Interface::DoUnify(Interface* that, bool* ok, Zone* zone) { // Try to merge all members from that into this. ZoneHashMap* map = that->exports_; - if (map != NULL) { - for (ZoneHashMap::Entry* p = map->Start(); p != NULL; p = map->Next(p)) { + if (map != nullptr) { + for (ZoneHashMap::Entry* p = map->Start(); p != nullptr; p = map->Next(p)) { this->DoAdd(p->key, p->hash, static_cast(p->value), zone, ok); if (!*ok) return; } @@ -161,8 +208,8 @@ void Interface::DoUnify(Interface* that, bool* ok, Zone* zone) { // If the new interface is larger than that's, then there were members in // 'this' which 'that' didn't have. If 'that' was frozen that is an error. - int this_size = this->exports_ == NULL ? 0 : this->exports_->occupancy(); - int that_size = map == NULL ? 0 : map->occupancy(); + int this_size = this->exports_ == nullptr ? 0 : this->exports_->occupancy(); + int that_size = map == nullptr ? 0 : map->occupancy(); if (that->IsFrozen() && this_size > that_size) { *ok = false; return; @@ -174,14 +221,19 @@ void Interface::DoUnify(Interface* that, bool* ok, Zone* zone) { } +// --------------------------------------------------------------------------- +// Printing. + #ifdef DEBUG void Interface::Print(int n) { int n0 = n > 0 ? n : 0; if (FLAG_print_interface_details) { PrintF("%p", static_cast(this)); - for (Interface* link = this->forward_; link != NULL; link = link->forward_) + for (Interface* link = this->forward_; link != nullptr; + link = link->forward_) { PrintF("->%p", static_cast(link)); + } PrintF(" "); } @@ -194,14 +246,15 @@ void Interface::Print(int n) { } else if (IsModule()) { PrintF("module %d %s{", Index(), IsFrozen() ? "" : "(unresolved) "); ZoneHashMap* map = Chase()->exports_; - if (map == NULL || map->occupancy() == 0) { + if (map == nullptr || map->occupancy() == 0) { PrintF("}\n"); } else if (n < 0 || n0 >= 2 * FLAG_print_interface_depth) { // Avoid infinite recursion on cyclic types. PrintF("...}\n"); } else { PrintF("\n"); - for (ZoneHashMap::Entry* p = map->Start(); p != NULL; p = map->Next(p)) { + for (ZoneHashMap::Entry* p = map->Start(); + p != nullptr; p = map->Next(p)) { String* name = *static_cast(p->key); Interface* interface = static_cast(p->value); PrintF("%*s%s : ", n0 + 2, "", name->ToAsciiArray()); diff --git a/src/interface.h b/src/interface.h index 598d038142..ae549634c5 100644 --- a/src/interface.h +++ b/src/interface.h @@ -41,15 +41,9 @@ class Interface : public ZoneObject { return new(zone) Interface(NONE); } - static Interface* NewValue() { - static Interface value_interface(VALUE + FROZEN); // Cached. - return &value_interface; - } + static Interface* NewValue(); - static Interface* NewConst() { - static Interface value_interface(VALUE + CONST + FROZEN); // Cached. - return &value_interface; - } + static Interface* NewConst(); static Interface* NewModule(Zone* zone) { return new(zone) Interface(MODULE); @@ -178,6 +172,9 @@ class Interface : public ZoneObject { // --------------------------------------------------------------------------- // Implementation. private: + struct ValueCreate; + struct ConstCreate; + enum Flags { // All flags are monotonic NONE = 0, VALUE = 1, // This type describes a value