Skip to content

Commit 32e6455

Browse files
mi-acCommit bot
mi-ac
authored and
Commit bot
committed
Revert of Add script context with context-allocated "const this" (patchset v8#7 id:120001 of https://codereview.chromium.org/1179893002/)
Reason for revert: [Sheriff] Breaks gc mole: http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20gcmole/builds/2435 Original issue's description: > Add script context with context-allocated "const this" > > R=rossberg@chromium.org > LOG=N > BUG=498811 > > Committed: https://crrev.com/fa32d461c16a053cc6d48d3fb326016bc2765765 > Cr-Commit-Position: refs/heads/master@{#28988} TBR=rossberg@chromium.org,mstarzinger@chromium.org,wingo@igalia.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=498811 Review URL: https://codereview.chromium.org/1180043004 Cr-Commit-Position: refs/heads/master@{#28992}
1 parent 74534bb commit 32e6455

19 files changed

Lines changed: 204 additions & 259 deletions

‎src/arm/full-codegen-arm.cc‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3104,15 +3104,20 @@ void FullCodeGenerator::EmitCall(Call* expr, CallICState::CallType call_type) {
31043104

31053105

31063106
void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) {
3107-
// r4: copy of the first argument or undefined if it doesn't exist.
3107+
// r5: copy of the first argument or undefined if it doesn't exist.
31083108
if (arg_count > 0) {
3109-
__ ldr(r4, MemOperand(sp, arg_count * kPointerSize));
3109+
__ ldr(r5, MemOperand(sp, arg_count * kPointerSize));
31103110
} else {
3111-
__ LoadRoot(r4, Heap::kUndefinedValueRootIndex);
3111+
__ LoadRoot(r5, Heap::kUndefinedValueRootIndex);
31123112
}
31133113

3114+
// r4: the receiver of the enclosing function.
3115+
__ ldr(r4, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset));
3116+
31143117
// r3: the receiver of the enclosing function.
3115-
__ ldr(r3, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset));
3118+
Variable* this_var = scope()->LookupThis();
3119+
DCHECK_NOT_NULL(this_var);
3120+
__ ldr(r3, VarOperand(this_var, r3));
31163121

31173122
// r2: language mode.
31183123
__ mov(r2, Operand(Smi::FromInt(language_mode())));
@@ -3121,8 +3126,9 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) {
31213126
__ mov(r1, Operand(Smi::FromInt(scope()->start_position())));
31223127

31233128
// Do the runtime call.
3129+
__ Push(r5);
31243130
__ Push(r4, r3, r2, r1);
3125-
__ CallRuntime(Runtime::kResolvePossiblyDirectEval, 5);
3131+
__ CallRuntime(Runtime::kResolvePossiblyDirectEval, 6);
31263132
}
31273133

31283134

@@ -3154,9 +3160,10 @@ void FullCodeGenerator::VisitCall(Call* expr) {
31543160
Call::CallType call_type = expr->GetCallType(isolate());
31553161

31563162
if (call_type == Call::POSSIBLY_EVAL_CALL) {
3157-
// In a call to eval, we first call
3158-
// RuntimeHidden_asResolvePossiblyDirectEval to resolve the function we need
3159-
// to call. Then we call the resolved function using the given arguments.
3163+
// In a call to eval, we first call RuntimeHidden_ResolvePossiblyDirectEval
3164+
// to resolve the function we need to call and the receiver of the
3165+
// call. Then we call the resolved function using the given
3166+
// arguments.
31603167
ZoneList<Expression*>* args = expr->arguments();
31613168
int arg_count = args->length();
31623169

@@ -3176,8 +3183,10 @@ void FullCodeGenerator::VisitCall(Call* expr) {
31763183
__ push(r1);
31773184
EmitResolvePossiblyDirectEval(arg_count);
31783185

3179-
// Touch up the stack with the resolved function.
3186+
// The runtime call returns a pair of values in r0 (function) and
3187+
// r1 (receiver). Touch up the stack with the right values.
31803188
__ str(r0, MemOperand(sp, (arg_count + 1) * kPointerSize));
3189+
__ str(r1, MemOperand(sp, arg_count * kPointerSize));
31813190

31823191
PrepareForBailoutForId(expr->EvalOrLookupId(), NO_REGISTERS);
31833192
}
@@ -4774,10 +4783,9 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
47744783
context()->Plug(r0);
47754784
} else if (proxy != NULL) {
47764785
Variable* var = proxy->var();
4777-
// Delete of an unqualified identifier is disallowed in strict mode but
4778-
// "delete this" is allowed.
4779-
bool is_this = var->HasThisName(isolate());
4780-
DCHECK(is_sloppy(language_mode()) || is_this);
4786+
// Delete of an unqualified identifier is disallowed in strict mode
4787+
// but "delete this" is allowed.
4788+
DCHECK(is_sloppy(language_mode()) || var->is_this());
47814789
if (var->IsUnallocated()) {
47824790
__ ldr(r2, GlobalObjectOperand());
47834791
__ mov(r1, Operand(var->name()));
@@ -4788,7 +4796,7 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
47884796
} else if (var->IsStackAllocated() || var->IsContextSlot()) {
47894797
// Result of deleting non-global, non-dynamic variables is false.
47904798
// The subexpression does not have side effects.
4791-
context()->Plug(is_this);
4799+
context()->Plug(var->is_this());
47924800
} else {
47934801
// Non-global variable. Call the runtime to try to delete from the
47944802
// context where the variable was introduced.

‎src/arm64/full-codegen-arm64.cc‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2799,17 +2799,21 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) {
27992799
}
28002800

28012801
__ Ldr(x10, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset));
2802+
// Prepare to push the receiver of the enclosing function.
2803+
Variable* this_var = scope()->LookupThis();
2804+
DCHECK_NOT_NULL(this_var);
2805+
__ Ldr(x11, VarOperand(this_var, x11));
28022806

28032807
// Prepare to push the language mode.
2804-
__ Mov(x11, Smi::FromInt(language_mode()));
2808+
__ Mov(x12, Smi::FromInt(language_mode()));
28052809
// Prepare to push the start position of the scope the calls resides in.
2806-
__ Mov(x12, Smi::FromInt(scope()->start_position()));
2810+
__ Mov(x13, Smi::FromInt(scope()->start_position()));
28072811

28082812
// Push.
2809-
__ Push(x9, x10, x11, x12);
2813+
__ Push(x9, x10, x11, x12, x13);
28102814

28112815
// Do the runtime call.
2812-
__ CallRuntime(Runtime::kResolvePossiblyDirectEval, 5);
2816+
__ CallRuntime(Runtime::kResolvePossiblyDirectEval, 6);
28132817
}
28142818

28152819

@@ -2841,8 +2845,9 @@ void FullCodeGenerator::VisitCall(Call* expr) {
28412845

28422846
if (call_type == Call::POSSIBLY_EVAL_CALL) {
28432847
// In a call to eval, we first call RuntimeHidden_ResolvePossiblyDirectEval
2844-
// to resolve the function we need to call. Then we call the resolved
2845-
// function using the given arguments.
2848+
// to resolve the function we need to call and the receiver of the
2849+
// call. Then we call the resolved function using the given
2850+
// arguments.
28462851
ZoneList<Expression*>* args = expr->arguments();
28472852
int arg_count = args->length();
28482853

@@ -2863,8 +2868,9 @@ void FullCodeGenerator::VisitCall(Call* expr) {
28632868
__ Push(x10);
28642869
EmitResolvePossiblyDirectEval(arg_count);
28652870

2866-
// Touch up the stack with the resolved function.
2867-
__ Poke(x0, (arg_count + 1) * kPointerSize);
2871+
// The runtime call returns a pair of values in x0 (function) and
2872+
// x1 (receiver). Touch up the stack with the right values.
2873+
__ PokePair(x1, x0, arg_count * kPointerSize);
28682874

28692875
PrepareForBailoutForId(expr->EvalOrLookupId(), NO_REGISTERS);
28702876
}
@@ -4458,10 +4464,9 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
44584464
context()->Plug(x0);
44594465
} else if (proxy != NULL) {
44604466
Variable* var = proxy->var();
4461-
// Delete of an unqualified identifier is disallowed in strict mode but
4462-
// "delete this" is allowed.
4463-
bool is_this = var->HasThisName(isolate());
4464-
DCHECK(is_sloppy(language_mode()) || is_this);
4467+
// Delete of an unqualified identifier is disallowed in strict mode
4468+
// but "delete this" is allowed.
4469+
DCHECK(is_sloppy(language_mode()) || var->is_this());
44654470
if (var->IsUnallocated()) {
44664471
__ Ldr(x12, GlobalObjectMemOperand());
44674472
__ Mov(x11, Operand(var->name()));
@@ -4472,7 +4477,7 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
44724477
} else if (var->IsStackAllocated() || var->IsContextSlot()) {
44734478
// Result of deleting non-global, non-dynamic variables is false.
44744479
// The subexpression does not have side effects.
4475-
context()->Plug(is_this);
4480+
context()->Plug(var->is_this());
44764481
} else {
44774482
// Non-global variable. Call the runtime to try to delete from the
44784483
// context where the variable was introduced.

‎src/bootstrapper.cc‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -197,11 +197,6 @@ class Genesis BASE_EMBEDDED {
197197
// other objects in the snapshot.
198198
void HookUpGlobalObject(Handle<GlobalObject> global_object,
199199
Handle<FixedArray> outdated_contexts);
200-
// The native context has a ScriptContextTable that store declarative bindings
201-
// made in script scopes. Add a "this" binding to that table pointing to the
202-
// global proxy.
203-
void InstallGlobalThisBinding();
204-
void HookUpGlobalThisBinding(Handle<FixedArray> outdated_contexts);
205200
// New context initialization. Used for creating a context from scratch.
206201
void InitializeGlobal(Handle<GlobalObject> global_object,
207202
Handle<JSFunction> empty_function);
@@ -819,40 +814,6 @@ void Genesis::CreateRoots() {
819814
}
820815

821816

822-
void Genesis::InstallGlobalThisBinding() {
823-
Handle<ScriptContextTable> script_contexts(
824-
native_context()->script_context_table());
825-
Handle<ScopeInfo> scope_info = ScopeInfo::CreateGlobalThisBinding(isolate());
826-
Handle<JSFunction> closure(native_context()->closure());
827-
Handle<Context> context = factory()->NewScriptContext(closure, scope_info);
828-
829-
// Go ahead and hook it up while we're at it.
830-
int slot = scope_info->ReceiverContextSlotIndex();
831-
DCHECK_EQ(slot, Context::MIN_CONTEXT_SLOTS);
832-
context->set(slot, native_context()->global_proxy());
833-
834-
native_context()->set_script_context_table(
835-
*ScriptContextTable::Extend(script_contexts, context));
836-
}
837-
838-
839-
void Genesis::HookUpGlobalThisBinding(Handle<FixedArray> outdated_contexts) {
840-
// One of these contexts should be the one that declares the global "this"
841-
// binding.
842-
for (int i = 0; i < outdated_contexts->length(); ++i) {
843-
Context* context = Context::cast(outdated_contexts->get(i));
844-
if (context->IsScriptContext()) {
845-
ScopeInfo* scope_info = ScopeInfo::cast(context->extension());
846-
int slot = scope_info->ReceiverContextSlotIndex();
847-
if (slot >= 0) {
848-
DCHECK_EQ(slot, Context::MIN_CONTEXT_SLOTS);
849-
context->set(slot, native_context()->global_proxy());
850-
}
851-
}
852-
}
853-
}
854-
855-
856817
Handle<GlobalObject> Genesis::CreateNewGlobals(
857818
v8::Handle<v8::ObjectTemplate> global_proxy_template,
858819
Handle<JSGlobalProxy> global_proxy) {
@@ -1011,7 +972,6 @@ void Genesis::InitializeGlobal(Handle<GlobalObject> global_object,
1011972
Handle<ScriptContextTable> script_context_table =
1012973
factory->NewScriptContextTable();
1013974
native_context()->set_script_context_table(*script_context_table);
1014-
InstallGlobalThisBinding();
1015975

1016976
Handle<String> object_name = factory->Object_string();
1017977
JSObject::AddProperty(
@@ -3126,7 +3086,6 @@ Genesis::Genesis(Isolate* isolate,
31263086
HookUpGlobalObject(global_object, outdated_contexts);
31273087
native_context()->builtins()->set_global_proxy(
31283088
native_context()->global_proxy());
3129-
HookUpGlobalThisBinding(outdated_contexts);
31303089

31313090
if (!ConfigureGlobalObjects(global_proxy_template)) return;
31323091
} else {

‎src/compiler/ast-graph-builder.cc‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2420,17 +2420,37 @@ void AstGraphBuilder::VisitCall(Call* expr) {
24202420
// Create node to ask for help resolving potential eval call. This will
24212421
// provide a fully resolved callee and the corresponding receiver.
24222422
Node* function = GetFunctionClosure();
2423+
// TODO(wingo): ResolvePossibleDirectEval doesn't really need a receiver,
2424+
// now that eval scopes don't have "this" declarations. Remove this hack
2425+
// once ResolvePossibleDirectEval changes.
2426+
Node* receiver;
2427+
{
2428+
Variable* variable = info()->scope()->LookupThis();
2429+
if (variable->IsStackAllocated()) {
2430+
receiver = environment()->Lookup(variable);
2431+
} else {
2432+
DCHECK(variable->IsContextSlot());
2433+
int depth = current_scope()->ContextChainLength(variable->scope());
2434+
bool immutable = variable->maybe_assigned() == kNotAssigned;
2435+
const Operator* op =
2436+
javascript()->LoadContext(depth, variable->index(), immutable);
2437+
receiver = NewNode(op, current_context());
2438+
}
2439+
}
24232440
Node* language = jsgraph()->Constant(language_mode());
24242441
Node* position = jsgraph()->Constant(current_scope()->start_position());
24252442
const Operator* op =
2426-
javascript()->CallRuntime(Runtime::kResolvePossiblyDirectEval, 5);
2427-
Node* new_callee =
2428-
NewNode(op, callee, source, function, language, position);
2429-
PrepareFrameState(new_callee, expr->EvalOrLookupId(),
2443+
javascript()->CallRuntime(Runtime::kResolvePossiblyDirectEval, 6);
2444+
Node* pair =
2445+
NewNode(op, callee, source, function, receiver, language, position);
2446+
PrepareFrameState(pair, expr->EvalOrLookupId(),
24302447
OutputFrameStateCombine::PokeAt(arg_count + 1));
2448+
Node* new_callee = NewNode(common()->Projection(0), pair);
2449+
Node* new_receiver = NewNode(common()->Projection(1), pair);
24312450

2432-
// Patch callee on the environment.
2451+
// Patch callee and receiver on the environment.
24332452
environment()->Poke(arg_count + 1, new_callee);
2453+
environment()->Poke(arg_count + 0, new_receiver);
24342454
}
24352455

24362456
// Create node to perform the function call.
@@ -2853,9 +2873,7 @@ void AstGraphBuilder::VisitDelete(UnaryOperation* expr) {
28532873
// Delete of an unqualified identifier is only allowed in classic mode but
28542874
// deleting "this" is allowed in all language modes.
28552875
Variable* variable = expr->expression()->AsVariableProxy()->var();
2856-
// Delete of an unqualified identifier is disallowed in strict mode but
2857-
// "delete this" is allowed.
2858-
DCHECK(is_sloppy(language_mode()) || variable->HasThisName(isolate()));
2876+
DCHECK(is_sloppy(language_mode()) || variable->is_this());
28592877
value = BuildVariableDelete(variable, expr->id(),
28602878
ast_context()->GetStateCombine());
28612879
} else if (expr->expression()->IsProperty()) {
@@ -3303,10 +3321,9 @@ Node* AstGraphBuilder::BuildVariableDelete(Variable* variable,
33033321
}
33043322
case Variable::PARAMETER:
33053323
case Variable::LOCAL:
3306-
case Variable::CONTEXT: {
3324+
case Variable::CONTEXT:
33073325
// Local var, const, or let variable or context variable.
3308-
return jsgraph()->BooleanConstant(variable->HasThisName(isolate()));
3309-
}
3326+
return jsgraph()->BooleanConstant(variable->is_this());
33103327
case Variable::LOOKUP: {
33113328
// Dynamic lookup of context variable (anywhere in the chain).
33123329
Node* name = jsgraph()->Constant(variable->name());

‎src/hydrogen.cc‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10197,11 +10197,12 @@ void HOptimizedGraphBuilder::VisitDelete(UnaryOperation* expr) {
1019710197
if (var->IsUnallocated()) {
1019810198
Bailout(kDeleteWithGlobalVariable);
1019910199
} else if (var->IsStackAllocated() || var->IsContextSlot()) {
10200-
// Result of deleting non-global variables is false. 'this' is not really
10201-
// a variable, though we implement it as one. The subexpression does not
10202-
// have side effects.
10203-
HValue* value = var->HasThisName(isolate()) ? graph()->GetConstantTrue()
10204-
: graph()->GetConstantFalse();
10200+
// Result of deleting non-global variables is false. 'this' is not
10201+
// really a variable, though we implement it as one. The
10202+
// subexpression does not have side effects.
10203+
HValue* value = var->is_this()
10204+
? graph()->GetConstantTrue()
10205+
: graph()->GetConstantFalse();
1020510206
return ast_context()->ReturnValue(value);
1020610207
} else {
1020710208
Bailout(kDeleteWithNonGlobalVariable);

‎src/ia32/full-codegen-ia32.cc‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3006,15 +3006,18 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) {
30063006

30073007
// Push the enclosing function.
30083008
__ push(Operand(ebp, JavaScriptFrameConstants::kFunctionOffset));
3009-
3009+
// Push the receiver of the enclosing function.
3010+
Variable* this_var = scope()->LookupThis();
3011+
DCHECK_NOT_NULL(this_var);
3012+
__ push(VarOperand(this_var, ecx));
30103013
// Push the language mode.
30113014
__ push(Immediate(Smi::FromInt(language_mode())));
30123015

30133016
// Push the start position of the scope the calls resides in.
30143017
__ push(Immediate(Smi::FromInt(scope()->start_position())));
30153018

30163019
// Do the runtime call.
3017-
__ CallRuntime(Runtime::kResolvePossiblyDirectEval, 5);
3020+
__ CallRuntime(Runtime::kResolvePossiblyDirectEval, 6);
30183021
}
30193022

30203023

@@ -3046,8 +3049,8 @@ void FullCodeGenerator::VisitCall(Call* expr) {
30463049

30473050
if (call_type == Call::POSSIBLY_EVAL_CALL) {
30483051
// In a call to eval, we first call RuntimeHidden_ResolvePossiblyDirectEval
3049-
// to resolve the function we need to call. Then we call the resolved
3050-
// function using the given arguments.
3052+
// to resolve the function we need to call and the receiver of the call.
3053+
// Then we call the resolved function using the given arguments.
30513054
ZoneList<Expression*>* args = expr->arguments();
30523055
int arg_count = args->length();
30533056
{ PreservePositionScope pos_scope(masm()->positions_recorder());
@@ -3064,7 +3067,9 @@ void FullCodeGenerator::VisitCall(Call* expr) {
30643067
__ push(Operand(esp, (arg_count + 1) * kPointerSize));
30653068
EmitResolvePossiblyDirectEval(arg_count);
30663069

3067-
// Touch up the stack with the resolved function.
3070+
// The runtime call returns a pair of values in eax (function) and
3071+
// edx (receiver). Touch up the stack with the right values.
3072+
__ mov(Operand(esp, (arg_count + 0) * kPointerSize), edx);
30683073
__ mov(Operand(esp, (arg_count + 1) * kPointerSize), eax);
30693074

30703075
PrepareForBailoutForId(expr->EvalOrLookupId(), NO_REGISTERS);
@@ -4698,10 +4703,9 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
46984703
context()->Plug(eax);
46994704
} else if (proxy != NULL) {
47004705
Variable* var = proxy->var();
4701-
// Delete of an unqualified identifier is disallowed in strict mode but
4702-
// "delete this" is allowed.
4703-
bool is_this = var->HasThisName(isolate());
4704-
DCHECK(is_sloppy(language_mode()) || is_this);
4706+
// Delete of an unqualified identifier is disallowed in strict mode
4707+
// but "delete this" is allowed.
4708+
DCHECK(is_sloppy(language_mode()) || var->is_this());
47054709
if (var->IsUnallocated()) {
47064710
__ push(GlobalObjectOperand());
47074711
__ push(Immediate(var->name()));
@@ -4712,7 +4716,7 @@ void FullCodeGenerator::VisitUnaryOperation(UnaryOperation* expr) {
47124716
// Result of deleting non-global variables is false. 'this' is
47134717
// not really a variable, though we implement it as one. The
47144718
// subexpression does not have side effects.
4715-
context()->Plug(is_this);
4719+
context()->Plug(var->is_this());
47164720
} else {
47174721
// Non-global variable. Call the runtime to try to delete from the
47184722
// context where the variable was introduced.

0 commit comments

Comments
 (0)