Commit 53510f6a authored by machenbach's avatar machenbach Committed by Commit bot

Revert of [crankshaft] Protect against deopt loops from string length...

Revert of [crankshaft] Protect against deopt loops from string length overflows. (patchset #1 id:1 of https://codereview.chromium.org/2348293002/ )

Reason for revert:
Mean https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20arm64%20-%20sim%20-%20MSAN/builds/10910

Original issue's description:
> [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
>
> Committed: https://crrev.com/cb19257a926a55209a6d6858ce26d51a0447ba71
> Cr-Commit-Position: refs/heads/master@{#39511}

TBR=hpayer@chromium.org,jarin@chromium.org,bmeurer@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5404

Review-Url: https://codereview.chromium.org/2357433002
Cr-Commit-Position: refs/heads/master@{#39518}
parent 47f203e4
...@@ -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() || !isolate()->IsStringLengthOverflowIntact()) { if (top_info()->IsStub()) {
// 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,7 +2380,6 @@ HValue* HGraphBuilder::BuildAddStringLengths(HValue* left_length, ...@@ -2380,7 +2380,6 @@ 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,13 +440,6 @@ class HGraph final : public ZoneObject { ...@@ -440,13 +440,6 @@ 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;
...@@ -522,7 +515,6 @@ class HGraph final : public ZoneObject { ...@@ -522,7 +515,6 @@ 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,13 +1214,6 @@ Handle<Object> Factory::NewError(Handle<JSFunction> constructor, ...@@ -1214,13 +1214,6 @@ 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,7 +586,9 @@ class Factory final { ...@@ -586,7 +586,9 @@ 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,10 +2911,6 @@ void Heap::CreateInitialObjects() { ...@@ -2911,10 +2911,6 @@ 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,7 +166,6 @@ using v8::MemoryPressureLevel; ...@@ -166,7 +166,6 @@ 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,11 +147,6 @@ bool Isolate::IsHasInstanceLookupChainIntact() { ...@@ -147,11 +147,6 @@ 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,15 +2818,6 @@ void Isolate::InvalidateArraySpeciesProtector() { ...@@ -2818,15 +2818,6 @@ 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,7 +1010,6 @@ class Isolate { ...@@ -1010,7 +1010,6 @@ 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
...@@ -1029,7 +1028,6 @@ class Isolate { ...@@ -1029,7 +1028,6 @@ 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,7 +234,8 @@ RUNTIME_FUNCTION(Runtime_ThrowIncompatibleMethodReceiver) { ...@@ -234,7 +234,8 @@ RUNTIME_FUNCTION(Runtime_ThrowIncompatibleMethodReceiver) {
RUNTIME_FUNCTION(Runtime_ThrowInvalidStringLength) { RUNTIME_FUNCTION(Runtime_ThrowInvalidStringLength) {
HandleScope scope(isolate); HandleScope scope(isolate);
THROW_NEW_ERROR_RETURN_FAILURE(isolate, NewInvalidStringLengthError()); THROW_NEW_ERROR_RETURN_FAILURE(
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