Serializer enable/disable flags need thread safety.

BUG=
R=hpayer@chromium.org, svenpanne@chromium.org

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20855 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/assembler-arm.cc

@@ -48,6 +48,7 @@ namespace internal {
 #ifdef DEBUG
 bool CpuFeatures::initialized_ = false;
 #endif
+bool CpuFeatures::hint_creating_snapshot_ = false;
 unsigned CpuFeatures::supported_ = 0;
 unsigned CpuFeatures::found_by_runtime_probing_only_ = 0;
 unsigned CpuFeatures::cross_compile_ = 0;
@@ -100,10 +101,27 @@ const char* DwVfpRegister::AllocationIndexToString(int index) {
 }
 
 
+void CpuFeatures::SetHintCreatingSnapshot() {
+  hint_creating_snapshot_ = true;
+}
+
+
+void CpuFeatures::ProbeWithoutIsolate() {
+  Probe(hint_creating_snapshot_);
+}
+
+
 void CpuFeatures::Probe() {
+  // The Serializer can only be queried after isolate initialization.
+  Probe(Serializer::enabled());
+}
+
+
+void CpuFeatures::Probe(bool serializer_enabled) {
   uint64_t standard_features = static_cast<unsigned>(
       OS::CpuFeaturesImpliedByPlatform()) | CpuFeaturesImpliedByCompiler();
-  ASSERT(supported_ == 0 || supported_ == standard_features);
+  ASSERT(supported_ == 0 ||
+         (supported_ & standard_features) == standard_features);
 #ifdef DEBUG
   initialized_ = true;
 #endif
@@ -113,7 +131,7 @@ void CpuFeatures::Probe() {
   // snapshot.
   supported_ |= standard_features;
 
-  if (Serializer::enabled()) {
+  if (serializer_enabled) {
     // No probing for features if we might serialize (generate snapshot).
     printf("   ");
     PrintFeatures();
@@ -1079,11 +1097,6 @@ static bool fits_shifter(uint32_t imm32,
 // encoded.
 bool Operand::must_output_reloc_info(const Assembler* assembler) const {
   if (rmode_ == RelocInfo::EXTERNAL_REFERENCE) {
-#ifdef DEBUG
-    if (!Serializer::enabled()) {
-      Serializer::TooLateToEnableNow();
-    }
-#endif  // def DEBUG
     if (assembler != NULL && assembler->predictable_code_size()) return true;
     return Serializer::enabled();
   } else if (RelocInfo::IsNone(rmode_)) {
@@ -3267,11 +3280,6 @@ void Assembler::RecordRelocInfo(const RelocInfo& rinfo) {
   if (!RelocInfo::IsNone(rinfo.rmode())) {
     // Don't record external references unless the heap will be serialized.
     if (rinfo.rmode() == RelocInfo::EXTERNAL_REFERENCE) {
-#ifdef DEBUG
-      if (!Serializer::enabled()) {
-        Serializer::TooLateToEnableNow();
-      }
-#endif
       if (!Serializer::enabled() && !emit_debug_code()) {
         return;
       }

src/arm/assembler-arm.h

@@ -58,6 +58,11 @@ class CpuFeatures : public AllStatic {
   // is enabled (snapshots must be portable).
   static void Probe();
 
+  // A special case for printing target and features, which we want to do
+  // before initializing the isolate
+  static void SetHintCreatingSnapshot();
+  static void ProbeWithoutIsolate();
+
   // Display target use when compiling.
   static void PrintTarget();
 
@@ -94,6 +99,9 @@ class CpuFeatures : public AllStatic {
   }
 
  private:
+  static void Probe(bool serializer_enabled);
+  static bool hint_creating_snapshot_;
+
   static bool Check(CpuFeature f, unsigned set) {
     return (set & flag2set(f)) != 0;
   }

src/arm64/assembler-arm64.cc

@@ -273,11 +273,6 @@ void Operand::initialize_handle(Handle<Object> handle) {
 
 bool Operand::NeedsRelocation() const {
   if (rmode_ == RelocInfo::EXTERNAL_REFERENCE) {
-#ifdef DEBUG
-    if (!Serializer::enabled()) {
-      Serializer::TooLateToEnableNow();
-    }
-#endif
     return Serializer::enabled();
   }
 
@@ -1970,9 +1965,6 @@ void Assembler::debug(const char* message, uint32_t code, Instr params) {
   // Don't generate simulator specific code if we are building a snapshot, which
   // might be run on real hardware.
   if (!Serializer::enabled()) {
-#ifdef DEBUG
-    Serializer::TooLateToEnableNow();
-#endif
     // The arguments to the debug marker need to be contiguous in memory, so
     // make sure we don't try to emit pools.
     BlockPoolsScope scope(this);
@@ -2525,11 +2517,6 @@ void Assembler::RecordRelocInfo(RelocInfo::Mode rmode, intptr_t data) {
   if (!RelocInfo::IsNone(rmode)) {
     // Don't record external references unless the heap will be serialized.
     if (rmode == RelocInfo::EXTERNAL_REFERENCE) {
-#ifdef DEBUG
-      if (!Serializer::enabled()) {
-        Serializer::TooLateToEnableNow();
-      }
-#endif
       if (!Serializer::enabled() && !emit_debug_code()) {
         return;
       }

src/flags.cc

@@ -545,7 +545,7 @@ void FlagList::ResetAllFlags() {
 void FlagList::PrintHelp() {
 #if V8_TARGET_ARCH_ARM
   CpuFeatures::PrintTarget();
-  CpuFeatures::Probe();
+  CpuFeatures::ProbeWithoutIsolate();
   CpuFeatures::PrintFeatures();
 #endif  // V8_TARGET_ARCH_ARM
 

src/ia32/assembler-ia32.cc

@@ -2703,11 +2703,6 @@ void Assembler::RecordRelocInfo(RelocInfo::Mode rmode, intptr_t data) {
   ASSERT(!RelocInfo::IsNone(rmode));
   // Don't record external references unless the heap will be serialized.
   if (rmode == RelocInfo::EXTERNAL_REFERENCE) {
-#ifdef DEBUG
-    if (!Serializer::enabled()) {
-      Serializer::TooLateToEnableNow();
-    }
-#endif
     if (!Serializer::enabled() && !emit_debug_code()) {
       return;
     }

src/mips/assembler-mips.cc

@@ -105,7 +105,8 @@ const char* DoubleRegister::AllocationIndexToString(int index) {
 void CpuFeatures::Probe() {
   unsigned standard_features = (OS::CpuFeaturesImpliedByPlatform() |
                                 CpuFeaturesImpliedByCompiler());
-  ASSERT(supported_ == 0 || supported_ == standard_features);
+  ASSERT(supported_ == 0 ||
+         (supported_ & standard_features) == standard_features);
 #ifdef DEBUG
   initialized_ = true;
 #endif

src/mksnapshot.cc

@@ -41,6 +41,10 @@
 #include "serialize.h"
 #include "list.h"
 
+#if V8_TARGET_ARCH_ARM
+#include "arm/assembler-arm-inl.h"
+#endif
+
 using namespace v8;
 
 
@@ -272,6 +276,12 @@ int main(int argc, char** argv) {
   // By default, log code create information in the snapshot.
   i::FLAG_log_code = true;
 
+#if V8_TARGET_ARCH_ARM
+  // Printing flags on ARM requires knowing if we intend to enable
+  // the serializer or not.
+  v8::internal::CpuFeatures::SetHintCreatingSnapshot();
+#endif
+
   // Print the usage if an error occurs when parsing the command line
   // flags or if the help flag is set.
   int result = i::FlagList::SetFlagsFromCommandLine(&argc, argv, true);
@@ -293,7 +303,7 @@ int main(int argc, char** argv) {
   Isolate* isolate = v8::Isolate::New();
   isolate->Enter();
   i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(isolate);
-  i::Serializer::Enable(internal_isolate);
+  i::Serializer::RequestEnable(internal_isolate);
   Persistent<Context> context;
   {
     HandleScope handle_scope(isolate);

src/serialize.cc

@@ -649,10 +649,7 @@ ExternalReferenceDecoder::~ExternalReferenceDecoder() {
   DeleteArray(encodings_);
 }
 
-
-bool Serializer::serialization_enabled_ = false;
-bool Serializer::too_late_to_enable_now_ = false;
-
+AtomicWord Serializer::serialization_state_ = SERIALIZER_STATE_UNINITIALIZED;
 
 class CodeAddressMap: public CodeEventLogger {
  public:
@@ -765,22 +762,42 @@ class CodeAddressMap: public CodeEventLogger {
 CodeAddressMap* Serializer::code_address_map_ = NULL;
 
 
-void Serializer::Enable(Isolate* isolate) {
-  if (!serialization_enabled_) {
-    ASSERT(!too_late_to_enable_now_);
-  }
-  if (serialization_enabled_) return;
-  serialization_enabled_ = true;
+void Serializer::RequestEnable(Isolate* isolate) {
   isolate->InitializeLoggingAndCounters();
   code_address_map_ = new CodeAddressMap(isolate);
 }
 
 
-void Serializer::Disable() {
-  if (!serialization_enabled_) return;
-  serialization_enabled_ = false;
-  delete code_address_map_;
-  code_address_map_ = NULL;
+void Serializer::InitializeOncePerProcess() {
+  // InitializeOncePerProcess is called by V8::InitializeOncePerProcess, a
+  // method guaranteed to be called only once in a process lifetime.
+  // serialization_state_ is read by many threads, hence the use of
+  // Atomic primitives. Here, we don't need a barrier or mutex to
+  // write it because V8 initialization is done by one thread, and gates
+  // all reads of serialization_state_.
+  ASSERT(NoBarrier_Load(&serialization_state_) ==
+         SERIALIZER_STATE_UNINITIALIZED);
+  SerializationState state = code_address_map_
+      ? SERIALIZER_STATE_ENABLED
+      : SERIALIZER_STATE_DISABLED;
+  NoBarrier_Store(&serialization_state_, state);
+}
+
+
+void Serializer::TearDown() {
+  // TearDown is called by V8::TearDown() for the default isolate. It's safe
+  // to shut down the serializer by that point. Just to be safe, we restore
+  // serialization_state_ to uninitialized.
+  ASSERT(NoBarrier_Load(&serialization_state_) !=
+         SERIALIZER_STATE_UNINITIALIZED);
+  if (code_address_map_) {
+    ASSERT(NoBarrier_Load(&serialization_state_) ==
+           SERIALIZER_STATE_ENABLED);
+    delete code_address_map_;
+    code_address_map_ = NULL;
+  }
+
+  NoBarrier_Store(&serialization_state_, SERIALIZER_STATE_UNINITIALIZED);
 }
 
 

src/serialize.h

@@ -470,13 +470,16 @@ class Serializer : public SerializerDeserializer {
   }
 
   Isolate* isolate() const { return isolate_; }
-  static void Enable(Isolate* isolate);
-  static void Disable();
-
-  // Call this when you have made use of the fact that there is no serialization
-  // going on.
-  static void TooLateToEnableNow() { too_late_to_enable_now_ = true; }
-  static bool enabled() { return serialization_enabled_; }
+  static void RequestEnable(Isolate* isolate);
+  static void InitializeOncePerProcess();
+  static void TearDown();
+
+  static bool enabled() {
+    SerializationState state = static_cast<SerializationState>(
+        NoBarrier_Load(&serialization_state_));
+    ASSERT(state != SERIALIZER_STATE_UNINITIALIZED);
+    return state == SERIALIZER_STATE_ENABLED;
+  }
   SerializationAddressMapper* address_mapper() { return &address_mapper_; }
   void PutRoot(int index,
                HeapObject* object,
@@ -574,9 +577,15 @@ class Serializer : public SerializerDeserializer {
   int fullness_[LAST_SPACE + 1];
   SnapshotByteSink* sink_;
   ExternalReferenceEncoder* external_reference_encoder_;
-  static bool serialization_enabled_;
-  // Did we already make use of the fact that serialization was not enabled?
-  static bool too_late_to_enable_now_;
+
+  enum SerializationState {
+    SERIALIZER_STATE_UNINITIALIZED = 0,
+    SERIALIZER_STATE_DISABLED = 1,
+    SERIALIZER_STATE_ENABLED = 2
+  };
+
+  static AtomicWord serialization_state_;
+
   SerializationAddressMapper address_mapper_;
   intptr_t root_index_wave_front_;
   void Pad();

src/v8.cc

@@ -98,6 +98,7 @@ void V8::TearDown() {
   call_completed_callbacks_ = NULL;
 
   Sampler::TearDown();
+  Serializer::TearDown();
 
 #ifdef V8_USE_DEFAULT_PLATFORM
   DefaultPlatform* platform = static_cast<DefaultPlatform*>(platform_);
@@ -172,6 +173,7 @@ void V8::RunMicrotasks(Isolate* isolate) {
 
 void V8::InitializeOncePerProcessImpl() {
   FlagList::EnforceFlagImplications();
+  Serializer::InitializeOncePerProcess();
 
   if (FLAG_predictable && FLAG_random_seed == 0) {
     // Avoid random seeds in predictable mode.

src/x64/assembler-x64.cc

@@ -2953,11 +2953,6 @@ void Assembler::RecordRelocInfo(RelocInfo::Mode rmode, intptr_t data) {
   ASSERT(!RelocInfo::IsNone(rmode));
   if (rmode == RelocInfo::EXTERNAL_REFERENCE) {
     // Don't record external references unless the heap will be serialized.
-#ifdef DEBUG
-    if (!Serializer::enabled()) {
-      Serializer::TooLateToEnableNow();
-    }
-#endif
     if (!Serializer::enabled() && !emit_debug_code()) {
       return;
     }

src/x64/macro-assembler-x64.cc

@@ -75,7 +75,6 @@ Operand MacroAssembler::ExternalOperand(ExternalReference target,
   if (root_array_available_ && !Serializer::enabled()) {
     intptr_t delta = RootRegisterDelta(target);
     if (delta != kInvalidRootRegisterDelta && is_int32(delta)) {
-      Serializer::TooLateToEnableNow();
       return Operand(kRootRegister, static_cast<int32_t>(delta));
     }
   }
@@ -88,7 +87,6 @@ void MacroAssembler::Load(Register destination, ExternalReference source) {
   if (root_array_available_ && !Serializer::enabled()) {
     intptr_t delta = RootRegisterDelta(source);
     if (delta != kInvalidRootRegisterDelta && is_int32(delta)) {
-      Serializer::TooLateToEnableNow();
       movp(destination, Operand(kRootRegister, static_cast<int32_t>(delta)));
       return;
     }
@@ -107,7 +105,6 @@ void MacroAssembler::Store(ExternalReference destination, Register source) {
   if (root_array_available_ && !Serializer::enabled()) {
     intptr_t delta = RootRegisterDelta(destination);
     if (delta != kInvalidRootRegisterDelta && is_int32(delta)) {
-      Serializer::TooLateToEnableNow();
       movp(Operand(kRootRegister, static_cast<int32_t>(delta)), source);
       return;
     }
@@ -127,7 +124,6 @@ void MacroAssembler::LoadAddress(Register destination,
   if (root_array_available_ && !Serializer::enabled()) {
     intptr_t delta = RootRegisterDelta(source);
     if (delta != kInvalidRootRegisterDelta && is_int32(delta)) {
-      Serializer::TooLateToEnableNow();
       leap(destination, Operand(kRootRegister, static_cast<int32_t>(delta)));
       return;
     }
@@ -144,7 +140,6 @@ int MacroAssembler::LoadAddressSize(ExternalReference source) {
     // instruction below.
     intptr_t delta = RootRegisterDelta(source);
     if (delta != kInvalidRootRegisterDelta && is_int32(delta)) {
-      Serializer::TooLateToEnableNow();
       // Operand is leap(scratch, Operand(kRootRegister, delta));
       // Opcodes : REX.W 8D ModRM Disp8/Disp32  - 4 or 7.
       int size = 4;

test/cctest/test-serialize.cc

@@ -266,7 +266,7 @@ static void Serialize() {
 // Test that the whole heap can be serialized.
 TEST(Serialize) {
   if (!Snapshot::HaveASnapshotToStartFrom()) {
-    Serializer::Enable(CcTest::i_isolate());
+    Serializer::RequestEnable(CcTest::i_isolate());
     v8::V8::Initialize();
     Serialize();
   }
@@ -276,7 +276,7 @@ TEST(Serialize) {
 // Test that heap serialization is non-destructive.
 TEST(SerializeTwice) {
   if (!Snapshot::HaveASnapshotToStartFrom()) {
-    Serializer::Enable(CcTest::i_isolate());
+    Serializer::RequestEnable(CcTest::i_isolate());
     v8::V8::Initialize();
     Serialize();
     Serialize();
@@ -375,7 +375,7 @@ DEPENDENT_TEST(DeserializeFromSecondSerializationAndRunScript2,
 TEST(PartialSerialization) {
   if (!Snapshot::HaveASnapshotToStartFrom()) {
     Isolate* isolate = CcTest::i_isolate();
-    Serializer::Enable(isolate);
+    Serializer::RequestEnable(isolate);
     v8::V8::Initialize();
     v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate);
     Heap* heap = isolate->heap();
@@ -526,7 +526,7 @@ DEPENDENT_TEST(PartialDeserialization, PartialSerialization) {
 TEST(ContextSerialization) {
   if (!Snapshot::HaveASnapshotToStartFrom()) {
     Isolate* isolate = CcTest::i_isolate();
-    Serializer::Enable(isolate);
+    Serializer::RequestEnable(isolate);
     v8::V8::Initialize();
     v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate);
     Heap* heap = isolate->heap();