Skip to content

Commit

Permalink
Revert of Array length reduction should throw in strict mode if it ca…
Browse files Browse the repository at this point in the history 
…n't delete an element. (patchset #7 id:220001 of https://codereview.chromium.org/1587073003/ )

Reason for revert:
[Sheriff] Breaks layout tests. Please fix upstream.
https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/4077

Original issue's description:
> Array length reduction should throw in strict mode if it can't delete an element.
>
> When accessor getter callback is called the v8::PropertyCallbackInfo::ShouldThrowOnError() is always false, since according to ES6 there's no difference between strict and non-strict property loads. For the setter case the v8::PropertyCallbackInfo::ShouldThrowOnError() returns true if the property is set in strict context.
>
> Interceptors follow same idea: for getter, enumerator and query callbacks the v8::PropertyCallbackInfo::ShouldThrowOnError() is always false, and for setter and deleter callback the v8::PropertyCallbackInfo::ShouldThrowOnError() returns true in strict context.
>
> This CL also cleans up the CallApiGetterStub and removes bogus asserts from [arm] Push(reg1, reg2, ..., regN) that prevented from pushing a set of registers containing duplicates.
>
> BUG=v8:4267
> LOG=Y
>
> Committed: https://crrev.com/1d3e837fcbbd9d9fd5e72dfe85dfd47c025f3c9f
> Cr-Commit-Position: refs/heads/master@{#33438}

TBR=verwaest@chromium.org,ishell@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:4267

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

Cr-Commit-Position: refs/heads/master@{#33444}
  • Loading branch information
@mi-ac
mi-ac authored and Commit bot committed Jan 21, 2016
1 parent 6e0573c commit 575e90c
Show file tree
Hide file tree
Showing 29 changed files with 310 additions and 649 deletions.
22 changes: 7 additions & 15 deletions include/v8.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3191,21 +3191,19 @@ class PropertyCallbackInfo {
V8_INLINE Local<Object> This() const;
V8_INLINE Local<Object> Holder() const;
V8_INLINE ReturnValue<T> GetReturnValue() const;
V8_INLINE bool ShouldThrowOnError() const;
// This shouldn't be public, but the arm compiler needs it.
static const int kArgsLength = 7;
static const int kArgsLength = 6;

protected:
friend class MacroAssembler;
friend class internal::PropertyCallbackArguments;
friend class internal::CustomArguments<PropertyCallbackInfo>;
static const int kShouldThrowOnErrorIndex = 0;
static const int kHolderIndex = 1;
static const int kIsolateIndex = 2;
static const int kReturnValueDefaultValueIndex = 3;
static const int kReturnValueIndex = 4;
static const int kDataIndex = 5;
static const int kThisIndex = 6;
static const int kHolderIndex = 0;
static const int kIsolateIndex = 1;
static const int kReturnValueDefaultValueIndex = 2;
static const int kReturnValueIndex = 3;
static const int kDataIndex = 4;
static const int kThisIndex = 5;

V8_INLINE PropertyCallbackInfo(internal::Object** args) : args_(args) {}
internal::Object** args_;
Expand Down Expand Up @@ -8264,12 +8262,6 @@ ReturnValue<T> PropertyCallbackInfo<T>::GetReturnValue() const {
return ReturnValue<T>(&args_[kReturnValueIndex]);
}

template <typename T>
bool PropertyCallbackInfo<T>::ShouldThrowOnError() const {
typedef internal::Internals I;
return args_[kShouldThrowOnErrorIndex] != I::IntToSmi(0);
}


Local<Primitive> Undefined(Isolate* isolate) {
typedef internal::Object* S;
Expand Down
14 changes: 0 additions & 14 deletions src/accessors.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,19 +204,6 @@ void Accessors::ArrayLengthSetter(
if (JSArray::ObservableSetLength(array, length).is_null()) {
isolate->OptionalRescheduleException(false);
}

if (info.ShouldThrowOnError()) {
uint32_t actual_new_len = 0;
CHECK(array->length()->ToArrayLength(&actual_new_len));
// Throw TypeError if there were non-deletable elements.
if (actual_new_len != length) {
Factory* factory = isolate->factory();
isolate->Throw(*factory->NewTypeError(
MessageTemplate::kStrictDeleteProperty,
factory->NewNumberFromUint(actual_new_len - 1), array));
isolate->OptionalRescheduleException(false);
}
}
}


Expand Down Expand Up @@ -1417,7 +1404,6 @@ static void ModuleGetExport(v8::Local<v8::Name> property,
static void ModuleSetExport(v8::Local<v8::Name> property,
v8::Local<v8::Value> value,
const v8::PropertyCallbackInfo<void>& info) {
if (!info.ShouldThrowOnError()) return;
Handle<Name> name = v8::Utils::OpenHandle(*property);
Isolate* isolate = name->GetIsolate();
Handle<Object> exception =
Expand Down
18 changes: 10 additions & 8 deletions src/arguments.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,16 @@ v8::Local<v8::Value> FunctionCallbackArguments::Call(FunctionCallback f) {
}


#define WRITE_CALL_2_VOID(Function, ReturnValue, Arg1, Arg2) \
void PropertyCallbackArguments::Call(Function f, Arg1 arg1, Arg2 arg2) { \
Isolate* isolate = this->isolate(); \
VMState<EXTERNAL> state(isolate); \
ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f)); \
PropertyCallbackInfo<ReturnValue> info(begin()); \
f(arg1, arg2, info); \
}
#define WRITE_CALL_2_VOID(Function, ReturnValue, Arg1, Arg2) \
void PropertyCallbackArguments::Call(Function f, \
Arg1 arg1, \
Arg2 arg2) { \
Isolate* isolate = this->isolate(); \
VMState<EXTERNAL> state(isolate); \
ExternalCallbackScope call_scope(isolate, FUNCTION_ADDR(f)); \
PropertyCallbackInfo<ReturnValue> info(begin()); \
f(arg1, arg2, info); \
}


FOR_EACH_CALLBACK_TABLE_MAPPING_0(WRITE_CALL_0)
Expand Down
10 changes: 4 additions & 6 deletions src/arguments.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,19 +152,17 @@ class PropertyCallbackArguments
static const int kReturnValueDefaultValueIndex =
T::kReturnValueDefaultValueIndex;
static const int kIsolateIndex = T::kIsolateIndex;
static const int kShouldThrowOnErrorIndex = T::kShouldThrowOnErrorIndex;

PropertyCallbackArguments(Isolate* isolate, Object* data, Object* self,
JSObject* holder, Object::ShouldThrow should_throw)
PropertyCallbackArguments(Isolate* isolate,
Object* data,
Object* self,
JSObject* holder)
: Super(isolate) {
Object** values = this->begin();
values[T::kThisIndex] = self;
values[T::kHolderIndex] = holder;
values[T::kDataIndex] = data;
values[T::kIsolateIndex] = reinterpret_cast<Object*>(isolate);
values[T::kShouldThrowOnErrorIndex] =
Smi::FromInt(should_throw == Object::THROW_ON_ERROR ? 1 : 0);

// Here the hole is set as default value.
// It cannot escape into js as it's remove in Call below.
values[T::kReturnValueDefaultValueIndex] =
Expand Down
29 changes: 12 additions & 17 deletions src/arm/code-stubs-arm.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5377,39 +5377,34 @@ void CallApiAccessorStub::Generate(MacroAssembler* masm) {

void CallApiGetterStub::Generate(MacroAssembler* masm) {
// ----------- S t a t e -------------
// -- sp[0] : name
// -- sp[4 .. (4 + kArgsLength*4)] : v8::PropertyCallbackInfo::args_
// -- sp[0] : name
// -- sp[4 - kArgsLength*4] : PropertyCallbackArguments object
// -- ...
// -- r2 : api_function_address
// -- r2 : api_function_address
// -----------------------------------

Register api_function_address = ApiGetterDescriptor::function_address();
DCHECK(api_function_address.is(r2));

// v8::PropertyCallbackInfo::args_ array and name handle.
const int kStackUnwindSpace = PropertyCallbackArguments::kArgsLength + 1;

// Load address of v8::PropertyAccessorInfo::args_ array and name handle.
__ mov(r0, sp); // r0 = Handle<Name>
__ add(r1, r0, Operand(1 * kPointerSize)); // r1 = v8::PCI::args_
__ mov(r0, sp); // r0 = Handle<Name>
__ add(r1, r0, Operand(1 * kPointerSize)); // r1 = PCA

const int kApiStackSpace = 1;
FrameScope frame_scope(masm, StackFrame::MANUAL);
__ EnterExitFrame(false, kApiStackSpace);

// Create v8::PropertyCallbackInfo object on the stack and initialize
// it's args_ field.
// Create PropertyAccessorInfo instance on the stack above the exit frame with
// r1 (internal::Object** args_) as the data.
__ str(r1, MemOperand(sp, 1 * kPointerSize));
__ add(r1, sp, Operand(1 * kPointerSize)); // r1 = v8::PropertyCallbackInfo&
__ add(r1, sp, Operand(1 * kPointerSize)); // r1 = AccessorInfo&

const int kStackUnwindSpace = PropertyCallbackArguments::kArgsLength + 1;

ExternalReference thunk_ref =
ExternalReference::invoke_accessor_getter_callback(isolate());

// +3 is to skip prolog, return address and name handle.
MemOperand return_value_operand(
fp, (PropertyCallbackArguments::kReturnValueOffset + 3) * kPointerSize);
CallApiFunctionAndReturn(masm, api_function_address, thunk_ref,
kStackUnwindSpace, NULL, return_value_operand, NULL);
kStackUnwindSpace, NULL,
MemOperand(fp, 6 * kPointerSize), NULL);
}


Expand Down
4 changes: 4 additions & 0 deletions src/arm/macro-assembler-arm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ class MacroAssembler: public Assembler {

// Push two registers. Pushes leftmost register first (to highest address).
void Push(Register src1, Register src2, Condition cond = al) {
DCHECK(!src1.is(src2));
if (src1.code() > src2.code()) {
stm(db_w, sp, src1.bit() | src2.bit(), cond);
} else {
Expand All @@ -326,6 +327,7 @@ class MacroAssembler: public Assembler {

// Push three registers. Pushes leftmost register first (to highest address).
void Push(Register src1, Register src2, Register src3, Condition cond = al) {
DCHECK(!AreAliased(src1, src2, src3));
if (src1.code() > src2.code()) {
if (src2.code() > src3.code()) {
stm(db_w, sp, src1.bit() | src2.bit() | src3.bit(), cond);
Expand All @@ -345,6 +347,7 @@ class MacroAssembler: public Assembler {
Register src3,
Register src4,
Condition cond = al) {
DCHECK(!AreAliased(src1, src2, src3, src4));
if (src1.code() > src2.code()) {
if (src2.code() > src3.code()) {
if (src3.code() > src4.code()) {
Expand All @@ -369,6 +372,7 @@ class MacroAssembler: public Assembler {
// Push five registers. Pushes leftmost register first (to highest address).
void Push(Register src1, Register src2, Register src3, Register src4,
Register src5, Condition cond = al) {
DCHECK(!AreAliased(src1, src2, src3, src4, src5));
if (src1.code() > src2.code()) {
if (src2.code() > src3.code()) {
if (src3.code() > src4.code()) {
Expand Down
26 changes: 10 additions & 16 deletions src/arm64/code-stubs-arm64.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5817,21 +5817,17 @@ void CallApiAccessorStub::Generate(MacroAssembler* masm) {

void CallApiGetterStub::Generate(MacroAssembler* masm) {
// ----------- S t a t e -------------
// -- sp[0] : name
// -- sp[8 .. (8 + kArgsLength*8)] : v8::PropertyCallbackInfo::args_
// -- sp[0] : name
// -- sp[8 - kArgsLength*8] : PropertyCallbackArguments object
// -- ...
// -- x2 : api_function_address
// -- x2 : api_function_address
// -----------------------------------

Register api_function_address = ApiGetterDescriptor::function_address();
DCHECK(api_function_address.is(x2));

// v8::PropertyCallbackInfo::args_ array and name handle.
const int kStackUnwindSpace = PropertyCallbackArguments::kArgsLength + 1;

// Load address of v8::PropertyAccessorInfo::args_ array and name handle.
__ Mov(x0, masm->StackPointer()); // x0 = Handle<Name>
__ Add(x1, x0, 1 * kPointerSize); // x1 = v8::PCI::args_
__ Add(x1, x0, 1 * kPointerSize); // x1 = PCA

const int kApiStackSpace = 1;

Expand All @@ -5842,22 +5838,20 @@ void CallApiGetterStub::Generate(MacroAssembler* masm) {
FrameScope frame_scope(masm, StackFrame::MANUAL);
__ EnterExitFrame(false, x10, kApiStackSpace + kCallApiFunctionSpillSpace);

// Create v8::PropertyCallbackInfo object on the stack and initialize
// it's args_ field.
// Create PropertyAccessorInfo instance on the stack above the exit frame with
// x1 (internal::Object** args_) as the data.
__ Poke(x1, 1 * kPointerSize);
__ Add(x1, masm->StackPointer(), 1 * kPointerSize);
// x1 = v8::PropertyCallbackInfo&
__ Add(x1, masm->StackPointer(), 1 * kPointerSize); // x1 = AccessorInfo&

const int kStackUnwindSpace = PropertyCallbackArguments::kArgsLength + 1;

ExternalReference thunk_ref =
ExternalReference::invoke_accessor_getter_callback(isolate());

const int spill_offset = 1 + kApiStackSpace;
// +3 is to skip prolog, return address and name handle.
MemOperand return_value_operand(
fp, (PropertyCallbackArguments::kReturnValueOffset + 3) * kPointerSize);
CallApiFunctionAndReturn(masm, api_function_address, thunk_ref,
kStackUnwindSpace, NULL, spill_offset,
return_value_operand, NULL);
MemOperand(fp, 6 * kPointerSize), NULL);
}


Expand Down
42 changes: 15 additions & 27 deletions src/ia32/code-stubs-ia32.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5664,50 +5664,38 @@ void CallApiAccessorStub::Generate(MacroAssembler* masm) {

void CallApiGetterStub::Generate(MacroAssembler* masm) {
// ----------- S t a t e -------------
// -- esp[0] : return address
// -- esp[4] : name
// -- esp[8 .. (8 + kArgsLength*4)] : v8::PropertyCallbackInfo::args_
// -- esp[0] : return address
// -- esp[4] : name
// -- esp[8 - kArgsLength*4] : PropertyCallbackArguments object
// -- ...
// -- edx : api_function_address
// -- edx : api_function_address
// -----------------------------------
DCHECK(edx.is(ApiGetterDescriptor::function_address()));

// v8::PropertyCallbackInfo::args_ array and name handle.
const int kStackUnwindSpace = PropertyCallbackArguments::kArgsLength + 1;

// Allocate v8::PropertyCallbackInfo object, arguments for callback and
// space for optional callback address parameter (in case CPU profiler is
// active) in non-GCed stack space.
const int kApiArgc = 3 + 1;
// array for v8::Arguments::values_, handler for name and pointer
// to the values (it considered as smi in GC).
const int kStackSpace = PropertyCallbackArguments::kArgsLength + 2;
// Allocate space for opional callback address parameter in case
// CPU profiler is active.
const int kApiArgc = 2 + 1;

Register api_function_address = edx;
Register scratch = ebx;

// Load address of v8::PropertyAccessorInfo::args_ array.
__ lea(scratch, Operand(esp, 2 * kPointerSize));
// load address of name
__ lea(scratch, Operand(esp, 1 * kPointerSize));

PrepareCallApiFunction(masm, kApiArgc);
// Create v8::PropertyCallbackInfo object on the stack and initialize
// it's args_ field.
Operand info_object = ApiParameterOperand(3);
__ mov(info_object, scratch);

__ sub(scratch, Immediate(kPointerSize));
__ mov(ApiParameterOperand(0), scratch); // name.
__ lea(scratch, info_object);
__ add(scratch, Immediate(kPointerSize));
__ mov(ApiParameterOperand(1), scratch); // arguments pointer.
// Reserve space for optional callback address parameter.
Operand thunk_last_arg = ApiParameterOperand(2);

ExternalReference thunk_ref =
ExternalReference::invoke_accessor_getter_callback(isolate());

// +3 is to skip prolog, return address and name handle.
Operand return_value_operand(
ebp, (PropertyCallbackArguments::kReturnValueOffset + 3) * kPointerSize);
CallApiFunctionAndReturn(masm, api_function_address, thunk_ref,
thunk_last_arg, kStackUnwindSpace, nullptr,
return_value_operand, NULL);
ApiParameterOperand(2), kStackSpace, nullptr,
Operand(ebp, 7 * kPointerSize), NULL);
}


Expand Down
Loading

0 comments on commit 575e90c

Please sign in to comment.