Commit cb19257a authored by bmeurer's avatar bmeurer Committed by Commit bot

[crankshaft] Protect against deopt loops from string length overflows.

Crankshaft just unconditionally deoptimizes the code when the length of
a string addition result would overflow. In order to protect against
deopt loops we insert a global protector cell.

We will use the same mechanism for inlining certain string additions
into TurboFan as well, and protecting against overflow (we will also
extend this to deal with String.prototype.concat and friends once we
get there).

BUG=v8:5404
R=jarin@chromium.org,hpayer@chromium.org

Review-Url: https://codereview.chromium.org/2348293002
Cr-Commit-Position: refs/heads/master@{#39511}
parent 9e640b74
...@@ -2368,7 +2368,7 @@ HValue* HGraphBuilder::BuildAddStringLengths(HValue* left_length, ...@@ -2368,7 +2368,7 @@ HValue* HGraphBuilder::BuildAddStringLengths(HValue* left_length,
HValue* length = AddUncasted<HAdd>(left_length, right_length); HValue* length = AddUncasted<HAdd>(left_length, right_length);
// Check that length <= kMaxLength <=> length < MaxLength + 1. // Check that length <= kMaxLength <=> length < MaxLength + 1.
HValue* max_length = Add<HConstant>(String::kMaxLength + 1); HValue* max_length = Add<HConstant>(String::kMaxLength + 1);
if (top_info()->IsStub()) { if (top_info()->IsStub() || !isolate()->IsStringLengthOverflowIntact()) {
// This is a mitigation for crbug.com/627934; the real fix // This is a mitigation for crbug.com/627934; the real fix
// will be to migrate the StringAddStub to TurboFan one day. // will be to migrate the StringAddStub to TurboFan one day.
IfBuilder if_invalid(this); IfBuilder if_invalid(this);
...@@ -2380,6 +2380,7 @@ HValue* HGraphBuilder::BuildAddStringLengths(HValue* left_length, ...@@ -2380,6 +2380,7 @@ HValue* HGraphBuilder::BuildAddStringLengths(HValue* left_length,
} }
if_invalid.End(); if_invalid.End();
} else { } else {
graph()->MarkDependsOnStringLengthOverflow();
Add<HBoundsCheck>(length, max_length); Add<HBoundsCheck>(length, max_length);
} }
return length; return length;
......
...@@ -440,6 +440,13 @@ class HGraph final : public ZoneObject { ...@@ -440,6 +440,13 @@ class HGraph final : public ZoneObject {
return depends_on_empty_array_proto_elements_; return depends_on_empty_array_proto_elements_;
} }
void MarkDependsOnStringLengthOverflow() {
if (depends_on_string_length_overflow_) return;
info()->dependencies()->AssumePropertyCell(
isolate()->factory()->string_length_protector());
depends_on_string_length_overflow_ = true;
}
bool has_uint32_instructions() { bool has_uint32_instructions() {
DCHECK(uint32_instructions_ == NULL || !uint32_instructions_->is_empty()); DCHECK(uint32_instructions_ == NULL || !uint32_instructions_->is_empty());
return uint32_instructions_ != NULL; return uint32_instructions_ != NULL;
...@@ -515,6 +522,7 @@ class HGraph final : public ZoneObject { ...@@ -515,6 +522,7 @@ class HGraph final : public ZoneObject {
bool allow_code_motion_; bool allow_code_motion_;
bool use_optimistic_licm_; bool use_optimistic_licm_;
bool depends_on_empty_array_proto_elements_; bool depends_on_empty_array_proto_elements_;
bool depends_on_string_length_overflow_;
int type_change_checksum_; int type_change_checksum_;
int maximum_environment_size_; int maximum_environment_size_;
int no_side_effects_scope_count_; int no_side_effects_scope_count_;
......
...@@ -1214,6 +1214,13 @@ Handle<Object> Factory::NewError(Handle<JSFunction> constructor, ...@@ -1214,6 +1214,13 @@ Handle<Object> Factory::NewError(Handle<JSFunction> constructor,
return maybe_error.ToHandleChecked(); return maybe_error.ToHandleChecked();
} }
Handle<Object> Factory::NewInvalidStringLengthError() {
// Invalidate the "string length" protector.
if (isolate()->IsStringLengthOverflowIntact()) {
isolate()->InvalidateStringLengthOverflowProtector();
}
return NewRangeError(MessageTemplate::kInvalidStringLength);
}
#define DEFINE_ERROR(NAME, name) \ #define DEFINE_ERROR(NAME, name) \
Handle<Object> Factory::New##NAME(MessageTemplate::Template template_index, \ Handle<Object> Factory::New##NAME(MessageTemplate::Template template_index, \
......
...@@ -586,9 +586,7 @@ class Factory final { ...@@ -586,9 +586,7 @@ class Factory final {
Handle<Object> NewError(Handle<JSFunction> constructor, Handle<Object> NewError(Handle<JSFunction> constructor,
Handle<String> message); Handle<String> message);
Handle<Object> NewInvalidStringLengthError() { Handle<Object> NewInvalidStringLengthError();
return NewRangeError(MessageTemplate::kInvalidStringLength);
}
Handle<Object> NewURIError() { Handle<Object> NewURIError() {
return NewError(isolate()->uri_error_function(), return NewError(isolate()->uri_error_function(),
......
...@@ -2911,6 +2911,10 @@ void Heap::CreateInitialObjects() { ...@@ -2911,6 +2911,10 @@ void Heap::CreateInitialObjects() {
handle(Smi::FromInt(Isolate::kArrayProtectorValid), isolate())); handle(Smi::FromInt(Isolate::kArrayProtectorValid), isolate()));
set_species_protector(*species_cell); set_species_protector(*species_cell);
cell = factory->NewPropertyCell();
cell->set_value(Smi::FromInt(Isolate::kArrayProtectorValid));
set_string_length_protector(*cell);
set_serialized_templates(empty_fixed_array()); set_serialized_templates(empty_fixed_array());
set_weak_stack_trace_list(Smi::FromInt(0)); set_weak_stack_trace_list(Smi::FromInt(0));
......
...@@ -166,6 +166,7 @@ using v8::MemoryPressureLevel; ...@@ -166,6 +166,7 @@ using v8::MemoryPressureLevel;
V(Cell, is_concat_spreadable_protector, IsConcatSpreadableProtector) \ V(Cell, is_concat_spreadable_protector, IsConcatSpreadableProtector) \
V(PropertyCell, has_instance_protector, HasInstanceProtector) \ V(PropertyCell, has_instance_protector, HasInstanceProtector) \
V(Cell, species_protector, SpeciesProtector) \ V(Cell, species_protector, SpeciesProtector) \
V(PropertyCell, string_length_protector, StringLengthProtector) \
/* Special numbers */ \ /* Special numbers */ \
V(HeapNumber, nan_value, NanValue) \ V(HeapNumber, nan_value, NanValue) \
V(HeapNumber, hole_nan_value, HoleNanValue) \ V(HeapNumber, hole_nan_value, HoleNanValue) \
......
...@@ -147,6 +147,11 @@ bool Isolate::IsHasInstanceLookupChainIntact() { ...@@ -147,6 +147,11 @@ bool Isolate::IsHasInstanceLookupChainIntact() {
return has_instance_cell->value() == Smi::FromInt(kArrayProtectorValid); return has_instance_cell->value() == Smi::FromInt(kArrayProtectorValid);
} }
bool Isolate::IsStringLengthOverflowIntact() {
PropertyCell* has_instance_cell = heap()->string_length_protector();
return has_instance_cell->value() == Smi::FromInt(kArrayProtectorValid);
}
} // namespace internal } // namespace internal
} // namespace v8 } // namespace v8
......
...@@ -2818,6 +2818,15 @@ void Isolate::InvalidateArraySpeciesProtector() { ...@@ -2818,6 +2818,15 @@ void Isolate::InvalidateArraySpeciesProtector() {
DCHECK(!IsArraySpeciesLookupChainIntact()); DCHECK(!IsArraySpeciesLookupChainIntact());
} }
void Isolate::InvalidateStringLengthOverflowProtector() {
DCHECK(factory()->string_length_protector()->value()->IsSmi());
DCHECK(IsStringLengthOverflowIntact());
PropertyCell::SetValueWithInvalidation(
factory()->string_length_protector(),
handle(Smi::FromInt(kArrayProtectorInvalid), this));
DCHECK(!IsStringLengthOverflowIntact());
}
bool Isolate::IsAnyInitialArrayPrototype(Handle<JSArray> array) { bool Isolate::IsAnyInitialArrayPrototype(Handle<JSArray> array) {
DisallowHeapAllocation no_gc; DisallowHeapAllocation no_gc;
return IsInAnyContext(*array, Context::INITIAL_ARRAY_PROTOTYPE_INDEX); return IsInAnyContext(*array, Context::INITIAL_ARRAY_PROTOTYPE_INDEX);
......
...@@ -1010,6 +1010,7 @@ class Isolate { ...@@ -1010,6 +1010,7 @@ class Isolate {
inline bool IsHasInstanceLookupChainIntact(); inline bool IsHasInstanceLookupChainIntact();
bool IsIsConcatSpreadableLookupChainIntact(); bool IsIsConcatSpreadableLookupChainIntact();
bool IsIsConcatSpreadableLookupChainIntact(JSReceiver* receiver); bool IsIsConcatSpreadableLookupChainIntact(JSReceiver* receiver);
inline bool IsStringLengthOverflowIntact();
// On intent to set an element in object, make sure that appropriate // On intent to set an element in object, make sure that appropriate
// notifications occur if the set is on the elements of the array or // notifications occur if the set is on the elements of the array or
...@@ -1028,6 +1029,7 @@ class Isolate { ...@@ -1028,6 +1029,7 @@ class Isolate {
void InvalidateArraySpeciesProtector(); void InvalidateArraySpeciesProtector();
void InvalidateHasInstanceProtector(); void InvalidateHasInstanceProtector();
void InvalidateIsConcatSpreadableProtector(); void InvalidateIsConcatSpreadableProtector();
void InvalidateStringLengthOverflowProtector();
// Returns true if array is the initial array prototype in any native context. // Returns true if array is the initial array prototype in any native context.
bool IsAnyInitialArrayPrototype(Handle<JSArray> array); bool IsAnyInitialArrayPrototype(Handle<JSArray> array);
......
...@@ -234,8 +234,7 @@ RUNTIME_FUNCTION(Runtime_ThrowIncompatibleMethodReceiver) { ...@@ -234,8 +234,7 @@ RUNTIME_FUNCTION(Runtime_ThrowIncompatibleMethodReceiver) {
RUNTIME_FUNCTION(Runtime_ThrowInvalidStringLength) { RUNTIME_FUNCTION(Runtime_ThrowInvalidStringLength) {
HandleScope scope(isolate); HandleScope scope(isolate);
THROW_NEW_ERROR_RETURN_FAILURE( THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewInvalidStringLengthError());
isolate, NewRangeError(MessageTemplate::kInvalidStringLength));
} }
RUNTIME_FUNCTION(Runtime_ThrowIteratorResultNotAnObject) { RUNTIME_FUNCTION(Runtime_ThrowIteratorResultNotAnObject) {
......
// 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.
// Flags: --allow-natives-syntax
function foo(a, b) {
return a + "0123456789012";
}
foo("a");
foo("a");
%OptimizeFunctionOnNextCall(foo);
foo("a");
var a = "a".repeat(268435440);
assertThrows(function() { foo(a); });
%OptimizeFunctionOnNextCall(foo);
assertThrows(function() { foo(a); });
assertOptimized(foo);
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