Skip to content
Permalink
Browse files
2010-10-17 Oliver Hunt <oliver@apple.com>
        Reviewed by Sam Weinig.

        Strict mode: Assignment that would create a global should be a late ReferenceError, not a syntax failure
        https://bugs.webkit.org/show_bug.cgi?id=47788

        Fixing this required a couple of changes:
         * resolve_base now has a flag to indicate whether it is being used for a put in strict mode.
           this allows us to throw an exception when we're doing a completely generic resolve for
           assignment, and that assignment would create a new global.
         * There is a new opcode 'op_ensure_property_exists' that is used to determine whether
           the property being assigned to already exists on the global object.  This currently
           has no caching, but such caching could be added relatively trivially.  It is only used
           in the case where we know that a property will be placed on the global object, and
           we cannot verify that the property already exists.

        In the jit we plant a call to cti_op_resolve_base_strict_put in the effected case rather
        than making op_resolve_base have an additional runtime branch.

        There's also a new helper function to create the exception for the invalid assignment.

        * bytecode/CodeBlock.cpp:
        (JSC::CodeBlock::dump):
        * bytecode/Opcode.h:
        * bytecompiler/BytecodeGenerator.cpp:
        (JSC::BytecodeGenerator::emitResolveBase):
        (JSC::BytecodeGenerator::emitResolveBaseForPut):
        * bytecompiler/BytecodeGenerator.h:
        * bytecompiler/NodesCodegen.cpp:
        (JSC::AssignResolveNode::emitBytecode):
        (JSC::ForInNode::emitBytecode):
        * interpreter/Interpreter.cpp:
        (JSC::Interpreter::resolveBase):
        (JSC::Interpreter::privateExecute):
        * jit/JIT.cpp:
        (JSC::JIT::privateCompileMainPass):
        * jit/JIT.h:
        * jit/JITOpcodes.cpp:
        (JSC::JIT::emit_op_resolve_base):
        (JSC::JIT::emit_op_ensure_property_exists):
        * jit/JITOpcodes32_64.cpp:
        (JSC::JIT::emit_op_resolve_base):
        (JSC::JIT::emit_op_ensure_property_exists):
        * jit/JITStubs.cpp:
        (JSC::DEFINE_STUB_FUNCTION):
        * jit/JITStubs.h:
        * parser/JSParser.cpp:
        (JSC::JSParser::parseProgram):
        * runtime/ExceptionHelpers.cpp:
        (JSC::createErrorForInvalidGlobalAssignment):
        * runtime/ExceptionHelpers.h:
        * runtime/Operations.h:
        (JSC::resolveBase):
2010-10-17  Oliver Hunt  <oliver@apple.com>

        Reviewed by Sam Weinig.

        Strict mode: Assignment that would create a global should be a late ReferenceError, not a syntax failure
        https://bugs.webkit.org/show_bug.cgi?id=47788

        Update test to check for the correct behaviour.

        * fast/js/basic-strict-mode-expected.txt:
        * fast/js/script-tests/basic-strict-mode.js:

Canonical link: https://commits.webkit.org/60506@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@69940 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
ojhunt committed Oct 18, 2010
1 parent 667f033 commit ce9753d00c90a680f2f42187ec403bd27eab51a2
Showing 20 changed files with 197 additions and 22 deletions.
@@ -1,3 +1,58 @@
2010-10-17 Oliver Hunt <oliver@apple.com>

Reviewed by Sam Weinig.

Strict mode: Assignment that would create a global should be a late ReferenceError, not a syntax failure
https://bugs.webkit.org/show_bug.cgi?id=47788

Fixing this required a couple of changes:
* resolve_base now has a flag to indicate whether it is being used for a put in strict mode.
this allows us to throw an exception when we're doing a completely generic resolve for
assignment, and that assignment would create a new global.
* There is a new opcode 'op_ensure_property_exists' that is used to determine whether
the property being assigned to already exists on the global object. This currently
has no caching, but such caching could be added relatively trivially. It is only used
in the case where we know that a property will be placed on the global object, and
we cannot verify that the property already exists.

In the jit we plant a call to cti_op_resolve_base_strict_put in the effected case rather
than making op_resolve_base have an additional runtime branch.

There's also a new helper function to create the exception for the invalid assignment.

* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::dump):
* bytecode/Opcode.h:
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::emitResolveBase):
(JSC::BytecodeGenerator::emitResolveBaseForPut):
* bytecompiler/BytecodeGenerator.h:
* bytecompiler/NodesCodegen.cpp:
(JSC::AssignResolveNode::emitBytecode):
(JSC::ForInNode::emitBytecode):
* interpreter/Interpreter.cpp:
(JSC::Interpreter::resolveBase):
(JSC::Interpreter::privateExecute):
* jit/JIT.cpp:
(JSC::JIT::privateCompileMainPass):
* jit/JIT.h:
* jit/JITOpcodes.cpp:
(JSC::JIT::emit_op_resolve_base):
(JSC::JIT::emit_op_ensure_property_exists):
* jit/JITOpcodes32_64.cpp:
(JSC::JIT::emit_op_resolve_base):
(JSC::JIT::emit_op_ensure_property_exists):
* jit/JITStubs.cpp:
(JSC::DEFINE_STUB_FUNCTION):
* jit/JITStubs.h:
* parser/JSParser.cpp:
(JSC::JSParser::parseProgram):
* runtime/ExceptionHelpers.cpp:
(JSC::createErrorForInvalidGlobalAssignment):
* runtime/ExceptionHelpers.h:
* runtime/Operations.h:
(JSC::resolveBase):

2010-10-17 Simon Fraser <simon.fraser@apple.com>

First part of fix for Windows build failure. Will wait for the
@@ -761,7 +761,14 @@ void CodeBlock::dump(ExecState* exec, const Vector<Instruction>::const_iterator&
case op_resolve_base: {
int r0 = (++it)->u.operand;
int id0 = (++it)->u.operand;
printf("[%4d] resolve_base\t %s, %s\n", location, registerName(exec, r0).data(), idName(id0, m_identifiers[id0]).data());
int isStrict = (++it)->u.operand;
printf("[%4d] resolve_base%s\t %s, %s\n", location, isStrict ? "_strict" : "", registerName(exec, r0).data(), idName(id0, m_identifiers[id0]).data());
break;
}
case op_ensure_property_exists: {
int r0 = (++it)->u.operand;
int id0 = (++it)->u.operand;
printf("[%4d] ensure_property_exists\t %s, %s\n", location, registerName(exec, r0).data(), idName(id0, m_identifiers[id0]).data());
break;
}
case op_resolve_with_base: {
@@ -100,7 +100,8 @@ namespace JSC {
macro(op_put_scoped_var, 4) \
macro(op_get_global_var, 3) \
macro(op_put_global_var, 3) \
macro(op_resolve_base, 3) \
macro(op_resolve_base, 4) \
macro(op_ensure_property_exists, 3) \
macro(op_resolve_with_base, 4) \
macro(op_get_by_id, 8) \
macro(op_get_by_id_self, 8) \
@@ -1271,13 +1271,40 @@ RegisterID* BytecodeGenerator::emitResolveBase(RegisterID* dst, const Identifier
emitOpcode(op_resolve_base);
instructions().append(dst->index());
instructions().append(addConstant(property));
instructions().append(false);
return dst;
}

// Global object is the base
return emitLoad(dst, JSValue(globalObject));
}

RegisterID* BytecodeGenerator::emitResolveBaseForPut(RegisterID* dst, const Identifier& property)
{
if (!m_codeBlock->isStrictMode())
return emitResolveBase(dst, property);
size_t depth = 0;
int index = 0;
JSObject* globalObject = 0;
bool requiresDynamicChecks = false;
findScopedProperty(property, index, depth, false, requiresDynamicChecks, globalObject);
if (!globalObject || requiresDynamicChecks) {
// We can't optimise at all :-(
emitOpcode(op_resolve_base);
instructions().append(dst->index());
instructions().append(addConstant(property));
instructions().append(true);
return dst;
}

// Global object is the base
RefPtr<RegisterID> result = emitLoad(dst, JSValue(globalObject));
emitOpcode(op_ensure_property_exists);
instructions().append(dst->index());
instructions().append(addConstant(property));
return result.get();
}

RegisterID* BytecodeGenerator::emitResolveWithBase(RegisterID* baseDst, RegisterID* propDst, const Identifier& property)
{
size_t depth = 0;
@@ -329,6 +329,7 @@ namespace JSC {
RegisterID* emitPutScopedVar(size_t skip, int index, RegisterID* value, JSValue globalObject);

RegisterID* emitResolveBase(RegisterID* dst, const Identifier& property);
RegisterID* emitResolveBaseForPut(RegisterID* dst, const Identifier& property);
RegisterID* emitResolveWithBase(RegisterID* baseDst, RegisterID* propDst, const Identifier& property);

void emitMethodCheck();
@@ -1231,7 +1231,7 @@ RegisterID* AssignResolveNode::emitBytecode(BytecodeGenerator& generator, Regist
return value;
}

RefPtr<RegisterID> base = generator.emitResolveBase(generator.newTemporary(), m_ident);
RefPtr<RegisterID> base = generator.emitResolveBaseForPut(generator.newTemporary(), m_ident);
if (dst == generator.ignoredResult())
dst = 0;
RegisterID* value = generator.emitNode(dst, m_right);
@@ -1605,7 +1605,7 @@ RegisterID* ForInNode::emitBytecode(BytecodeGenerator& generator, RegisterID* ds
if (!propertyName) {
propertyName = generator.newTemporary();
RefPtr<RegisterID> protect = propertyName;
RegisterID* base = generator.emitResolveBase(generator.newTemporary(), ident);
RegisterID* base = generator.emitResolveBaseForPut(generator.newTemporary(), ident);

generator.emitExpressionInfo(divot(), startOffset(), endOffset());
generator.emitPutById(base, ident, propertyName);
@@ -280,8 +280,14 @@ NEVER_INLINE void Interpreter::resolveBase(CallFrame* callFrame, Instruction* vP
{
int dst = vPC[1].u.operand;
int property = vPC[2].u.operand;
callFrame->r(dst) = JSValue(JSC::resolveBase(callFrame, callFrame->codeBlock()->identifier(property), callFrame->scopeChain()));
ASSERT(callFrame->r(dst).jsValue());
bool isStrictPut = vPC[3].u.operand;
const Identifier ident = callFrame->codeBlock()->identifier(property);
JSValue result = JSC::resolveBase(callFrame, ident, callFrame->scopeChain(), isStrictPut);
if (!result) {
callFrame->r(dst) = result;
ASSERT(callFrame->r(dst).jsValue());
} else
callFrame->globalData().exception = createErrorForInvalidGlobalAssignment(callFrame, ident.ustring());
}

NEVER_INLINE bool Interpreter::resolveBaseAndProperty(CallFrame* callFrame, Instruction* vPC, JSValue& exceptionValue)
@@ -2432,6 +2438,24 @@ JSValue Interpreter::privateExecute(ExecutionFlag flag, RegisterFile* registerFi
vPC += OPCODE_LENGTH(op_resolve_base);
NEXT_INSTRUCTION();
}
DEFINE_OPCODE(op_ensure_property_exists) {
/* ensure_property_exists base(r) property(id)
Throws an exception if property does not exist on base
*/
int base = vPC[1].u.operand;
int property = vPC[2].u.operand;
Identifier& ident = codeBlock->identifier(property);

JSValue baseVal = callFrame->r(base).jsValue();
JSObject* baseObject = asObject(baseVal);
PropertySlot slot(baseVal);
if (!baseObject->getPropertySlot(callFrame, ident, slot)) {
exceptionValue = createErrorForInvalidGlobalAssignment(callFrame, ident.ustring());
goto vm_throw;
}
NEXT_INSTRUCTION();
}
DEFINE_OPCODE(op_resolve_with_base) {
/* resolve_with_base baseDst(r) propDst(r) property(id)
@@ -302,6 +302,7 @@ void JIT::privateCompileMainPass()
DEFINE_OP(op_put_setter)
DEFINE_OP(op_resolve)
DEFINE_OP(op_resolve_base)
DEFINE_OP(op_ensure_property_exists)
DEFINE_OP(op_resolve_global)
DEFINE_OP(op_resolve_global_dynamic)
DEFINE_OP(op_resolve_skip)
@@ -811,6 +811,7 @@ namespace JSC {
void emit_op_put_setter(Instruction*);
void emit_op_resolve(Instruction*);
void emit_op_resolve_base(Instruction*);
void emit_op_ensure_property_exists(Instruction*);
void emit_op_resolve_global(Instruction*, bool dynamic = false);
void emit_op_resolve_global_dynamic(Instruction*);
void emit_op_resolve_skip(Instruction*);
@@ -644,7 +644,15 @@ void JIT::emit_op_strcat(Instruction* currentInstruction)

void JIT::emit_op_resolve_base(Instruction* currentInstruction)
{
JITStubCall stubCall(this, cti_op_resolve_base);
JITStubCall stubCall(this, currentInstruction[3].u.operand ? cti_op_resolve_base_strict_put : cti_op_resolve_base);
stubCall.addArgument(ImmPtr(&m_codeBlock->identifier(currentInstruction[2].u.operand)));
stubCall.call(currentInstruction[1].u.operand);
}

void JIT::emit_op_ensure_property_exists(Instruction* currentInstruction)
{
JITStubCall stubCall(this, cti_op_ensure_property_exists);
stubCall.addArgument(Imm32(currentInstruction[1].u.operand));
stubCall.addArgument(ImmPtr(&m_codeBlock->identifier(currentInstruction[2].u.operand)));
stubCall.call(currentInstruction[1].u.operand);
}
@@ -737,7 +737,15 @@ void JIT::emit_op_strcat(Instruction* currentInstruction)

void JIT::emit_op_resolve_base(Instruction* currentInstruction)
{
JITStubCall stubCall(this, cti_op_resolve_base);
JITStubCall stubCall(this, currentInstruction[3].u.operand ? cti_op_resolve_base_strict_put : cti_op_resolve_base);
stubCall.addArgument(ImmPtr(&m_codeBlock->identifier(currentInstruction[2].u.operand)));
stubCall.call(currentInstruction[1].u.operand);
}

void JIT::emit_op_ensure_property_exists(Instruction* currentInstruction)
{
JITStubCall stubCall(this, cti_op_ensure_property_exists);
stubCall.addArgument(Imm32(currentInstruction[1].u.operand));
stubCall.addArgument(ImmPtr(&m_codeBlock->identifier(currentInstruction[2].u.operand)));
stubCall.call(currentInstruction[1].u.operand);
}
@@ -2697,9 +2697,35 @@ DEFINE_STUB_FUNCTION(EncodedJSValue, op_resolve_base)
{
STUB_INIT_STACK_FRAME(stackFrame);
return JSValue::encode(JSC::resolveBase(stackFrame.callFrame, stackFrame.args[0].identifier(), stackFrame.callFrame->scopeChain()));
return JSValue::encode(JSC::resolveBase(stackFrame.callFrame, stackFrame.args[0].identifier(), stackFrame.callFrame->scopeChain(), false));
}
DEFINE_STUB_FUNCTION(EncodedJSValue, op_resolve_base_strict_put)
{
STUB_INIT_STACK_FRAME(stackFrame);
JSValue base = JSC::resolveBase(stackFrame.callFrame, stackFrame.args[0].identifier(), stackFrame.callFrame->scopeChain(), true);
if (!base) {
stackFrame.globalData->exception = createErrorForInvalidGlobalAssignment(stackFrame.callFrame, stackFrame.args[0].identifier().ustring());
VM_THROW_EXCEPTION();
}
return JSValue::encode(base);
}
DEFINE_STUB_FUNCTION(EncodedJSValue, op_ensure_property_exists)
{
STUB_INIT_STACK_FRAME(stackFrame);
JSValue base = stackFrame.callFrame->r(stackFrame.args[0].int32()).jsValue();
JSObject* object = asObject(base);
PropertySlot slot(object);
ASSERT(stackFrame.callFrame->codeBlock()->isStrictMode());
if (!object->getPropertySlot(stackFrame.callFrame, stackFrame.args[1].identifier(), slot)) {
stackFrame.globalData->exception = createErrorForInvalidGlobalAssignment(stackFrame.callFrame, stackFrame.args[1].identifier().ustring());
VM_THROW_EXCEPTION();
}
return JSValue::encode(base);
}
DEFINE_STUB_FUNCTION(EncodedJSValue, op_resolve_skip)
{
STUB_INIT_STACK_FRAME(stackFrame);
@@ -338,6 +338,8 @@ extern "C" {
EncodedJSValue JIT_STUB cti_op_pre_inc(STUB_ARGS_DECLARATION);
EncodedJSValue JIT_STUB cti_op_resolve(STUB_ARGS_DECLARATION);
EncodedJSValue JIT_STUB cti_op_resolve_base(STUB_ARGS_DECLARATION);
EncodedJSValue JIT_STUB cti_op_resolve_base_strict_put(STUB_ARGS_DECLARATION);
EncodedJSValue JIT_STUB cti_op_ensure_property_exists(STUB_ARGS_DECLARATION);
EncodedJSValue JIT_STUB cti_op_resolve_global(STUB_ARGS_DECLARATION);
EncodedJSValue JIT_STUB cti_op_resolve_global_dynamic(STUB_ARGS_DECLARATION);
EncodedJSValue JIT_STUB cti_op_resolve_skip(STUB_ARGS_DECLARATION);
@@ -529,18 +529,10 @@ bool JSParser::parseProgram(JSGlobalObject* lexicalGlobalObject)
if (!sourceElements || !consume(EOFTOK))
return true;
if (!m_syntaxAlreadyValidated) {
IdentifierSet writtenVariables;
scope->getUncapturedWrittenVariables(writtenVariables);
IdentifierSet::const_iterator end = writtenVariables.end();
for (IdentifierSet::const_iterator ptr = writtenVariables.begin(); ptr != end; ++ptr) {
PropertySlot slot(lexicalGlobalObject);
if (!lexicalGlobalObject->getPropertySlot(lexicalGlobalObject->globalExec(), Identifier(m_globalData, *ptr), slot))
return true;
}
IdentifierSet deletedVariables;
if (!scope->getDeletedVariables(deletedVariables))
return true;
end = deletedVariables.end();
IdentifierSet::const_iterator end = deletedVariables.end();
SymbolTable& globalEnvRecord = lexicalGlobalObject->symbolTable();
for (IdentifierSet::const_iterator ptr = deletedVariables.begin(); ptr != end; ++ptr) {
if (!globalEnvRecord.get(*ptr).isNull())
@@ -192,4 +192,9 @@ JSValue throwOutOfMemoryError(ExecState* exec)
return throwError(exec, createError(exec, "Out of memory"));
}

JSObject* createErrorForInvalidGlobalAssignment(ExecState* exec, const UString& propertyName)
{
return createReferenceError(exec, makeUString("Strict mode forbids implicit creation of global property '", propertyName, "'"));
}

} // namespace JSC
@@ -54,6 +54,7 @@ namespace JSC {
JSValue createNotAFunctionError(ExecState*, JSValue, unsigned bytecodeOffset, CodeBlock*);
JSObject* createNotAnObjectError(ExecState*, JSNotAnObjectErrorStub*, unsigned bytecodeOffset, CodeBlock*);
JSValue throwOutOfMemoryError(ExecState*);
JSObject* createErrorForInvalidGlobalAssignment(ExecState*, const UString&);

} // namespace JSC

@@ -460,7 +460,7 @@ namespace JSC {
}
}

ALWAYS_INLINE JSValue resolveBase(CallFrame* callFrame, Identifier& property, ScopeChainNode* scopeChain)
ALWAYS_INLINE JSValue resolveBase(CallFrame* callFrame, Identifier& property, ScopeChainNode* scopeChain, bool isStrictPut)
{
ScopeChainIterator iter = scopeChain->begin();
ScopeChainIterator next = iter;
@@ -472,7 +472,9 @@ namespace JSC {
JSObject* base;
while (true) {
base = *iter;
if (next == end || base->getPropertySlot(callFrame, property, slot))
if (next == end)
return isStrictPut ? JSValue() : base;
if (base->getPropertySlot(callFrame, property, slot))
return base;

iter = next;
@@ -1,3 +1,15 @@
2010-10-17 Oliver Hunt <oliver@apple.com>

Reviewed by Sam Weinig.

Strict mode: Assignment that would create a global should be a late ReferenceError, not a syntax failure
https://bugs.webkit.org/show_bug.cgi?id=47788

Update test to check for the correct behaviour.

* fast/js/basic-strict-mode-expected.txt:
* fast/js/script-tests/basic-strict-mode.js:

2010-10-17 Antonio Gomes <agomes@rim.com>

Reviewed by Simon Fraser.
@@ -51,7 +51,8 @@ PASS 'use strict'; '\007'; threw exception SyntaxError: Parse error.
PASS '\007'; 'use strict'; threw exception SyntaxError: Parse error.
PASS 'use strict'; (function (){ delete someDeclaredGlobal;}) threw exception SyntaxError: Parse error.
PASS (function (){ 'use strict'; delete someDeclaredGlobal;}) threw exception SyntaxError: Parse error.
PASS 'use strict'; someGlobal = 'Shouldn\'t be able to assign this.'; someGlobal; threw exception SyntaxError: Parse error.
PASS 'use strict'; if (0) { someGlobal = 'Shouldn\'t be able to assign this.'; }; true; is true
PASS 'use strict'; someGlobal = 'Shouldn\'t be able to assign this.'; threw exception ReferenceError: Strict mode forbids implicit creation of global property 'someGlobal'.
PASS 'use strict'; eval('var introducedVariable = "FAIL: variable introduced into containing scope";'); introducedVariable threw exception ReferenceError: Can't find variable: introducedVariable.
PASS 'use strict'; objectWithReadonlyProperty.prop = 'fail' threw exception TypeError: Attempted to assign to readonly property..
PASS 'use strict'; delete objectWithReadonlyProperty.prop threw exception TypeError: Unable to delete property..
@@ -60,7 +60,8 @@ shouldThrow("'\\007'; 'use strict';");
var someDeclaredGlobal;
shouldThrow("'use strict'; (function (){ delete someDeclaredGlobal;})");
shouldThrow("(function (){ 'use strict'; delete someDeclaredGlobal;})");
shouldThrow("'use strict'; someGlobal = 'Shouldn\\'t be able to assign this.'; someGlobal;");
shouldBeTrue("'use strict'; if (0) { someGlobal = 'Shouldn\\'t be able to assign this.'; }; true;");
shouldThrow("'use strict'; someGlobal = 'Shouldn\\'t be able to assign this.'; ");
shouldThrow("'use strict'; eval('var introducedVariable = \"FAIL: variable introduced into containing scope\";'); introducedVariable");
var objectWithReadonlyProperty = {};
Object.defineProperty(objectWithReadonlyProperty, "prop", {value: "value", writable:false});

0 comments on commit ce9753d

Please sign in to comment.