Reviewers: Jakob,
Message:
Hi Jakob,
Could you have a look?
I'd be keen to get this in a chrome build.
Thanks for the help,
--Michael
Description:
Some extra checks to debug a crash failure related to type feedback vector.
It appears that we have some path that recompiles code that looks quite
different from the first compile. The vector structure is different. I've
added
extra CHECKs() to catch the problem a bit sooner, and also made it clear
that
setting code in the SharedFunctionInfo requires setting a vector, too.
BUG=503565
Please review this at https://codereview.chromium.org/1219533003/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+58, -39 lines):
M src/compiler.cc
M src/debug.cc
M src/factory.cc
M src/heap/mark-compact.cc
M src/objects.h
M src/objects.cc
M src/objects-inl.h
M src/runtime/runtime-function.cc
M src/type-feedback-vector.h
M test/cctest/test-heap.cc
Index: src/compiler.cc
diff --git a/src/compiler.cc b/src/compiler.cc
index
0f3ebe0e6726666acba53cb8745366e992aed288..9598351c62abdbaf2b411f93011e1ef896f554bd
100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -213,6 +213,7 @@ bool CompilationInfo::ShouldSelfOptimize() {
void CompilationInfo::EnsureFeedbackVector() {
if (feedback_vector_.is_null() ||
feedback_vector_->SpecDiffersFrom(function()->feedback_vector_spec())) {
+ CHECK(feedback_vector_.is_null());
feedback_vector_ = isolate()->factory()->NewTypeFeedbackVector(
function()->feedback_vector_spec());
}
@@ -684,8 +685,7 @@ MUST_USE_RESULT static MaybeHandle<Code>
GetUnoptimizedCodeCommon(
shared->set_scope_info(*scope_info);
// Update the code and feedback vector for the shared function info.
- shared->ReplaceCode(*info->code());
- shared->set_feedback_vector(*info->feedback_vector());
+ shared->ReplaceCode(*info->code(), *info->feedback_vector());
return info->code();
}
@@ -957,9 +957,8 @@ bool
Compiler::EnsureDeoptimizationSupport(CompilationInfo* info) {
}
if (!FullCodeGenerator::MakeCode(&unoptimized)) return false;
- shared->EnableDeoptimizationSupport(*unoptimized.code());
- shared->set_feedback_vector(*unoptimized.feedback_vector());
-
+ shared->EnableDeoptimizationSupport(*unoptimized.code(),
+ *unoptimized.feedback_vector());
info->MarkAsCompiled();
// The scope info might not have been set if a lazily compiled
@@ -1461,9 +1460,8 @@ Handle<SharedFunctionInfo>
Compiler::GetSharedFunctionInfo(
} else if (!lazy) {
// We have additional data from compilation now.
DCHECK(!existing->is_compiled());
- existing->ReplaceCode(*info.code());
+ existing->ReplaceCode(*info.code(), *info.feedback_vector());
existing->set_scope_info(*scope_info);
- existing->set_feedback_vector(*info.feedback_vector());
}
return existing;
}
@@ -1501,7 +1499,7 @@ MaybeHandle<Code>
Compiler::GetOptimizedCode(Handle<JSFunction> function,
if (!GetUnoptimizedCodeCommon(&unoptimized).ToHandle(¤t_code)) {
return MaybeHandle<Code>();
}
- shared->ReplaceCode(*current_code);
+ shared->ReplaceCode(*current_code, *unoptimized.feedback_vector());
}
current_code->set_profiler_ticks(0);
Index: src/debug.cc
diff --git a/src/debug.cc b/src/debug.cc
index
e952fe7ebb88a9dc07893f4e973be4fc5fc9ffab..5e043abe8681c4eaf66bc5a2fd269b549a8c1d0d
100644
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -1873,7 +1873,9 @@ void Debug::PrepareForBreakPoints() {
if (SkipSharedFunctionInfo(shared, active_code_marker)) continue;
if (shared->is_generator()) continue;
if (HasDebugBreakSlots(shared->code())) continue;
- shared->ReplaceCode(*lazy_compile);
+ shared->ReplaceCode(
+ *lazy_compile,
+
TypeFeedbackVector::cast(isolate_->heap()->empty_fixed_array()));
}
}
@@ -1890,12 +1892,14 @@ void Debug::PrepareForBreakPoints() {
// Mark generator functions that didn't have suspended activations for
lazy
// recompilation. Note that this set does not include any active
functions.
+ Heap* heap = isolate_->heap();
for (int i = 0; i < generator_functions.length(); i++) {
Handle<JSFunction> &function = generator_functions[i];
if (function->code()->kind() != Code::FUNCTION) continue;
if (function->code()->has_debug_break_slots()) continue;
function->ReplaceCode(*lazy_compile);
- function->shared()->ReplaceCode(*lazy_compile);
+ function->shared()->ReplaceCode(
+ *lazy_compile,
TypeFeedbackVector::cast(heap->empty_fixed_array()));
}
// Now recompile all functions with activation frames and and
Index: src/factory.cc
diff --git a/src/factory.cc b/src/factory.cc
index
2b79a5bf8f25a74a8ac37a35efa75fe7427ea86e..10d57e74948d4820698cd0237123b89a78dc867d
100644
--- a/src/factory.cc
+++ b/src/factory.cc
@@ -2158,6 +2158,9 @@ Handle<SharedFunctionInfo>
Factory::NewSharedFunctionInfo(
MaybeHandle<Code> maybe_code) {
Handle<Map> map = shared_function_info_map();
Handle<SharedFunctionInfo> share = New<SharedFunctionInfo>(map,
OLD_SPACE);
+ FeedbackVectorSpec empty_spec(0);
+ Handle<TypeFeedbackVector> feedback_vector =
+ NewTypeFeedbackVector(&empty_spec);
// Set pointer fields.
share->set_name(*name);
@@ -2166,6 +2169,7 @@ Handle<SharedFunctionInfo>
Factory::NewSharedFunctionInfo(
code = handle(isolate()->builtins()->builtin(Builtins::kIllegal));
}
share->set_code(*code);
+ share->set_feedback_vector(*feedback_vector, SKIP_WRITE_BARRIER);
share->set_optimized_code_map(Smi::FromInt(0));
share->set_scope_info(ScopeInfo::Empty(isolate()));
Code* construct_stub =
@@ -2176,10 +2180,6 @@ Handle<SharedFunctionInfo>
Factory::NewSharedFunctionInfo(
share->set_script(*undefined_value(), SKIP_WRITE_BARRIER);
share->set_debug_info(*undefined_value(), SKIP_WRITE_BARRIER);
share->set_inferred_name(*empty_string(), SKIP_WRITE_BARRIER);
- FeedbackVectorSpec empty_spec(0);
- Handle<TypeFeedbackVector> feedback_vector =
- NewTypeFeedbackVector(&empty_spec);
- share->set_feedback_vector(*feedback_vector, SKIP_WRITE_BARRIER);
#if TRACE_MAPS
share->set_unique_id(isolate()->GetNextUniqueSharedFunctionInfoId());
#endif
Index: src/heap/mark-compact.cc
diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
index
8bbfeb173f7ba3ac5e55b63145e62d3085338727..e833eb7e2c0c99081cd205be11e0de2928f94c37
100644
--- a/src/heap/mark-compact.cc
+++ b/src/heap/mark-compact.cc
@@ -851,6 +851,7 @@ void MarkCompactCollector::Finish() {
void CodeFlusher::ProcessJSFunctionCandidates() {
Code* lazy_compile =
isolate_->builtins()->builtin(Builtins::kCompileLazy);
Object* undefined = isolate_->heap()->undefined_value();
+ FixedArray* empty_fixed_array = isolate_->heap()->empty_fixed_array();
JSFunction* candidate = jsfunction_candidates_head_;
JSFunction* next_candidate;
@@ -873,6 +874,7 @@ void CodeFlusher::ProcessJSFunctionCandidates() {
shared->ClearOptimizedCodeMap();
}
shared->set_code(lazy_compile);
+
shared->set_feedback_vector(TypeFeedbackVector::cast(empty_fixed_array));
candidate->set_code(lazy_compile);
} else {
DCHECK(Marking::IsBlack(code_mark));
@@ -900,6 +902,7 @@ void CodeFlusher::ProcessJSFunctionCandidates() {
void CodeFlusher::ProcessSharedFunctionInfoCandidates() {
Code* lazy_compile =
isolate_->builtins()->builtin(Builtins::kCompileLazy);
+ FixedArray* empty_fixed_array = isolate_->heap()->empty_fixed_array();
SharedFunctionInfo* candidate = shared_function_info_candidates_head_;
SharedFunctionInfo* next_candidate;
@@ -920,6 +923,8 @@ void CodeFlusher::ProcessSharedFunctionInfoCandidates()
{
candidate->ClearOptimizedCodeMap();
}
candidate->set_code(lazy_compile);
+ candidate->set_feedback_vector(
+ TypeFeedbackVector::cast(empty_fixed_array));
}
Object** code_slot =
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index
0e43e1d3a0fbd434bf311fa3dffc579b3b73a3ee..277a8af4dee35c822ba5920d68cb5ef5cc7afa1c
100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -5443,7 +5443,7 @@ void SharedFunctionInfo::set_code(Code* value,
WriteBarrierMode mode) {
}
-void SharedFunctionInfo::ReplaceCode(Code* value) {
+void SharedFunctionInfo::ReplaceCode(Code* value, TypeFeedbackVector*
vector) {
// If the GC metadata field is already used then the function was
// enqueued as a code flushing candidate and we remove it now.
if (code()->gc_metadata() != NULL) {
@@ -5454,7 +5454,7 @@ void SharedFunctionInfo::ReplaceCode(Code* value) {
DCHECK(code()->gc_metadata() == NULL && value->gc_metadata() == NULL);
set_code(value);
-
+ set_feedback_vector(vector);
if (is_compiled()) set_never_compiled(false);
}
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
b14281a41bbbe9648a6838a292b811264596392b..3094f5d4157109aa88fd7dba061a1d5574f90df2
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -10559,7 +10559,8 @@ static bool IsCodeEquivalent(Code* code, Code*
recompiled) {
}
-void SharedFunctionInfo::EnableDeoptimizationSupport(Code* recompiled) {
+void SharedFunctionInfo::EnableDeoptimizationSupport(
+ Code* recompiled, TypeFeedbackVector* vector) {
DCHECK(!has_deoptimization_support());
DisallowHeapAllocation no_allocation;
Code* code = this->code();
@@ -10572,7 +10573,7 @@ void
SharedFunctionInfo::EnableDeoptimizationSupport(Code* recompiled) {
// old code, we have to replace it. We should try to avoid this
// altogether because it flushes valuable type feedback by
// effectively resetting all IC state.
- ReplaceCode(recompiled);
+ ReplaceCode(recompiled, vector);
}
DCHECK(has_deoptimization_support());
}
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
dc96725023d6c7f0bf14b0383303d3ef04f2b8ab..5add9261c2c318ed035954ddf45cd4ce0579ddda
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -6580,7 +6580,7 @@ class SharedFunctionInfo: public HeapObject {
// [code]: Function code.
DECL_ACCESSORS(code, Code)
- inline void ReplaceCode(Code* code);
+ inline void ReplaceCode(Code* code, TypeFeedbackVector* vector);
// [optimized_code_map]: Map from native context to optimized code
// and a shared literals array or Smi(0) if none.
@@ -6848,7 +6848,8 @@ class SharedFunctionInfo: public HeapObject {
inline bool has_deoptimization_support();
// Enable deoptimization support through recompiled code.
- void EnableDeoptimizationSupport(Code* recompiled);
+ void EnableDeoptimizationSupport(Code* recompiled,
+ TypeFeedbackVector* vector);
// Disable (further) attempted optimization of all functions sharing this
// shared function info.
Index: src/runtime/runtime-function.cc
diff --git a/src/runtime/runtime-function.cc
b/src/runtime/runtime-function.cc
index
749e16b9ed12dc37b1cf0461606839c06d92c510..026b2939672548ba5944c8430c85703514e910a9
100644
--- a/src/runtime/runtime-function.cc
+++ b/src/runtime/runtime-function.cc
@@ -226,10 +226,10 @@ RUNTIME_FUNCTION(Runtime_SetCode) {
// Set the code, scope info, formal parameter count, and the length
// of the target shared function info.
- target_shared->ReplaceCode(source_shared->code());
+ target_shared->ReplaceCode(source_shared->code(),
+ source_shared->feedback_vector());
target_shared->set_scope_info(source_shared->scope_info());
target_shared->set_length(source_shared->length());
- target_shared->set_feedback_vector(source_shared->feedback_vector());
target_shared->set_internal_formal_parameter_count(
source_shared->internal_formal_parameter_count());
target_shared->set_start_position_and_type(
Index: src/type-feedback-vector.h
diff --git a/src/type-feedback-vector.h b/src/type-feedback-vector.h
index
a6f72210fc9f2545d77ea92e885a6f801071fadd..89970c6580dbcfcb3c4509e69e8b34c2f4201231
100644
--- a/src/type-feedback-vector.h
+++ b/src/type-feedback-vector.h
@@ -146,39 +146,47 @@ class TypeFeedbackVector : public FixedArray {
// Conversion from a slot or ic slot to an integer index to the
underlying
// array.
int GetIndex(FeedbackVectorSlot slot) const {
- DCHECK(slot.ToInt() < first_ic_slot_index());
+ CHECK(slot.ToInt() < first_ic_slot_index());
return kReservedIndexCount + ic_metadata_length() + slot.ToInt();
}
int GetIndex(FeedbackVectorICSlot slot) const {
int first_ic_slot = first_ic_slot_index();
- DCHECK(slot.ToInt() < ICSlots());
+ CHECK(slot.ToInt() < ICSlots());
return first_ic_slot + slot.ToInt() * elements_per_ic_slot();
}
// Conversion from an integer index to either a slot or an ic slot. The
caller
// should know what kind she expects.
FeedbackVectorSlot ToSlot(int index) const {
- DCHECK(index >= kReservedIndexCount && index < first_ic_slot_index());
+ CHECK(index >= kReservedIndexCount && index < first_ic_slot_index());
return FeedbackVectorSlot(index - ic_metadata_length() -
kReservedIndexCount);
}
FeedbackVectorICSlot ToICSlot(int index) const {
- DCHECK(index >= first_ic_slot_index() && index < length());
+ CHECK(index >= first_ic_slot_index() && index < length());
int ic_slot = (index - first_ic_slot_index()) / elements_per_ic_slot();
return FeedbackVectorICSlot(ic_slot);
}
- Object* Get(FeedbackVectorSlot slot) const { return get(GetIndex(slot));
}
+ Object* Get(FeedbackVectorSlot slot) const {
+ CHECK(GetIndex(slot) < length());
+ return get(GetIndex(slot));
+ }
void Set(FeedbackVectorSlot slot, Object* value,
WriteBarrierMode mode = UPDATE_WRITE_BARRIER) {
+ CHECK(GetIndex(slot) < length());
set(GetIndex(slot), value, mode);
}
- Object* Get(FeedbackVectorICSlot slot) const { return
get(GetIndex(slot)); }
+ Object* Get(FeedbackVectorICSlot slot) const {
+ CHECK(GetIndex(slot) < length());
+ return get(GetIndex(slot));
+ }
void Set(FeedbackVectorICSlot slot, Object* value,
WriteBarrierMode mode = UPDATE_WRITE_BARRIER) {
+ CHECK(GetIndex(slot) < length());
set(GetIndex(slot), value, mode);
}
@@ -344,11 +352,11 @@ class CallICNexus : public FeedbackNexus {
CallICNexus(Handle<TypeFeedbackVector> vector, FeedbackVectorICSlot slot)
: FeedbackNexus(vector, slot) {
- DCHECK(vector->GetKind(slot) == Code::CALL_IC);
+ CHECK(vector->GetKind(slot) == Code::CALL_IC);
}
CallICNexus(TypeFeedbackVector* vector, FeedbackVectorICSlot slot)
: FeedbackNexus(vector, slot) {
- DCHECK(vector->GetKind(slot) == Code::CALL_IC);
+ CHECK(vector->GetKind(slot) == Code::CALL_IC);
}
void Clear(Code* host);
@@ -377,11 +385,11 @@ class LoadICNexus : public FeedbackNexus {
public:
LoadICNexus(Handle<TypeFeedbackVector> vector, FeedbackVectorICSlot slot)
: FeedbackNexus(vector, slot) {
- DCHECK(vector->GetKind(slot) == Code::LOAD_IC);
+ CHECK(vector->GetKind(slot) == Code::LOAD_IC);
}
LoadICNexus(TypeFeedbackVector* vector, FeedbackVectorICSlot slot)
: FeedbackNexus(vector, slot) {
- DCHECK(vector->GetKind(slot) == Code::LOAD_IC);
+ CHECK(vector->GetKind(slot) == Code::LOAD_IC);
}
void Clear(Code* host);
@@ -398,11 +406,11 @@ class KeyedLoadICNexus : public FeedbackNexus {
public:
KeyedLoadICNexus(Handle<TypeFeedbackVector> vector, FeedbackVectorICSlot
slot)
: FeedbackNexus(vector, slot) {
- DCHECK(vector->GetKind(slot) == Code::KEYED_LOAD_IC);
+ CHECK(vector->GetKind(slot) == Code::KEYED_LOAD_IC);
}
KeyedLoadICNexus(TypeFeedbackVector* vector, FeedbackVectorICSlot slot)
: FeedbackNexus(vector, slot) {
- DCHECK(vector->GetKind(slot) == Code::KEYED_LOAD_IC);
+ CHECK(vector->GetKind(slot) == Code::KEYED_LOAD_IC);
}
void Clear(Code* host);
@@ -423,11 +431,11 @@ class StoreICNexus : public FeedbackNexus {
public:
StoreICNexus(Handle<TypeFeedbackVector> vector, FeedbackVectorICSlot
slot)
: FeedbackNexus(vector, slot) {
- DCHECK(vector->GetKind(slot) == Code::STORE_IC);
+ CHECK(vector->GetKind(slot) == Code::STORE_IC);
}
StoreICNexus(TypeFeedbackVector* vector, FeedbackVectorICSlot slot)
: FeedbackNexus(vector, slot) {
- DCHECK(vector->GetKind(slot) == Code::STORE_IC);
+ CHECK(vector->GetKind(slot) == Code::STORE_IC);
}
void Clear(Code* host);
@@ -445,11 +453,11 @@ class KeyedStoreICNexus : public FeedbackNexus {
KeyedStoreICNexus(Handle<TypeFeedbackVector> vector,
FeedbackVectorICSlot slot)
: FeedbackNexus(vector, slot) {
- DCHECK(vector->GetKind(slot) == Code::KEYED_STORE_IC);
+ CHECK(vector->GetKind(slot) == Code::KEYED_STORE_IC);
}
KeyedStoreICNexus(TypeFeedbackVector* vector, FeedbackVectorICSlot slot)
: FeedbackNexus(vector, slot) {
- DCHECK(vector->GetKind(slot) == Code::KEYED_STORE_IC);
+ CHECK(vector->GetKind(slot) == Code::KEYED_STORE_IC);
}
void Clear(Code* host);
Index: test/cctest/test-heap.cc
diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index
6229670da848efbbf75c6d50e3605d0e052180a7..f0b82a1e4c647cf31d35bfd2d8d845e18826b417
100644
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -5933,7 +5933,9 @@ static void RemoveCodeAndGC(const
v8::FunctionCallbackInfo<v8::Value>& args) {
Handle<Object> obj = v8::Utils::OpenHandle(*args[0]);
Handle<JSFunction> fun = Handle<JSFunction>::cast(obj);
fun->ReplaceCode(*isolate->builtins()->CompileLazy());
- fun->shared()->ReplaceCode(*isolate->builtins()->CompileLazy());
+ fun->shared()->ReplaceCode(
+ *isolate->builtins()->CompileLazy(),
+ TypeFeedbackVector::cast(isolate->heap()->empty_fixed_array()));
isolate->heap()->CollectAllAvailableGarbage("remove code and gc");
}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
