From fdba189730471ffb560ed8284cf6d05b24de2c07 Mon Sep 17 00:00:00 2001 From: "fschneider@chromium.org" <fschneider@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00> Date: Tue, 12 Jan 2010 17:22:57 +0000 Subject: [PATCH] Fix a problem with const initialization in the top-level code generator. When initializing the special local variable containing the reference to the enclosing function in named functions we now (correctly) emit an INIT_CONST instead of INIT_VAR, and we correctly bail out in the top-level code generator. Also part of this change is adding missing statement position information for some statements in the top-level code generator. Review URL: http://codereview.chromium.org/536029 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3587 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/codegen-arm.cc | 166 +++++++++++++++++++++------------------ src/arm/codegen-arm.h | 3 + src/compiler.cc | 3 + src/fast-codegen.cc | 6 ++ src/ia32/codegen-ia32.cc | 6 ++ src/parser.cc | 2 +- src/x64/codegen-x64.cc | 6 ++ 7 files changed, 114 insertions(+), 78 deletions(-) diff --git a/src/arm/codegen-arm.cc b/src/arm/codegen-arm.cc index 2af2f6bf662..6636a3777d8 100644 --- a/src/arm/codegen-arm.cc +++ b/src/arm/codegen-arm.cc @@ -263,6 +263,13 @@ void CodeGenerator::GenCode(FunctionLiteral* fun) { frame_->Drop(); // Value is no longer needed. } + // Initialize ThisFunction reference if present. + if (scope_->is_function_scope() && scope_->function() != NULL) { + __ mov(ip, Operand(Factory::the_hole_value())); + frame_->EmitPush(ip); + StoreToSlot(scope_->function()->slot(), NOT_CONST_INIT); + } + // Generate code to 'execute' declarations and initialize functions // (source elements). In case of an illegal redeclaration we need to // handle that instead of processing the declarations. @@ -2446,6 +2453,87 @@ void CodeGenerator::LoadFromSlot(Slot* slot, TypeofState typeof_state) { } +void CodeGenerator::StoreToSlot(Slot* slot, InitState init_state) { + ASSERT(slot != NULL); + if (slot->type() == Slot::LOOKUP) { + ASSERT(slot->var()->is_dynamic()); + + // For now, just do a runtime call. + frame_->EmitPush(cp); + __ mov(r0, Operand(slot->var()->name())); + frame_->EmitPush(r0); + + if (init_state == CONST_INIT) { + // Same as the case for a normal store, but ignores attribute + // (e.g. READ_ONLY) of context slot so that we can initialize + // const properties (introduced via eval("const foo = (some + // expr);")). Also, uses the current function context instead of + // the top context. + // + // Note that we must declare the foo upon entry of eval(), via a + // context slot declaration, but we cannot initialize it at the + // same time, because the const declaration may be at the end of + // the eval code (sigh...) and the const variable may have been + // used before (where its value is 'undefined'). Thus, we can only + // do the initialization when we actually encounter the expression + // and when the expression operands are defined and valid, and + // thus we need the split into 2 operations: declaration of the + // context slot followed by initialization. + frame_->CallRuntime(Runtime::kInitializeConstContextSlot, 3); + } else { + frame_->CallRuntime(Runtime::kStoreContextSlot, 3); + } + // Storing a variable must keep the (new) value on the expression + // stack. This is necessary for compiling assignment expressions. + frame_->EmitPush(r0); + + } else { + ASSERT(!slot->var()->is_dynamic()); + + JumpTarget exit; + if (init_state == CONST_INIT) { + ASSERT(slot->var()->mode() == Variable::CONST); + // Only the first const initialization must be executed (the slot + // still contains 'the hole' value). When the assignment is + // executed, the code is identical to a normal store (see below). + Comment cmnt(masm_, "[ Init const"); + __ ldr(r2, SlotOperand(slot, r2)); + __ LoadRoot(ip, Heap::kTheHoleValueRootIndex); + __ cmp(r2, ip); + exit.Branch(ne); + } + + // We must execute the store. Storing a variable must keep the + // (new) value on the stack. This is necessary for compiling + // assignment expressions. + // + // Note: We will reach here even with slot->var()->mode() == + // Variable::CONST because of const declarations which will + // initialize consts to 'the hole' value and by doing so, end up + // calling this code. r2 may be loaded with context; used below in + // RecordWrite. + frame_->EmitPop(r0); + __ str(r0, SlotOperand(slot, r2)); + frame_->EmitPush(r0); + if (slot->type() == Slot::CONTEXT) { + // Skip write barrier if the written value is a smi. + __ tst(r0, Operand(kSmiTagMask)); + exit.Branch(eq); + // r2 is loaded with context when calling SlotOperand above. + int offset = FixedArray::kHeaderSize + slot->index() * kPointerSize; + __ mov(r3, Operand(offset)); + __ RecordWrite(r2, r3, r1); + } + // If we definitely did not jump over the assignment, we do not need + // to bind the exit label. Doing so can defeat peephole + // optimization. + if (init_state == CONST_INIT || slot->type() == Slot::CONTEXT) { + exit.Bind(); + } + } +} + + void CodeGenerator::LoadFromGlobalSlotCheckExtensions(Slot* slot, TypeofState typeof_state, Register tmp, @@ -4262,83 +4350,7 @@ void Reference::SetValue(InitState init_state) { case SLOT: { Comment cmnt(masm, "[ Store to Slot"); Slot* slot = expression_->AsVariableProxy()->AsVariable()->slot(); - ASSERT(slot != NULL); - if (slot->type() == Slot::LOOKUP) { - ASSERT(slot->var()->is_dynamic()); - - // For now, just do a runtime call. - frame->EmitPush(cp); - __ mov(r0, Operand(slot->var()->name())); - frame->EmitPush(r0); - - if (init_state == CONST_INIT) { - // Same as the case for a normal store, but ignores attribute - // (e.g. READ_ONLY) of context slot so that we can initialize - // const properties (introduced via eval("const foo = (some - // expr);")). Also, uses the current function context instead of - // the top context. - // - // Note that we must declare the foo upon entry of eval(), via a - // context slot declaration, but we cannot initialize it at the - // same time, because the const declaration may be at the end of - // the eval code (sigh...) and the const variable may have been - // used before (where its value is 'undefined'). Thus, we can only - // do the initialization when we actually encounter the expression - // and when the expression operands are defined and valid, and - // thus we need the split into 2 operations: declaration of the - // context slot followed by initialization. - frame->CallRuntime(Runtime::kInitializeConstContextSlot, 3); - } else { - frame->CallRuntime(Runtime::kStoreContextSlot, 3); - } - // Storing a variable must keep the (new) value on the expression - // stack. This is necessary for compiling assignment expressions. - frame->EmitPush(r0); - - } else { - ASSERT(!slot->var()->is_dynamic()); - - JumpTarget exit; - if (init_state == CONST_INIT) { - ASSERT(slot->var()->mode() == Variable::CONST); - // Only the first const initialization must be executed (the slot - // still contains 'the hole' value). When the assignment is - // executed, the code is identical to a normal store (see below). - Comment cmnt(masm, "[ Init const"); - __ ldr(r2, cgen_->SlotOperand(slot, r2)); - __ LoadRoot(ip, Heap::kTheHoleValueRootIndex); - __ cmp(r2, ip); - exit.Branch(ne); - } - - // We must execute the store. Storing a variable must keep the - // (new) value on the stack. This is necessary for compiling - // assignment expressions. - // - // Note: We will reach here even with slot->var()->mode() == - // Variable::CONST because of const declarations which will - // initialize consts to 'the hole' value and by doing so, end up - // calling this code. r2 may be loaded with context; used below in - // RecordWrite. - frame->EmitPop(r0); - __ str(r0, cgen_->SlotOperand(slot, r2)); - frame->EmitPush(r0); - if (slot->type() == Slot::CONTEXT) { - // Skip write barrier if the written value is a smi. - __ tst(r0, Operand(kSmiTagMask)); - exit.Branch(eq); - // r2 is loaded with context when calling SlotOperand above. - int offset = FixedArray::kHeaderSize + slot->index() * kPointerSize; - __ mov(r3, Operand(offset)); - __ RecordWrite(r2, r3, r1); - } - // If we definitely did not jump over the assignment, we do not need - // to bind the exit label. Doing so can defeat peephole - // optimization. - if (init_state == CONST_INIT || slot->type() == Slot::CONTEXT) { - exit.Bind(); - } - } + cgen_->StoreToSlot(slot, init_state); break; } diff --git a/src/arm/codegen-arm.h b/src/arm/codegen-arm.h index 0b8d67a61f1..b62bc36d705 100644 --- a/src/arm/codegen-arm.h +++ b/src/arm/codegen-arm.h @@ -272,6 +272,9 @@ class CodeGenerator: public AstVisitor { // Read a value from a slot and leave it on top of the expression stack. void LoadFromSlot(Slot* slot, TypeofState typeof_state); + // Store the value on top of the stack to a slot. + void StoreToSlot(Slot* slot, InitState init_state); + void LoadFromGlobalSlotCheckExtensions(Slot* slot, TypeofState typeof_state, Register tmp, diff --git a/src/compiler.cc b/src/compiler.cc index 519362c5660..420b809e7d2 100644 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -888,6 +888,9 @@ void CodeGenSelector::VisitAssignment(Assignment* expr) { Property* prop = expr->target()->AsProperty(); ASSERT(var == NULL || prop == NULL); if (var != NULL) { + if (var->mode() == Variable::CONST) { + BAILOUT("Assignment to const"); + } // All global variables are supported. if (!var->is_global()) { ASSERT(var->slot() != NULL); diff --git a/src/fast-codegen.cc b/src/fast-codegen.cc index a76ae9ca36f..6d20df33d9e 100644 --- a/src/fast-codegen.cc +++ b/src/fast-codegen.cc @@ -288,6 +288,7 @@ void FastCodeGenerator::VisitEmptyStatement(EmptyStatement* stmt) { void FastCodeGenerator::VisitIfStatement(IfStatement* stmt) { Comment cmnt(masm_, "[ IfStatement"); + SetStatementPosition(stmt); // Expressions cannot recursively enter statements, there are no labels in // the state. ASSERT_EQ(NULL, true_label_); @@ -315,6 +316,7 @@ void FastCodeGenerator::VisitIfStatement(IfStatement* stmt) { void FastCodeGenerator::VisitContinueStatement(ContinueStatement* stmt) { Comment cmnt(masm_, "[ ContinueStatement"); + SetStatementPosition(stmt); NestedStatement* current = nesting_stack_; int stack_depth = 0; while (!current->IsContinueTarget(stmt->target())) { @@ -330,6 +332,7 @@ void FastCodeGenerator::VisitContinueStatement(ContinueStatement* stmt) { void FastCodeGenerator::VisitBreakStatement(BreakStatement* stmt) { Comment cmnt(masm_, "[ BreakStatement"); + SetStatementPosition(stmt); NestedStatement* current = nesting_stack_; int stack_depth = 0; while (!current->IsBreakTarget(stmt->target())) { @@ -345,6 +348,7 @@ void FastCodeGenerator::VisitBreakStatement(BreakStatement* stmt) { void FastCodeGenerator::VisitReturnStatement(ReturnStatement* stmt) { Comment cmnt(masm_, "[ ReturnStatement"); + SetStatementPosition(stmt); Expression* expr = stmt->expression(); // Complete the statement based on the type of the subexpression. if (expr->AsLiteral() != NULL) { @@ -406,6 +410,7 @@ void FastCodeGenerator::VisitSwitchStatement(SwitchStatement* stmt) { void FastCodeGenerator::VisitDoWhileStatement(DoWhileStatement* stmt) { Comment cmnt(masm_, "[ DoWhileStatement"); + SetStatementPosition(stmt); Label body, stack_limit_hit, stack_check_success; Iteration loop_statement(this, stmt); @@ -443,6 +448,7 @@ void FastCodeGenerator::VisitDoWhileStatement(DoWhileStatement* stmt) { void FastCodeGenerator::VisitWhileStatement(WhileStatement* stmt) { Comment cmnt(masm_, "[ WhileStatement"); + SetStatementPosition(stmt); Label body, stack_limit_hit, stack_check_success; Iteration loop_statement(this, stmt); diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc index 819231a43b6..49b32c251ea 100644 --- a/src/ia32/codegen-ia32.cc +++ b/src/ia32/codegen-ia32.cc @@ -251,6 +251,12 @@ void CodeGenerator::GenCode(FunctionLiteral* fun) { StoreArgumentsObject(true); } + // Initialize ThisFunction reference if present. + if (scope_->is_function_scope() && scope_->function() != NULL) { + frame_->Push(Factory::the_hole_value()); + StoreToSlot(scope_->function()->slot(), NOT_CONST_INIT); + } + // Generate code to 'execute' declarations and initialize functions // (source elements). In case of an illegal redeclaration we need to // handle that instead of processing the declarations. diff --git a/src/parser.cc b/src/parser.cc index 1c2818c972c..4090a080fd3 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -3598,7 +3598,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name, top_scope_->NewUnresolved(function_name, inside_with()); fproxy->BindTo(fvar); body.Add(new ExpressionStatement( - new Assignment(Token::INIT_VAR, fproxy, + new Assignment(Token::INIT_CONST, fproxy, NEW(ThisFunction()), RelocInfo::kNoPosition))); } diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc index 904d02d4f8a..5184b3d9224 100644 --- a/src/x64/codegen-x64.cc +++ b/src/x64/codegen-x64.cc @@ -393,6 +393,12 @@ void CodeGenerator::GenCode(FunctionLiteral* function) { StoreArgumentsObject(true); } + // Initialize ThisFunction reference if present. + if (scope_->is_function_scope() && scope_->function() != NULL) { + frame_->Push(Factory::the_hole_value()); + StoreToSlot(scope_->function()->slot(), NOT_CONST_INIT); + } + // Generate code to 'execute' declarations and initialize functions // (source elements). In case of an illegal redeclaration we need to // handle that instead of processing the declarations. -- 2.18.1