From b5c542bac8660ada1b55acd6f10fd0cb1cb8115e Mon Sep 17 00:00:00 2001 From: adamk Date: Thu, 6 Oct 2016 11:19:44 -0700 Subject: [PATCH] Avoid static initializers in PropertyAccessCompiler Introduce AccessCompilerData which hangs off the Isolate, and initialize it when the first PropertyAccessCompiler is instantiated. This avoids TSAN failures when trying to access load/store calling convention arrays. BUG=v8:5427 Review-Url: https://codereview.chromium.org/2389313002 Cr-Commit-Position: refs/heads/master@{#40055} --- BUILD.gn | 1 + src/ic/access-compiler-data.h | 48 +++++++++++++++++++++++++ src/ic/access-compiler.cc | 13 ++++--- src/ic/access-compiler.h | 12 +++---- src/ic/arm/access-compiler-arm.cc | 22 ++++++------ src/ic/arm64/access-compiler-arm64.cc | 27 +++++++------- src/ic/ia32/access-compiler-ia32.cc | 21 ++++++----- src/ic/mips/access-compiler-mips.cc | 22 ++++++------ src/ic/mips64/access-compiler-mips64.cc | 22 ++++++------ src/ic/ppc/access-compiler-ppc.cc | 22 ++++++------ src/ic/s390/access-compiler-s390.cc | 21 +++++------ src/ic/x64/access-compiler-x64.cc | 23 ++++++------ src/ic/x87/access-compiler-x87.cc | 21 ++++++----- src/isolate.cc | 5 +++ src/isolate.h | 4 +++ src/v8.gyp | 1 + 16 files changed, 166 insertions(+), 119 deletions(-) create mode 100644 src/ic/access-compiler-data.h diff --git a/BUILD.gn b/BUILD.gn index faff97eb68..26ef84bdb8 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1378,6 +1378,7 @@ v8_source_set("v8_base") { "src/heap/store-buffer.h", "src/i18n.cc", "src/i18n.h", + "src/ic/access-compiler-data.h", "src/ic/access-compiler.cc", "src/ic/access-compiler.h", "src/ic/call-optimization.cc", diff --git a/src/ic/access-compiler-data.h b/src/ic/access-compiler-data.h new file mode 100644 index 0000000000..dffcac7d05 --- /dev/null +++ b/src/ic/access-compiler-data.h @@ -0,0 +1,48 @@ +// Copyright 2016 the V8 project 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 V8_IC_ACCESS_COMPILER_DATA_H_ +#define V8_IC_ACCESS_COMPILER_DATA_H_ + +#include + +#include "src/allocation.h" +#include "src/base/macros.h" + +namespace v8 { +namespace internal { + +class AccessCompilerData { + public: + AccessCompilerData() {} + + bool IsInitialized() const { return load_calling_convention_ != nullptr; } + void Initialize(int load_register_count, const Register* load_registers, + int store_register_count, const Register* store_registers) { + load_calling_convention_.reset(NewArray(load_register_count)); + for (int i = 0; i < load_register_count; ++i) { + load_calling_convention_[i] = load_registers[i]; + } + store_calling_convention_.reset(NewArray(store_register_count)); + for (int i = 0; i < store_register_count; ++i) { + store_calling_convention_[i] = store_registers[i]; + } + } + + Register* load_calling_convention() { return load_calling_convention_.get(); } + Register* store_calling_convention() { + return store_calling_convention_.get(); + } + + private: + std::unique_ptr load_calling_convention_; + std::unique_ptr store_calling_convention_; + + DISALLOW_COPY_AND_ASSIGN(AccessCompilerData); +}; + +} // namespace internal +} // namespace v8 + +#endif // V8_IC_ACCESS_COMPILER_DATA_H_ diff --git a/src/ic/access-compiler.cc b/src/ic/access-compiler.cc index bb6b5e50d9..d92f9c0c53 100644 --- a/src/ic/access-compiler.cc +++ b/src/ic/access-compiler.cc @@ -4,7 +4,6 @@ #include "src/ic/access-compiler.h" - namespace v8 { namespace internal { @@ -42,13 +41,17 @@ void PropertyAccessCompiler::TailCallBuiltin(MacroAssembler* masm, GenerateTailCall(masm, code); } - -Register* PropertyAccessCompiler::GetCallingConvention(Code::Kind kind) { +Register* PropertyAccessCompiler::GetCallingConvention(Isolate* isolate, + Code::Kind kind) { + AccessCompilerData* data = isolate->access_compiler_data(); + if (!data->IsInitialized()) { + InitializePlatformSpecific(data); + } if (kind == Code::LOAD_IC || kind == Code::KEYED_LOAD_IC) { - return load_calling_convention(); + return data->load_calling_convention(); } DCHECK(kind == Code::STORE_IC || kind == Code::KEYED_STORE_IC); - return store_calling_convention(); + return data->store_calling_convention(); } diff --git a/src/ic/access-compiler.h b/src/ic/access-compiler.h index ecc5c08a59..3d488e82ea 100644 --- a/src/ic/access-compiler.h +++ b/src/ic/access-compiler.h @@ -6,13 +6,13 @@ #define V8_IC_ACCESS_COMPILER_H_ #include "src/code-stubs.h" +#include "src/ic/access-compiler-data.h" #include "src/macro-assembler.h" #include "src/objects.h" namespace v8 { namespace internal { - class PropertyAccessCompiler BASE_EMBEDDED { public: static Builtins::Name MissBuiltin(Code::Kind kind) { @@ -36,7 +36,7 @@ class PropertyAccessCompiler BASE_EMBEDDED { protected: PropertyAccessCompiler(Isolate* isolate, Code::Kind kind, CacheHolderFlag cache_holder) - : registers_(GetCallingConvention(kind)), + : registers_(GetCallingConvention(isolate, kind)), kind_(kind), cache_holder_(cache_holder), isolate_(isolate), @@ -59,11 +59,6 @@ class PropertyAccessCompiler BASE_EMBEDDED { Register scratch1() const { return registers_[2]; } Register scratch2() const { return registers_[3]; } - static Register* GetCallingConvention(Code::Kind); - static Register* load_calling_convention(); - static Register* store_calling_convention(); - static Register* keyed_store_calling_convention(); - Register* registers_; static void GenerateTailCall(MacroAssembler* masm, Handle code); @@ -72,6 +67,9 @@ class PropertyAccessCompiler BASE_EMBEDDED { Handle GetCodeWithFlags(Code::Flags flags, Handle name); private: + static Register* GetCallingConvention(Isolate* isolate, Code::Kind kind); + static void InitializePlatformSpecific(AccessCompilerData* data); + Code::Kind kind_; CacheHolderFlag cache_holder_; diff --git a/src/ic/arm/access-compiler-arm.cc b/src/ic/arm/access-compiler-arm.cc index 9ce485ed46..e501cdcc8b 100644 --- a/src/ic/arm/access-compiler-arm.cc +++ b/src/ic/arm/access-compiler-arm.cc @@ -17,24 +17,22 @@ void PropertyAccessCompiler::GenerateTailCall(MacroAssembler* masm, __ Jump(code, RelocInfo::CODE_TARGET); } - -Register* PropertyAccessCompiler::load_calling_convention() { - // receiver, name, scratch1, scratch2, scratch3. +void PropertyAccessCompiler::InitializePlatformSpecific( + AccessCompilerData* data) { Register receiver = LoadDescriptor::ReceiverRegister(); Register name = LoadDescriptor::NameRegister(); - static Register registers[] = {receiver, name, r3, r0, r4}; - return registers; -} + // Load calling convention. + // receiver, name, scratch1, scratch2, scratch3. + Register load_registers[] = {receiver, name, r3, r0, r4}; -Register* PropertyAccessCompiler::store_calling_convention() { + // Store calling convention. // receiver, name, scratch1, scratch2. - Register receiver = StoreDescriptor::ReceiverRegister(); - Register name = StoreDescriptor::NameRegister(); - static Register registers[] = {receiver, name, r3, r4}; - return registers; -} + Register store_registers[] = {receiver, name, r3, r4}; + data->Initialize(arraysize(load_registers), load_registers, + arraysize(store_registers), store_registers); +} #undef __ } // namespace internal diff --git a/src/ic/arm64/access-compiler-arm64.cc b/src/ic/arm64/access-compiler-arm64.cc index 6273633822..8cbb5278ea 100644 --- a/src/ic/arm64/access-compiler-arm64.cc +++ b/src/ic/arm64/access-compiler-arm64.cc @@ -25,24 +25,23 @@ void PropertyAccessCompiler::GenerateTailCall(MacroAssembler* masm, // registers are actually scratch registers, and which are important. For now, // we use the same assignments as ARM to remain on the safe side. -Register* PropertyAccessCompiler::load_calling_convention() { - // receiver, name, scratch1, scratch2, scratch3. +void PropertyAccessCompiler::InitializePlatformSpecific( + AccessCompilerData* data) { Register receiver = LoadDescriptor::ReceiverRegister(); Register name = LoadDescriptor::NameRegister(); - static Register registers[] = {receiver, name, x3, x0, x4}; - return registers; + + // Load calling convention. + // receiver, name, scratch1, scratch2, scratch3. + Register load_registers[] = {receiver, name, x3, x0, x4}; + + // Store calling convention. + // receiver, name, scratch1, scratch2. + Register store_registers[] = {receiver, name, x3, x4}; + + data->Initialize(arraysize(load_registers), load_registers, + arraysize(store_registers), store_registers); } - -Register* PropertyAccessCompiler::store_calling_convention() { - // receiver, value, scratch1, scratch2. - Register receiver = StoreDescriptor::ReceiverRegister(); - Register name = StoreDescriptor::NameRegister(); - static Register registers[] = {receiver, name, x3, x4}; - return registers; -} - - #undef __ } // namespace internal } // namespace v8 diff --git a/src/ic/ia32/access-compiler-ia32.cc b/src/ic/ia32/access-compiler-ia32.cc index 3219f3d1cb..411c744659 100644 --- a/src/ic/ia32/access-compiler-ia32.cc +++ b/src/ic/ia32/access-compiler-ia32.cc @@ -16,22 +16,21 @@ void PropertyAccessCompiler::GenerateTailCall(MacroAssembler* masm, __ jmp(code, RelocInfo::CODE_TARGET); } - -Register* PropertyAccessCompiler::load_calling_convention() { - // receiver, name, scratch1, scratch2, scratch3. +void PropertyAccessCompiler::InitializePlatformSpecific( + AccessCompilerData* data) { Register receiver = LoadDescriptor::ReceiverRegister(); Register name = LoadDescriptor::NameRegister(); - static Register registers[] = {receiver, name, ebx, eax, edi}; - return registers; -} + // Load calling convention. + // receiver, name, scratch1, scratch2, scratch3. + Register load_registers[] = {receiver, name, ebx, eax, edi}; -Register* PropertyAccessCompiler::store_calling_convention() { + // Store calling convention. // receiver, name, scratch1, scratch2. - Register receiver = StoreDescriptor::ReceiverRegister(); - Register name = StoreDescriptor::NameRegister(); - static Register registers[] = {receiver, name, ebx, edi}; - return registers; + Register store_registers[] = {receiver, name, ebx, edi}; + + data->Initialize(arraysize(load_registers), load_registers, + arraysize(store_registers), store_registers); } #undef __ diff --git a/src/ic/mips/access-compiler-mips.cc b/src/ic/mips/access-compiler-mips.cc index 2aa0283485..1c97ca3cad 100644 --- a/src/ic/mips/access-compiler-mips.cc +++ b/src/ic/mips/access-compiler-mips.cc @@ -17,24 +17,22 @@ void PropertyAccessCompiler::GenerateTailCall(MacroAssembler* masm, __ Jump(code, RelocInfo::CODE_TARGET); } - -Register* PropertyAccessCompiler::load_calling_convention() { - // receiver, name, scratch1, scratch2, scratch3. +void PropertyAccessCompiler::InitializePlatformSpecific( + AccessCompilerData* data) { Register receiver = LoadDescriptor::ReceiverRegister(); Register name = LoadDescriptor::NameRegister(); - static Register registers[] = {receiver, name, a3, a0, t0}; - return registers; -} + // Load calling convention. + // receiver, name, scratch1, scratch2, scratch3. + Register load_registers[] = {receiver, name, a3, a0, t0}; -Register* PropertyAccessCompiler::store_calling_convention() { + // Store calling convention. // receiver, name, scratch1, scratch2. - Register receiver = StoreDescriptor::ReceiverRegister(); - Register name = StoreDescriptor::NameRegister(); - static Register registers[] = {receiver, name, a3, t0}; - return registers; -} + Register store_registers[] = {receiver, name, a3, t0}; + data->Initialize(arraysize(load_registers), load_registers, + arraysize(store_registers), store_registers); +} #undef __ } // namespace internal diff --git a/src/ic/mips64/access-compiler-mips64.cc b/src/ic/mips64/access-compiler-mips64.cc index bf6c73e86f..16d7a3d790 100644 --- a/src/ic/mips64/access-compiler-mips64.cc +++ b/src/ic/mips64/access-compiler-mips64.cc @@ -17,24 +17,22 @@ void PropertyAccessCompiler::GenerateTailCall(MacroAssembler* masm, __ Jump(code, RelocInfo::CODE_TARGET); } - -Register* PropertyAccessCompiler::load_calling_convention() { - // receiver, name, scratch1, scratch2, scratch3. +void PropertyAccessCompiler::InitializePlatformSpecific( + AccessCompilerData* data) { Register receiver = LoadDescriptor::ReceiverRegister(); Register name = LoadDescriptor::NameRegister(); - static Register registers[] = {receiver, name, a3, a0, a4}; - return registers; -} + // Load calling convention. + // receiver, name, scratch1, scratch2, scratch3. + Register load_registers[] = {receiver, name, a3, a0, a4}; -Register* PropertyAccessCompiler::store_calling_convention() { + // Store calling convention. // receiver, name, scratch1, scratch2. - Register receiver = StoreDescriptor::ReceiverRegister(); - Register name = StoreDescriptor::NameRegister(); - static Register registers[] = {receiver, name, a3, a4}; - return registers; -} + Register store_registers[] = {receiver, name, a3, a4}; + data->Initialize(arraysize(load_registers), load_registers, + arraysize(store_registers), store_registers); +} #undef __ } // namespace internal diff --git a/src/ic/ppc/access-compiler-ppc.cc b/src/ic/ppc/access-compiler-ppc.cc index 6143b4ce47..f78ef57e74 100644 --- a/src/ic/ppc/access-compiler-ppc.cc +++ b/src/ic/ppc/access-compiler-ppc.cc @@ -17,24 +17,22 @@ void PropertyAccessCompiler::GenerateTailCall(MacroAssembler* masm, __ Jump(code, RelocInfo::CODE_TARGET); } - -Register* PropertyAccessCompiler::load_calling_convention() { - // receiver, name, scratch1, scratch2, scratch3. +void PropertyAccessCompiler::InitializePlatformSpecific( + AccessCompilerData* data) { Register receiver = LoadDescriptor::ReceiverRegister(); Register name = LoadDescriptor::NameRegister(); - static Register registers[] = {receiver, name, r6, r3, r7}; - return registers; -} + // Load calling convention. + // receiver, name, scratch1, scratch2, scratch3. + Register load_registers[] = {receiver, name, r6, r3, r7}; -Register* PropertyAccessCompiler::store_calling_convention() { + // Store calling convention. // receiver, name, scratch1, scratch2. - Register receiver = StoreDescriptor::ReceiverRegister(); - Register name = StoreDescriptor::NameRegister(); - static Register registers[] = {receiver, name, r6, r7}; - return registers; -} + Register store_registers[] = {receiver, name, r6, r7}; + data->Initialize(arraysize(load_registers), load_registers, + arraysize(store_registers), store_registers); +} #undef __ } // namespace internal diff --git a/src/ic/s390/access-compiler-s390.cc b/src/ic/s390/access-compiler-s390.cc index 0a3285d5aa..ed8c089b9c 100644 --- a/src/ic/s390/access-compiler-s390.cc +++ b/src/ic/s390/access-compiler-s390.cc @@ -18,20 +18,21 @@ void PropertyAccessCompiler::GenerateTailCall(MacroAssembler* masm, __ Jump(code, RelocInfo::CODE_TARGET); } -Register* PropertyAccessCompiler::load_calling_convention() { - // receiver, name, scratch1, scratch2, scratch3. +void PropertyAccessCompiler::InitializePlatformSpecific( + AccessCompilerData* data) { Register receiver = LoadDescriptor::ReceiverRegister(); Register name = LoadDescriptor::NameRegister(); - static Register registers[] = {receiver, name, r5, r2, r6}; - return registers; -} -Register* PropertyAccessCompiler::store_calling_convention() { + // Load calling convention. + // receiver, name, scratch1, scratch2, scratch3. + Register load_registers[] = {receiver, name, r5, r2, r6}; + + // Store calling convention. // receiver, name, scratch1, scratch2. - Register receiver = StoreDescriptor::ReceiverRegister(); - Register name = StoreDescriptor::NameRegister(); - static Register registers[] = {receiver, name, r5, r6}; - return registers; + Register store_registers[] = {receiver, name, r5, r6}; + + data->Initialize(arraysize(load_registers), load_registers, + arraysize(store_registers), store_registers); } #undef __ diff --git a/src/ic/x64/access-compiler-x64.cc b/src/ic/x64/access-compiler-x64.cc index 2b292528c8..9e95b9506c 100644 --- a/src/ic/x64/access-compiler-x64.cc +++ b/src/ic/x64/access-compiler-x64.cc @@ -11,30 +11,27 @@ namespace internal { #define __ ACCESS_MASM(masm) - void PropertyAccessCompiler::GenerateTailCall(MacroAssembler* masm, Handle code) { __ jmp(code, RelocInfo::CODE_TARGET); } - -Register* PropertyAccessCompiler::load_calling_convention() { - // receiver, name, scratch1, scratch2, scratch3. +void PropertyAccessCompiler::InitializePlatformSpecific( + AccessCompilerData* data) { Register receiver = LoadDescriptor::ReceiverRegister(); Register name = LoadDescriptor::NameRegister(); - static Register registers[] = {receiver, name, rax, rbx, rdi}; - return registers; -} + // Load calling convention. + // receiver, name, scratch1, scratch2, scratch3. + Register load_registers[] = {receiver, name, rax, rbx, rdi}; -Register* PropertyAccessCompiler::store_calling_convention() { + // Store calling convention. // receiver, name, scratch1, scratch2. - Register receiver = StoreDescriptor::ReceiverRegister(); - Register name = StoreDescriptor::NameRegister(); - static Register registers[] = {receiver, name, rbx, rdi}; - return registers; -} + Register store_registers[] = {receiver, name, rbx, rdi}; + data->Initialize(arraysize(load_registers), load_registers, + arraysize(store_registers), store_registers); +} #undef __ } // namespace internal diff --git a/src/ic/x87/access-compiler-x87.cc b/src/ic/x87/access-compiler-x87.cc index e528de65ba..d1867553cd 100644 --- a/src/ic/x87/access-compiler-x87.cc +++ b/src/ic/x87/access-compiler-x87.cc @@ -16,22 +16,21 @@ void PropertyAccessCompiler::GenerateTailCall(MacroAssembler* masm, __ jmp(code, RelocInfo::CODE_TARGET); } - -Register* PropertyAccessCompiler::load_calling_convention() { - // receiver, name, scratch1, scratch2, scratch3. +void PropertyAccessCompiler::InitializePlatformSpecific( + AccessCompilerData* data) { Register receiver = LoadDescriptor::ReceiverRegister(); Register name = LoadDescriptor::NameRegister(); - static Register registers[] = {receiver, name, ebx, eax, edi}; - return registers; -} + // Load calling convention. + // receiver, name, scratch1, scratch2, scratch3. + Register load_registers[] = {receiver, name, ebx, eax, edi}; -Register* PropertyAccessCompiler::store_calling_convention() { + // Store calling convention. // receiver, name, scratch1, scratch2. - Register receiver = StoreDescriptor::ReceiverRegister(); - Register name = StoreDescriptor::NameRegister(); - static Register registers[] = {receiver, name, ebx, edi}; - return registers; + Register store_registers[] = {receiver, name, ebx, edi}; + + data->Initialize(arraysize(load_registers), load_registers, + arraysize(store_registers), store_registers); } #undef __ diff --git a/src/isolate.cc b/src/isolate.cc index c3e5eb6b37..b8fe6b993b 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -26,6 +26,7 @@ #include "src/deoptimizer.h" #include "src/external-reference-table.h" #include "src/frames-inl.h" +#include "src/ic/access-compiler-data.h" #include "src/ic/stub-cache.h" #include "src/interface-descriptors.h" #include "src/interpreter/interpreter.h" @@ -2211,6 +2212,9 @@ Isolate::~Isolate() { delete[] call_descriptor_data_; call_descriptor_data_ = NULL; + delete access_compiler_data_; + access_compiler_data_ = NULL; + delete regexp_stack_; regexp_stack_ = NULL; @@ -2378,6 +2382,7 @@ bool Isolate::Init(Deserializer* des) { date_cache_ = new DateCache(); call_descriptor_data_ = new CallInterfaceDescriptorData[CallDescriptors::NUMBER_OF_DESCRIPTORS]; + access_compiler_data_ = new AccessCompilerData(); cpu_profiler_ = new CpuProfiler(this); heap_profiler_ = new HeapProfiler(heap()); interpreter_ = new interpreter::Interpreter(this); diff --git a/src/isolate.h b/src/isolate.h index ce7b6f209e..9fdff50549 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -33,6 +33,7 @@ class RandomNumberGenerator; namespace internal { +class AccessCompilerData; class BasicBlockProfiler; class Bootstrapper; class CancelableTaskManager; @@ -1029,6 +1030,8 @@ class Isolate { CallInterfaceDescriptorData* call_descriptor_data(int index); + AccessCompilerData* access_compiler_data() { return access_compiler_data_; } + void IterateDeferredHandles(ObjectVisitor* visitor); void LinkDeferredHandles(DeferredHandles* deferred_handles); void UnlinkDeferredHandles(DeferredHandles* deferred_handles); @@ -1342,6 +1345,7 @@ class Isolate { List regexp_indices_; DateCache* date_cache_; CallInterfaceDescriptorData* call_descriptor_data_; + AccessCompilerData* access_compiler_data_; base::RandomNumberGenerator* random_number_generator_; base::AtomicValue rail_mode_; diff --git a/src/v8.gyp b/src/v8.gyp index d633451a73..9d87345a42 100644 --- a/src/v8.gyp +++ b/src/v8.gyp @@ -950,6 +950,7 @@ 'i18n.h', 'icu_util.cc', 'icu_util.h', + 'ic/access-compiler-data.h', 'ic/access-compiler.cc', 'ic/access-compiler.h', 'ic/call-optimization.cc',